Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Skills are applied automatically when the agent detects relevant tasks. Each ski
### React Frontend Development

- **react-sdk-patterns** – Complete `@miden-sdk/react` hook API reference: MidenProvider, query hooks, mutation hooks, transaction stages, signer integration, utilities
- **frontend-pitfalls** – Critical frontend pitfalls: WASM init race, recursive access crash, COOP/COEP headers, BigInt handling, Bech32 mismatch, IndexedDB state loss
- **frontend-pitfalls** – Critical frontend pitfalls: client-readiness gating, multi-step WASM sequences and pointer lifetimes, COOP/COEP headers, BigInt boundaries, Bech32 network prefixes, IndexedDB state loss, the Web Worker shim, and structured error codes
- **vite-wasm-setup** – Vite + WASM configuration: required plugins, deployment headers (Nginx, Vercel, Cloudflare), TypeScript config, troubleshooting
- **frontend-source-guide** – Advanced frontend development guide: AI practices and miden-client source repository map for discovering patterns beyond basic skills
- **signer-integration** – Integrating external signers (Para, Turnkey, MidenFi wallet adapter) and building custom signers for Miden React frontends
Expand All @@ -29,14 +29,15 @@ Skills are applied automatically when the agent detects relevant tasks. Each ski
- **masm-inline-comments** – Inline commenting conventions for .masm files (lowercase, avoid over-commenting)
- **masm-doc-comments** – Procedure documentation format (`#!` doc blocks with Inputs, Outputs, Where, Panics, Invocation)
- **masm-padding** – Stack padding conventions for `call` vs `exec` procedures
- **masm-formatting** – Orchestrator covering capitalization, `(N)` span notation, cross-repo doc-comment divergences, `Cycles:`, and chained assertion style
- **masm-formatting** – Orchestrator covering capitalization, `(N)` span notation, cross-repo doc-comment divergences, `Cycles:`, chained assertion style, and the `miden-format` formatter
- **masm-proc-type-signatures** – Type-signature conventions for `pub proc`: parameter and return types, semantic type aliases, struct/array/tuple types, and how a signature maps onto the operand stack

### Miden Client (Web SDK & Internals)

- **rust-client-patterns** – Rust conventions for the `miden-client` crate: error handling (`thiserror` + `ErrorHint`), `Store` trait (adding methods across `SqliteStore`/`WebStore` with cross-platform `async_trait`), `Client<AUTH>` generic pattern with the `Keystore` super-trait, `no_std` imports (`alloc::`/`core::`), `ClientBuilder` network constructors (`for_testnet()`, `for_devnet()`, `for_localhost()`), and section header formatting (`// ===` top-level, `// ---` subsections)
- **wasm-bridge** – Rust↔JS WASM boundary conventions for the `web-client` crate: `#[wasm_bindgen]` method exposure with `js_name`, newtype wrappers with `From` conversions, `js_error_with_context` error chaining with `ErrorHint`, promise handling (`await_js`/`await_ok`/`await_js_value`), data transfer objects with `getter_with_clone`, JS function imports, and the `MidenClient`/`WasmWebClient` (`WebClient`) two-layer JS API
- **idxdb-patterns** – IndexedDB/Dexie persistence conventions for the `idxdb-store` crate: Dexie transactions (`db.dexie.transaction("rw", tables, ...)`), schema interfaces (`IAccount`, `IAccountCode`), database registry (`getDatabase`/`openDatabase`), `logWebStoreError` error handling, forward-only state updates, and the TS→JS dual-commit build workflow
- **web-client-usage** – Developer-facing patterns for using the `@miden-sdk/miden-sdk` npm package: `MidenClient.create()` initialization, the resource-based API (`client.accounts`, `client.transactions`, `client.notes`, `client.tags`, `client.settings`, `client.compile`, `client.keystore`), sync ordering, type conversions (`AccountId.fromHex`, `BigInt` amounts, `NoteVisibility`), transaction flows (mint, send, consume, swap, custom contracts), private note transport, querying, import/export, and pitfall avoidance
- **web-client-usage** – Developer-facing patterns for using the `@miden-sdk/miden-sdk` npm package: `MidenClient.create()` initialization, the resource-based API (`client.accounts`, `client.transactions`, `client.notes`, `client.pswap`, `client.tags`, `client.settings`, `client.compile`, `client.keystore`), sync ordering, type conversions (`AccountId.fromHex`, `BigInt` amounts, `NoteVisibility`), transaction flows (mint, send, consume, swap, custom contracts), private note transport, querying, import/export, and pitfall avoidance

