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) : multicall from CLI #2679

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
b384a89
Update execute.rs
Manush-2005 Nov 3, 2024
ae378aa
Few minor changes
Manush-2005 Nov 3, 2024
5751ee7
Keeping vec string
Manush-2005 Nov 4, 2024
8677707
Sperating each call using commas
Manush-2005 Nov 6, 2024
eebf651
Implementing Invoker before for loop and creating sep function for co…
Manush-2005 Nov 8, 2024
1ee5e0a
Using the Fromstr trait for string operations
Manush-2005 Nov 8, 2024
b3ed182
Checking local manifest for contract tag
Manush-2005 Nov 10, 2024
ce9e1b4
dummy commit
Manush-2005 Nov 10, 2024
ef4eee5
Dummy commit
Manush-2005 Nov 11, 2024
6e4a960
Merge conflict solved commit
Manush-2005 Nov 11, 2024
9a4eeb9
Getting contracts the right way and also letting string pass if there…
Manush-2005 Nov 12, 2024
0363f73
Update execute.rs
Manush-2005 Nov 3, 2024
1c9a191
Few minor changes
Manush-2005 Nov 3, 2024
7a10e6c
Keeping vec string
Manush-2005 Nov 4, 2024
58c5560
Sperating each call using commas
Manush-2005 Nov 6, 2024
9dbf346
Implementing Invoker before for loop and creating sep function for co…
Manush-2005 Nov 8, 2024
0bdfc44
Using the Fromstr trait for string operations
Manush-2005 Nov 8, 2024
2264ba1
Checking local manifest for contract tag
Manush-2005 Nov 10, 2024
b421952
dummy commit
Manush-2005 Nov 10, 2024
a3480ff
Dummy commit
Manush-2005 Nov 11, 2024
a7996a2
Merge conflict solved commit
Manush-2005 Nov 11, 2024
0f742d6
Getting contracts the right way and also letting string pass if there…
Manush-2005 Nov 12, 2024
46e4412
Merge branch 'Sozo-mutiple-execute' of https://github.com/Manush-2005…
Manush-2005 Nov 12, 2024
970e489
Problems of using self in the resolve_contract_address function
Manush-2005 Nov 12, 2024
e25f6c7
Fmt and also fixed some errors
Manush-2005 Nov 13, 2024
608bd9f
Some final changes
Manush-2005 Nov 13, 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
209 changes: 125 additions & 84 deletions bin/sozo/src/commands/execute.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
use super::options::account::AccountOptions;
use super::options::starknet::StarknetOptions;
use super::options::transaction::TransactionOptions;
use super::options::world::WorldOptions;
use crate::utils;
use anyhow::{anyhow, Result};
use clap::Args;
use dojo_utils::{Invoker, TxnConfig};
Expand All @@ -8,35 +13,19 @@ use sozo_scarbext::WorkspaceExt;
use sozo_walnut::WalnutDebugger;
use starknet::core::types::Call;
use starknet::core::utils as snutils;
use tracing::trace;

use super::options::account::AccountOptions;
use super::options::starknet::StarknetOptions;
use super::options::transaction::TransactionOptions;
use super::options::world::WorldOptions;
use crate::utils;
use dojo_world::diff::WorldDiff;
use scarb::core::Workspace;
use starknet::core::types::Address;
use tracing::trace;

#[derive(Debug, Args)]
#[command(about = "Execute a system with the given calldata.")]
pub struct ExecuteArgs {
#[arg(
help = "The address or the tag (ex: dojo_examples:actions) of the contract to be executed."
help = "List of calls to execute. Each call should be in format: <CONTRACT_ADDRESS/TAG>,<ENTRYPOINT>,<ARG1>,<ARG2>,... (ex: dojo_examples:actions,execute,1,2)"
)]
pub tag_or_address: ResourceDescriptor,

#[arg(help = "The name of the entrypoint to be executed.")]
pub entrypoint: String,

#[arg(short, long)]
#[arg(help = "The calldata to be passed to the system. Comma separated values e.g., \
0x12345,128,u256:9999999999. Sozo supports some prefixes that you can use to \
automatically parse some types. The supported prefixes are:
- u256: A 256-bit unsigned integer.
- sstr: A cairo short string.
- str: A cairo string (ByteArray).
- int: A signed integer.
- no prefix: A cairo felt or any type that fit into one felt.")]
pub calldata: Option<String>,
pub calls: Vec<String>,

#[arg(long)]
#[arg(help = "If true, sozo will compute the diff of the world from the chain to translate \
Expand All @@ -56,6 +45,69 @@ pub struct ExecuteArgs {
pub transaction: TransactionOptions,
}

