Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bench): fix sozo bench #2601

Merged
merged 3 commits into from
Oct 31, 2024
Merged

fix(bench): fix sozo bench #2601

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
80dea8e
wip
glihm Oct 31, 2024
268684c
Merge remote-tracking branch 'dojo/main' into bench-fixes
glihm Oct 31, 2024
b63ea45
benches: fix sozo benchmark
glihm Oct 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions crates/benches/contracts/Scarb.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@ version = 1

[[package]]
name = "benches"
version = "0.4.4"
version = "1.0.0-rc.0"
dependencies = [
"dojo",
]

[[package]]
name = "dojo"
version = "0.4.4"
source = "git+https://github.com/dojoengine/dojo#37f41d585f549013a73ca189034b0471f1e81731"
version = "1.0.0-rc.0"
source = "git+https://github.com/dojoengine/dojo?tag=v1.0.0-rc.0#f4199aec570a395278b8c8748bc46e2f6be3d0c7"
dependencies = [
"dojo_plugin",
]

[[package]]
name = "dojo_plugin"
version = "0.3.11"
source = "git+https://github.com/dojoengine/dojo?tag=v0.3.11#1e651b5d4d3b79b14a7d8aa29a92062fcb9e6659"
version = "2.8.4"
source = "git+https://github.com/dojoengine/dojo?tag=v1.0.0-rc.0#f4199aec570a395278b8c8748bc46e2f6be3d0c7"
25 changes: 6 additions & 19 deletions crates/benches/contracts/Scarb.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,11 @@
[package]
cairo-version = "2.4.0"
cairo-version = "=2.8.4"
name = "benches"
version = "0.4.4"
version = "1.0.0-rc.0"

[cairo]
sierra-replace-ids = true
[[target.starknet-contract]]
build-external-contracts = ["dojo::world::world_contract::world"]

[dependencies]
dojo = { git = "https://github.com/dojoengine/dojo", version = "0.4.4" }

[[target.dojo]]

[tool.dojo]
initializer_class_hash = "0xbeef"

[tool.dojo.env]
# rpc_url = "http://localhost:5050/"
# Default account for katana with seed = 0
account_address = "0x2af9427c5a277474c079a1283c880ee8a6f0f8fbf73ce969c08d88befec1bba"
private_key = "0x1800000000300000180000000000030000000000003006001800006600"

[tool.dojo.world.namespace]
default = "benches"
dojo = { git = "https://github.com/dojoengine/dojo", tag = "v1.0.0-rc.0" }
starknet = "=2.8.4"
Comment on lines +10 to +11
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Dependencies are mostly aligned, with one minor inconsistency sensei!

Most of the project uses starknet 2.8.4, but there's one outlier in crates/katana/contracts/Scarb.toml using version 2.8.2. The dojo dependencies are properly configured, with the benches using the git tag that matches the package version (v1.0.0-rc.0).

  • crates/katana/contracts/Scarb.toml: Consider updating starknet from "2.8.2" to "=2.8.4" to maintain consistency
🔗 Analysis chain

Dependencies alignment check needed.

The dojo dependency tag matches the package version, which is good practice. The addition of starknet dependency with a specific version is also appropriate.

Let's verify dependency consistency:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for consistent dependency versions across the project
# Expect: All dojo and starknet dependencies should use compatible versions

# Search for dojo dependencies
echo "Checking dojo dependency declarations:"
rg "dojo = \{" -g "Scarb.toml" -A 2

# Search for starknet dependencies
echo -e "\nChecking starknet dependency declarations:"
rg "^starknet = " -g "Scarb.toml"

Length of output: 1486

Script:

#!/bin/bash
# Let's try a more focused search for dependencies in the specific file and related ones

# Check the specific file's dependencies
echo "Checking dependencies in benches/contracts/Scarb.toml:"
cat crates/benches/contracts/Scarb.toml

# Check dojo dependencies across all Scarb.toml files
echo -e "\nChecking all dojo dependencies:"
rg "dojo.*=.*" -g "Scarb.toml"

