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 2 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 @@ impl ExecuteArgs {
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(
self.transaction.walnut,
self.starknet.url(profile_config.env.as_ref())?,
);
Expand Down Expand Up @@ -132,9 +132,13 @@ impl ExecuteArgs {
.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)?;
}
glihm marked this conversation as resolved.
Show resolved Hide resolved

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

use build::BuildArgs;
use call::CallArgs;
Expand All @@ -33,6 +34,7 @@ use inspect::InspectArgs;
use migrate::MigrateArgs;
use model::ModelArgs;
use test::TestArgs;
use verify::VerifyArgs;

#[derive(Debug, Subcommand)]
pub enum Commands {
Expand Down Expand Up @@ -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>),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@glihm I've changed it from "Verify" to "WalnutVerify". Please note that after this change calling walnut verification is calling sozo walnut-verify.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

}

impl fmt::Display for Commands {
Expand All @@ -81,6 +85,7 @@ impl fmt::Display for Commands {
Commands::Init(_) => write!(f, "Init"),
Commands::Model(_) => write!(f, "Model"),
Commands::Events(_) => write!(f, "Events"),
Commands::WalnutVerify(_) => write!(f, "WalnutVerify"),
}
}
}
Expand All @@ -107,6 +112,7 @@ pub fn run(command: Commands, config: &Config) -> Result<()> {
Commands::Init(args) => args.run(config),
Commands::Model(args) => args.run(config),
Commands::Events(args) => args.run(config),
Commands::WalnutVerify(args) => args.run(config),
}
}

Expand Down
17 changes: 17 additions & 0 deletions bin/sozo/src/commands/verify.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
use anyhow::Result;
use clap::Args;
use scarb::core::Config;
use sozo_walnut::WalnutDebugger;

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

impl VerifyArgs {
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 also have this one walnut specific. May be into the walnut crate actually. 👍

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(())
})
}
}
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 @@ impl WalnutDebugger {

/// 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))
} else {
None
}
}

/// 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,
TransactionResult::Noop => {
return Ok(());
}
TransactionResult::HashReceipt(transaction_hash, _) => transaction_hash,
};
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
}
}
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::io;
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();

// 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();

// 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();

Ok(())
}
let source_code = _collect_source_code(root_dir)?;
glihm marked this conversation as resolved.
Show resolved Hide resolved
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;

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() };

#[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())),
}

Ok(())
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?;
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