#[derive(Debug)]
pub struct CallArgs {
pub tag_or_address: ResourceDescriptor, // Contract address or tag
pub entrypoint: String, // Entrypoint to call
pub calldata: Option<String>, // Calldata to pass to the entrypoint
}

impl std::str::FromStr for CallArgs {
type Err = anyhow::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let s = s.trim();

let parts: Vec<&str> = s.split(',').collect();
if parts.len() < 2 {
return Err(anyhow!("Invalid call format. Expected format: <CONTRACT_NAME>,<ENTRYPOINT_NAME>,<ARG1>,<ARG2>,..."));
}

let entrypoint = parts[1].trim();
if entrypoint.is_empty() {
Comment on lines +55 to +67
Copy link
Member

Choose a reason for hiding this comment

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

please write a unit test for this function.

Copy link
Author

Choose a reason for hiding this comment

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

Hey @kariy, can you tell me the steps to write a unit test for this function? I am kinda new to this.

Copy link
Author

Choose a reason for hiding this comment

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

Hey @glihm , I am actually kinda getting an error running the test
This errors are like this:
Compiling zstd v0.13.2 Compiling nextest-runner v0.65.0 Finishedreleaseprofile [optimized] target(s) in 2m 27s Installing /home/manush2005/.cargo/bin/cargo-nextest Installed packagecargo-nextest v0.9.82(executablecargo-nextest) manush2005@Manush:/mnt/c/Users/Manush/OneDrive/Desktop/Manush/dojo$ cargo nextest run --all-features -p sozo Updating crates.io index Updating git repository https://github.com/dojoengine/dojo`
error: failed to get dojo-lang as a dependency of package scarb v2.8.4 (https://github.com/dojoengine/scarb?rev=7eac49b3e61236ce466e712225d9c989f9db1ef3#7eac49b3)
... which satisfies git dependency scarb (locked to 2.8.4) of package dojo-test-utils v1.0.0 (/mnt/c/Users/Manush/OneDrive/Desktop/Manush/dojo/crates/dojo/test-utils)
... which satisfies path dependency dojo-test-utils (locked to 1.0.0) of package dojo-utils v1.0.0 (/mnt/c/Users/Manush/OneDrive/Desktop/Manush/dojo/crates/dojo/utils)
... which satisfies path dependency dojo-utils (locked to 1.0.0) of package katana v1.0.0 (/mnt/c/Users/Manush/OneDrive/Desktop/Manush/dojo/bin/katana)

Caused by:
failed to load source for dependency dojo-lang

Caused by:
Unable to update https://github.com/dojoengine/dojo?rev=6725a8f20af56213fa7382aa1e158817f3ee623c#6725a8f2

Caused by:
failed to fetch into: /home/manush2005/.cargo/git/db/dojo-10cac2e09298cf35

Caused by:
network failure seems to have happened
if a proxy or similar is necessary net.git-fetch-with-cli may help here
https://doc.rust-lang.org/cargo/reference/config.html#netgit-fetch-with-cli

Caused by:
SSL error: unknown error; class=Ssl (16)`

Copy link
Author

Choose a reason for hiding this comment

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

Also , @glihm and @kariy , can you please highlight some changes I need to make in the current code. It is diffcult for me to grasp as it is my frist time contributing to dojo

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Manush-2005 how are you trying to compile the code locally?

You should follow the CONTRIBUTING.md guide to help you out.
If you have any question, you're welcome in the dojo discord and happy to have a call to show you the flow if you still have complications to build.

return Err(anyhow!("Empty entrypoint"));
}
if !entrypoint.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') {
return Err(anyhow!("Invalid entrypoint format. Must contain only alphanumeric characters and underscores"));
}

Ok(CallArgs {
tag_or_address: parts[0].parse()?,
entrypoint: entrypoint.to_string(),
calldata: if parts.len() > 2 { Some(parts[2..].join(",")) } else { None },
Comment on lines +75 to +77
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Improve calldata parsing to handle commas within arguments.

Ohayo, sensei! The current parsing logic joins calldata arguments with commas, which may cause issues if an argument itself contains a comma. Consider enhancing the parsing mechanism to handle such cases, possibly by using a different delimiter or supporting argument escaping.

})
}
}
Comment on lines +55 to +80
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance the parsing logic for better robustness.

Ohayo, sensei! The current parsing implementation has some potential issues:

  1. It doesn't handle escaped commas in arguments
  2. Missing validation for empty tag/address
  3. Individual parts after split aren't trimmed
 fn from_str(s: &str) -> Result<Self, Self::Err> {
     let s = s.trim();
+    if s.is_empty() {
+        return Err(anyhow!("Empty call string"));
+    }
 
-    let parts: Vec<&str> = s.split(',').collect();
+    // Handle escaped commas
+    let mut parts = Vec::new();
+    let mut current = String::new();
+    let mut escaped = false;
+    
+    for c in s.chars() {
+        match (c, escaped) {
+            ('\\', false) => escaped = true,
+            (',', false) => {
+                parts.push(current.trim().to_string());
+                current.clear();
+            },
+            (c, _) => {
+                current.push(c);
+                escaped = false;
+            }
+        }
+    }
+    parts.push(current.trim().to_string());
+
     if parts.len() < 2 {
         return Err(anyhow!("Invalid call format. Expected format: <CONTRACT_NAME>,<ENTRYPOINT_NAME>,<ARG1>,<ARG2>,..."));
     }
 
-    let entrypoint = parts[1].trim();
+    let tag_or_address = parts[0].trim();
+    if tag_or_address.is_empty() {
+        return Err(anyhow!("Empty contract address/tag"));
+    }
+
+    let entrypoint = parts[1].trim();
     if entrypoint.is_empty() {
         return Err(anyhow!("Empty entrypoint"));
     }
📝 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
impl std::str::FromStr for CallArgs {
type Err = anyhow::Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
let s = s.trim();
let parts: Vec<&str> = s.split(',').collect();
if parts.len() < 2 {
return Err(anyhow!("Invalid call format. Expected format: <CONTRACT_NAME>,<ENTRYPOINT_NAME>,<ARG1>,<ARG2>,..."));
}
let entrypoint = parts[1].trim();
if entrypoint.is_empty() {
return Err(anyhow!("Empty entrypoint"));
}
if !entrypoint.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') {
return Err(anyhow!("Invalid entrypoint format. Must contain only alphanumeric characters and underscores"));
}
Ok(CallArgs {
tag_or_address: parts[0].parse()?,
entrypoint: entrypoint.to_string(),
calldata: if parts.len() > 2 { Some(parts[2..].join(",")) } else { None },
})
}
}
impl std::str::FromStr for CallArgs {
type Err = anyhow::Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
let s = s.trim();
if s.is_empty() {
return Err(anyhow!("Empty call string"));
}
// Handle escaped commas
let mut parts = Vec::new();
let mut current = String::new();
let mut escaped = false;
for c in s.chars() {
match (c, escaped) {
('\\', false) => escaped = true,
(',', false) => {
parts.push(current.trim().to_string());
current.clear();
},
(c, _) => {
current.push(c);
escaped = false;
}
}
}
parts.push(current.trim().to_string());
if parts.len() < 2 {
return Err(anyhow!("Invalid call format. Expected format: <CONTRACT_NAME>,<ENTRYPOINT_NAME>,<ARG1>,<ARG2>,..."));
}
let tag_or_address = parts[0].trim();
if tag_or_address.is_empty() {
return Err(anyhow!("Empty contract address/tag"));
}
let entrypoint = parts[1].trim();
if entrypoint.is_empty() {
return Err(anyhow!("Empty entrypoint"));
}
if !entrypoint.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') {
return Err(anyhow!("Invalid entrypoint format. Must contain only alphanumeric characters and underscores"));
}
Ok(CallArgs {
tag_or_address: parts[0].parse()?,
entrypoint: entrypoint.to_string(),
calldata: if parts.len() > 2 { Some(parts[2..].join(",")) } else { None },
})
}
}


async fn resolve_contract_address(
descriptor: &ResourceDescriptor,
world_diff: &WorldDiff,
options: &ExecuteArgs,
ws: &Workspace,
) -> Result<Address> {
match descriptor {
ResourceDescriptor::Address(address) => Ok(*address),
ResourceDescriptor::Tag(tag) => {
let contracts = utils::contracts_from_manifest_or_diff(
options.account.clone(),
options.starknet.clone(),
options.world,
&ws,
options.diff,
)
.await?;

contracts
.get(tag)
.map(|c| c.address)
.ok_or_else(|| anyhow!("Contract {descriptor} not found in the world diff."))
}
ResourceDescriptor::Name(_) => {
unimplemented!("Expected to be a resolved tag with default namespace.")
}
Comment on lines +105 to +107
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Implement the Name variant resolution.

Ohayo, sensei! The Name variant is currently unimplemented, which could cause runtime panics.

-        ResourceDescriptor::Name(_) => {
-            unimplemented!("Expected to be a resolved tag with default namespace.")
-        }
+        ResourceDescriptor::Name(name) => {
+            let tag = format!("{}:{}", profile_config.namespace.default, name);
+            resolve_contract_address(
+                &ResourceDescriptor::Tag(tag),
+                world_diff,
+                options,
+                ws
+            ).await
+        }

Committable suggestion skipped: line range outside the PR's diff.

}
}

impl ExecuteArgs {
pub fn run(self, config: &Config) -> Result<()> {
trace!(args = ?self);
Expand All @@ -64,8 +116,6 @@ impl ExecuteArgs {

let profile_config = ws.load_profile_config()?;

let descriptor = self.tag_or_address.ensure_namespace(&profile_config.namespace.default);

#[cfg(feature = "walnut")]
let _walnut_debugger = WalnutDebugger::new_from_flag(
self.transaction.walnut,
Expand All @@ -75,66 +125,57 @@ impl ExecuteArgs {
let txn_config: TxnConfig = self.transaction.try_into()?;

config.tokio_handle().block_on(async {
let (contract_address, contracts) = match &descriptor {
ResourceDescriptor::Address(address) => (Some(*address), Default::default()),
ResourceDescriptor::Tag(tag) => {
let contracts = utils::contracts_from_manifest_or_diff(
self.account.clone(),
self.starknet.clone(),
self.world,
&ws,
self.diff,
)
.await?;

(contracts.get(tag).map(|c| c.address), contracts)
}
ResourceDescriptor::Name(_) => {
unimplemented!("Expected to be a resolved tag with default namespace.")
}
};

let contract_address = contract_address.ok_or_else(|| {
let mut message = format!("Contract {descriptor} not found in the manifest.");
if self.diff {
message.push_str(
" Run the command again with `--diff` to force the fetch of data from the \
chain.",
);
}
anyhow!(message)
})?;

trace!(
contract=?descriptor,
entrypoint=self.entrypoint,
calldata=?self.calldata,
"Executing Execute command."
);

let calldata = if let Some(cd) = self.calldata {
calldata_decoder::decode_calldata(&cd)?
} else {
vec![]
};

let call = Call {
calldata,
to: contract_address,
selector: snutils::get_selector_from_name(&self.entrypoint)?,
};

let (provider, _) = self.starknet.provider(profile_config.env.as_ref())?;

let account = self
.account
.account(provider, profile_config.env.as_ref(), &self.starknet, &contracts)
.await?;

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

// We could save the world diff computation extracting the account directly from the options.
let (world_diff, account, _) = utils::get_world_diff_and_account(
self.account,
self.starknet.clone(),
self.world,
&ws,
&mut None,
)
.await?;

let mut invoker = Invoker::new(&account, txn_config);

// Parse the Vec<String> into Vec<CallArgs> using FromStr
let call_args_list: Vec<CallArgs> =
self.calls.iter().map(|s| s.parse()).collect::<Result<Vec<_>>>()?;

for call_args in call_args_list {
let descriptor =
call_args.tag_or_address.ensure_namespace(&profile_config.namespace.default);

// Checking the contract tag in local manifest
let contract_address =
if let Some(local_address) = ws.get_contract_address(&descriptor) {
local_address
} else {
resolve_contract_address(&descriptor, &world_diff, &ws).await?;
};
Comment on lines +153 to +154
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect arguments in resolve_contract_address call.

Ohayo, sensei! The resolve_contract_address call is missing the options argument.

-                        resolve_contract_address(&descriptor, &world_diff, &ws).await?;
+                        resolve_contract_address(&descriptor, &world_diff, &self, &ws).await?;
📝 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
resolve_contract_address(&descriptor, &world_diff, &ws).await?;
};
resolve_contract_address(&descriptor, &world_diff, &self, &ws).await?;
};


trace!(
contract=?descriptor,
entrypoint=call_args.entrypoint,
calldata=?call_args.calldata,
"Executing Execute command."
);

let calldata = if let Some(cd) = call_args.calldata {
calldata_decoder::decode_calldata(&cd)?
} else {
vec![]
};

let call = Call {
calldata,
to: contract_address,
selector: snutils::get_selector_from_name(&call_args.entrypoint)?,
};

invoker.add_call(call); // Adding each call to the Invoker
}

let tx_result = invoker.invoke(call).await?; // Invoking the multi-call
println!("{}", tx_result);
Ok(())
})
Expand Down
Loading