# Check starknet dependencies across all Scarb.toml files
echo -e "\nChecking all starknet dependencies:"
rg "starknet.*=.*" -g "Scarb.toml"

Length of output: 2046

30 changes: 30 additions & 0 deletions crates/benches/contracts/dojo_dev.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
[world]
description = "Bench world."
name = "bench"
seed = "bench"

[env]
rpc_url = "http://localhost:5050/"
# Default account for katana with seed = 0
account_address = "0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec"
private_key = "0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912"
#world_address = "0x077c0dc7c1aba7f8842aff393ce6aa71fa675b4ced1bc927f7fc971b6acd92fc"
Comment on lines +6 to +11
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Security: Move sensitive data to environment variables.

Ohayo sensei! While this is a development configuration, it's still risky to hardcode private keys in configuration files as they might accidentally be committed to version control.

Consider this safer approach:

[env]
rpc_url = "http://localhost:5050/"
# Default account for katana with seed = 0
-account_address = "0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec"
-private_key = "0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912"
+account_address = "${BENCH_ACCOUNT_ADDRESS}"
+private_key = "${BENCH_PRIVATE_KEY}"

Then provide these values through environment variables during benchmarking.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
[env]
rpc_url = "http://localhost:5050/"
# Default account for katana with seed = 0
account_address = "0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec"
private_key = "0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912"
#world_address = "0x077c0dc7c1aba7f8842aff393ce6aa71fa675b4ced1bc927f7fc971b6acd92fc"
[env]
rpc_url = "http://localhost:5050/"
# Default account for katana with seed = 0
account_address = "${BENCH_ACCOUNT_ADDRESS}"
private_key = "${BENCH_PRIVATE_KEY}"
#world_address = "0x077c0dc7c1aba7f8842aff393ce6aa71fa675b4ced1bc927f7fc971b6acd92fc"
🧰 Tools
🪛 Gitleaks

10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


[namespace]
default = "ns"
mappings = { "ns" = ["c1", "M"], "ns2" = ["c1", "M"] }

[init_call_args]
"ns-c1" = ["0xfffe"]
"ns2-c1" = ["0xfffe"]

[writers]
"ns" = ["ns-c1", "ns-c2"]
"ns-M" = ["ns-c2", "ns-c1", "ns2-c1"]

[owners]
"ns" = ["ns-c1"]

[migration]
order_inits = ["ns-c2", "ns-c1"]
skip_contracts = ["ns-c3"]
9 changes: 1 addition & 8 deletions crates/dojo/test-utils/src/compiler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use std::{env, fs, io};

use assert_fs::TempDir;
use camino::{Utf8Path, Utf8PathBuf};
use scarb::compiler::plugin::CairoPluginRepository;
use scarb::compiler::{CompilationUnit, CompilerRepository, Profile};
use scarb::compiler::{CompilationUnit, Profile};
use scarb::core::{Config, TargetKind};
use scarb::ops;
use scarb::ops::{CompileOpts, FeaturesOpts, FeaturesSelector};
Expand Down Expand Up @@ -297,10 +296,6 @@ pub fn copy_project_temp(
/// * `path` - The path to the Scarb.toml file to build the config for.
/// * `profile` - The profile to use for the config.
pub fn build_test_config(path: &str, profile: Profile) -> anyhow::Result<Config> {
let compilers = CompilerRepository::empty();

let cairo_plugins = CairoPluginRepository::empty();

// If the cache_dir is not overriden, we can't run tests in parallel.
let cache_dir = TempDir::new().unwrap();

Expand All @@ -310,9 +305,7 @@ pub fn build_test_config(path: &str, profile: Profile) -> anyhow::Result<Config>
.global_cache_dir_override(Some(Utf8Path::from_path(cache_dir.path()).unwrap()))
.ui_verbosity(Verbosity::Verbose)
.log_filter_directive(env::var_os("SCARB_LOG"))
.compilers(compilers)
.profile(profile)
.cairo_plugins(cairo_plugins)
.build()
}

Expand Down
1 change: 1 addition & 0 deletions output_sozo.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test build/Sozo.Cold ... bench: 4521852125 ns/iter (+/- 0)
Loading