## Commands

Expand Down
55 changes: 21 additions & 34 deletions skills/advice-provider-hygiene/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Readers retrieve the entry by recomputing the same hash from data they already t

### 3. Missing advice is an error

A missing advice-map entry, an empty advice stack, or an absent required value is an error — not a default. `adv.push_mapval` / `adv.push_mapvaln` already abort execution when the key is missing (the VM returns `MapKeyNotFound`), so don't paper over it with a fallback. When you branch on presence yourself, surface the failure with `assert.err=ERR_...`. Don't substitute zero / empty / a fallback and continue.
A missing advice-map entry, an empty advice stack, or an absent required value is an error — not a default. Surface it with `assert.err=ERR_...`. Don't substitute zero / empty / a fallback and continue.

## Why

Expand All @@ -45,30 +45,24 @@ Advice data is tied to a commitment by piping it into memory. There are two mech
`adv_pipe`, `adv_loadw`, and `mem::pipe_double_words_to_memory` copy advice data into memory but do *not* check it against any commitment. Hash the loaded region with Poseidon2 yourself and assert it equals a commitment the kernel already trusts.

```masm
# Good: pipe words while hashing, then assert against a trusted commitment.
# This is the input-note-assets path from the transaction prologue:
# pipe_double_words_to_memory runs `adv_pipe exec.poseidon2::permute` internally
# (no commitment check of its own), then you squeeze and assert.
# Good: pipe words while hashing, then assert against the trusted commitment
# (permute rounds abbreviated; the real path pipes the full region before squeezing)
exec.poseidon2::init_no_padding
adv_pipe exec.poseidon2::permute
# ... one permute per piped block ...
exec.poseidon2::squeeze_digest
# => [COMPUTED_COMMITMENT, ...]
exec.memory::get_ref_block_commitment
assert_eqw.err=ERR_PROLOGUE_GLOBAL_INPUTS_PROVIDED_DO_NOT_MATCH_BLOCK_COMMITMENT

# Good: pipe double words while hashing, then assert against the provided commitment
exec.poseidon2::init_no_padding
exec.mem::pipe_double_words_to_memory
exec.poseidon2::squeeze_digest
# => [COMPUTED_ASSETS_COMMITMENT, ...]
exec.memory::get_input_note_assets_commitment
assert_eqw.err=ERR_PROLOGUE_PROVIDED_INPUT_ASSETS_INFO_DOES_NOT_MATCH_ITS_COMMITMENT

# Good: drive the adv_pipe + permute loop yourself, squeeze, then assert.
# (The block-data prologue squeezes a SUB_COMMITMENT here, merges it with the
# trusted NOTE_ROOT to form the block commitment, and only THEN asserts — i.e.
# the squeezed digest is combined with trusted data before the equality check.)
exec.poseidon2::init_no_padding
adv_pipe exec.poseidon2::permute
# ... one `adv_pipe exec.poseidon2::permute` per piped block ...
exec.poseidon2::squeeze_digest
# => [SUB_COMMITMENT, ...] (combine with trusted data as needed, e.g. merge a root)
# ... eventually ...
exec.memory::get_block_commitment
assert_eqw.err=ERR_PROLOGUE_GLOBAL_INPUTS_PROVIDED_DO_NOT_MATCH_BLOCK_COMMITMENT

# Bad: pipe advice into memory and use it without the hash/assert step
adv_pipe
# ... data could be anything the prover supplied
Expand All @@ -87,28 +81,21 @@ exec.mem::pipe_preimage_to_memory

### Content-addressed keys and missing entries

An advice-map key is a full word (4 felts) sitting on top of the operand stack. `adv.push_mapval` reads that word as the key and pushes the looked-up value onto the *advice* stack (the operand stack is unchanged), so you typically follow it with `adv_loadw` or a pipe to bring the value into the operand stack or memory.

```masm
# Good: key the advice map entry by the commitment itself
# (NOTE_DATA_COMMITMENT is the trusted word already on the operand-stack top)
adv.push_mapval # value pushed onto the advice stack, keyed by the commitment word
adv_loadw # => [NOTE_DATA, ...] pull the value into the operand stack
push.NOTE_DATA_COMMITMENT
adv.push_mapval

