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.

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

feat(sozo): introduce walnut verify command + execute "--walnut" flag enabled #2734

Merged
merged 5 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions bin/sozo/src/commands/execute.rs
Copy link
Collaborator

Choose a reason for hiding this comment

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

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;

Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
let descriptor = self.tag_or_address.ensure_namespace(&profile_config.namespace.default);

#[cfg(feature = "walnut")]
let _walnut_debugger = WalnutDebugger::new_from_flag(
let walnut_debugger = WalnutDebugger::new_from_flag(

Check warning on line 70 in bin/sozo/src/commands/execute.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/execute.rs#L70

Added line #L70 was not covered by tests
self.transaction.walnut,
self.starknet.url(profile_config.env.as_ref())?,
);
Expand Down Expand Up @@ -132,9 +132,13 @@
.await?;

let invoker = Invoker::new(&account, txn_config);
// TODO: add walnut back, perhaps at the invoker level.
let tx_result = invoker.invoke(call).await?;

#[cfg(feature = "walnut")]
if let Some(walnut_debugger) = walnut_debugger {
walnut_debugger.debug_transaction(&config.ui(), &tx_result)?;
}

Check warning on line 140 in bin/sozo/src/commands/execute.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/execute.rs#L138-L140

Added lines #L138 - L140 were not covered by tests

println!("{}", tx_result);
Ok(())
})
Expand Down
6 changes: 6 additions & 0 deletions bin/sozo/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
pub(crate) mod model;
pub(crate) mod options;
pub(crate) mod test;
pub(crate) mod walnut;

use build::BuildArgs;
use call::CallArgs;
Expand All @@ -33,6 +34,7 @@
use migrate::MigrateArgs;
use model::ModelArgs;
use test::TestArgs;
use walnut::WalnutArgs;

#[derive(Debug, Subcommand)]
pub enum Commands {
Expand Down Expand Up @@ -63,6 +65,8 @@
Model(Box<ModelArgs>),
#[command(about = "Inspect events emitted by the world")]
Events(Box<EventsArgs>),
#[command(about = "Interact with walnut.dev - transactions debugger and simulator")]
Walnut(Box<WalnutArgs>),
}

impl fmt::Display for Commands {
Expand All @@ -81,6 +85,7 @@
Commands::Init(_) => write!(f, "Init"),
Commands::Model(_) => write!(f, "Model"),
Commands::Events(_) => write!(f, "Events"),
Commands::Walnut(_) => write!(f, "WalnutVerify"),

Check warning on line 88 in bin/sozo/src/commands/mod.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/mod.rs#L88

Added line #L88 was not covered by tests
}
}
}
Expand All @@ -107,6 +112,7 @@
Commands::Init(args) => args.run(config),
Commands::Model(args) => args.run(config),
Commands::Events(args) => args.run(config),
Commands::Walnut(args) => args.run(config),

Check warning on line 115 in bin/sozo/src/commands/mod.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/mod.rs#L115

Added line #L115 was not covered by tests
}
}

Expand Down
33 changes: 33 additions & 0 deletions bin/sozo/src/commands/walnut.rs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's move this file into the sozo/walnut crate.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use anyhow::Result;
use clap::{Args, Subcommand};
use scarb::core::Config;
use sozo_walnut::WalnutDebugger;

#[derive(Debug, Args)]
pub struct WalnutArgs {
#[command(subcommand)]
pub command: WalnutVerifyCommand,
}

#[derive(Debug, Subcommand)]
pub enum WalnutVerifyCommand {
#[command(about = "Verify contracts in walnut.dev - essential for debugging source code in Walnut")]
Verify(WalnutVerifyOptions),
}

#[derive(Debug, Args)]
pub struct WalnutVerifyOptions {}

impl WalnutArgs {
pub fn run(self, config: &Config) -> Result<()> {
let ws = scarb::ops::read_workspace(config.manifest_path(), config)?;
config.tokio_handle().block_on(async {
match self.command {
WalnutVerifyCommand::Verify(_options) => {
WalnutDebugger::verify(&ws).await?;

Check warning on line 27 in bin/sozo/src/commands/walnut.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/walnut.rs#L22-L27

Added lines #L22 - L27 were not covered by tests
}
}
Ok(())
})
}

Check warning on line 32 in bin/sozo/src/commands/walnut.rs

View check run for this annotation

Codecov / codecov/patch

bin/sozo/src/commands/walnut.rs#L30-L32

Added lines #L30 - L32 were not covered by tests
}
1 change: 1 addition & 0 deletions crates/sozo/walnut/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ thiserror.workspace = true
url.workspace = true
urlencoding = "2.1.3"
walkdir.workspace = true
dojo-utils.workspace = true

[dev-dependencies]
starknet.workspace = true
37 changes: 21 additions & 16 deletions crates/sozo/walnut/src/debugger.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use dojo_world::diff::WorldDiff;
use dojo_utils::TransactionResult;
use scarb::core::Workspace;
use scarb_ui::Ui;
use starknet::core::types::Felt;
use url::Url;

