diff --git a/.claude/skills/advice-provider-hygiene/SKILL.md b/.claude/skills/advice-provider-hygiene/SKILL.md index 878b84e655..2ea9f09cff 100644 --- a/.claude/skills/advice-provider-hygiene/SKILL.md +++ b/.claude/skills/advice-provider-hygiene/SKILL.md @@ -81,6 +81,8 @@ exec.mem::pipe_preimage_to_memory ### Content-addressed keys and missing entries +Illustrative fragments (with the key already on the operand stack): + ```masm # Good: key the advice map entry by the commitment itself push.NOTE_DATA_COMMITMENT @@ -90,12 +92,13 @@ adv.push_mapval push.0x1234_5678_0000_0001 adv.push_mapval -# Good: a missing required entry is an error +# Good: move the advice-stack presence flag before asserting it adv.has_mapkey +adv_push assert.err=ERR_MISSING_REQUIRED_ADVICE -# Bad: silent zero on missing key -adv.push_mapval # no-op if key absent; proceed as if zero +# A direct lookup also errors if the key is absent; it never substitutes a default +adv.push_mapval ``` For the Rust analog (returning `Err` on bad/missing external input rather than panicking or defaulting), see `return-error-not-panic`. diff --git a/.claude/skills/cheap-masm-equivalents/SKILL.md b/.claude/skills/cheap-masm-equivalents/SKILL.md index 8d2d6cb8c5..1534a12b1c 100644 --- a/.claude/skills/cheap-masm-equivalents/SKILL.md +++ b/.claude/skills/cheap-masm-equivalents/SKILL.md @@ -36,15 +36,15 @@ push.0 gt # same answer, 16 cycles ```masm # Good: cdrop for ternary selection -# stack: [b, a, cond] +# stack: [c, b, a] cdrop -# stack: [a if cond else b] +# stack: [b if c = 1 else a] # Bad: branchy equivalent if.true - drop # drop b, keep a -else swap drop # drop a, keep b +else + drop # drop b, keep a end ``` diff --git a/.claude/skills/decouple-component-from-storage/SKILL.md b/.claude/skills/decouple-component-from-storage/SKILL.md index 6fa822adfc..3522118b56 100644 --- a/.claude/skills/decouple-component-from-storage/SKILL.md +++ b/.claude/skills/decouple-component-from-storage/SKILL.md @@ -1,19 +1,21 @@ --- name: decouple-component-from-storage -description: Use when writing a MASM procedure inside a reusable account component that accesses account storage — receive the storage slot as a parameter so the component is portable across storage layouts. +description: Use when writing a generic MASM utility that accesses a caller-selected account storage slot — receive the slot as a parameter so the utility is portable across storage layouts. --- -# Decouple Component Procedures from Storage Layout +# Decouple Generic Utilities from Storage Layout ## Rule -A reusable account component must not bake a storage-slot index into its procedure bodies. The same component can be installed into many accounts, each mapping it to a different slot, so a hard-coded slot index only works for one layout. +A generic storage utility that operates on a caller-selected slot must not bake that slot into its procedure body. Hard-coding one caller's slot makes the utility work for only that storage layout. Instead, take the storage slot as a parameter — the slot id, split into its `slot_id_prefix` / `slot_id_suffix` felts — and pass it into the storage-access procedure (`active_account::get_item` / `get_map_item`, `native_account::set_item` / `set_map_item`). The account-level glue procedure that knows the real layout supplies the slot id. +Component-owned named slots are different. A named slot's ID is derived deterministically from its stable name, so a component may reference its own slot by that name. The `authority` component follows this pattern with `AUTHORITY_SLOT`; generic array utilities accept caller-supplied slot IDs instead. + ## Why -A component installed into different accounts sits at a different storage slot in each. Hard-coding the slot ties the component to one layout and silently misreads storage everywhere else; taking the slot id as a parameter makes the procedure portable. +Taking a caller-selected slot as a parameter makes a generic utility portable. Referencing a component-owned slot by its stable name keeps that component's storage identity deterministic across accounts. ## Examples @@ -30,7 +32,8 @@ end push.index push.MY_SLOT_ID_PREFIX push.MY_SLOT_ID_SUFFIX exec.get -# Bad: the component hard-codes its own slot, so it only works at that one layout +# Also good: a component references its own deterministic named slot +pub const AUTHORITY_SLOT = word("miden::standards::access::authority::authority_config") pub proc get_authority push.AUTHORITY_SLOT[0..2] exec.active_account::get_item end diff --git a/.claude/skills/felt-construction/SKILL.md b/.claude/skills/felt-construction/SKILL.md index 0f1b627f8e..774b20bdce 100644 --- a/.claude/skills/felt-construction/SKILL.md +++ b/.claude/skills/felt-construction/SKILL.md @@ -1,23 +1,23 @@ --- name: felt-construction -description: Use when constructing a `Felt` from a numeric value in Rust — avoid silently truncating values that may exceed the field modulus. +description: Use when constructing a `Felt` from a numeric value in Rust — use checked construction unless the canonical bound is already proved. --- # Felt Construction From Untrusted Numeric Inputs ## Rule -Do not call `Felt::new(x)` when `x` could exceed the field modulus. `Felt::new` silently truncates oversized values, which produces a valid-looking `Felt` that no longer equals the original input — a classic source of hard-to-attribute bugs. +`Felt::new(x)` is checked and returns `Result`, rejecting values greater than or equal to `Felt::ORDER`. Use it when a `u64` input may exceed the field modulus. Use one of: - `Felt::from(x)` where `x` is a `u32` or smaller (infallible). -- `Felt::try_from(x)` for `u64`-and-larger inputs, returning `Result`. -- An explicit `assert!(x < Felt::MODULUS)` before `Felt::new(x)` if you have already proven the bound. +- `Felt::new(x)` or `Felt::try_from(x)` for `u64` inputs; both return `Result` and check the bound. +- `Felt::new_unchecked(x)` only when `x < Felt::ORDER` has already been proved. ## Why -The field modulus sits just below `2^64`, so `Felt::new` truncates only for a narrow band of large values — most tests pass and production hits the bad input as a value mismatch far from the call. `Felt::from(u32)` cannot truncate and `Felt::try_from` forces the bound check. +The field modulus sits just below `2^64`, so out-of-range inputs occupy a narrow band that tests can miss. Checked construction makes those inputs explicit errors; `new_unchecked` skips that protection. ## Examples @@ -25,9 +25,13 @@ The field modulus sits just below `2^64`, so `Felt::new` truncates only for a na // Good: u32 input, infallible conversion let f = Felt::from(slot_index as u32); -// Good: untrusted u64 input, checked conversion -let f = Felt::try_from(user_value).map_err(|_| Error::FeltOverflow)?; +// Good: untrusted u64 input, checked conversion returning Result +let f = Felt::new(user_value).map_err(|_| Error::FeltOverflow)?; -// Bad: silent truncation on any value >= MODULUS -let f = Felt::new(user_value); +// Good: unchecked construction only after proving the canonical bound +assert!(bounded_value < Felt::ORDER); +let f = Felt::new_unchecked(bounded_value); + +// Bad: unchecked construction on an untrusted value +let f = Felt::new_unchecked(user_value); ``` diff --git a/.claude/skills/masm-inline-comments/SKILL.md b/.claude/skills/masm-inline-comments/SKILL.md index 165a09ad14..874047729f 100644 --- a/.claude/skills/masm-inline-comments/SKILL.md +++ b/.claude/skills/masm-inline-comments/SKILL.md @@ -14,7 +14,7 @@ Inline comments (single `#`) should begin with a lowercase letter. ```masm # good: lowercase start exec.native_account::remove_asset -# => [ASSET, note_idx, pad(11)] +# => [FINAL_ASSET_VALUE, note_idx, pad(11)] # Bad: uppercase start (avoid) # Remove the asset from the account @@ -31,7 +31,7 @@ Only apply this rule to new code you write. Do not remove comments that are pres - Standard control flow: `if.true`, `while.true`, `end` **Do comment:** -- Stack state after complex operations: `# => [ptr, ASSET, end_ptr]` +- Stack state after complex operations: `# => [ptr, ASSET_ID, ASSET_VALUE, end_ptr]` - Purpose of a code block: `# compute the pointer at which we should stop iterating` - Non-obvious logic or business rules - TODO items and references to external specs @@ -50,11 +50,16 @@ This pairs each stack state visually with the operation that produced it and let **Good:** ```masm +# => [ASSET_ID, ASSET_VALUE, note_idx, pad(7)] + +dupw.1 dupw.1 +# => [ASSET_ID, ASSET_VALUE, ASSET_ID, ASSET_VALUE, note_idx, pad(7)] + exec.native_account::remove_asset -# => [ASSET, note_idx, pad(11)] +# => [FINAL_ASSET_VALUE, ASSET_ID, ASSET_VALUE, note_idx, pad(7)] -dupw dup.8 movdn.4 -# => [ASSET, note_idx, ASSET, note_idx, pad(11)] +dropw +# => [ASSET_ID, ASSET_VALUE, note_idx, pad(7)] ``` **Also OK (no blank line before `end` or control flow):** @@ -69,7 +74,7 @@ end An inline `# => [...]` tracker uses the same item names, capitalization, and `(N)` span notation as the `#!` doc block for the enclosing procedure (see masm-doc-comments skill): - Single-felt names stay lowercase: `note_idx`, `final_nonce`. -- Word names stay UPPERCASE: `ASSET`, `RECIPIENT`. +- Word names stay UPPERCASE: `ASSET_ID`, `ASSET_VALUE`, `RECIPIENT`. - `(N)` spans stay lowercase: `pad(12)`, `foreign_procedure_inputs(15)`. Composite names like `account_id_{suffix,prefix}` are a doc-block shorthand for a group of felts. In inline trackers they decompose into their individual felts since each felt occupies one stack slot: @@ -136,15 +141,18 @@ dup **Good:** ```masm -# remove the asset from the account +# preserve the asset before removing it from the account +dupw.1 dupw.1 +# => [ASSET_ID, ASSET_VALUE, ASSET_ID, ASSET_VALUE, note_idx, pad(7)] + exec.native_account::remove_asset -# => [ASSET, note_idx, pad(11)] +# => [FINAL_ASSET_VALUE, ASSET_ID, ASSET_VALUE, note_idx, pad(7)] -dupw dup.8 movdn.4 -# => [ASSET, note_idx, ASSET, note_idx, pad(11)] +dropw +# => [ASSET_ID, ASSET_VALUE, note_idx, pad(7)] exec.output_note::add_asset -# => [ASSET, note_idx, pad(11)] +# => [pad(16)] ``` **Avoid:** diff --git a/.claude/skills/masm-locals-over-globals/SKILL.md b/.claude/skills/masm-locals-over-globals/SKILL.md index 2533d451d0..61df586805 100644 --- a/.claude/skills/masm-locals-over-globals/SKILL.md +++ b/.claude/skills/masm-locals-over-globals/SKILL.md @@ -7,7 +7,7 @@ description: Use when a MASM procedure needs temporary scratch storage — keep ## Rule -When a MASM procedure needs scratch storage that lives only for the duration of one invocation, use procedure-local memory (`loc_store`, `loc_load`, `loc_storew`, `loc_loadw`) rather than allocating in a shared global memory region. +When a MASM procedure needs scratch storage that lives only for the duration of one invocation, use procedure-local memory (`loc_store`, `loc_load`, `loc_storew_be` / `loc_loadw_be`, or the corresponding `_le` word forms) rather than allocating in a shared global memory region. Global memory regions are reserved for state that crosses procedure boundaries (kernel inputs, account data, advice-keyed state). Stashing per-call scratch there leaks an implementation detail into a shared namespace and ties the procedure to a fixed address. @@ -21,8 +21,9 @@ Procedure locals are allocated and freed by the VM, so two callers of the same p ```masm # Good -proc compute_hash - # allocate two local slots +@locals(2) +proc compute_hash(a: felt, b: felt) -> (felt, felt) + # store two scratch values in local slots loc_store.0 loc_store.1 # ... @@ -32,7 +33,7 @@ end # Bad: scratch in a shared region const SCRATCH_PTR = 0x4000 -proc compute_hash +proc compute_hash(value: felt) mem_store.SCRATCH_PTR # collides with anyone else using SCRATCH_PTR end ``` diff --git a/.claude/skills/masm-padding/SKILL.md b/.claude/skills/masm-padding/SKILL.md index 149be39e9b..fed49e474c 100644 --- a/.claude/skills/masm-padding/SKILL.md +++ b/.claude/skills/masm-padding/SKILL.md @@ -44,7 +44,8 @@ Not: This shows up most often at the start of note scripts that don't use their input arguments: ```masm -begin +@note_script +pub proc main(args: NoteArgs) dropw # => [pad(16)] ... @@ -124,29 +125,22 @@ These extra elements must be explicitly dropped before the procedure returns (di ## Debugging Stack Depth -When unsure whether the stack matches the depth you expect, use the assembly's debug instructions to inspect it at runtime. These cost zero VM cycles, do not affect the program hash, and are stripped at compile time when the assembler is not in debug mode. - -- `debug.stack` – print the full operand stack. -- `debug.stack.N` – print only the top N elements (1 ≤ N < 256). -- `sdepth` – push the current stack depth onto the stack as a felt; useful when you need depth as a runtime value, e.g. to assert it: +Use the event-based procedures in `miden::core::debug` to inspect VM state. These are ordinary procedure calls: they emit print events whenever invoked, affect the program being executed, and consume cycles (`print_stack` costs 3 cycles). Remove them from production programs. - ```masm - sdepth push.16 eq assert.err="depth must be 16 here" - ``` - -Run with the `--debug` flag to see output: +```masm +use miden::core::debug -```bash -miden-vm run program.masm --debug +begin + exec.debug::print_stack + sdepth push.16 eq assert.err="depth must be 16 here" +end ``` -Without `--debug`, debug instructions are silently removed. Remove or comment out `debug.*` lines before committing production MASM. - ## Validation Checklist For all invocation types: - [ ] Inline `# =>` trackers reflect the post-auto-pad depth (never below 16) at boundaries that enforce the floor (`call`, note scripts, tx scripts) -- [ ] No `debug.*` instruction is left in production MASM +- [ ] No `miden::core::debug` procedure call is left in production MASM For `call` procedures: - [ ] Inputs doc comment shows exactly 16 elements with `pad(N)` diff --git a/.claude/skills/u32-assert-before-u32-ops/SKILL.md b/.claude/skills/u32-assert-before-u32-ops/SKILL.md index ea22f5397b..999d50cf60 100644 --- a/.claude/skills/u32-assert-before-u32-ops/SKILL.md +++ b/.claude/skills/u32-assert-before-u32-ops/SKILL.md @@ -1,39 +1,41 @@ --- name: u32-assert-before-u32-ops -description: Use when writing MASM `u32*` instructions on values from user input or untrusted sources — ensure the operands are valid u32s first. +description: Use when writing MASM `u32*` instructions whose operands must already fit in 32 bits, especially for user input or untrusted values. --- -# Validate u32 Operands Before u32 Instructions +# Validate Required u32 Operands ## Rule -MASM's `u32*` instructions assume their operands are valid `u32` values (i.e. fit in 32 bits). Operating on a non-u32 value silently produces garbage or traps with a generic message. +Most MASM `u32*` arithmetic instructions require operands that fit in 32 bits. Their behavior on an out-of-range operand is undefined, so the executor may trap and the resulting proof is not valid. -Before applying any `u32*` instruction to a value that is not already known to be a valid u32 (e.g. it came from the stack as input, was read from memory, or arose from a non-u32 arithmetic op), assert the bound: +Before applying a `u32*` instruction whose documented precondition requires valid u32 operands, assert the bound of any operand that is not already known-valid (e.g. it came from the stack as input, was read from memory, or arose from a non-u32 arithmetic op): ```masm u32assert # one value u32assert2 # two top values -u32assert4 # four top values +u32assertw # one word (four values) ``` -If the operand is already known-valid (just produced by another `u32*` op, or a value loaded from a slot whose layout is u32 by construction), skip the assert. +If the operand is already known-valid (just produced as a valid-u32 output of another operation, or loaded from a slot whose layout is u32 by construction), skip the assert. + +`u32test`, `u32testw`, `u32cast`, and `u32split` accept arbitrary field values, so they do not require a prior u32 assertion. ## Why -`u32*` instructions are tuned for the precondition that operands fit in 32 bits, and the VM does not check it for you. Skipping `u32assert*` lets a non-u32 input silently produce a wrong result or trap uninformatively; the assert gives the bug a named failure mode. +Arithmetic instructions do not check the u32 precondition for you. An explicit assertion prevents undefined behavior and gives an out-of-range input a clear failure mode. ## Examples ```masm -# Good: assert u32 before the u32 op -u32assert.err=ERR_VALUE_NOT_U32 -u32add +# Good: assert both operands before producing one wrapping sum +u32assert2.err=ERR_VALUES_NOT_U32 +u32wrapping_add # Good: both operands at once u32assert2.err=ERR_VALUES_NOT_U32 u32lt # Bad: u32 op on untrusted input -u32add # one operand could be >2^32; silently wraps or traps +u32wrapping_add # either operand could be greater than or equal to 2^32 ```