# Bad: hard-coded magic key (a key is a Word — 4 felts — and this one is meaningless)
push.0x0001.0x0000.0x0000.0x1234
# Bad: hard-coded magic key
push.0x1234_5678_0000_0001
adv.push_mapval

# Good: a missing required entry is an error.
# adv.has_mapkey pushes the presence flag onto the ADVICE stack, so move it to the
# operand stack with adv_push before asserting.
# stack: [KEY, ...]
adv.has_mapkey # advice stack: [has_key, ...]; operand stack unchanged
adv_push # => [has_key, KEY, ...]
# Good: a missing required entry is an error
adv.has_mapkey
assert.err=ERR_MISSING_REQUIRED_ADVICE
Comment on lines +93 to 95

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P2] Move the membership flag from advice to the operand stack

adv.has_mapkey places its result on the advice stack. The following assert therefore checks an existing operand-stack element instead of checking whether the key exists. With a missing key whose top limb is 1, the documented check succeeds. Insert adv_push before assert and retain the key's actual stack shape. Also remove the claim that adv.push_mapval is a no-op for an absent key: the VM raises a missing-advice-map-key error.

Validation: Reproduced the false success with rc.7/devnet and final v0.16/mock. The control with adv_push fails as expected; an absent adv.push_mapval also fails.

Sources: Advice event implementation.


# (adv.push_mapval itself already aborts with MapKeyNotFound on an absent key
# never assume it returns zero and continue.)
# Bad: silent zero on missing key
adv.push_mapval # no-op if key absent; proceed as if zero
```

The Rust analog is to return `Err` on bad or missing external input rather than panicking or silently defaulting.
For the Rust analog (returning `Err` on bad/missing external input rather than panicking or defaulting), see `return-error-not-panic`.
71 changes: 49 additions & 22 deletions skills/cheap-masm-equivalents/SKILL.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: cheap-masm-equivalents
description: Use when writing or reviewing MASM hot paths — prefer the cheaper equivalent instruction: `neq.0` over `push.0 gt` for non-zero checks, `cdrop` over an `if/else` selecting between two values, `dup.N` over `loc_load` for a value still on the stack, `eqw` over hand-rolled element-wise word comparison, `u32gt`/`u32lt` over generic `gt`/`lt` on known-u32 operands.
description: Use when writing or reviewing MASM hot paths or loops — prefer the cheaper equivalent: loop counters and pointers on the operand stack instead of procedure locals, `neq.0` over `push.0 gt` for non-zero checks, `cdrop` over an `if/else` selecting between two values, `dup.N` over `loc_load` for a value still on the stack, `eqw` over hand-rolled element-wise word comparison, `u32gt`/`u32lt` over generic `gt`/`lt` on known-u32 operands.
---

# Prefer Cheap MASM Equivalents
Expand All @@ -9,44 +9,71 @@ description: Use when writing or reviewing MASM hot paths — prefer the cheaper

Several MASM idioms have a cheap and an expensive form. Use the cheap one when both produce the same result on the inputs the procedure can see:

- Non-zero check: `neq.0` (2 cycles) over a comparison-based check like `push.0 gt` (~17 cycles). `neq.0` lowers to `eqz not`; `push.0 gt` does a full field comparison just to learn "not zero".
- Selecting between two values on a flag: `cdrop` (2 cycles) over an `if.true ... else ... end` branch with the same effect.
- Re-fetch a recently-pushed value: `dup.N` (1-3 cycles) over `loc_load.N` (which costs more) when the value is still on the stack.
- Whole-word equality: the single `eqw` instruction (15 cycles) over a hand-rolled element-wise sequence of `eq`/`and`.
- u32-known operands: `u32lt` (3 cycles) / `u32gt` (4 cycles) over generic `lt` (17 cycles) / `gt` (16 cycles).
- Loop variables (counters, pointers, indices): keep them on the operand stack across iterations instead of in procedure locals. See below.
- Non-zero check: `neq.0` (3 cycles) over `gt.0` (16 cycles).
- Selecting between two values on a flag: `cdrop` over an `if.true ... else ... end` branch with the same effect.
- Re-fetch a recently-pushed value: `dup.N` over `loc_load.N` when the value is still on the stack.
- Whole-word equality: `eqw` over element-wise comparisons.
- u32-known operands: `u32gt`/`u32lt` over generic `gt`/`lt`.

Don't apply the cheap form when the operands violate its precondition. `u32lt`/`u32gt` are undefined if either operand is >= 2^32, so the operands must be known (or asserted) to be valid u32s first.

Also note that `gt.0` is only sugar for `push.0 gt`: it parses, but tests `a > 0` via a full field comparison, not `a != 0`. For a genuine non-zero check use `neq.0`; reach for `push.0 gt` / `gt.0` only when you actually want strictly-greater-than-zero.
Don't apply the cheap form when the operands violate its precondition (e.g. `u32gt` on a value that might exceed `u32::MAX`).

## Why

MASM cycle costs are not uniform — `gt`/`lt` do full 64-bit comparison work that `neq` skips, so a hot path using the expensive form pays for it on every call. The swaps are semantically equivalent under their preconditions, so the saving is free. The protocol does this in practice: a loop in `account_delta.masm` carries the comment `# we use neq instead of lt for efficiency`. The Miden assembly docs make the same point for branches: an `if.true ... else ... end` incurs non-negligible overhead, so when both branches just select a value (no incompatible side effects), compute both and select with `cdrop`.
MASM cycle costs are not uniform — `gt.0` does signed-comparison work that `neq.0` skips, so a hot path using the expensive form pays for it on every call. The swaps are semantically equivalent under their preconditions, so the saving is free.

