chore(claude skills): aggregated skills from 1y of PRs

.claude/skills/advice-provider-hygiene/SKILL.md

@@ -0,0 +1,86 @@
+---
+name: advice-provider-hygiene
+description: Use when writing kernel, account, or note MASM code that reads from or writes to the advice provider (advice stack / advice map) — key entries by content hash, validate every piece of advice data against a commitment the kernel already trusts, and treat missing required entries as errors rather than silent zeros.
+---
+
+# Advice Provider Hygiene
+
+## Rules
+
+The advice provider is untrusted input supplied by the (potentially adversarial) prover. Any kernel, account, or note procedure that touches it must follow three rules.
+
+### 1. Validate advice data against a commitment
+
+Before consuming data loaded from the advice provider:
+
+1. Read the data from the advice stack / advice map into memory.
+2. Compute its hash with the appropriate hasher (typically `Rpo256` over the loaded region).
+3. Assert the computed hash equals an expected commitment that the kernel already trusts — on-chain storage, a prior input, or a value already on the stack.
+
+Do not consume advice data before this check passes. The advice provider's only role is to supply witness data for commitments the kernel has already received.
+
+### 2. Key advice map entries by content hash
+
+When inserting into the advice map, the key must be a hash of the value it indexes (or a derived commitment of the same data):
+
+- Use `Rpo256(value)` (or whichever hash matches the consumer's check) as the key.
+- Do not hard-code keys like `0x0000_0000_0000_0001`, `ADVICE_KEY_NOTE_DATA`, or per-procedure magic constants.
+
+Readers retrieve the entry by recomputing the same hash from data they already trust; rule 1's commitment check binds the lookup result to that trusted hash.
+
+### 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. Surface it with `assert.err=ERR_...`. Don't substitute zero / empty / a fallback and continue.
+
+## Why
+
+The advice provider is filled by the prover, who is potentially adversarial. Each of the three rules closes a different attack:
+
+- Without commitment validation, a malicious prover injects any data and the kernel runs against it.
+- Without content-addressed keys, two procedures (or one procedure plus an adversarial write) can silently collide on the same key in the global per-transaction namespace.
+- Without erroring on missing data, the prover skips inputs the kernel would have used to enforce a check; the kernel runs to completion against zeros and produces a "proof" that doesn't actually witness the intended property.
+
+Content addressing makes collisions cryptographically negligible. The load/hash/assert pattern binds untrusted advice to trusted state. Treating absence as an error preserves the kernel's invariants under adversarial input.
+
+The pattern is universal in the kernel and account procedures: **load → hash → assert against commitment → use**, with content-addressed keys throughout and `assert.err=...` on absence.
+
+## Examples
+
+```masm
+# Good: load → hash → check commitment → use
+adv_pipe                                # load advice into memory at the pointer
+exec.hash_data                          # compute hash of the loaded region
+push.EXPECTED_COMMITMENT
+assert_eqw.err=ERR_COMMITMENT_MISMATCH
+# ... safe to use the data here
+
+# Good: content-addressed advice map insert
+push.NOTE_DATA_COMMITMENT
+adv.push_mapval                         # key is the commitment itself
+
+# Good: missing entry is an error
+adv.has_mapkey
+assert.err=ERR_MISSING_REQUIRED_ADVICE
+
+# Bad: consume advice data without validating it
+adv_pipe
+exec.consume_data                       # data could be anything the prover wants
+
+# Bad: hard-coded magic key
+push.0x1234_5678_0000_0001
+adv.push_mapval
+
+# Bad: silent zero on missing key
+adv.push_mapval                         # no-op if key absent; proceed as if zero
+```
+
+For the Rust analog (returning `Err` on bad/missing external input rather than panicking or defaulting), see `return-error-not-panic`.
+
+## Evidence
+
+- PR #1648 (bobbinth): "Verify that data received from the advice provider matches an expected hash to prevent malicious injection." / "Validate the advice data against the on-chain commitment before using it."
+- PR #1871 (bobbinth): "Avoid hard-coded advice-map keys; the advice map is a global per-transaction namespace and predictable keys risk collisions." / "The advice data must be checked against the on-chain commitment."
+- PR #1896 (PhilippGackstatter): "Use the data commitment as the key rather than a fixed constant." / "Don't silently treat missing data as zero."
+- PR #1995 (PhilippGackstatter): "Add a commitment check after the advice pipe." / "Absent advice key must produce a typed error."
+- PR #1360 (bobbinth): "Advice map keys should be the hash of the underlying data they index."
+- PR #2439 (PhilippGackstatter): "When piping advice data into memory, validate that the supplied hash matches the data rather than blindly inserting from the advice provider."

.claude/skills/assert-specific-error-in-tests/SKILL.md

@@ -0,0 +1,52 @@
+---
+name: assert-specific-error-in-tests
+description: Use when writing a Rust test that exercises a failure path or a MASM test that expects a `panic` / `assert` — assert on the specific expected error variant or `ERR_*` code, not merely that some failure occurred.
+---
+
+# Negative Tests Must Pin the Expected Error
+
+## Rule
+
+A test that exercises an error path must assert on the specific error returned:
+
+- In Rust: use `assert_matches!(result, Err(MyError::SpecificVariant { .. }))` or destructure the error and assert on its fields. Don't accept "any `Err`" via `assert!(result.is_err())`.
+- In MASM: assert that the trapping error code matches the expected `ERR_*` constant, not just that the transaction failed.
+
+If multiple error conditions could plausibly fire on the same input, assert on the one this test is actually exercising.
+
+## Why
+
+A test that only checks `is_err()` passes even when an unrelated bug breaks the function. The test isn't validating the failure mode it claims to — it's validating "this code path doesn't reach the happy path", which a `panic!("not implemented")` would also satisfy.
+
+Specific-variant assertions catch regressions where one error path starts firing instead of another (e.g. a validation reorder that surfaces `InvalidFormat` before `EmptyInput`), which the looser assertion would silently accept.
+
+## Examples
+
+```rust
+// Good
+use assert_matches::assert_matches;
+
+let result = AccountId::try_from(&bytes);
+assert_matches!(result, Err(AccountError::InvalidLength { expected: 32, got: 5 }));
+
+// Bad
+let result = AccountId::try_from(&bytes);
+assert!(result.is_err());
+```
+
+```rust
+// Good (MASM test)
+let err = run_kernel(...).unwrap_err();
+assert_eq!(err.code(), ERR_NOTE_NOT_FOUND);
+
+// Bad
+assert!(run_kernel(...).is_err());
+```
+
+## Evidence
+
+- PR #2740 (PhilippGackstatter): "Negative tests must assert the exact expected error code/variant, not merely that some failure occurred."
+- PR #2636 (PhilippGackstatter): "Assert on a specific expected error in negative-path tests instead of asserting only that some error occurred."
+- PR #1604 (bobbinth): "Assert on the variant; is_err is too loose."
+- PR #1599 (PhilippGackstatter): "Match the specific ERR_ constant."
+- PR #1759 (PhilippGackstatter): "Use assert_matches with the specific variant."

.claude/skills/cheap-masm-equivalents/SKILL.md

@@ -0,0 +1,58 @@
+---
+name: cheap-masm-equivalents
+description: Use when writing or reviewing MASM hot paths — prefer the cheaper instruction sequence when it is semantically equivalent (`neq.0` over `gt.0` for non-zero checks, `dup` over `loc_load`, `cdrop` over branches selecting between two values).
+---
+
+# Prefer Cheap MASM Equivalents
+
+## Rule
+
+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` (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 (e.g. `u32gt` on a value that might exceed `u32::MAX`).
+
+## Why
+
+MASM cycle costs are not uniform. `gt.0` does signed-comparison work that `neq.0` skips entirely; both produce the same boolean for the typical "is it non-zero?" question on non-negative inputs. A hot kernel path that uses `gt.0` everywhere pays the full 16-cycle cost on each call for a check `neq.0` would do in 3.
+
+These swaps are also free of risk in most cases — the semantics are equivalent under typical preconditions.
+
+## Examples
+
+```masm
+# Good
+push.0 neq          # non-zero check, 3 cycles
+# or simply
+neq.0
+
+# Bad
+push.0 gt           # same answer, 16 cycles
+```
+
+```masm
+# Good: cdrop for ternary selection
+# stack: [b, a, cond]
+cdrop
+# stack: [a if cond else b]
+
+# Bad: branchy equivalent
+if.true
+    drop      # drop b, keep a
+else
+    swap drop # drop a, keep b
+end
+```
+
+## Evidence
+
+- PR #2636 (PhilippGackstatter): "Prefer neq.0 (3 cycles) over gt.0 (16 cycles) when checking a value is non-zero in MASM."
+- PR #2712 (PhilippGackstatter): "Use cdrop here instead of the branch."
+- PR #1681 (bobbinth): "Replace this branch with cdrop."
+- PR #1968 (bobbinth): "eqw is cheaper than the per-element compare."
+- PR #2123 (PhilippGackstatter): "u32gt; we know both operands are u32."

.claude/skills/checked-arithmetic/SKILL.md

@@ -0,0 +1,38 @@
+---
+name: checked-arithmetic
+description: Use when writing Rust arithmetic on amounts, balances, supplies, or other quantities derived from external/user input — use `checked_*` or `overflowing_*` operations and handle the overflow flag explicitly; do not rely on wrapping semantics for untrusted values.
+---
+
+# Checked Arithmetic on User-Supplied Values
+
+## Rule
+
+Any arithmetic where one or more operands originates from external input (transaction body, advice provider, user RPC, deserialized payload) must use checked or overflowing arithmetic and surface the overflow:
+
+- Prefer `checked_add` / `checked_sub` / `checked_mul` and return an error on `None`.
+- Use `overflowing_add` / `widening_mul` when you need the wrapping value *and* the overflow flag; then `assert!(!overflow)` (or branch) before using the result.
+- Do not use the default `+`, `-`, `*` operators on untrusted values in release builds — debug-only overflow checks are not enough.
+
+## Why
+
+Default arithmetic on `u64`/`u128` wraps in release. Two amounts that fit individually can multiply to overflow, producing a fungible-asset amount that is silently far smaller than the user intended. Checked arithmetic forces the caller to acknowledge the overflow path; wrapping arithmetic hides it.
+
+## Examples
+
+```rust
+// Good: checked
+let total = balance.checked_add(amount).ok_or(Error::Overflow)?;
+
+// Good: overflowing with explicit flag check
+let (product, overflow) = a.widening_mul(b);
+if overflow { return Err(Error::Overflow); }
+
+// Bad: wraps on overflow in release
+let total = balance + amount;
+```
+
+## Evidence
+
+- PR #2636 (PhilippGackstatter): "Use checked or overflowing arithmetic (overflowing_add, widening_mul) on user-supplied amounts and assert the overflow flag rather than relying on wrapping semantics."
+- PR #2712 (PhilippGackstatter): "This addition can overflow on user input — use checked_add and handle the None case."
+- PR #1654 (bobbinth): "We should use checked arithmetic here."

.claude/skills/conversion-method-naming/SKILL.md

@@ -0,0 +1,56 @@
+---
+name: conversion-method-naming
+description: Use when naming a conversion or accessor method in Rust — follow the API guideline prefixes (`as_` for free borrows, `to_` for expensive conversions, `into_` for ownership transfers, `from_` for type-associated constructors) and don't overload `with_` for non-builder methods.
+---
+
+# Rust Conversion Method Naming
+
+## Rule
+
+Pick the prefix that matches the cost and ownership of the conversion:
+
+- `as_<type>` — free or near-free, returns a borrow (e.g. `as_bytes() -> &[u8]`).
+- `to_<type>` — non-trivial cost, returns an owned value, leaves `self` intact (e.g. `to_string() -> String`).
+- `into_<type>` — consumes `self`, returns the inner/converted value.
+- `from_<type>` — associated function on the target type that constructs it.
+- `with_<field>` — *only* for builder-style methods that take and return `Self` with a field set.
+
+Do not name a non-borrowing method `as_*`. Do not name a non-builder method `with_*`. Do not use `to_*` for a free borrow.
+
+## Why
+
+The conventions come from the Rust API guidelines and are deeply baked into the standard library and the wider ecosystem. A reader expects `as_x` to be cheap, `to_x` to allocate, `into_x` to consume — violating that costs every reader a moment of confusion and trains them to distrust the rest of the API.
+
+`with_*` is especially load-bearing: when readers see `foo.with_bar(b)`, they expect `foo` back with `bar` set. Naming an unrelated method `with_` derails that intuition.
+
+## Examples
+
+```rust
+// Good
+impl Header {
+    pub fn as_bytes(&self) -> &[u8] { ... }              // cheap borrow
+    pub fn to_vec(&self) -> Vec<u8> { ... }              // allocates
+    pub fn into_payload(self) -> Payload { self.payload } // consumes
+}
+
+impl HeaderBuilder {
+    pub fn with_version(mut self, v: u8) -> Self { self.version = v; self }
+}
+
+// Bad
+impl Header {
+    pub fn as_vec(&self) -> Vec<u8> { ... }  // allocates, should be to_vec
+    pub fn to_bytes(&self) -> &[u8] { ... }  // borrow, should be as_bytes
+}
+
+// Bad: with_* on a non-builder method
+fn with_seed(rng: &mut Rng, seed: u64) { ... }   // not returning Self
+```
+
+## Evidence
+
+- PR #2053 (PhilippGackstatter): "Follow Rust API naming guidelines for conversions: as_ for free borrows, to_ for expensive conversions, into_ for ownership transfers."
+- PR #2712 (PhilippGackstatter): "Reserve with_* for builder methods that return Self; if a method does not return Self, give it a different verb or implement a conversion trait."
+- PR #2569 (PhilippGackstatter): "This should be to_, not as_, because it allocates."
+- PR #2205 (bobbinth): "Rename as_ to to_ here."
+- PR #1426 (PhilippGackstatter): "with_ implies builder semantics; rename."

.claude/skills/decouple-component-from-storage/SKILL.md

@@ -0,0 +1,47 @@
+---
+name: decouple-component-from-storage
+description: Use when writing a MASM procedure that lives inside an account component and currently reads from a hard-coded storage slot — accept the required values as stack parameters instead, so the procedure works regardless of where the component is mapped in the account's storage layout.
+---
+
+# Decouple Component Procedures from Storage Layout
+
+## Rule
+
+Procedures that belong to an account component must not bake the component's storage-slot index into their bodies. Instead:
+
+- Accept the required values as stack parameters (see `masm-explicit-stack-inputs`).
+- Let the caller — the account-level glue procedure that knows the actual slot layout — read from storage and push the values.
+
+If a procedure truly needs to read from "its own" storage, it should take the slot index (or a pointer to it) on the stack, not hard-code it.
+
+## Why
+
+A component can be installed into many accounts, each with different storage layouts. Hard-coding a slot index in the component's body ties that component to a single layout — installing it in any other account silently misreads memory.
+
+Stack-parameter inputs make the component portable: the account-level caller decides where the slot lives and passes the data in. The component then operates on what it was given, with no implicit assumption about where it came from.
+
+## Examples
+
+```masm
+# Good: component procedure takes its inputs on the stack
+proc transfer_asset
+    # => [ASSET, recipient_id, current_balance]
+    # operates on what was passed in
+end
+
+# Account-level caller reads from the actual slot, then invokes:
+mem_loadw.OUR_VAULT_SLOT
+exec.transfer_asset
+
+# Bad: component procedure reads a hard-coded slot
+proc transfer_asset
+    # => [ASSET, recipient_id]
+    mem_loadw.4   # only works if THIS component is at slot 4
+end
+```
+
+## Evidence
+
+- PR #1712 (PhilippGackstatter): "Decouple component procedures from storage layout assumptions by accepting required values as input parameters."
+- PR #2664 (PhilippGackstatter): "This component reads slot 4 directly — pass it as a parameter."
+- PR #1599 (bobbinth): "The component shouldn't know its own slot."

.claude/skills/domain-newtypes-over-primitives/SKILL.md

@@ -0,0 +1,54 @@
+---
+name: domain-newtypes-over-primitives
+description: Use when introducing API parameters, struct fields, or return types that conceptually carry a domain value (asset amount, faucet ID, slot index, fee rate) — wrap them in a newtype that enforces invariants, instead of accepting raw `Word`, `u64`, or `(prefix, suffix)` tuples.
+---
+
+# Use Domain Newtypes, Not Raw Primitives
+
+## Rule
+
+When an API boundary takes or returns a value with a domain meaning, define a newtype that:
+
+- Validates the value at construction time (see `validate-in-constructor`).
+- Exposes the inner representation only through deliberate accessors.
+- Is used at every API boundary touching the concept (not raw `u64`/`Word`/tuples).
+
+Raw `(AccountId, u64)` tuples, bare `Word` parameters, and primitive-typed amounts must be replaced with a named type like `FungibleAsset`, `FaucetId`, `BlockNumber`.
+
+## Why
+
+A typed wrapper is the only place invariants get enforced. Once a function accepts `u64` for "amount", every caller is responsible for not passing a value above the max supply, and every reviewer has to check. With `FungibleAsset` (or similar) the type system enforces the rule once and everyone benefits.
+
+Newtypes also make refactors safer: changing a representation (e.g. moving from `(prefix, suffix)` to a single `[u8; 16]`) touches one type, not every signature.
+
+## Examples
+
+```rust
+// Good
+pub fn mint(asset: FungibleAsset, to: AccountId) -> Result<Receipt, Error>;
+
+// Bad
+pub fn mint(faucet_id: AccountId, amount: u64, to: AccountId) -> Result<Receipt, Error>;
+```
+
+```rust
+// Good: validated wrapper with explicit constructor
+pub struct BlockNumber(u32);
+impl BlockNumber {
+    pub fn new(n: u32) -> Result<Self, Error> {
+        if n > MAX_BLOCK_NUMBER { return Err(Error::OutOfRange); }
+        Ok(Self(n))
+    }
+}
+
+// Bad: raw u32 leaks into every signature, every caller checks the bound
+pub fn lookup_block(n: u32) -> Option<Block>;
+```
+
+## Evidence
+
+- PR #2439 (PhilippGackstatter): "Wrap domain-validated values in newtypes that enforce invariants at construction time; avoid raw Word or u64 in APIs."
+- PR #2636 (PhilippGackstatter): "Accept strongly-typed domain values (e.g. FungibleAsset) at API boundaries instead of raw (AccountId, u64) tuples or Word arrays."
+- PR #2890 (bobbinth): "We should introduce a newtype here rather than passing the raw value."
+- PR #1978 (PhilippGackstatter): "Use a typed wrapper instead of a bare Word."
+- PR #2246 (bobbinth): "This should be a typed wrapper, not a raw integer."