use crate::transaction::walnut_debug_transaction;
use crate::verification::walnut_verify_migration_strategy;
use crate::verification::walnut_verify;
use crate::{utils, Error};

/// A debugger for Starknet transactions embedding the walnut configuration.
Expand All @@ -22,29 +22,34 @@

/// Creates a new Walnut debugger if the `use_walnut` flag is set.
pub fn new_from_flag(use_walnut: bool, rpc_url: Url) -> Option<Self> {
if use_walnut { Some(Self::new(rpc_url)) } else { None }
if use_walnut {
Some(Self::new(rpc_url))

Check warning on line 26 in crates/sozo/walnut/src/debugger.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/debugger.rs#L25-L26

Added lines #L25 - L26 were not covered by tests
} else {
None

Check warning on line 28 in crates/sozo/walnut/src/debugger.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/debugger.rs#L28

Added line #L28 was not covered by tests
}
}

/// Debugs a transaction with Walnut by printing a link to the Walnut debugger page.
pub fn debug_transaction(&self, ui: &Ui, transaction_hash: &Felt) -> Result<(), Error> {
pub fn debug_transaction(
&self,
ui: &Ui,
transaction_result: &TransactionResult,
) -> Result<(), Error> {
let transaction_hash = match transaction_result {
TransactionResult::Hash(transaction_hash) => transaction_hash,

Check warning on line 39 in crates/sozo/walnut/src/debugger.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/debugger.rs#L33-L39

Added lines #L33 - L39 were not covered by tests
TransactionResult::Noop => {
return Ok(());

Check warning on line 41 in crates/sozo/walnut/src/debugger.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/debugger.rs#L41

Added line #L41 was not covered by tests
}
TransactionResult::HashReceipt(transaction_hash, _) => transaction_hash,

Check warning on line 43 in crates/sozo/walnut/src/debugger.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/debugger.rs#L43

Added line #L43 was not covered by tests
};
let url = walnut_debug_transaction(&self.rpc_url, transaction_hash)?;
ui.print(format!("Debug transaction with Walnut: {url}"));
Ok(())
}

/// Verifies a migration strategy with Walnut by uploading the source code of the contracts and
/// models in the strategy.
pub async fn verify_migration_strategy(
&self,
ws: &Workspace<'_>,
world_diff: &WorldDiff,
) -> anyhow::Result<()> {
walnut_verify_migration_strategy(ws, self.rpc_url.to_string(), world_diff).await
}

/// Checks if the Walnut API key is set.
pub fn check_api_key() -> Result<(), Error> {
let _ = utils::walnut_get_api_key()?;
Ok(())
pub async fn verify(ws: &Workspace<'_>) -> anyhow::Result<()> {
walnut_verify(ws).await

Check warning on line 53 in crates/sozo/walnut/src/debugger.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/debugger.rs#L52-L53

Added lines #L52 - L53 were not covered by tests
}
}
6 changes: 1 addition & 5 deletions crates/sozo/walnut/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
use std::env;

use crate::{Error, WALNUT_API_KEY_ENV_VAR, WALNUT_API_URL, WALNUT_API_URL_ENV_VAR};

pub fn walnut_get_api_key() -> Result<String, Error> {
env::var(WALNUT_API_KEY_ENV_VAR).map_err(|_| Error::MissingApiKey)
}
use crate::{WALNUT_API_URL, WALNUT_API_URL_ENV_VAR};

pub fn walnut_get_api_url() -> String {
env::var(WALNUT_API_URL_ENV_VAR).unwrap_or_else(|_| WALNUT_API_URL.to_string())
Expand Down
117 changes: 32 additions & 85 deletions crates/sozo/walnut/src/verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,114 +3,61 @@
use std::path::Path;

use console::{pad_str, Alignment, Style, StyledObject};
use dojo_world::diff::{ResourceDiff, WorldDiff};
use dojo_world::local::ResourceLocal;
use dojo_world::remote::ResourceRemote;
use dojo_world::ResourceType;
use reqwest::StatusCode;
use scarb::core::Workspace;
use serde::Serialize;
use serde_json::Value;
use sozo_scarbext::WorkspaceExt;
use walkdir::WalkDir;

use crate::utils::{walnut_get_api_key, walnut_get_api_url};
use crate::utils::walnut_get_api_url;
use crate::Error;

/// Verifies all classes declared during migration.
/// Only supported on hosted networks (non-localhost).
///
/// This function verifies all contracts and models in the strategy. For every contract and model,
/// it sends a request to the Walnut backend with the class name, class hash, RPC URL, and source
/// code. Walnut will then build the project with Sozo, compare the Sierra bytecode with the
/// bytecode on the network, and if they are equal, it will store the source code and associate it
/// with the class hash.
pub async fn walnut_verify_migration_strategy(
ws: &Workspace<'_>,
rpc_url: String,
world_diff: &WorldDiff,
) -> anyhow::Result<()> {
let ui = ws.config().ui();
// Check if rpc_url is localhost
if rpc_url.contains("localhost") || rpc_url.contains("127.0.0.1") {
ui.print(" ");
ui.warn("Verifying classes with Walnut is only supported on hosted networks.");
ui.print(" ");
return Ok(());
}

// Check if there are any contracts or models in the strategy
if world_diff.is_synced() {
ui.print(" ");
ui.print("🌰 No contracts or models to verify.");
ui.print(" ");
return Ok(());
}
#[derive(Debug, Serialize)]
struct VerificationPayload {
/// JSON that contains a map where the key is the path to the file and the value is the content
/// of the file. It should contain all files required to build the Dojo project with Sozo.
pub source_code: Value,

let _profile_config = ws.load_profile_config()?;
pub cairo_version: String,
}

for (_selector, resource) in world_diff.resources.iter() {
if resource.resource_type() == ResourceType::Contract {
match resource {
ResourceDiff::Created(ResourceLocal::Contract(_contract)) => {
// Need to verify created.
}
ResourceDiff::Updated(_, ResourceRemote::Contract(_contract)) => {
// Need to verify updated.
}
_ => {
// Synced, we don't need to verify.
}
}
}
}
/// Verifies all classes in the workspace.
///
/// This function verifies all contracts and models in the workspace. It sends a single request to
/// the Walnut backend with the source code. Walnut will then build the project and store
/// the source code associated with the class hashes.
pub async fn walnut_verify(ws: &Workspace<'_>) -> anyhow::Result<()> {
let ui = ws.config().ui();

Check warning on line 30 in crates/sozo/walnut/src/verification.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/verification.rs#L29-L30

Added lines #L29 - L30 were not covered by tests

// Notify start of verification
ui.print(" ");
ui.print("🌰 Verifying classes with Walnut...");
ui.print(" ");

// Retrieve the API key and URL from environment variables
let _api_key = walnut_get_api_key()?;
let _api_url = walnut_get_api_url();
let api_url = walnut_get_api_url();

Check warning on line 38 in crates/sozo/walnut/src/verification.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/verification.rs#L38

Added line #L38 was not covered by tests

// Collect source code
// TODO: now it's the same output as scarb, need to update the dojo fork to output the source
// code, or does scarb supports it already?
// its path to a file so `parent` should never return `None`
let root_dir: &Path = ws.manifest_path().parent().unwrap().as_std_path();

Check warning on line 41 in crates/sozo/walnut/src/verification.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/verification.rs#L40-L41

Added lines #L40 - L41 were not covered by tests

Ok(())
}
let source_code = _collect_source_code(root_dir)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove the leading _ since the functions are now used. Every function that is used must not be prefixed by _.

let cairo_version = scarb::version::get().version;

Check warning on line 44 in crates/sozo/walnut/src/verification.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/verification.rs#L43-L44

Added lines #L43 - L44 were not covered by tests

fn _get_class_name_from_artifact_path(path: &Path, namespace: &str) -> Result<String, Error> {
let file_name = path.file_stem().and_then(OsStr::to_str).ok_or(Error::InvalidFileName)?;
let class_name = file_name.strip_prefix(namespace).ok_or(Error::NamespacePrefixNotFound)?;
Ok(class_name.to_string())
}
let verification_payload =
VerificationPayload { source_code, cairo_version: cairo_version.to_string() };

Check warning on line 47 in crates/sozo/walnut/src/verification.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/verification.rs#L46-L47

Added lines #L46 - L47 were not covered by tests

#[derive(Debug, Serialize)]
struct _VerificationPayload {
/// The names of the classes we want to verify together with the selector.
pub class_names: Vec<String>,
/// The hashes of the Sierra classes.
pub class_hashes: Vec<String>,
/// The RPC URL of the network where these classes are declared (can only be a hosted network).
pub rpc_url: String,
/// JSON that contains a map where the key is the path to the file and the value is the content
/// of the file. It should contain all files required to build the Dojo project with Sozo.
pub source_code: Value,
// Send verification request
match verify_classes(verification_payload, &api_url).await {
Ok(message) => ui.print(_subtitle(message)),
Err(e) => ui.print(_subtitle(e.to_string())),

Check warning on line 52 in crates/sozo/walnut/src/verification.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/verification.rs#L49-L52

Added lines #L49 - L52 were not covered by tests
}

Ok(())

Check warning on line 55 in crates/sozo/walnut/src/verification.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/verification.rs#L55

Added line #L55 was not covered by tests
Comment on lines +50 to +55
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.

Suggested change
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(())

}

async fn _verify_classes(
payload: _VerificationPayload,
api_url: &str,
api_key: &str,
) -> Result<String, Error> {
let res = reqwest::Client::new()
.post(format!("{api_url}/v1/verify"))
.header("x-api-key", api_key)
.json(&payload)
.send()
.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?;

Check warning on line 60 in crates/sozo/walnut/src/verification.rs

View check run for this annotation

Codecov / codecov/patch

crates/sozo/walnut/src/verification.rs#L58-L60

Added lines #L58 - L60 were not covered by tests
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

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.


if res.status() == StatusCode::OK {
Ok(res.text().await?)
Expand Down
Loading