diff --git a/CHANGELOG.md b/CHANGELOG.md index 91ca948a4..87dc54477 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Enhancements + +* [FEATURE][cli] `call` reads the procedure's signature from the package manifest: it prints the signature with type names (`add-points(point, point) -> point`), takes each argument as one token of its own type — an `account-id` or an `asset` is written as `0x..` or `::` instead of raw field elements — and renders the result the same way. Arguments that occupy more stack values than a called procedure can see are now rejected instead of reaching the procedure as zeros, the state delta is reported again (the script dropped only the arguments, leaving every call deeper than it started), and a procedure that only reads reports that the transaction was rejected for having no effects instead of printing the transaction kernel's assertion chain ([#2179](https://github.com/0xMiden/rust-sdk/pull/2179)). + ## 0.16.0-alpha.1 (2026-07-17) ### Breaking Changes diff --git a/Cargo.lock b/Cargo.lock index 5429ba0e2..96dd3377b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -36,6 +36,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "allocator-api2" +version = "0.2.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" + [[package]] name = "alloy-primitives" version = "1.6.0" @@ -1160,6 +1166,12 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d9c4f5dac5e15c24eb999c26181a6ca40b39fe946cbe4c263c7209467bc83af2" +[[package]] +name = "foldhash" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "77ce24cb58228fbb8aa041425bb1050850ac19177686ea6e0f41a70416f56fdb" + [[package]] name = "fs-err" version = "3.3.0" @@ -1387,7 +1399,7 @@ version = "0.15.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9229cfe53dfd69f0609a49f65461bd93001ea1ef889cd5529dd176593f5338a1" dependencies = [ - "foldhash", + "foldhash 0.1.5", ] [[package]] @@ -1395,6 +1407,11 @@ name = "hashbrown" version = "0.17.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a" +dependencies = [ + "allocator-api2", + "equivalent", + "foldhash 0.2.0", +] [[package]] name = "hashlink" @@ -1891,9 +1908,8 @@ checksum = "88904434abc2901f197fe8cc55f0445e7ded921dba5911dad2e2b39b48e663c4" [[package]] name = "miden-ace-codegen" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b96b70c94b9dbb4a5dd9b03c52bf59d2fae254eb3fee4d9338889d8b5b82b9d9" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "miden-core", "miden-crypto", @@ -1925,9 +1941,8 @@ dependencies = [ [[package]] name = "miden-air" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1f1fc5bffab96b788524758d72c3457726357d1397874fac5ea841c6753b28e9" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "miden-ace-codegen", "miden-core", @@ -1940,9 +1955,8 @@ dependencies = [ [[package]] name = "miden-assembly" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0d08306e2cc2fe3fa50aef600d434ad67a780807374cbda60fe4450ca4dad37e" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "env_logger", "log", @@ -1958,9 +1972,8 @@ dependencies = [ [[package]] name = "miden-assembly-syntax" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2609e82bffb5d9e0d37591db837d659224dec4e82e13575daa6203280ba6c14f" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "env_logger", "log", @@ -1981,9 +1994,8 @@ dependencies = [ [[package]] name = "miden-assembly-syntax-cst" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b442514653b58191f4ed0fd9ed14853d977bd44702cf04064e0eca7cc5c77b79" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "miden-debug-types", "miden-rowan", @@ -2069,6 +2081,7 @@ dependencies = [ "miden-client-sqlite-store", "miden-debug", "miden-mast-package", + "miden-protocol", "midenc-hir-type", "miette", "predicates", @@ -2140,9 +2153,8 @@ dependencies = [ [[package]] name = "miden-core" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "93203254d349bb0d7b4302853331c57b66a38bb4289fd2f56bee0c624b05e1a1" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "derive_more", "log", @@ -2159,9 +2171,8 @@ dependencies = [ [[package]] name = "miden-core-lib" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d062b282d71d2f847bf372a070929f8238c5199b9756e36d4d11f22acd58ad1" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "env_logger", "fs-err", @@ -2230,9 +2241,9 @@ dependencies = [ [[package]] name = "miden-debug" -version = "0.9.0" +version = "0.9.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d3f9a821ae2e7f30b007ac13a747b0713fe0d5b5c23c7fc81320de827fa25b10" +checksum = "d4e4971fefa10e310c76ca0dc5037dd7fe0c1a097ce17a20b17265cff3ecd591" dependencies = [ "clap", "compact_str", @@ -2255,9 +2266,9 @@ dependencies = [ [[package]] name = "miden-debug-dap" -version = "0.9.0" +version = "0.9.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f76e7d4b6570ae61af7bd5e6bd3da052dcd6ae47c87ff212175c2be618f77eec" +checksum = "0a694b0b59aeb4fb82adfec83e03763d9e370dce7d9287f908b9064b5d31ef34" dependencies = [ "serde", "serde_json", @@ -2266,9 +2277,9 @@ dependencies = [ [[package]] name = "miden-debug-engine" -version = "0.9.0" +version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "27c7cdb32831b608fa8f23a37d935e18ff84dee10d54c6e0d0f162f7fba8eb5a" +checksum = "338b0d4003123733d51a7a4fe19d39437bef3ce84fb81a62471851b9bc36e3ff" dependencies = [ "clap", "glob", @@ -2296,9 +2307,8 @@ dependencies = [ [[package]] name = "miden-debug-types" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f75a72da10ddadeb9a56542b675431b9cb93f5f25ea180ff8f0c311aafee922" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "memchr", "miden-crypto", @@ -2311,6 +2321,7 @@ dependencies = [ "serde", "serde_spanned 1.1.1", "thiserror", + "zerocopy", ] [[package]] @@ -2380,16 +2391,19 @@ dependencies = [ [[package]] name = "miden-mast-package" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "942d744f2606ad9d146fc9d817fccc5913d9cc8b8b9e53666779f6c415a174ce" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ + "hashbrown 0.17.1", "log", "miden-assembly-syntax", "miden-core", "miden-debug-types", + "miden-utils-indexing", + "rustc-hash", "serde", "thiserror", + "zerocopy", ] [[package]] @@ -2456,9 +2470,8 @@ dependencies = [ [[package]] name = "miden-package-registry" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01202970e634f258502ed082d776c906289b6394eb5ac6b8067be0d7261a55a3" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "miden-assembly-syntax", "miden-core", @@ -2472,9 +2485,8 @@ dependencies = [ [[package]] name = "miden-processor" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d2e4776a16e18c5317ae31cfb149f30a5b454a25b6e6fefd3ccee943227514db" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "itertools", "miden-air", @@ -2491,9 +2503,8 @@ dependencies = [ [[package]] name = "miden-project" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7de67303b1f197ad49f023fad38ca1c4be8fb55ef6098c95f254b09a8ad6b040" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "miden-assembly-syntax", "miden-core", @@ -2539,9 +2550,8 @@ dependencies = [ [[package]] name = "miden-prover" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a18511f219eeb6decc384f8a39d1428ed6d3eb313b3ff778a988635f61b84ece" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "miden-air", "miden-core", @@ -2550,7 +2560,6 @@ dependencies = [ "serde", "serde-wincode", "tracing", - "wincode", ] [[package]] @@ -2664,9 +2673,8 @@ dependencies = [ [[package]] name = "miden-utils-core-derive" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dd084f8c9b3fbd173536a794850e1f29789eaeaff0f87e7852505f0fa9aa8556" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "proc-macro2", "quote", @@ -2675,9 +2683,8 @@ dependencies = [ [[package]] name = "miden-utils-diagnostics" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "048bbc18d446d1bc29b5e3ceac69000f1488a0ea1b15b18f48a0c702c2c1e706" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "miden-debug-types", "miden-miette", @@ -2686,11 +2693,10 @@ dependencies = [ [[package]] name = "miden-utils-indexing" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "98a713931678d5cdc23ab388cda048cc055079767780799b1d56f35b14ae6957" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ - "miden-crypto", + "miden-serde-utils", "proptest", "serde", "thiserror", @@ -2698,9 +2704,8 @@ dependencies = [ [[package]] name = "miden-utils-sync" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a7fe110d6c5a90a27747d051c6d20cbbbc2d425c4276e96ce7bf82270d23cc3" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "lock_api", "loom", @@ -2710,9 +2715,8 @@ dependencies = [ [[package]] name = "miden-verifier" -version = "0.25.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c5bbb6c5af707266e4844460f5b55fc3c80d96a6a80b5e2a479d3cc95ad6d483" +version = "0.25.8" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "miden-air", "miden-core", @@ -2721,14 +2725,12 @@ dependencies = [ "serde-wincode", "thiserror", "tracing", - "wincode", ] [[package]] name = "midenc-hir-type" version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f67fe32b429d499d0ae713e044b593d866b1ab51540de6ca8881413aba42e858" +source = "git+https://github.com/walnuthq/miden-vm?branch=pr%2Fdebug_info_encoder_decoder#d6230b541f83dbdbb552d173f8253f8d560fa664" dependencies = [ "miden-formatting", "miden-serde-utils", diff --git a/Cargo.toml b/Cargo.toml index 3f82c46ac..d9d3168cc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,8 +52,9 @@ miden-node-proto-build = { default-features = false, version = "0.16.0 miden-note-transport-proto-build = { default-features = false, version = "0.5.0-alpha.1" } # Miden debug dependency -miden-debug = { default-features = false, features = ["dap", "std"], version = "0.9" } -miden-processor = { default-features = false, version = "0.25" } +miden-debug = { default-features = false, features = ["dap", "std"], version = "0.9" } +miden-mast-package = { default-features = false, version = "0.25" } +miden-processor = { default-features = false, version = "0.25" } # External dependencies anyhow = { default-features = false, version = "1.0" } @@ -80,6 +81,33 @@ tracing = { version = "0.1" } tracing-subscriber = { default-features = false, version = "0.3" } uuid = { version = "1.10" } +# TEMPORARY (do not merge): `miden call` needs two things that are not in a published miden-vm +# release yet — the `miden_mast_package::types` re-export it names manifest signature types +# through, and the fix for procedure names in package manifest exports, without which linking a +# compiler-built package panics. Point the whole miden-vm crate set at the branch carrying both; +# patching `mast-package` alone would pull a second `miden-core` and break type unification. Delete +# this entire block once both land in a release — the caret `"0.25"` requirement then picks them up +# from crates.io. +[patch.crates-io] +miden-ace-codegen = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-air = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-assembly = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-assembly-syntax = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-core = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-core-lib = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-debug-types = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-mast-package = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-package-registry = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-processor = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-project = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-prover = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-utils-core-derive = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-utils-diagnostics = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-utils-indexing = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-utils-sync = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +miden-verifier = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } +midenc-hir-type = { branch = "pr/debug_info_encoder_decoder", git = "https://github.com/walnuthq/miden-vm" } + # Lints are set to warn for development, which are promoted to errors in CI. [workspace.lints.clippy] # Pedantic lints are set to a lower priority which allows lints in the group to be selectively enabled. diff --git a/bin/miden-cli/Cargo.toml b/bin/miden-cli/Cargo.toml index 92c819220..7b4be4642 100644 --- a/bin/miden-cli/Cargo.toml +++ b/bin/miden-cli/Cargo.toml @@ -26,6 +26,8 @@ testing = ["miden-client/testing"] miden-client = { features = ["tonic"], workspace = true } miden-client-sqlite-store = { workspace = true } miden-debug = { optional = true, workspace = true } +miden-mast-package = { workspace = true } +miden-protocol = { workspace = true } # External dependencies clap = { features = ["derive"], workspace = true } @@ -48,7 +50,7 @@ miden-client = { features = ["std"], workspace = true } anyhow = { workspace = true } assert_cmd = { workspace = true } miden-client = { features = ["testing"], workspace = true } -miden-mast-package = { version = "0.25" } +miden-mast-package = { workspace = true } midenc-hir-type = { version = "0.9" } predicates = { workspace = true } regex = { workspace = true } diff --git a/bin/miden-cli/src/codecs/account_id.rs b/bin/miden-cli/src/codecs/account_id.rs new file mode 100644 index 000000000..367e396ac --- /dev/null +++ b/bin/miden-cli/src/codecs/account_id.rs @@ -0,0 +1,85 @@ +//! The `account-id` codec for typed `call` rendering. +//! +//! `account-id` felts are validated with protocol-level rules, so the CLI registers this codec +//! (via [`TypedProcInfo::with_scalar_codec`]) to encode one hex token into the two stack felts the +//! procedure expects and render the returned felts back as `account-id(0x..)`. +//! +//! [`TypedProcInfo::with_scalar_codec`]: miden_mast_package::typed::TypedProcInfo::with_scalar_codec + +use miden_mast_package::typed::{MIDEN_CORE_TYPES, TypedError, WitScalarCodec}; +use miden_protocol::Felt; +use miden_protocol::account::AccountId; + +use crate::codecs::invalid_scalar; + +/// Bare WIT type name the typed encoder matches this codec against, regardless of the package and +/// version in the full type name (e.g. `miden:base/core-types@1.0.0/account-id`). +const ACCOUNT_ID_WIT_NAME: &str = "account-id"; + +/// Encodes and renders the WIT `account-id` type: one hex token, two stack felts. +pub struct AccountIdCodec; + +impl WitScalarCodec for AccountIdCodec { + fn wit_name(&self) -> &str { + ACCOUNT_ID_WIT_NAME + } + + fn wit_interface(&self) -> Option<&str> { + Some(MIDEN_CORE_TYPES) + } + + fn encode(&self, token: &str) -> Result, TypedError> { + let id = AccountId::from_hex(token) + .map_err(|err| invalid_scalar(ACCOUNT_ID_WIT_NAME, token, &err))?; + let [prefix, suffix]: [Felt; 2] = id.into(); + Ok(vec![prefix, suffix]) + } + + fn decode(&self, felts: &[Felt]) -> Result { + // The caller passes as many felts as the type occupies, so any other count means the + // signature and this codec disagree about the value's width. + let [prefix, suffix] = felts else { + return Err(TypedError::MalformedResult { + ty: ACCOUNT_ID_WIT_NAME.to_string(), + reason: "an account id occupies exactly two felts", + }); + }; + let id = AccountId::try_from_elements(*suffix, *prefix).map_err(|_| { + TypedError::MalformedResult { + ty: ACCOUNT_ID_WIT_NAME.to_string(), + reason: "the felts are not a valid account id", + } + })?; + Ok(format!("account-id({})", id.to_hex())) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn account_id_one_hex_token_roundtrips() { + let codec = AccountIdCodec; + let hex = "0xaa0000000000bb110000cc000000dd"; + + // Compared against the felts the account id itself carries: a round-trip alone would also + // pass if `encode` and `decode` had the two fields the same way around. + let expected: [Felt; 2] = AccountId::from_hex(hex).unwrap().into(); + assert_eq!(codec.encode(hex).unwrap(), expected); + + assert_eq!(codec.decode(&expected).unwrap(), format!("account-id({hex})")); + } + + #[test] + fn felts_that_are_not_an_account_id_are_rejected() { + let err = AccountIdCodec.decode(&[Felt::from(1u32), Felt::from(2u32)]).unwrap_err(); + assert!(matches!(err, TypedError::MalformedResult { .. })); + } + + #[test] + fn invalid_account_id_token_is_rejected() { + let err = AccountIdCodec.encode("not-hex").unwrap_err(); + assert!(matches!(err, TypedError::InvalidScalar { .. })); + } +} diff --git a/bin/miden-cli/src/codecs/asset.rs b/bin/miden-cli/src/codecs/asset.rs new file mode 100644 index 000000000..dcf6161bd --- /dev/null +++ b/bin/miden-cli/src/codecs/asset.rs @@ -0,0 +1,124 @@ +//! The `asset` codec for typed `call` rendering. +//! +//! On the stack a WIT `asset` is its eight felts — the id word followed by the value word, i.e. +//! [`Asset::as_elements`]. The CLI registers this codec so an asset argument can be given as a +//! single `::` token instead of two raw word hexes, and so a returned asset +//! renders back the same way. The token form matches the one the rest of the CLI takes for +//! fungible assets, minus the token symbol and address spellings: resolving those needs the +//! client, and a codec only sees the text. + +use miden_mast_package::typed::{MIDEN_CORE_TYPES, TypedError, WitScalarCodec}; +use miden_protocol::account::AccountId; +use miden_protocol::asset::{Asset, FungibleAsset}; +use miden_protocol::{Felt, Word}; + +use crate::codecs::invalid_scalar; + +/// Bare WIT type name the typed encoder matches this codec against (e.g. the leaf of +/// `miden:base/core-types@1.0.0/asset`). +const ASSET_WIT_NAME: &str = "asset"; + +/// Encodes and renders the WIT `asset` type: one `::` token, eight stack felts. +/// Only fungible assets are supported by this token form. +pub struct AssetCodec; + +impl WitScalarCodec for AssetCodec { + fn wit_name(&self) -> &str { + ASSET_WIT_NAME + } + + fn wit_interface(&self) -> Option<&str> { + Some(MIDEN_CORE_TYPES) + } + + fn encode(&self, token: &str) -> Result, TypedError> { + let (amount, faucet) = token.split_once("::").ok_or_else(|| { + invalid_scalar(ASSET_WIT_NAME, token, "expected `::`") + })?; + let amount: u64 = amount.parse().map_err(|e: core::num::ParseIntError| { + invalid_scalar(ASSET_WIT_NAME, token, &format!("invalid amount: {e}")) + })?; + let faucet_id = + AccountId::from_hex(faucet).map_err(|e| invalid_scalar(ASSET_WIT_NAME, token, &e))?; + let asset: Asset = FungibleAsset::new(faucet_id, amount) + .map_err(|e| invalid_scalar(ASSET_WIT_NAME, token, &e))? + .into(); + Ok(asset.as_elements().to_vec()) + } + + fn decode(&self, felts: &[Felt]) -> Result { + // As in `AccountIdCodec`, any count other than the type's width means the signature and + // this codec disagree. + let [k0, k1, k2, k3, v0, v1, v2, v3] = felts else { + return Err(malformed_asset("an asset occupies exactly eight felts")); + }; + let id = Word::from([*k0, *k1, *k2, *k3]); + let value = Word::from([*v0, *v1, *v2, *v3]); + let asset = Asset::from_id_and_value_words(id, value) + .map_err(|_| malformed_asset("the felts are not a valid asset"))?; + Ok(match asset { + Asset::Fungible(f) => format!("asset({}::{})", f.amount(), f.faucet_id().to_hex()), + Asset::NonFungible(_) => "asset(non-fungible)".to_string(), + }) + } +} + +/// Builds the error for result felts that are not an asset this codec can render. +fn malformed_asset(reason: &'static str) -> TypedError { + TypedError::MalformedResult { ty: ASSET_WIT_NAME.to_string(), reason } +} + +#[cfg(test)] +mod tests { + use miden_client::testing::account_id::ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET; + + use super::*; + + fn hex(id: u128) -> String { + AccountId::try_from(id).unwrap().to_hex() + } + + fn faucet_token(amount: u64) -> String { + format!("{amount}::{}", hex(ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET)) + } + + #[test] + fn a_fungible_asset_token_roundtrips() { + let token = faucet_token(100); + + let felts = AssetCodec.encode(&token).unwrap(); + assert_eq!(felts.len(), 8); + + assert_eq!(AssetCodec.decode(&felts).unwrap(), format!("asset({token})")); + } + + #[test] + fn a_token_with_a_single_colon_is_rejected() { + let err = AssetCodec.encode(&faucet_token(100).replace("::", ":")).unwrap_err(); + assert!(matches!(err, TypedError::InvalidScalar { .. })); + } + + #[test] + fn a_token_in_the_reverse_order_is_rejected() { + let token = format!("{}::100", hex(ACCOUNT_ID_PUBLIC_FUNGIBLE_FAUCET)); + let err = AssetCodec.encode(&token).unwrap_err(); + assert!(matches!(err, TypedError::InvalidScalar { .. })); + } + + #[test] + fn felts_that_are_not_an_asset_are_rejected() { + let felts: Vec = (1..=8u32).map(Felt::from).collect(); + let err = AssetCodec.decode(&felts).unwrap_err(); + assert!(matches!(err, TypedError::MalformedResult { .. })); + } + + #[test] + fn a_felt_count_other_than_eight_is_rejected() { + let felts = AssetCodec.encode(&faucet_token(100)).unwrap(); + + for len in [0, 4, 7] { + let err = AssetCodec.decode(&felts[..len]).unwrap_err(); + assert!(matches!(err, TypedError::MalformedResult { .. }), "len {len} was accepted"); + } + } +} diff --git a/bin/miden-cli/src/codecs/mod.rs b/bin/miden-cli/src/codecs/mod.rs new file mode 100644 index 000000000..a5c4c282a --- /dev/null +++ b/bin/miden-cli/src/codecs/mod.rs @@ -0,0 +1,44 @@ +//! Protocol-aware WIT scalar codecs. +//! +//! The encode/decode engine and the [`WitScalarCodec`] trait live in `miden-mast-package`, which +//! does not depend on `miden-protocol` and should not: the VM does not depend on the protocol. So +//! it ships the two codecs it can write itself, `word` and `felt`, and leaves the trait for the +//! rest. +//! +//! `account-id` and `asset` are the rest. `AccountId::from_hex` says what a valid id is, and +//! `Asset` says what a valid asset is, so both codecs live on this side. +//! +//! [`with_cli_codecs`] registers them in one place, so the commands that render typed signatures +//! do not know the individual types. +//! +//! [`WitScalarCodec`]: miden_mast_package::typed::WitScalarCodec +//! [`TypedProcInfo`]: miden_mast_package::typed::TypedProcInfo + +mod account_id; +mod asset; + +pub use account_id::AccountIdCodec; +pub use asset::AssetCodec; +use miden_mast_package::typed::{TypedError, TypedProcInfo}; + +/// Builds the `InvalidScalar` error a codec returns when it can't parse `token`. Shared so every +/// codec reports the same error shape from one place. +pub(crate) fn invalid_scalar( + wit_name: &str, + token: &str, + reason: &(impl ToString + ?Sized), +) -> TypedError { + TypedError::InvalidScalar { + wit_name: wit_name.to_string(), + token: token.to_string(), + reason: reason.to_string(), + } +} + +/// Registers every CLI scalar codec onto `typed`. New codecs are added here so the commands that +/// render typed signatures stay agnostic of the individual WIT types. +pub fn with_cli_codecs(typed: TypedProcInfo) -> TypedProcInfo { + typed + .with_scalar_codec(Box::new(AccountIdCodec)) + .with_scalar_codec(Box::new(AssetCodec)) +} diff --git a/bin/miden-cli/src/commands/call.rs b/bin/miden-cli/src/commands/call.rs index 9cbe1eea7..d626113fa 100644 --- a/bin/miden-cli/src/commands/call.rs +++ b/bin/miden-cli/src/commands/call.rs @@ -6,16 +6,25 @@ use clap::Parser; use miden_client::assembly::CodeBuilder; use miden_client::keystore::Keystore; use miden_client::transaction::{AdviceInputs, TransactionRequestBuilder, TransactionScript}; -use miden_client::vm::{Package, PackageExport}; +use miden_client::vm::{MIN_STACK_DEPTH, Package, PackageExport, PackageManifest, ProcedureExport}; use miden_client::{Client, Deserializable, Felt, Word}; +use miden_mast_package::typed::TypedProcInfo; use crate::advice_inputs::load_advice_map_from_file; +use crate::codecs::with_cli_codecs; use crate::errors::CliError; -use crate::utils::{parse_account_id, print_executed_program_stack, print_executed_transaction}; +use crate::utils::{parse_account_id, print_executed_transaction}; // CALL COMMAND // ================================================================================================ +/// Part of the transaction kernel's assertion message for a transaction that changes nothing. +/// +/// The assertion carries a message rather than a code the executor exposes, so this is matched as +/// text. It only decides whether an explanatory line is printed: if the kernel ever rewords it, +/// the line stops appearing and nothing else changes. +const EMPTY_TRANSACTION_ASSERTION: &str = "neither changed the account state"; + #[derive(Debug, Clone, Parser)] #[command(about = "Call a procedure on a local account and display the result and state delta")] pub struct CallCmd { @@ -48,9 +57,7 @@ impl CallCmd { mut client: Client, ) -> Result<(), CliError> { if client.get_sync_height().await? == 0.into() { - return Err(CliError::InvalidArgument( - "Client has not been synced yet. Run `miden-client sync` first.".to_string(), - )); + return Err(CliError::NotSynced); } let (account_str, procedure) = self.target.split_once(':').ok_or_else(|| { @@ -65,45 +72,52 @@ impl CallCmd { 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 export = resolve_procedure_export(&package.manifest, procedure)?; + let digest = export.digest; + let signature = export.signature.clone().ok_or_else(|| missing_signature(procedure))?; + // The signature prints under the name the package carries, not the one the user typed, so + // `call increment_by` shows `increment-by(felt) -> felt`. + let name = export.path.last().ok_or_else(|| { + CliError::InvalidArgument(format!( + "The export matching '{procedure}' has an empty path, so it names no procedure." + )) + })?; + let typed = with_cli_codecs(TypedProcInfo::new(name, signature)); + + println!("Signature: {typed}\n"); + + // The script has to clear the stack it grew, which needs the result size: without it the + // drop sequence cannot be built at all, in either execution below. + let Some(result_felts) = typed.output_felt_count() else { + return Err(CliError::InvalidArgument(format!( + "Procedure '{procedure}' returns a value whose size on the stack is not known." + ))); + }; - let args = parse_args(&self.args)?; + // Checks the argument count as well, and names the procedure and both counts when it is + // wrong, so there is nothing to check here first. + let args = typed.encode_args(&self.args)?; let advice_entries = match &self.inputs_path { Some(path) => load_advice_map_from_file(path)?, None => vec![], }; - 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." - ); - }, - _ => {}, - } - - // 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. + // The account's code is loaded from the client's store at runtime, so the library does not + // need to be embedded in the compiled script. 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)?; - // 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)?; + // 1) Read-only execution to get return values. + let read_tx_script = generate_tx_script( + linked_builder.clone(), + &digest, + &args, + result_felts, + Cleanup::KeepResults, + )?; let advice_inputs = AdviceInputs::default().with_map(advice_entries.clone()); @@ -111,10 +125,16 @@ impl CallCmd { .execute_program(account_id, read_tx_script, advice_inputs, BTreeMap::new()) .await?; - print_executed_program_stack(&output_stack, result_count); + // A procedure that returns nothing has no result to show; anything else that cannot be + // rendered is an error, since a raw stack dump would hide that the result is not a valid + // value of its type. + if let Some(rendered) = typed.decode_result(output_stack.as_slice())? { + println!("Result: {rendered}"); + } - // 2) Transaction execution to get state delta. - let delta_tx_script = generate_tx_script(linked_builder, &digest, &args, Some(0))?; + // 2) Transaction execution to get the state delta. + let delta_tx_script = + generate_tx_script(linked_builder, &digest, &args, result_felts, Cleanup::DropAll)?; let tx_request = TransactionRequestBuilder::new() .custom_script(delta_tx_script) @@ -129,7 +149,31 @@ impl CallCmd { print_executed_transaction(&mut client, tx_result.executed_transaction()).await?; }, Err(e) => { - println!("\n(Could not compute state delta: {e})"); + let mut report = String::new(); + let mut cause = std::error::Error::source(&e); + while let Some(err) = cause { + writeln!(report, " caused by: {err}").unwrap(); + cause = err.source(); + } + + if report.contains(EMPTY_TRANSACTION_ASSERTION) { + // A procedure that only reads, on an account whose components write nothing, + // leaves the transaction with no effects at all, and the kernel refuses those. + // For a read-only call that is the expected outcome rather than a fault, so it + // is reported instead of dumping the assertion chain. The kernel rejects only + // when the account was left unchanged and nothing was consumed, so that is all + // this can report; it says nothing about created notes. + println!(); + println!("The transaction was rejected because it had no effects:\n"); + println!("No notes were consumed."); + println!(); + println!("Account Storage was not changed."); + println!("Account Vault was not changed."); + println!("Account nonce was not changed."); + } else { + println!("\n(Could not compute state delta: {e})"); + print!("{report}"); + } }, } @@ -153,27 +197,47 @@ fn load_package(path: &Path) -> Result { }) } -fn resolve_procedure_digest(package: &Package, procedure_name: &str) -> Result { +/// Finds the export `procedure_name` names, which carries both the digest to call and the +/// signature the arguments are encoded against. +/// +/// The compiler writes two exports for the same Component Model procedure: one with its WIT +/// signature, `add-points(point, point) -> point`, and one lowered to the C ABI, +/// `fn(felt, felt, felt, felt) -> i32`. Arguments are encoded and results are rendered from the +/// signature this picks, so it has to be the WIT one. The lowered signature describes the ABI +/// plumbing instead: its parameters are the flattened felts, and its `i32` result is a pointer to +/// the value rather than the value. +fn resolve_procedure_export<'a>( + manifest: &'a PackageManifest, + procedure_name: &str, +) -> Result<&'a ProcedureExport, CliError> { // The user passes a bare name (e.g. `get_count`); match it // against each export's name without the module path. Export names may be kebab (Rust/WIT) or // snake (hand-written MASM bare identifiers), so compare with `_` and `-` treated as equal. let target = procedure_name.replace('_', "-"); let mut available = Vec::new(); - for export in package.manifest.exports() { + let mut found_without_signature = false; + + for export in manifest.exports() { let PackageExport::Procedure(proc) = export else { continue; }; + // Every procedure goes on the list, so a "not found" error shows the whole surface. + available.push(format!(" {}", proc.path)); + if export.name().replace('_', "-") != target { - // Not the requested procedure; keep it for the "not found" error list. - available.push(format!(" {}", proc.path)); continue; } // The same leaf name is exported both as a `C`-ABI lowering (for `exec`) and as the // `ComponentModel` export (the cross-context `call` target); pick the latter. if proc.signature.as_ref().is_some_and(|sig| sig.abi.is_wasm_canonical_abi()) { - return Ok(proc.digest); + return Ok(proc); } + found_without_signature = true; + } + + if found_without_signature { + return Err(missing_signature(procedure_name)); } Err(CliError::InvalidArgument(format!( @@ -182,101 +246,176 @@ fn resolve_procedure_digest(package: &Package, procedure_name: &str) -> Result Result, CliError> { - args.iter() - .map(|arg| { - let n = arg.parse::().map_err(|_| { - CliError::InvalidArgument(format!("Invalid argument '{arg}'. Expected u64.")) - })?; - Felt::try_from(n) - .map_err(|_| CliError::InvalidArgument(format!("Argument '{arg}' is too large."))) - }) - .collect() +/// The error for a procedure the package exports but does not describe well enough to call: `call` +/// encodes its arguments from the signature, so a procedure without one cannot be reached. +fn missing_signature(procedure_name: &str) -> CliError { + CliError::InvalidArgument(format!( + "Procedure '{procedure_name}' is exported without a type signature, so its arguments \ + cannot be encoded. Only procedures built from a WIT interface carry one." + )) } -/// Parameter and result counts from a procedure's manifest signature. `None` means the -/// information is unavailable (procedure missing from manifest or export lacks type info). -struct ProcedureSignature { - param_count: Option, - result_count: Option, -} +// TESTS +// ================================================================================================ -/// Prints the signature of `procedure_name` from the package manifest and returns its parameter -/// and result counts. If the procedure is missing, prints the list of available exports. -fn print_manifest_signature(package: &Package, procedure_name: &str) -> ProcedureSignature { - const UNKNOWN: ProcedureSignature = - ProcedureSignature { param_count: None, result_count: None }; +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use miden_mast_package::PathBuf; + use midenc_hir_type::{CallConv, FunctionType, Type}; + + use super::*; + + /// A manifest exporting every `(path, signature)` pair. Resolution matches on the path and + /// reads the signature, so the digest is left zero. + fn manifest_with_exports(exports: &[(&str, Option)]) -> PackageManifest { + let exports = exports.iter().map(|(path, signature)| { + let path: Arc<_> = path.parse::().expect("path should parse").into(); + PackageExport::Procedure(ProcedureExport::new( + path, + None, + Word::default(), + signature.clone(), + )) + }); - let kebab_name = procedure_name.replace('_', "-"); - let quoted_kebab = format!("\"{kebab_name}\""); - let quoted_name = format!("\"{procedure_name}\""); + PackageManifest::new(exports).expect("manifest should be valid") + } - for export in package.manifest.exports() { - let PackageExport::Procedure(proc_export) = export else { - continue; - }; + /// The interface form of a Component Model export. It keeps the WIT types. + fn interface_form() -> (&'static str, Option) { + ( + "::\"miden:counter/counter@0.1.0\"::\"increment-by\"", + Some(FunctionType::new(CallConv::ComponentModel, [Type::Felt], [Type::Felt])), + ) + } - let path_str = proc_export.path.to_string(); - if !path_str.ends_with(&kebab_name) - && !path_str.ends_with(procedure_name) - && !path_str.ends_with("ed_kebab) - && !path_str.ends_with("ed_name) - { - continue; + /// The lowered form of the same export. The C ABI flattens the types and returns the big value + /// by reference: an `i32` pointer, not the value. + fn lowered_form() -> (&'static str, Option) { + ( + "::\"miden:counter/counter@0.1.0\"::cc::\"miden:counter/counter@0.1.0#increment-by\"", + Some(FunctionType::new(CallConv::C, [Type::Felt], [Type::I32])), + ) + } + + #[test] + fn the_interface_form_wins_over_the_lowered_one() { + // The compiler is free to write the two exports in either order, so neither may decide it. + for exports in [[interface_form(), lowered_form()], [lowered_form(), interface_form()]] { + let manifest = manifest_with_exports(&exports); + + let export = resolve_procedure_export(&manifest, "increment-by").unwrap(); + assert_eq!(export.signature, interface_form().1); } + } - if let Some(sig) = &proc_export.signature { - let params: Vec = sig.params.iter().map(|p| format!("{p:?}")).collect(); - let results: Vec = sig.results.iter().map(|r| format!("{r:?}")).collect(); + #[test] + fn a_lowered_name_is_not_reachable_by_the_bare_procedure_name() { + // The last part of the lowered path holds the whole interface, so it never equals the + // plain name. Were it found, its `i32` return would be printed as a value. + let manifest = manifest_with_exports(&[lowered_form()]); + + let err = resolve_procedure_export(&manifest, "increment-by").unwrap_err(); + assert_eq!( + err.to_string(), + "invalid argument: Procedure 'increment-by' not found. Available:\n \ + ::\"miden:counter/counter@0.1.0\"::cc::\"miden:counter/counter@0.1.0#increment-by\"" + ); + } - let ret_str = if results.is_empty() { - String::new() - } else { - format!(" -> ({})", results.join(", ")) - }; + #[test] + fn an_underscore_query_finds_a_kebab_export() { + let manifest = manifest_with_exports(&[interface_form(), lowered_form()]); - let params_str = params.join(", "); - println!("Raw Signature: {procedure_name}({params_str}){ret_str}\n"); + let export = resolve_procedure_export(&manifest, "increment_by").unwrap(); + assert_eq!(export.signature, interface_form().1); + } - return ProcedureSignature { - param_count: Some(sig.params.len()), - result_count: Some(sig.results.len()), - }; + #[test] + fn a_hand_written_masm_export_does_not_shadow_the_component_model_one() { + // A MASM `increment_by` matches the query by name, but `call` needs the Component Model + // signature: only that one describes the values the user passes and reads. + let masm = + || ("::mix::increment_by", Some(FunctionType::new(CallConv::Fast, [], [Type::U32]))); + for exports in [[interface_form(), masm()], [masm(), interface_form()]] { + let manifest = manifest_with_exports(&exports); + + let export = resolve_procedure_export(&manifest, "increment_by").unwrap(); + assert_eq!(export.signature, interface_form().1); } - println!("Raw Signature: {procedure_name}(...) [no type info]\n"); - return UNKNOWN; } - println!("(procedure '{procedure_name}' not found in manifest exports)"); - println!("Available exports:"); - for export in package.manifest.exports() { - if let PackageExport::Procedure(p) = export { - println!(" {}", p.path); - } + #[test] + fn an_unknown_procedure_lists_the_whole_export_surface() { + let manifest = manifest_with_exports(&[interface_form(), lowered_form()]); + + let err = resolve_procedure_export(&manifest, "no-such-proc").unwrap_err(); + assert_eq!( + err.to_string(), + "invalid argument: Procedure 'no-such-proc' not found. Available:\n \ + ::\"miden:counter/counter@0.1.0\"::\"increment-by\"\n \ + ::\"miden:counter/counter@0.1.0\"::cc::\"miden:counter/counter@0.1.0#increment-by\"" + ); + } + + #[test] + fn an_export_without_a_signature_is_reported_as_such() { + // MASM written by hand: the export has the name we ask for, but no type info. The error + // has to say that, not that the procedure is missing. + let manifest = manifest_with_exports(&[("::mix::\"increment-by\"", None)]); + + let err = resolve_procedure_export(&manifest, "increment-by").unwrap_err(); + assert_eq!(err.to_string(), missing_signature("increment-by").to_string()); } - println!(); - UNKNOWN } -/// Builds a transaction script that pushes `args`, calls the procedure at `digest`, and optionally -/// drops the pushed args from under the results. `Some(n)` keeps the top `n` values; `None` skips -/// drops. +/// What the script leaves on the stack for the caller to read. +#[derive(Clone, Copy)] +enum Cleanup { + /// Drop the arguments from under the results, leaving the results on top. + KeepResults, + /// Drop the results as well. Dropping past the point where the script started is harmless, + /// since the stack never goes below its minimum depth. + DropAll, +} + +/// Builds a transaction script that pushes `args`, calls the procedure at `digest`, and clears the +/// arguments it pushed. +/// +/// The script has to return at the stack depth it was entered with, so the arguments always have +/// to go; `cleanup` only decides whether the `result_felts` results go with them. fn generate_tx_script( code_builder: CodeBuilder, digest: &Word, args: &[Felt], - result_count: Option, + result_felts: usize, + cleanup: Cleanup, ) -> Result { - // MASM `movup.n` only works for n in 2..=15. The VM stack exposes only the top - // 16 elements; anything deeper lives in the overflow table and cannot be reached - // by `movup`. So we can't drop args from under more than 15 results. - // See miden-vm/docs/src/user_docs/assembly/instruction_reference.md (movup row) - // and miden-vm/docs/src/design/stack/stack_ops.md (MOVUP/MOVDN sections). - if let Some(n) = result_count - && n > 15 + // The script passes every argument on the stack, and `MIN_STACK_DEPTH` is what the callee + // sees of it. An argument below that reaches the callee as a zero and the call still succeeds, + // so without this check a procedure with wide arguments would run on the wrong values. + if args.len() > MIN_STACK_DEPTH { + return Err(CliError::InvalidArgument(format!( + "The arguments occupy {} stack values, and a called procedure can only see {}.", + args.len(), + MIN_STACK_DEPTH + ))); + } + + // Each argument is dropped with `movup.{result_felts}`, which only reaches the directly + // addressable part of the stack. That bounds the results the script can step over, but only + // when there is an argument under them to reach in the first place. + // See miden-vm/docs/src/user_docs/assembly/instruction_reference.md (movup row). + let max_stepped_over = MIN_STACK_DEPTH - 1; + if matches!(cleanup, Cleanup::KeepResults) + && !args.is_empty() + && result_felts > max_stepped_over { return Err(CliError::InvalidArgument(format!( - "Procedure returns {n} values; only up to 15 are supported." + "Procedure takes arguments and returns {result_felts} values on the stack; the \ + arguments can only be cleared from under at most {max_stepped_over} results." ))); } @@ -290,25 +429,30 @@ fn generate_tx_script( writeln!(script, " call.{}", digest.to_hex()).unwrap(); let to_drop = args.len(); - if to_drop > 0 { - match result_count { - Some(0) => { + match cleanup { + Cleanup::DropAll => { + for _ in 0..to_drop + result_felts { + script.push_str(" drop\n"); + } + }, + // The results sit above the arguments, so each argument has to be stepped over. + Cleanup::KeepResults => match result_felts { + 0 => { for _ in 0..to_drop { script.push_str(" drop\n"); } }, - Some(1) => { + 1 => { for _ in 0..to_drop { script.push_str(" swap drop\n"); } }, - Some(n) => { + n => { for _ in 0..to_drop { writeln!(script, " movup.{n} drop").unwrap(); } }, - None => {}, - } + }, } script.push_str("end\n"); diff --git a/bin/miden-cli/src/errors.rs b/bin/miden-cli/src/errors.rs index 76e7d7df8..9cd0ab2a6 100644 --- a/bin/miden-cli/src/errors.rs +++ b/bin/miden-cli/src/errors.rs @@ -14,6 +14,7 @@ use miden_client::{ ErrorHint, NetworkIdError, }; +use miden_mast_package::typed::TypedError; use miette::Diagnostic; use thiserror::Error; @@ -95,8 +96,20 @@ pub enum CliError { MissingFlag(String), #[error("network id error")] NetworkIdError(#[from] NetworkIdError), + #[error("client has not been synced yet")] + #[diagnostic( + code(cli::not_synced), + help("Run `{} sync` first.", client_binary_name().display()) + )] + NotSynced, #[error("invalid argument: {0}")] InvalidArgument(String), + // Covers both directions of the typed path: encoding arguments and decoding results. The + // inner error already states the whole problem, so it is shown in place of a wrapper message + // rather than under one, where it would be printed twice. + #[error(transparent)] + #[diagnostic(code(cli::typed_error))] + Typed(#[from] TypedError), #[error("parse error: {1}")] #[diagnostic(code(cli::parse_error), help("Check the inputs."))] Parse(#[source] SourceError, String), diff --git a/bin/miden-cli/src/lib.rs b/bin/miden-cli/src/lib.rs index dbd73b515..099a7b2eb 100644 --- a/bin/miden-cli/src/lib.rs +++ b/bin/miden-cli/src/lib.rs @@ -276,6 +276,7 @@ impl DerefMut for CliClient { } mod advice_inputs; +mod codecs; pub mod config; // These modules intentionally shadow the miden_client re-exports - CLI has its own errors/utils #[allow(hidden_glob_reexports)] diff --git a/bin/miden-cli/tests/cli.rs b/bin/miden-cli/tests/cli.rs index eafd1be84..23b08274a 100644 --- a/bin/miden-cli/tests/cli.rs +++ b/bin/miden-cli/tests/cli.rs @@ -1539,9 +1539,19 @@ fn call_test_exports( library: &miden_client::assembly::Library, ) -> Vec { use miden_client::vm::{PackageExport, ProcedureExport, QualifiedProcedureName}; - use midenc_hir_type::{CallConv, FunctionType, Type}; - - let signature_overrides: [(&str, FunctionType); 3] = [ + use midenc_hir_type::{CallConv, FunctionType, StructType, Type}; + + // The `account-id` core type as the compiler records it: a named record of two field + // elements. Its name is what the CLI's `account-id` codec matches against. + let account_id = Type::Struct(Arc::new(StructType::named( + Arc::from("miden:base/core-types@1.0.0/account-id"), + [ + (Arc::::from("prefix"), Type::Felt), + (Arc::::from("suffix"), Type::Felt), + ], + ))); + + let signature_overrides: [(&str, FunctionType); 5] = [ ( "add", FunctionType::new(CallConv::ComponentModel, [Type::Felt, Type::Felt], [Type::Felt]), @@ -1555,6 +1565,14 @@ fn call_test_exports( ), ), ("read_advice", FunctionType::new(CallConv::ComponentModel, [], [Type::Felt])), + ( + "take_account_id", + FunctionType::new(CallConv::ComponentModel, [account_id.clone()], [account_id.clone()]), + ), + ( + "account_id_suffix", + FunctionType::new(CallConv::ComponentModel, [account_id.clone()], [Type::Felt]), + ), ]; let mut exports = Vec::new(); @@ -1621,6 +1639,20 @@ fn build_call_test_masp(out_path: &Path) { add exec.sys::truncate_stack end + + @account_procedure + pub proc take_account_id + # Identity over the two felts of an account id, so the typed decoder can be checked + # against the value that was encoded. + nop + end + + @account_procedure + pub proc account_id_suffix + # Drops the prefix and returns the suffix, so a swapped field order cannot pass + # unnoticed the way it does through the identity above. + drop + end "#; let library: Library = CodeBuilder::default() @@ -1829,10 +1861,86 @@ fn call_with_advice_inputs() { ); let stdout = String::from_utf8_lossy(&output.stdout); + assert_eq!(output_line(&stdout, "Result:"), "Result: 22felt"); +} + +/// Returns the single line of `stdout` that starts with `prefix`, so a test can compare the whole +/// line. A fragment match would also accept a longer value that starts the same way. +fn output_line<'a>(stdout: &'a str, prefix: &str) -> &'a str { + let mut matching = stdout.lines().filter(|line| line.starts_with(prefix)); + let line = matching + .next() + .unwrap_or_else(|| panic!("no line starts with `{prefix}`:\n{stdout}")); + assert!( + matching.next().is_none(), + "more than one line starts with `{prefix}`:\n{stdout}" + ); + line +} + +/// Tests the typed encode/decode path: an `account-id` hex token is expanded to two felts on +/// the way in and rendered back as `account-id(0x..)` on the way out. +#[test] +fn call_typed_account_id_roundtrip() { + let (temp_dir, account_id, masp_path) = setup_call_test_account(); + + let acct_hex = "0xaa0000000000bb110000cc000000dd"; + let mut cmd = cargo_bin_cmd!("miden-client"); + cmd.args([ + "call", + &format!("{account_id}:take_account_id"), + acct_hex, + "--package", + masp_path.to_str().unwrap(), + ]); + + let output = cmd.current_dir(&temp_dir).output().unwrap(); + assert!( + output.status.success(), + "Call failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert_eq!( + output_line(&stdout, "Signature:"), + "Signature: take_account_id(account-id) -> account-id" + ); + assert_eq!(output_line(&stdout, "Result:"), format!("Result: account-id({acct_hex})")); +} + +/// Tests that the two felts of an `account-id` argument reach the procedure in signature order. +/// The identity round-trip above cannot show this: encoding and decoding would agree even if both +/// had the fields the wrong way around. +#[test] +fn call_typed_account_id_field_order() { + let (temp_dir, account_id, masp_path) = setup_call_test_account(); + + let acct_hex = "0xaa0000000000bb110000cc000000dd"; + let [_prefix, suffix]: [Felt; 2] = AccountId::from_hex(acct_hex).unwrap().into(); + + let mut cmd = cargo_bin_cmd!("miden-client"); + cmd.args([ + "call", + &format!("{account_id}:account_id_suffix"), + acct_hex, + "--package", + masp_path.to_str().unwrap(), + ]); + + let output = cmd.current_dir(&temp_dir).output().unwrap(); assert!( - stdout.contains("Result: 22"), - "Expected advice-derived result in output:\n{stdout}" + output.status.success(), + "Call failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + assert_eq!( + output_line(&stdout, "Signature:"), + "Signature: account_id_suffix(account-id) -> felt" ); + assert_eq!(output_line(&stdout, "Result:"), format!("Result: {suffix}felt")); } /// Tests that calling a `add` with the wrong number of arguments fails diff --git a/docs/external/src/rust-client/cli/index.md b/docs/external/src/rust-client/cli/index.md index 4014d5be8..73fe7e5bd 100644 --- a/docs/external/src/rust-client/cli/index.md +++ b/docs/external/src/rust-client/cli/index.md @@ -440,7 +440,21 @@ Usage: `miden-client call : [ARGS]... --package The target is a single argument of the form `:`. The account ID may be given as a partial ID. The procedure name is matched against the package's exports with `_` and `-` treated as equivalent, so it can be written in either snake_case or kebab-case (`get_count` matches the export `get-count`). -Arguments are passed positionally after the target. Each one is a `u64` field element, and they are pushed onto the stack so that the first argument ends up on top. Their number is checked against the procedure's signature in the package manifest. If the package does not record a signature, the check is skipped and a warning is printed, in which case passing the wrong number of arguments may fail or produce a wrong result. +Arguments are passed positionally after the target, one token per value in the procedure's signature. The signature comes from the package manifest, and it also decides how each token is read and how the result is printed: + +| Type | Token form | Example | +| ---- | ---------- | ------- | +| `felt` | decimal field element | `42` | +| integers (`u8`…`u128`, `i8`…`i128`) | decimal, range-checked against the type | `-1` | +| `bool` | `true`, `false`, `1` or `0` | `true` | +| `word` | hex | `0x00..` | +| `account-id` | hex account ID | `0x4614b8bf575eab71455e97bd394e90` | +| `asset` | `::`, fungible only | `100::0xabcdef0123456789` | +| records and fixed arrays | one token per field, in order | `3 4` for `point { x, y }` | + +Only procedures exported from a WIT interface carry a signature. A procedure without one cannot be called, because there is nothing to encode its arguments against. + +The arguments are pushed onto the stack so that the first one ends up on top, and together they may occupy at most 16 stack values — that is all a called procedure can see. `--inputs-path` takes the same TOML format as [`exec`](#exec). The entries are loaded into the VM's advice map and are visible to the called procedure. @@ -455,7 +469,7 @@ miden-client call 0x4614b8bf575eab71455e97bd394e90:increment-count --package tar The command first prints the procedure's signature and its return values, then the effects the call has on the account: ```sh -Raw Signature: increment-count() -> (Felt) +Signature: increment-count() -> felt Result: 1 The transaction will have the following effects: @@ -477,6 +491,8 @@ Account Vault will not be changed. Nonce incremented by: 1. ``` +A procedure that only reads leaves the transaction with no effects at all, which the transaction kernel does not allow. The result is still printed, followed by a note that the transaction was rejected for having no effects. + :::note The call is executed locally. No proof is generated, nothing is submitted to the network, and the account's stored state is left unchanged. :::