feat(sozo): introduce walnut verify command + execute "--walnut" flag enabled #2734
Conversation
WalkthroughOhayo, sensei! The pull request integrates the Walnut debugger into the Dojo framework, enhancing transaction debugging and verification capabilities. Key changes include the introduction of a new Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant WalnutDebugger
participant Backend
User->>CLI: Execute sozo command with --walnut flag
CLI->>WalnutDebugger: Initialize debugger
alt Transaction Execution
CLI->>WalnutDebugger: Debug transaction
WalnutDebugger->>Backend: Send transaction details
Backend-->>WalnutDebugger: Return debugging information
end
alt Verification
CLI->>WalnutDebugger: Verify source code
WalnutDebugger->>Backend: Send source code payload
Backend-->>WalnutDebugger: Return verification result
end
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (7)
bin/sozo/src/commands/verify.rs (2)
6-7: Consider adding configuration options to the VerifyArgs struct, sensei!The empty struct might be too restrictive. Consider adding options that would enhance the verification workflow, such as:
- Path to specific contracts to verify
- Verification mode or level
- Output format preferences
Example enhancement:
#[derive(Debug, Args)] pub struct VerifyArgs { + /// Path to specific contract(s) to verify + #[arg(short, long)] + pub contracts: Option<Vec<String>>, + + /// Verification mode (basic, thorough) + #[arg(short, long, default_value = "basic")] + pub mode: String, }
1-17: Solid architectural foundation, sensei!The verify command follows the established patterns in the sozo CLI:
- Proper separation of concerns between argument parsing and execution
- Consistent async handling with other commands
- Clean integration with the WalnutDebugger
The structure allows for future extensibility while maintaining the codebase's architectural integrity.
bin/sozo/src/commands/execute.rs (1)
Line range hint
70-74: Ohayo sensei! Consider adding error handling for WalnutDebugger initialization.The WalnutDebugger initialization looks clean, but we might want to handle potential initialization failures gracefully.
Consider wrapping the initialization in a Result:
- let walnut_debugger = WalnutDebugger::new_from_flag( - self.transaction.walnut, - self.starknet.url(profile_config.env.as_ref())?, - ); + let walnut_debugger = WalnutDebugger::new_from_flag( + self.transaction.walnut, + self.starknet.url(profile_config.env.as_ref())?, + ).map_err(|e| anyhow!("Failed to initialize WalnutDebugger: {}", e))?;crates/sozo/walnut/src/debugger.rs (1)
25-29: Consider simplifyingnew_from_flagwiththenOhayo, sensei! You can make the
new_from_flagmethod more concise by utilizing thethenmethod onbool.Apply this diff to simplify:
pub fn new_from_flag(use_walnut: bool, rpc_url: Url) -> Option<Self> { - if use_walnut { - Some(Self::new(rpc_url)) - } else { - None - } + use_walnut.then(|| Self::new(rpc_url)) }crates/sozo/walnut/src/verification.rs (3)
58-60: Reusereqwest::Clientfor improved performanceOhayo, sensei! Creating a new
reqwest::Clientfor each request can be inefficient. Consider reusing a single instance ofClientto improve performance.Apply this diff to reuse the client:
-async fn verify_classes(payload: VerificationPayload, api_url: &str) -> Result<String, Error> { - let res = - reqwest::Client::new().post(format!("{api_url}/v1/verify")).json(&payload).send().await?; +async fn verify_classes( + client: &reqwest::Client, + payload: VerificationPayload, + api_url: &str, +) -> Result<String, Error> { + let res = client.post(format!("{api_url}/v1/verify")).json(&payload).send().await?; // Existing code... } // In `walnut_verify` function: pub async fn walnut_verify(ws: &Workspace<'_>) -> anyhow::Result<()> { // Existing code... + let client = reqwest::Client::new(); match verify_classes(&client, verification_payload, &api_url).await { // Existing code... } // Existing code... }
15-22: Ensure comprehensive documentation forVerificationPayloadOhayo, sensei! The
VerificationPayloadstruct plays a crucial role in the verification process. Consider adding more detailed documentation for its fields to enhance maintainability and readability.
Line range hint
62-79: Rename functions to adhere to Rust naming conventionsOhayo, sensei! In Rust, leading underscores are typically used for unused variables or to silence warnings. Since these functions are used within the module, consider renaming
_collect_source_code,_subtitle, and_dimmed_messageto remove the leading underscores for clarity.Apply this diff to rename the functions:
-fn _collect_source_code(root_dir: &Path) -> Result<Value, Error> { +fn collect_source_code(root_dir: &Path) -> Result<Value, Error> { -fn _subtitle<D: AsRef<str>>(message: D) -> String { +fn subtitle<D: AsRef<str>>(message: D) -> String { -fn _dimmed_message<D>(message: D) -> StyledObject<D> { +fn dimmed_message<D>(message: D) -> StyledObject<D> {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
bin/sozo/src/commands/execute.rs(2 hunks)bin/sozo/src/commands/mod.rs(5 hunks)bin/sozo/src/commands/verify.rs(1 hunks)crates/sozo/walnut/Cargo.toml(1 hunks)crates/sozo/walnut/src/debugger.rs(2 hunks)crates/sozo/walnut/src/utils.rs(1 hunks)crates/sozo/walnut/src/verification.rs(1 hunks)
🔇 Additional comments (14)
crates/sozo/walnut/src/utils.rs (2)
3-3: LGTM! Clean and focused imports.
Ohayo! The imports are well-organized and specifically bring in only the required constants.
Line range hint 5-7: Consider adding validation and documentation.
Ohayo sensei! While the function implementation is clean and provides good fallback behavior, here are some suggestions for improvement:
- Add documentation explaining the function's purpose and behavior
- Consider validating the URL format before returning
Here's a suggested implementation:
+/// Returns the Walnut API URL from the environment variable WALNUT_API_URL_ENV_VAR
+/// or falls back to the default WALNUT_API_URL constant.
+///
+/// # Returns
+/// A valid URL string for the Walnut API endpoint
pub fn walnut_get_api_url() -> String {
- env::var(WALNUT_API_URL_ENV_VAR).unwrap_or_else(|_| WALNUT_API_URL.to_string())
+ let url = env::var(WALNUT_API_URL_ENV_VAR).unwrap_or_else(|_| WALNUT_API_URL.to_string());
+ // Basic URL validation
+ debug_assert!(url.starts_with("http://") || url.starts_with("https://"),
+ "Invalid API URL format: {}", url);
+ url
}Also, since the API key functionality was removed, let's verify there are no remaining references to it.
bin/sozo/src/commands/verify.rs (2)
1-4: Ohayo! The imports look well-organized, sensei!
The chosen dependencies align perfectly with the command's requirements - error handling with anyhow, CLI parsing with clap, and integration with the walnut debugger.
9-17: 🛠️ Refactor suggestion
Enhance error handling and user feedback, sensei!
The implementation could benefit from the following improvements:
- Add user-friendly error messages
- Simplify the async block
- Add progress indication for verification
Consider this enhancement:
impl VerifyArgs {
pub fn run(self, config: &Config) -> Result<()> {
let ws = scarb::ops::read_workspace(config.manifest_path(), config)?;
- config.tokio_handle().block_on(async {
- WalnutDebugger::verify(&ws).await?;
- Ok(())
- })
+ println!("Starting contract verification...");
+ config.tokio_handle().block_on(async {
+ WalnutDebugger::verify(&ws)
+ .await
+ .map_err(|e| anyhow::anyhow!("Verification failed: {}", e))?;
+ println!("✅ Verification completed successfully!");
+ Ok(())
+ })
}
}Let's check how WalnutDebugger::verify is implemented to understand its error cases:
crates/sozo/walnut/Cargo.toml (1)
23-23: Ohayo! Clean dependency addition, sensei! 🎋
The addition of dojo-utils as a workspace dependency follows the project's consistent pattern of workspace version management and aligns well with the new verification functionality.
Let's confirm the dependency usage with this script:
✅ Verification successful
Ohayo! The dojo-utils dependency is well-utilized across the codebase, sensei! 🎋
The search results show extensive usage of dojo-utils throughout the project, including in the walnut crate (crates/sozo/walnut/src/debugger.rs) where it's importing TransactionResult. The dependency is actively used for transaction handling, configuration, and utility functions across multiple crates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that dojo-utils is actually used in the codebase
# Expected: Find usage in the new verification-related code
# Search for dojo_utils usage in Rust files
rg -t rust "use.*dojo_utils|dojo_utils::"
Length of output: 7023
bin/sozo/src/commands/execute.rs (1)
137-140: LGTM! Clean implementation of conditional debugging.
The conditional debug transaction execution is well-structured and aligns with the PR objective of enabling the --walnut flag functionality.
Let's verify the WalnutDebugger usage across the codebase:
✅ Verification successful
Ohayo! The WalnutDebugger implementation is consistent and well-integrated, sensei!
The verification shows that:
- The
debug_transactionfunctionality is properly encapsulated in thewalnutcrate - The implementation follows a consistent pattern across the codebase
- The feature is well-tested with both hosted and localhost scenarios
- Usage in both
execute.rsandregister.rsfollows the same pattern usingWalnutDebugger::new_from_flag
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify WalnutDebugger integration patterns
# Look for other potential debug_transaction calls to ensure consistent usage
rg -A 3 "WalnutDebugger::new_from_flag|debug_transaction"
Length of output: 3484
bin/sozo/src/commands/mod.rs (4)
24-24: LGTM! Module declaration and import follow existing patterns.
Ohayo sensei! The new verify module is properly declared and imported, maintaining consistency with other command modules.
Also applies to: 37-37
68-69: LGTM! Command definition aligns with PR objectives.
The new Verify command is properly boxed and includes a descriptive about message that clearly indicates its purpose for contract verification.
88-88: LGTM! Display implementation maintains consistency.
The Display implementation for the Verify command follows the established pattern used for other commands.
115-115: Verify command execution pattern matches existing implementation.
The run function properly delegates to the VerifyArgs implementation, maintaining consistency with other commands.
Let's confirm the VerifyArgs implementation exists:
✅ Verification successful
Command implementation verified and matches existing pattern
The VerifyArgs implementation in bin/sozo/src/commands/verify.rs follows the expected pattern with:
- Empty struct definition:
pub struct VerifyArgs {} - Proper run method implementation that takes config and returns Result
- Consistent with other command implementations in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and structure of VerifyArgs
# Check for VerifyArgs struct definition
ast-grep --pattern 'struct VerifyArgs {
$$$
}'
# Check for run method implementation
ast-grep --pattern 'impl VerifyArgs {
$$$
fn run($$$) -> Result<$$$> {
$$$
}
$$$
}'
Length of output: 148
Script:
#!/bin/bash
# Let's try a different approach to find VerifyArgs
# First, let's find the file containing VerifyArgs
rg -l "VerifyArgs" --type rust
# Then let's look at the implementation details
rg "VerifyArgs" -A 10 --type rust
# Also search for the run method specifically
rg "fn run" -A 5 --type rust
Length of output: 16350
crates/sozo/walnut/src/debugger.rs (4)
1-1: Approved: Correct import of TransactionResult
Ohayo, sensei! The import statement correctly brings in TransactionResult from dojo_utils.
8-8: Approved: Importing walnut_verify
Ohayo, sensei! The addition of walnut_verify to the imports is spot on.
52-53: Approved: Added new verify method
Ohayo, sensei! The new asynchronous verify method integrates walnut_verify effectively.
33-44: Ensure all TransactionResult variants are handled
Ohayo, sensei! In the debug_transaction method, please verify that all possible variants of TransactionResult are accounted for in the match statement to prevent any unexpected behavior.
Run the following script to list all variants of TransactionResult:
This will help confirm that all variants are handled appropriately.
✅ Verification successful
All TransactionResult variants are properly handled
Ohayo, sensei! After checking the enum definition, I can confirm that the match statement in debug_transaction correctly handles all three variants of TransactionResult:
Noop-> returns early withOk(())Hash(transaction_hash)-> extracts the hashHashReceipt(transaction_hash, _)-> extracts the hash
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: List all variants of the TransactionResult enum.
# Search the codebase for the definition of TransactionResult
rg -A 10 'enum TransactionResult' --type rust
Length of output: 804
| .await?; | ||
| async fn verify_classes(payload: VerificationPayload, api_url: &str) -> Result<String, Error> { | ||
| let res = | ||
| reqwest::Client::new().post(format!("{api_url}/v1/verify")).json(&payload).send().await?; |
There was a problem hiding this comment.
Ensure that the api_url uses HTTPS for secure communication
Ohayo, sensei! To protect the sensitive source code being transmitted, please ensure that the api_url uses HTTPS to secure the communication with the Walnut backend.
| match verify_classes(verification_payload, &api_url).await { | ||
| Ok(message) => ui.print(_subtitle(message)), | ||
| Err(e) => ui.print(_subtitle(e.to_string())), | ||
| } | ||
|
|
||
| Ok(()) |
There was a problem hiding this comment.
Consider propagating errors from verify_classes
Ohayo, sensei! Currently, the walnut_verify function always returns Ok(()), even if verify_classes fails. This may mask errors from the caller and lead to unexpected behavior. Consider returning the error from verify_classes to ensure that the caller can handle it appropriately.
Apply this diff to propagate errors:
pub async fn walnut_verify(ws: &Workspace<'_>) -> anyhow::Result<()> {
// Existing code...
match verify_classes(verification_payload, &api_url).await {
Ok(message) => ui.print(_subtitle(message)),
Err(e) => {
ui.print(_subtitle(e.to_string()));
+ return Err(e.into());
}
}
- Ok(())
+ Ok(())
}📝 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.
| match verify_classes(verification_payload, &api_url).await { | |
| Ok(message) => ui.print(_subtitle(message)), | |
| Err(e) => ui.print(_subtitle(e.to_string())), | |
| } | |
| Ok(()) | |
| match verify_classes(verification_payload, &api_url).await { | |
| Ok(message) => ui.print(_subtitle(message)), | |
| Err(e) => { | |
| ui.print(_subtitle(e.to_string())); | |
| return Err(e.into()); | |
| } | |
| } | |
| Ok(()) |
There was a problem hiding this comment.
Hey @PeerZetZzZzZ nice to meet you and thank you for the PR!
Quick question first:
- Did you updated your scarb in the backend to support the dojo plugin?
- Our fork is here: https://github.com/dojoengine/scarb.
- We are adding support to proc macro. Once they are supported by the default language server, we will migrate to it. You'll have to disable then the dojo plugin in your scarb since it will no longer be required and proc macro will be preferred.
|
Hey @glihm. Here are the answers (and some questions):
|
bin/sozo/src/commands/mod.rs
Outdated
| #[command(about = "Verify a contract")] | ||
| Verify(Box<VerifyArgs>), |
There was a problem hiding this comment.
This command is too generic, since we don't know if it's explorer verify or walnut.
Could you please introduce again a walnut feature and actually gate this argument under the feature with a name like WalnutVerify for instance?
bin/sozo/src/commands/mod.rs
Outdated
| @@ -63,6 +65,8 @@ pub enum Commands { | |||
| Model(Box<ModelArgs>), | |||
| #[command(about = "Inspect events emitted by the world")] | |||
| Events(Box<EventsArgs>), | |||
| #[command(about = "Verify contracts in walnut.dev - transactions debugger and simulator")] | |||
| WalnutVerify(Box<VerifyArgs>), | |||
There was a problem hiding this comment.
@glihm I've changed it from "Verify" to "WalnutVerify". Please note that after this change calling walnut verification is calling sozo walnut-verify.
There was a problem hiding this comment.
If walnut is meant to be updated in the future and we may have something like sozo walnut verify or sozo walnut debug, then consider adding an other level.
If not, it's good like so. 👍
And even for the commands, they can be coded into the walnut crate. And here, only imported and used if the feature is actually enabled.
|
@glihm I've pushed fixes after your comments. Pls take a look, thanks ! |
bin/sozo/src/commands/verify.rs
Outdated
| #[derive(Debug, Args)] | ||
| pub struct VerifyArgs {} | ||
|
|
||
| impl VerifyArgs { |
There was a problem hiding this comment.
Let's also have this one walnut specific. May be into the walnut crate actually. 👍
bin/sozo/src/commands/mod.rs
Outdated
| @@ -63,6 +65,8 @@ pub enum Commands { | |||
| Model(Box<ModelArgs>), | |||
| #[command(about = "Inspect events emitted by the world")] | |||
| Events(Box<EventsArgs>), | |||
| #[command(about = "Verify contracts in walnut.dev - transactions debugger and simulator")] | |||
| WalnutVerify(Box<VerifyArgs>), | |||
There was a problem hiding this comment.
If walnut is meant to be updated in the future and we may have something like sozo walnut verify or sozo walnut debug, then consider adding an other level.
If not, it's good like so. 👍
And even for the commands, they can be coded into the walnut crate. And here, only imported and used if the feature is actually enabled.
|
@glihm Thanks for great suggestions, I went to this direction - so now we have |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
bin/sozo/src/commands/walnut.rs (4)
6-10: Add documentation for the WalnutArgs structureConsider adding rustdoc comments to explain the purpose and usage of this structure. This will help other developers understand how to use the CLI.
+/// Main structure for handling Walnut-related CLI commands. +/// This structure is used to parse and execute Walnut verification commands. #[derive(Debug, Args)] pub struct WalnutArgs { + /// The subcommand to execute #[command(subcommand)] pub command: WalnutVerifyCommand, }
12-16: Consider future command extensibilityThe enum is well-structured for the current
Verifycommand. As this is a new feature, you might want to consider adding a comment about potential future commands that could be added here.#[derive(Debug, Subcommand)] +/// Enum representing all available Walnut commands. +/// Note: Future commands (e.g., status, list) can be added here as new variants. pub enum WalnutVerifyCommand { #[command(about = "Verify contracts in walnut.dev - essential for debugging source code in Walnut")] Verify(WalnutVerifyOptions), }
18-19: Consider adding verification configuration optionsThe empty
WalnutVerifyOptionsstructure suggests room for future configuration options. Consider what verification parameters might be useful to expose to users.Examples of potential options:
#[derive(Debug, Args)] +/// Options for contract verification in Walnut pub struct WalnutVerifyOptions { + // TODO: Consider adding options like: + // #[arg(long, help = "Verification timeout in seconds")] + // timeout: Option<u64>, + // #[arg(long, help = "Skip verification of specific contracts")] + // skip: Vec<String>, }
21-33: Enhance error handling and user feedbackThe implementation looks good but could benefit from more detailed error handling and progress feedback for users.
Consider applying these improvements:
impl WalnutArgs { pub fn run(self, config: &Config) -> Result<()> { let ws = scarb::ops::read_workspace(config.manifest_path(), config)?; + println!("Reading workspace configuration..."); config.tokio_handle().block_on(async { match self.command { WalnutVerifyCommand::Verify(_options) => { + println!("Starting contract verification..."); WalnutDebugger::verify(&ws).await?; + println!("Verification completed successfully!"); } } Ok(()) - }) + }).map_err(|e| anyhow::anyhow!("Verification failed: {}", e)) } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bin/sozo/src/commands/mod.rs(5 hunks)bin/sozo/src/commands/walnut.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- bin/sozo/src/commands/mod.rs
🔇 Additional comments (1)
bin/sozo/src/commands/walnut.rs (1)
1-5: Ohayo! The imports look well-organized, sensei!
The necessary dependencies are properly imported and follow Rust conventions.
|
Hey @glihm, how is it going? Could you please review the latest changes? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2734 +/- ##
==========================================
- Coverage 56.26% 55.85% -0.42%
==========================================
Files 415 447 +32
Lines 53183 57704 +4521
==========================================
+ Hits 29926 32232 +2306
- Misses 23257 25472 +2215 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Awesome, sorry for the delay I didn't see the modification right after the latest review, thanks for the ping.
Wondering how this crate could be tested locally without the test depending on external resources to be online. But may be not required for this PR.
You may run those scripts to check the missing linting:
bash scripts/clippy.sh
bash scripts/rust_fmt.sh
bin/sozo/src/commands/walnut.rs
Outdated
There was a problem hiding this comment.
Let's move this file into the sozo/walnut crate.
|
|
||
| Ok(()) | ||
| } | ||
| let source_code = _collect_source_code(root_dir)?; |
There was a problem hiding this comment.
Let's remove the leading _ since the functions are now used. Every function that is used must not be prefixed by _.
|
Hey @glihm. I made requested changes. Hope it's good. Let me know pls, thanks for all support! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
crates/sozo/walnut/src/walnut.rs (1)
5-6: Ohayo sensei! Consider fallback or guard forWalnutDebuggerimport.
IfWalnutDebuggerbecomes optional or behind a feature flag, ensure any usage here gracefully detects unavailability.crates/sozo/walnut/src/debugger.rs (1)
47-48: Streamline error messages for verification.
It might be helpful to provide more context in the event of verification failures (e.g., which contract or step triggered the error).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
bin/sozo/src/commands/mod.rs(4 hunks)crates/sozo/walnut/Cargo.toml(1 hunks)crates/sozo/walnut/src/debugger.rs(2 hunks)crates/sozo/walnut/src/lib.rs(1 hunks)crates/sozo/walnut/src/verification.rs(2 hunks)crates/sozo/walnut/src/walnut.rs(1 hunks)
🔇 Additional comments (13)
crates/sozo/walnut/src/walnut.rs (3)
1-2: Ohayo sensei! Good usage ofanyhow::Result.
This approach simplifies error handling and is consistent with the rest of the codebase.
9-10: Ensure subcommands are properly tested.
Unit tests or integration tests would be helpful, confirming that the correct commands are parsed and executed.
25-34: Check error propagation in async block.
When using?inside the block, confirm that your upper layers handle these errors properly for good user feedback.crates/sozo/walnut/src/lib.rs (1)
28-28: Ohayo sensei! Module visibility looks good.
Exposingwalnutat this level integrates well with your multi-file architecture.crates/sozo/walnut/Cargo.toml (1)
21-22: Ohayo sensei! Great job adding theclapdependency.
This aligns with your new command-line functionality. Also, be sure to track potential version conflicts in the workspace.crates/sozo/walnut/src/debugger.rs (1)
28-39: Ohayo sensei! Smart handling ofTransactionResultvariants.
Bailing early onNoopis efficient. If additional variants are introduced in the future, ensure you handle them in the match.crates/sozo/walnut/src/verification.rs (4)
15-22: LGTM! Well-structured payload definition.Ohayo, sensei! The
VerificationPayloadstruct is well-documented and properly encapsulates the necessary data for verification.
29-55: Improve error handling inwalnut_verifyOhayo, sensei! The function should propagate errors from
verify_classesto allow proper error handling by the caller.Apply this diff to propagate errors:
match verify_classes(verification_payload, &api_url).await { Ok(message) => ui.print(_subtitle(message)), - Err(e) => ui.print(_subtitle(e.to_string())), + Err(e) => { + ui.print(_subtitle(e.to_string())); + return Err(e.into()); + } } - - Ok(())
Line range hint
58-67: Ensure secure communication with Walnut backendOhayo, sensei! The API URL should use HTTPS to protect sensitive source code during transmission.
Let's verify the API URL configuration:
#!/bin/bash # Description: Check if the API URL is configured to use HTTPS rg -l "http://" --glob "**/*.rs" --glob "**/*.toml"
Line range hint
69-108: LGTM! Well-implemented source code collection.Ohayo, sensei! The function properly handles file traversal, path manipulation, and error cases.
bin/sozo/src/commands/mod.rs (3)
67-68: LGTM! Command structure aligns with previous discussions.Ohayo, sensei! The Walnut command is well-documented and follows the agreed-upon structure.
87-87: LGTM! Consistent display implementation.Ohayo, sensei! The display string "WalnutVerify" is consistent with the command's purpose.
114-114: LGTM! Proper command execution handling.Ohayo, sensei! The run implementation follows the established pattern for command execution.
There was a problem hiding this comment.
Thanks for the updates! I can't push some modifications, I've sent you some diffs in the review.
Also, when I try to verify something, this is the output I have:
https://app.walnut.dev/verification/status/8044f0c7-ae1d-43cb-8a0b-7566b6427439
Also, could you show an example of working with this command on the dojo_starter example: https://github.com/dojoengine/dojo-starter? Or you may already have this actually?
This could be in a separate PR or even if Walnut documentation, just linking this from the dojo repo.
Happy to merge once the conditional compilation flags are solved. 👍
There was a problem hiding this comment.
Missing conditional compilation flags:
diff --git a/bin/sozo/src/commands/execute.rs b/bin/sozo/src/commands/execute.rs
index a69977a0..fe5e37f6 100644
--- a/bin/sozo/src/commands/execute.rs
+++ b/bin/sozo/src/commands/execute.rs
@@ -5,6 +5,7 @@ use dojo_world::config::calldata_decoder;
use scarb::core::Config;
use sozo_ops::resource_descriptor::ResourceDescriptor;
use sozo_scarbext::WorkspaceExt;
+#[cfg(feature = "walnut")]
use sozo_walnut::WalnutDebugger;
use starknet::core::types::Call;
use starknet::core::utils as snutils;
|
Hey @glihm, just pushed requested changes. Added compilation flags in all places pointed. Regarding your failed verification (https://app.walnut.dev/verification/status/8044f0c7-ae1d-43cb-8a0b-7566b6427439): unfortunately your project is using relative path dependency ( Here is example dojo-starter project which I just verified to check all works: https://app.walnut.dev/verification/status/65fe0291-f0b1-465d-9b71-2aa22c87a6b4 Here you can see a source of this verified project: https://app.walnut.dev/classes/0x02283c68ecba5c60bbbbd3b00659808a02244468e41a1d2cdba1312d65b83594 I hope everything is good now, thanks for your support! |
|
Ouch, didn't check the paths earlier, that makes total sense. Thank you for pointing that out! |
There was a problem hiding this comment.
Thanks for the iterations and the work on that @mobycrypt!
Let's move on and this will be included in the next patch version. 👍
glihm
changed the title
This PR includes:
sozo verifywhich verifies contracts in Walnut. Existing verification workflow during migration is removed.--walnutflag is now enabled.Summary by CodeRabbit
New Features
--walnutflag forsozo migrate applyandsozo executecommands.Walnutcommand to the CLI for enhanced functionality.Improvements
Changes