# feat(sdk): typed transaction script arguments for #[tx_script] - #1296
Open
greenhat wants to merge 13 commits into
Open
# feat(sdk): typed transaction script arguments for #[tx_script]#1296greenhat wants to merge 13 commits into
#[tx_script]#1296greenhat wants to merge 13 commits into
Conversation
Collaborator
|
Sadly this has to be rebased again. I really think we should remove the |
Contributor
Author
We tried removing them before, but I had to add them back due to the protocol crates drift. See #1126 |
greenhat
force-pushed
the
i1291-tx-script-redesign
branch
from
August 3, 2026 05:26
26c64fe to
e787518
Compare
Contributor
Author
|
@bitwalker Rebased and ready |
Transaction scripts received the raw TX_SCRIPT_ARGS word and had to hand-roll the advice-provider plumbing for anything larger than one word, with the host mirroring the same layout, padding, and hash by hand — silent agreements that could drift apart. Implements option B of #1291. The new miden-tx-script-args crate defines ScriptArgs/EncodedScriptArgs with a blanket impl for every FromFeltRepr + ToFeltRepr type: encodings of at most 4 felts travel packed directly in the args word, longer or variable-length ones travel through the advice provider hash-verified against the args word, and the mode is a compile-time property derived from the new FromFeltRepr::FIXED_LEN constant (required on manual impls; the derive computes it). encode is pure and the guest-only transport is a wasm-target-scoped dependency, so off-chain code can build script arguments without depending on any on-chain SDK crate.
A required FIXED_LEN associated constant was a source break for every manual FromFeltRepr implementation, and a hand-written value is the one place the length invariant can silently go wrong. Default the constant to None instead: manual implementations keep compiling unchanged, and None is fail-safe — consumers that dispatch on the length, like the tx-script args transport, just fall back to the always-correct commitment mode. The derive still computes exact values, so derived types keep word-mode transport. This also removes the migration section and the BREAKING changelog entry for the constant.
…ormatting in-VM A manual FromFeltRepr implementation with a wrong FIXED_LEN of at most 4 felts previously hit only a debug_assert on the host, after which release builds silently truncated the encoding to the args word — building a transaction with wrong arguments and no error anywhere. Promote the length check to a hard assert that runs before the transport mode is selected, and cover the lying-implementation case with a unit test. Slim the guest decode path: panic messages are unobservable in-VM (the generated panic handler traps without formatting), so decode failures now trap directly under cfg(miden) while native builds keep a formatted panic using FeltReprError's Display. The transport-mode branch is const-evaluated so the dead path is never codegenned, zero felts use Felt::ZERO, and the helpers are inlined. Net effect on the basic-wallet example script: 5813 -> 5742 cycles and 16628 -> 16113 bytes. Extract the post-load commitment decoding into a target-independent decode_preimage helper so the canonical-padding rules (only zero felts, fewer than one word, may remain) are unit-tested natively.
The negative mock-chain tests accepted any execution failure, and the commitment tamper test altered a felt the guest itself asserts on — so removing the in-VM hash verification entirely would have left the suite green. Each negative case now asserts the error code of the specific mechanism that must fire, and the cases isolate the properties one by one: a valid preimage registered under a wrong args word (fails only because of the hash check), a tampered felt the script never reads (new unasserted Vec field on the commitment fixture, which also covers variable-length decoding in the VM), and self-consistent malformed preimages — non-zero padding, a whole extra zero word, and a non-word-multiple advice value — that pass the hash check and must be rejected by the decode canonicality rules. The word-mode fixture entrypoint is renamed to a non-run name to cover the macro's method_path/export_name split end-to-end, and the host mirrors pin their encoded sizes so layout drift against the guest structs fails loudly.
The migration guide's host-side snippet elided the felt-type conversion between the miden-field types that EncodedScriptArgs carries and the protocol crates' felt type, so pasting it produced type errors with no hint; the snippet now shows the conversion. The Preimage variant docs claimed commitment mode implies the encoding exceeds one word (variable-length types use it at any size) and never named the hash the host must apply — Poseidon2 is the contract, so the variant and trait docs now name it. The trait doc also overclaimed that encoder and decoder always agree on how the args word is interpreted; only the transport mode is guaranteed, the type definitions themselves must match. Also corrects the felt-repr derive doc's claim that any non-zero felt decodes as true (only 0/1 are accepted).
Renames the example script-args struct to TxScriptArgs with an args binding, matching the feature's own vocabulary (ScriptArgs, tx_script_args) and avoiding a collision with the SDK's distinct note-inputs concept; the macro rustdoc, migration guide, and harness mirror follow. Hygiene from review: the felt-repr const-fold helpers are doc(hidden) derive plumbing; the generated guest entrypoint ident is built from EXPORT_NAME instead of a hardcoded fn run next to the const; is_unit_return_type and is_type_named are deduplicated from script/note/component macros into util; the unreachable receiver arm and an unused Clone derive are cleaned up; WORD_FELTS aliases Word::NUM_ELEMENTS; the duplicate check-cfg declaration is dropped from the crate manifest (build.rs already registers it); and the tx-script fixture generator documents why it omits internal-wit-emit.
The felt count computation truncated the u64 word count to usize (32-bit on wasm32) and multiplied unchecked, while the unsafe extern write still received the original num_words felt — an advice value of 2^30 words or more would have the VM write past the buffer sized from the truncated value. Not practically reachable (the host would need to materialize a ~128 GB advice value), but commitment-mode ScriptArgs decoding now routes a host-supplied advice length through this function for every transaction script, and the unsafe call's capacity precondition should not depend on that. Guard with a single native felt comparison against 2^30 before the conversion, which makes the truncating cast provably lossless. Full checked casts and checked_mul lower expensively on the VM (+330 cycles and +2.3 KB on the basic-wallet example script); the felt comparison costs 38 cycles and 196 bytes, reflected in the re-baselined expects and the changelog cost note. The guard also pushes adv_load_preimage over the inliner threshold: the body falls out of line, and each advice load in the batch kernel pays call overhead, +3222 cycles (+10%) on a two-transaction batch. Force #[inline(always)] to bound the guard cost at the per-call felt comparison, and re-baseline the batch-kernel expects for it: +241/+202 cycles on the batch scenarios (~0.7%), +1022 bytes of MAST from cross-site inlining.
Baking the panic into ScriptArgs::decode forced cfg'd trap machinery into the transport crate and left off-chain callers (and the native unit tests) with panics where they want values. decode now returns Result<_, ScriptArgsError> — Decode(FeltReprError) plus the transport-specific NonZeroPadding, TrailingData, and NonWordMultipleLength — and the #[tx_script]-generated wrapper matches on it and panics with a static message, so the on-chain behavior is unchanged (a malformed encoding fails the transaction) while off-chain code handles errors as values. The trap helpers disappear from the crate, the canonicality unit tests assert error values instead of panics, and off-VM commitment decoding stays unimplemented!() since the advice provider is a capability, not a data error. All in-VM decode failures now funnel through the wrapper's single panic, so the e2e negative cases share one error code while the stdlib hash-verification failure keeps its own, preserving the hash-isolation property. Keeping error values alive (instead of dead-ending into traps that let the optimizer discard error construction) costs 6133 vs 5780 cycles and 19110 vs 16309 bytes on the basic-wallet example; the changelog cost note is updated accordingly.
…cies The dependency updates that landed on next resolve differently in the examples' and test fixtures' standalone lockfiles, which regenerate on their first compile after the rebase — mostly collapsing previously duplicated dependency versions. Committing the refreshed locks keeps test runs from dirtying the working tree.
Off-chain ergonomics: commitment-mode decode off the VM now returns ScriptArgsError::AdviceProviderUnavailable instead of panicking, so decode is uniformly fallible regardless of the type's transport mode, and decode_preimage is public — the pure half of commitment decoding for host-side tests and tooling. Dependency cone: the guest transport dependency on miden-stdlib-sys is optional behind a new miden-vm-guest feature that the miden SDK crate enables, so wasm hosts (browser dapps, WASI services) no longer pull the on-chain bindings and the crate's off-chain claim holds on every target. Contract hardening: ScriptArgs is sealed to its blanket implementation so the documented transport guarantees hold for every implementor; decode now verifies a manual implementation consumed exactly its declared FIXED_LEN (the encode side was already checked); and Vec's felt-repr decoding bounds its reservation by the reader's remaining input, so a hash-consistent preimage with a lying length prefix errors instead of aborting on allocation. Macro contract edges: the generated wrapper calls the entrypoint through self:: so a function named arg no longer collides with the wrapper parameter, unsafe fn and explicit ABIs are rejected with clean macro errors, and parenthesized parameter types are accepted.
…ort pairing The migration guide's mirror recipe now says what actually must match — the felt-repr wire sequence, not the Rust fields — and tells hosts to pin the mirror with a FIXED_LEN assert and a golden encoding, because one drift direction fails silently: a word-mode guest type whose host mirror grows past 4 felts switches the host to commitment mode and the guest decodes the hash limbs as argument values. The in-repo harness gains the golden-encoding pin it recommends. Also records the adv_load_preimage behavior change (truncating huge word counts into an undersized buffer replaced by a trap at 2^30 words) in the stdlib-sys changelog and on the function's doc, explains why the miden facade exports the felt-repr crate under two names, and documents that a manual FromFeltRepr impl inheriting the defaulted FIXED_LEN silently makes containing types variable-length.
Structural and readability items from review: the encode padding loops become resize calls and its length assert drops the formatting variant; the two decode_commitment cfg variants sit adjacent with decode_preimage below them; the hoisted is_type_named predicate reverts to the strict PathArguments::None check it replaced (empty angle brackets no longer match) matching the component-macro copies; the guest-wrapper scaffolding becomes private to the script module and its trait_impls parameter is named for what it receives; the e2e tests name the shared decode-panic error code once; and the harness test module and util helpers move out of the middle of their files. The reviewed fixed-length fast path for commitment decoding was implemented, measured, and dropped: it saved 30 cycles but cost 1044 bytes of package size on the example (both u64 length paths codegen), which is the wrong trade.
… base Rebasing onto the released base left three follow-ups the replayed commits could not carry: the compiler improvements on next shrank the compiled tx-script example, the changelog cost note quoted the old size delta, and the workspace lock still recorded miden-tx-script-args at its pre-release version. Re-baseline the example package-size expect from 19110 to 13556 bytes (cycle pins are unaffected), restate the typed-decoding overhead against the new hand-rolled baseline (about 4.4 KB), and refresh the lock entry to 0.14.0-rc.1.
greenhat
force-pushed
the
i1291-tx-script-redesign
branch
from
August 7, 2026 14:15
e787518 to
c573b3a
Compare
Contributor
Author
|
Rebased and ready. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1291
Implements option B of #1291: a transaction script's inputs are a plain data struct passed to the entrypoint by value and unpacked automatically, replacing the raw
Wordargument that scripts had to decode by hand.The entrypoint is a free function of any name taking a required by-value script-args parameter and an optional
#[account(...)]reference, in either order. Existingfn run(arg: Word, ...)entrypoints keep compiling and behave unchanged, sinceWorddecodes as itself.Transport lives in the
ScriptArgstrait, implemented for everyFromFeltRepr + ToFeltReprtype via a blanket impl, with the mode selected at compile time from the newFromFeltRepr::FIXED_LENconstant: encodings of at most 4 felts are packed directly into theTX_SCRIPT_ARGSword (unused felts are zero and asserted so), while longer or variable-length encodings travel through the advice provider keyed by their Poseidon2 hash, whichadv_load_preimageverifies in-VM so the host cannot substitute values. Making the mode a compile-time property of the type guarantees the host-side encoder and guest-side decoder cannot disagree on how the args word is interpreted.ScriptArgs::decodeis fallible (ScriptArgsError): the generated entrypoint wrapper panics on decode errors, failing the transaction, while off-chain code handles them as values.ScriptArgs::encodereturnsEncodedScriptArgs::{Word, Preimage}and the caller computes the commitment hash, keeping crypto dependencies out of the SDK.The trait lives in a new
miden-tx-script-argscrate (re-exported frommiden) whose off-chain dependency cone is onlymiden-fieldandmiden-field-repr— the guest-side transport is a wasm-target-scoped dependency, so hosts encoding script arguments never build the on-chain SDK bindings.