diff --git a/bin/miden-cli/src/commands/call.rs b/bin/miden-cli/src/commands/call.rs index a5dc8f1b65..5dbd86b4b0 100644 --- a/bin/miden-cli/src/commands/call.rs +++ b/bin/miden-cli/src/commands/call.rs @@ -3,9 +3,17 @@ use std::fmt::Write as _; use std::path::{Path, PathBuf}; use clap::Parser; +use miden_client::account::AccountId; use miden_client::assembly::CodeBuilder; use miden_client::keystore::Keystore; -use miden_client::transaction::{AdviceInputs, TransactionRequestBuilder, TransactionScript}; +use miden_client::rpc::domain::account::AccountStorageRequirements; +use miden_client::transaction::{ + AdviceInputs, + ForeignAccount, + TransactionRequestBuilder, + TransactionRequestError, + TransactionScript, +}; use miden_client::vm::{Package, PackageExport}; use miden_client::{Client, Deserializable, Felt, Word}; @@ -26,9 +34,10 @@ pub struct CallCmd { #[arg(value_name = "args")] args: Vec, - /// Path to the package (.masp) file containing the procedure. + /// Path to the package (.masp) file containing the procedure. If omitted, `` must + /// be a hex digest and the output stack is shown as raw felts. #[arg(long, short)] - package: PathBuf, + package: Option, } impl CallCmd { @@ -49,79 +58,212 @@ impl CallCmd { )) })?; - let account_id = parse_account_id(&client, account_str).await?; - client.try_get_account(account_id).await?; - - let package = load_package(&self.package)?; - - let digest = resolve_procedure_digest(&package, procedure)?; - let ProcedureSignature { param_count, result_count } = - print_manifest_signature(&package, procedure); - + let target_id = parse_account_id(&client, account_str).await?; + let call_target = resolve_call_target(&client, target_id).await?; let args = parse_args(&self.args)?; + let call_code = self.resolve_call_code(&client, procedure, &args)?; - match param_count { - Some(expected) if args.len() != expected => { - return Err(CliError::InvalidArgument(format!( - "Procedure '{procedure}' expects {expected} argument(s), got {}.", - args.len() - ))); - }, - None => { - println!( - "Warning: no type info for procedure '{procedure}'. Skipping argument \ - count check. Passing a wrong number of arguments may cause errors or \ - wrong results." - ); - }, - _ => {}, + if call_target.is_remote() { + run_remote_call(&mut client, &call_target, target_id, call_code, &args).await + } else { + run_local_call(&mut client, call_target.executor, call_code, &args).await } + } - // The account's code is loaded into the from the client's store in th VM runtime, so we - // don't need the library into the compiled script. But the assembler still needs - // it at compile time to resolve `call.` to a known procedure — otherwise it - // emits a "phantom target" warning. Dynamic linking provides that resolution without - // embedding the library bytes in the script. - let linked_builder = - client.code_builder().with_dynamically_linked_library(package.mast.as_ref())?; - - // 1) Read-only execution to get return values. If `result_count` is unknown we skip - // the drop sequence and let `print_output_stack` auto-detect results from the stack. - let read_tx_script = - generate_tx_script(linked_builder.clone(), &digest, &args, result_count)?; - - let output_stack = client - .execute_program(account_id, read_tx_script, AdviceInputs::default(), BTreeMap::new()) - .await?; - - print_executed_program_stack(&output_stack, result_count); - - // 2) Transaction execution to get state delta. - let delta_tx_script = generate_tx_script(linked_builder, &digest, &args, Some(0))?; - - let tx_request = TransactionRequestBuilder::new() - .custom_script(delta_tx_script) - .build() - .map_err(|err| { - CliError::Transaction(err.into(), "Failed to build transaction".to_string()) + /// Resolves the procedure digest and code builder either from `--package` (calling by name) + /// or from a hex digest when no package is given. + fn resolve_call_code( + &self, + client: &Client, + procedure: &str, + args: &[Felt], + ) -> Result { + if let Some(pkg_path) = &self.package { + let package = load_package(pkg_path)?; + let digest = resolve_procedure_digest(&package, procedure)?; + let ProcedureSignature { param_count, result_count } = + print_manifest_signature(&package, procedure); + + match param_count { + Some(expected) if args.len() != expected => { + return Err(CliError::InvalidArgument(format!( + "Procedure '{procedure}' expects {expected} argument(s), got {}.", + args.len() + ))); + }, + None => { + println!( + "Warning: no type info for procedure '{procedure}'. Skipping \ + argument count check. Passing a wrong number of arguments may \ + cause errors or wrong results." + ); + }, + _ => {}, + } + + // Dynamic linking lets the assembler resolve `call.` without embedding + // the library bytes in the script. + let builder = + client.code_builder().with_dynamically_linked_library(package.mast.as_ref())?; + Ok(CallCode { builder, digest, result_count }) + } else { + let digest = Word::try_from(procedure).map_err(|_| { + CliError::InvalidArgument(format!( + "'{procedure}' is not a hex digest. Pass `--package .masp` to \ + call a procedure by name, or give its hex digest to call without a \ + package." + )) })?; - - match client.execute_transaction(account_id, tx_request).await { - Ok(tx_result) => { - print_executed_transaction(tx_result.executed_transaction())?; - }, - Err(e) => { - println!("\n(Could not compute state delta: {e})"); - }, + println!( + "No `--package` provided; output will be raw felts. Pass \ + `--package .masp` for typed output." + ); + Ok(CallCode { + builder: client.code_builder(), + digest, + result_count: None, + }) } - - Ok(()) } } // HELPERS // ================================================================================================ +/// Resolved call code: the linked builder, the procedure digest, and the result count when known. +struct CallCode { + builder: CodeBuilder, + digest: Word, + result_count: Option, +} + +/// Runs a remote call via FPI. FPI cannot mutate the foreign account, so there is no state delta +/// to compute — only the read phase runs. +async fn run_remote_call( + client: &mut Client, + call_target: &CallTarget, + target_id: AccountId, + call_code: CallCode, + args: &[Felt], +) -> Result<(), CliError> { + let CallCode { builder, digest, result_count } = call_code; + let tx_script = generate_fpi_tx_script(builder, target_id, &digest, args)?; + + let output_stack = client + .execute_program( + call_target.executor, + tx_script, + AdviceInputs::default(), + call_target.foreign_accounts.clone(), + ) + .await?; + + print_executed_program_stack(&output_stack, result_count); + println!("\nRemote calls are read-only; no state delta."); + Ok(()) +} + +/// Runs a local call: a read phase for the return values, then a transaction for the state delta. +/// The executor is the target account itself, so the procedure may mutate it. +async fn run_local_call( + client: &mut Client, + executor: AccountId, + call_code: CallCode, + args: &[Felt], +) -> Result<(), CliError> { + let CallCode { builder, digest, result_count } = call_code; + let read_tx_script = generate_tx_script(builder.clone(), &digest, args, result_count)?; + let delta_tx_script = generate_tx_script(builder, &digest, args, Some(0))?; + + // 1) Read-only execution to get return values. + let output_stack = client + .execute_program(executor, read_tx_script, AdviceInputs::default(), BTreeMap::new()) + .await?; + print_executed_program_stack(&output_stack, result_count); + + // 2) Transaction execution to get the state delta. + let tx_request = TransactionRequestBuilder::new() + .custom_script(delta_tx_script) + .build() + .map_err(|err| { + CliError::Transaction(err.into(), "Failed to build transaction".to_string()) + })?; + + match client.execute_transaction(executor, tx_request).await { + Ok(tx_result) => print_executed_transaction(tx_result.executed_transaction())?, + Err(e) => println!("\n(Could not compute state delta: {e})"), + } + Ok(()) +} + +/// Resolved call target. Local accounts run themselves; remote accounts are read via FPI +/// using a local account as executor. +struct CallTarget { + executor: AccountId, + foreign_accounts: BTreeMap, +} + +impl CallTarget { + fn is_remote(&self) -> bool { + !self.foreign_accounts.is_empty() + } +} + +async fn resolve_call_target( + client: &Client, + target_id: AccountId, +) -> Result { + if client.get_account(target_id).await?.is_some() { + return Ok(CallTarget { + executor: target_id, + foreign_accounts: BTreeMap::new(), + }); + } + + let executor = pick_local_executor(client).await?; + + let foreign_account = ForeignAccount::public(target_id, AccountStorageRequirements::default()) + .map_err(|err| match err { + TransactionRequestError::InvalidForeignAccountId(_) => { + CliError::InvalidArgument(format!( + "Account {target_id} is not in the local store and is not a public account; \ + remote calls require an account with public state." + )) + }, + other => CliError::InvalidArgument(format!( + "Failed to construct foreign account for {target_id}: {other}" + )), + })?; + + println!( + "Account {target_id} not found locally; reading from network via FPI \ + (executor: {executor})." + ); + + Ok(CallTarget { + executor, + foreign_accounts: BTreeMap::from([(target_id, foreign_account)]), + }) +} + +/// Picks the first local regular account to use as the FPI executor. +async fn pick_local_executor( + client: &Client, +) -> Result { + let headers = client.get_account_headers().await?; + headers + .into_iter() + .map(|(header, _status)| header.id()) + .find(|id| id.account_type().is_regular_account()) + .ok_or_else(|| { + CliError::InvalidArgument( + "No local regular account found to make the remote call from. Create one with \ + `miden-client new-wallet`/`new-account`) and re-run." + .to_string(), + ) + }) +} + fn load_package(path: &Path) -> Result { if !path.exists() { return Err(CliError::InvalidArgument(format!( @@ -254,7 +396,7 @@ fn generate_tx_script( ))); } - let mut script = String::from("begin\n"); + let mut script = String::from("use miden::core::sys\n\nbegin\n"); // Push args in reverse so the first arg ends up on top. for arg in args.iter().rev() { @@ -285,6 +427,53 @@ fn generate_tx_script( } } + // Without a known result count we can't drop the pushed args from under the results, so the + // stack ends up with extra elements. `truncate_stack` enforces the 16-element exit invariant. + script.push_str(" exec.sys::truncate_stack\n"); + script.push_str("end\n"); + Ok(code_builder.compile_tx_script(&script)?) +} + +/// Builds a script that invokes `proc_digest` on `foreign_id` via FPI. Args are pushed so +/// args[0] ends up on top, matching the direct-call convention. `truncate_stack` enforces the +/// 16-element exit invariant required by FPI component exports. +fn generate_fpi_tx_script( + code_builder: CodeBuilder, + foreign_id: AccountId, + proc_digest: &Word, + args: &[Felt], +) -> Result { + const FPI_INPUT_SLOTS: usize = 16; + if args.len() > FPI_INPUT_SLOTS { + return Err(CliError::InvalidArgument(format!( + "FPI supports up to {FPI_INPUT_SLOTS} input felts; got {}", + args.len() + ))); + } + + let mut script = String::from("use miden::protocol::tx\nuse miden::core::sys\n\nbegin\n"); + + // Pad the deeper input slots with zeros, then push args so args[0] lands on top. + let pad_count = FPI_INPUT_SLOTS - args.len(); + let full_words = pad_count / 4; + let remainder = pad_count % 4; + for _ in 0..full_words { + script.push_str(" padw\n"); + } + for _ in 0..remainder { + script.push_str(" push.0\n"); + } + for arg in args.iter().rev() { + writeln!(script, " push.{arg}").unwrap(); + } + + writeln!(script, " push.{}", proc_digest.to_hex()).unwrap(); + writeln!(script, " push.{}", foreign_id.prefix().as_u64()).unwrap(); + writeln!(script, " push.{}", foreign_id.suffix()).unwrap(); + + script.push_str(" exec.tx::execute_foreign_procedure\n"); + script.push_str(" exec.sys::truncate_stack\n"); script.push_str("end\n"); + Ok(code_builder.compile_tx_script(&script)?) } diff --git a/bin/miden-cli/tests/cli.rs b/bin/miden-cli/tests/cli.rs index 0ec47e1bcb..3c30ba6b06 100644 --- a/bin/miden-cli/tests/cli.rs +++ b/bin/miden-cli/tests/cli.rs @@ -1818,6 +1818,103 @@ fn call_rejects_wrong_arg_count() { ); } +/// Helper: sets up two isolated clients. The first owns a public `call-test` account deployed +/// on-chain; the second has only a local wallet to act as the FPI executor. Returns the second +/// client's (`caller_dir`, `account_id`, `masp_path`). +fn setup_remote_call_test() -> (PathBuf, String, PathBuf) { + // Client A: owns and deploys the call-test account. + let target_dir = init_cli().1; + + let masp_path = target_dir.join("call_test.masp"); + build_call_test_masp(&masp_path); + + let init_toml = r#" +"miden::testing::call_test::stored_value" = "0x0000000000000000000000000000000000000000000000000000000000000000" +"#; + let init_path = target_dir.join("call_test_init.toml"); + fs::write(&init_path, init_toml).unwrap(); + + sync_cli(&target_dir); + + // `--deploy` submits a transaction that commits the public account on-chain, which is what + // makes it readable from another client via FPI. + let mut create_cmd = cargo_bin_cmd!("miden-client"); + create_cmd.args([ + "new-account", + "--account-type", + "regular-account-immutable-code", + "-s", + "public", + "-p", + "auth/no-auth", + "-p", + masp_path.to_str().unwrap(), + "-i", + init_path.to_str().unwrap(), + "--deploy", + ]); + + let output = create_cmd.current_dir(&target_dir).output().unwrap(); + assert!( + output.status.success(), + "Failed to create and deploy account: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + let account_id = stdout + .split_whitespace() + .skip_while(|&w| w != "-s") + .nth(1) + .expect("Could not parse account ID from new-account output") + .to_string(); + + // Wait for the deploy transaction to commit before the account can be read remotely. + sync_until_committed_transaction(&target_dir); + + // Client B: only a local wallet, used as the FPI executor. + let caller_dir = init_cli().1; + new_wallet_cli(&caller_dir, AccountStorageMode::Private); + sync_cli(&caller_dir); + + (caller_dir, account_id, masp_path) +} + +/// Tests calling a procedure on a public account that is not in the caller's local store. The +/// call is routed through FPI using the caller's local wallet as the executor. +#[test] +fn call_remote_account_via_fpi() { + let (caller_dir, account_id, masp_path) = setup_remote_call_test(); + + let mut cmd = cargo_bin_cmd!("miden-client"); + cmd.args([ + "call", + &format!("{account_id}:add"), + "3", + "7", + "--package", + masp_path.to_str().unwrap(), + ]); + + let output = cmd.current_dir(&caller_dir).output().unwrap(); + assert!( + output.status.success(), + "Remote call failed.\nstdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains("reading from network via FPI"), + "Expected FPI routing message in output:\n{stdout}" + ); + assert!( + stdout.contains("Result: 10"), + "Expected `add(3, 7)` result in output:\n{stdout}" + ); +} + // AUTH COMPONENT TESTS // ================================================================================================