## Examples

```masm
# Good: non-zero check, 2 cycles (lowers to `eqz not`)
# Good
push.0 neq # non-zero check, 3 cycles
# or simply
neq.0

# Bad: full field comparison just to test "not zero", ~17 cycles
push.0 gt
# Bad
push.0 gt # same answer, 16 cycles
```

```masm
# Good: cdrop for ternary selection (condition on TOP), 2 cycles
# stack: [cond, b, a]
# Good: cdrop for ternary selection
# stack: [b, a, cond]
cdrop
# stack: [b if cond else a] (cond=1 keeps b, cond=0 keeps a; fails if cond > 1)
# stack: [a if cond else b]
Comment on lines +38 to +41

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[P2] Correct cdrop's stack order and branch equivalence

cdrop consumes the condition from the top of the stack, so the documented [b, a, cond] arrangement tests b as a boolean and can trap. Moving the condition to the top still does not make the two snippets equivalent: cdrop selects the opposite operand from the shown if.true/drop/else/swap/drop sequence. Correct both the precondition and selected value before recommending this substitution; otherwise an optimization can change contract behavior.

Validation: Both rc.7 and final v0.16 reject the documented stack with a non-binary operand. With the condition correctly on top, cdrop returns 30 while the shown branch returns 20 for the same input.


# Bad: branchy equivalent for the same condition-on-top layout
# stack: [cond, b, a]
# Bad: branchy equivalent
if.true
swap drop # cond true: keep b
drop # drop b, keep a
else
drop # cond false: keep a
swap drop # drop a, keep b
end
# stack: [b if cond else a]
```

Note on `cdrop` stack order: the condition is consumed from the top of the stack and `b` (the value just below it) is kept when the condition is 1, `a` when it is 0. `cdrop` fails if the condition is > 1. `if.true`/`if.false` likewise pop their condition from the top of the stack.
## Loop Variables Belong on the Stack

A procedure local is not a register: `loc_load.i` costs 5 cycles and `loc_store.i` costs 6. Reaching the same value on the stack
with `dup.n`, `swap`, `movup.n` or `movdn.n` (usually) costs 1 cycle. So a loop that keeps its counter and pointer in locals pays 5-11 cycles per access, per iteration, for data the stack could hold for 1.

Read once, mutate in place:

```masm
# Good: item_ptr lives on the stack next to the loop counter
# => [items_left, item_ptr, ...]
# 1 cycle: read the pointer
dup.1
# ... use it ...
# 4 cycles: advance it
swap add.ITEM_NUM_ELEMENTS swap
sub.1 dup neq.0

# Bad: same loop through a local
# 5 cycles
loc_load.ITEM_PTR_LOC
# ... use it ...
# 13 cycles
loc_load.ITEM_PTR_LOC add.ITEM_NUM_ELEMENTS loc_store.ITEM_PTR_LOC
sub.1 dup neq.0
```

### Working around `call`

The reason to reach for a local is a `call`: the callee takes the top 16 elements, so while those 16 slots are being filled, nothing below them is addressable by `dup.n`. Values that only have to *survive* the call are fine on the stack - they sit in the overflow and come back untouched. Only a value that must be re-read *while* the frame is being built has to live in a local, and even then it is one local, not one per loop variable.
Loading