Conversation
* fix(ci): install Rust 1.91 for cargo-msrv compatibility cargo-msrv 0.19.3 requires rustc 1.91.1 or newer, but the stable toolchain currently ships 1.90.0. Install the 1.91 toolchain and set RUSTUP_TOOLCHAIN=1.91 on the cargo-msrv install and MSRV check steps so the tool can build and run. * Revert "fix(ci): install Rust 1.91 for cargo-msrv compatibility" This reverts commit 4485176. * chore: bump toolchain * chore: clippy
Add scheme-specific constructors that accept typed public keys instead of a generic PublicKeyCommitment, preventing mismatches between key type and authentication scheme: - falcon512_poseidon2(pub_key): creates component from a Falcon512 key - ecdsa_k256_keccak(pub_key): creates component from an ECDSA K256 key - from_public_key(pub_key): creates component from a PublicKey enum Also add From<ecdsa_k256_keccak::PublicKey> for PublicKeyCommitment for parity with the existing Falcon512 conversion.
* chore: rename from PSM to Guardian * fix: prefer state guardian in comments * fix: rename guardian multisig to guarded multisig * fix: restore changelog note and apply nightly formatting * fix: move guardian rename changelog notes to v0.15 * style: clarify guardian masm comments * fix: comment
…#2736) * refactor: lowercase note script filenames in miden-agglayer * docs(agglayer): update SPEC.md note script filenames to lowercase Agent-Logs-Url: https://github.com/0xMiden/protocol/sessions/4f7eb749-9710-4a36-8caf-486f12ae21d7 Co-authored-by: partylikeits1983 <77119221+partylikeits1983@users.noreply.github.com> * fix: update SPEC.md to reference .masm --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
…and `RoleSymbol` types (#2690)
* feat(asset): add AssetAmount wrapper type for fungible amounts Introduce a validated AssetAmount(u64) newtype that encapsulates the max-amount check (2^63 - 2^31) at construction time. - Add AssetAmount with MAX constant, new(), as_u64(), From<u8/u16/u32>, TryFrom<u64>, Display, Serializable, Deserializable - Integrate into FungibleAsset: replace raw u64 amount field - Delegate validation in FungibleAsset::new() to AssetAmount::new() - Keep FungibleAsset::MAX_AMOUNT as backward-compatible alias - Keep amount() returning u64 to minimize downstream churn Closes #2532 * refactor: remove as_u64, delegate add/sub to AssetAmount Remove the redundant as_u64() accessor since From<AssetAmount> for u64 already provides the same conversion. Replace all call sites with u64::from(). Add add() and sub() methods to AssetAmount so that FungibleAsset can delegate arithmetic to the inner type instead of performing it inline. Add unit tests for both new methods. * style: fix rustfmt and clippy lints Expand import list to one-per-line, merge duplicate use super statements, collapse single-line assert_eq, and suppress should_implement_trait on AssetAmount::add/sub. * docs: add changelog entry for AssetAmount wrapper * refactor: make AssetAmount::MAX a u64 constant, use .into() in tests * fix: add type annotations for .into() in tests to resolve ambiguity * refactor: implement Add and Sub traits for AssetAmount --------- Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com> Co-authored-by: Marti <marti@miden.team>
* feat: add `metadata_into_note_type` procedure to note.masm Add a procedure to extract the note type from the metadata header and unit tests for both public and private note types. * refactor: address PR review — move stack layout to Where docs, consolidate tests with rstest * chore: add CHANGELOG entry for metadata_into_note_type * refactor: address review — clarify bit position, add stack layout docs, use NoteType comparison * refactoring changes Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * docs: link CHANGELOG entry for metadata_into_note_type to PR #2738 --------- Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com>
…er build scripts (#2538) * refactor: avoid unnecessary asm/ tree copy in standards build script miden-standards: Remove the full asm/ directory copy to OUT_DIR entirely. This crate never mutates its source tree, so the assembler and error extractor can read directly from the crate's asm/ directory. miden-protocol: Replace the bulk copy of the entire asm/ tree with targeted staging of only the two directories that need modification (kernels/transaction/ and protocol/). The assembler requires shared modules to be physically present alongside these source files, but shared_utils/ and shared_modules/ themselves don't need to be copied. Event extraction now reads directly from the original source. Also simplifies copy_dir_recursive (replacing the old copy_directory with its awkward prefix-stripping API) and removes dead code. https://claude.ai/code/session_01HDd5o3XxcgZiGrvBDFsUr1 refactor: scope change to standards build only, leave protocol as-is The protocol crate needs source-level staging because the assembler's `$kernel::` import resolution requires shared modules to be physically co-located with kernel source. This cannot be avoided without assembler changes, so revert the protocol build.rs to the base branch version. https://claude.ai/code/session_01HDd5o3XxcgZiGrvBDFsUr1 * refactor: remove copy_directory from agglayer build.rs Read MASM sources directly from the crate's asm/ directory instead of copying them to OUT_DIR first. The copy is unnecessary because agglayer doesn't mutate the directory structure during compilation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: generate agglayer error constants into OUT_DIR Write error constants to OUT_DIR instead of src/errors/agglayer.rs, matching the pattern used by miden-standards. The include now uses concat!(env!("OUT_DIR"), ...) so error generation always runs regardless of BUILD_GENERATED_FILES_IN_SRC. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: validate canonical_zeros.masm instead of generating it The canonical zeros are deterministic Keccak256 values that never change. Instead of generating the file at build time (guarded by BUILD_GENERATED_FILES_IN_SRC), the committed file is now validated against the expected content. To regenerate, run with REGENERATE_CANONICAL_ZEROS=1. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: remove BUILD_GENERATED_FILES_IN_SRC from agglayer Both error constants and canonical zeros no longer write to src/, so BUILD_GENERATED_FILES_IN_SRC is unused and can be removed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: fix trailing empty doc comment line Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
* refactor: reduce max assets per note from 255 to 64 * feat: add add_assets_exceeding_max_per_note_fails test * Update crates/miden-protocol/asm/kernels/transaction/lib/note.masm Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update crates/miden-protocol/asm/kernels/transaction/lib/memory.masm Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: correct CHANGELOG issue link from miden-base to protocol repo Agent-Logs-Url: https://github.com/0xMiden/protocol/sessions/7e834550-7f38-4d90-9c05-d46e3a7e37fa Co-authored-by: partylikeits1983 <77119221+partylikeits1983@users.noreply.github.com> * refactor: address PR comments: add max assets success test, fix memory layout table * chore: fix changelog * Update crates/miden-protocol/src/transaction/kernel/memory.rs Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com> * Update crates/miden-testing/src/kernel_tests/tx/test_output_note.rs Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com> * refactor: use rstest for tests & update changelog --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
…onstruction (#2758) * chore: make note procedure names consistent * chore: add changelog * chore: update swap tag construction
* Add cycle counts to successful and failed notes * Changelog * Cycle count order comment * Make fields private
* fix: improve deserialization error handling in AccountCode * chore: update changelog * refactor: removed AccountCode::from_bytes()
* refactor: always enable concurrent on std in miden-tx * chore: delete miden-protocol-macros crate * chore: update changelog
* Added the pswap masm contract and its supportive functions * Added the pswap masm contract and its supportive functions * refactor: rename swapp_tag to pswap_tag, NUM_ITEMS to NUM_STORAGE_ITEMS, remove create_ wrappers, add builder finish_fn - Rename swapp_tag -> pswap_tag and SWAPp -> PSWAP throughout - Rename NUM_ITEMS -> NUM_STORAGE_ITEMS for clarity - Remove create_p2id_payback_note and create_remainder_note wrappers, make build_ functions public instead - Compute p2id_tag inside build_p2id_payback_note from self.storage - Add #[builder(finish_fn(vis = "", name = build_internal))] to PswapNote * refactor: remove redundant test helpers, use pswap lib functions directly Replace all test helper wrappers with direct calls to library functions: - create_pswap_note -> PswapNote::create() - create_expected_pswap_p2id_note + create_expected_pswap_remainder_note -> pswap.execute() - build_pswap_storage -> PswapNoteStorage::from_parts() - Remove make_pswap_tag, make_note_assets, make_note_args, compute_p2id_tag_* - Inline calculate_output_amount as PswapNote::calculate_output_amount() * docs: polish pswap doc comments to match sibling note style - Replace storage layout list with markdown table - Remove trivial "Returns the X" docs on simple getters - Add # Errors sections where relevant - Rewrite method docs to describe intent, not implementation - Add one-line docs on From/TryFrom conversion impls - Tighten PswapNote struct doc * refactor: make assets a required field on PswapNote builder * fix: adapt pswap to upstream API changes after rebase - Rename RpoRandomCoin to RandomCoin (miden-crypto 0.23 rename) - Store full ASSET_KEY instead of prefix/suffix to preserve callback metadata in faucet_id_suffix_and_metadata - Replace create_fungible_asset calls with direct ASSET_KEY + manual ASSET_VALUE construction, avoiding the new enable_callbacks parameter - Update hardcoded P2ID script root to match current P2idNote::script_root() * refactor: use procref for P2ID script root, clean up pswap MASM - Replace hardcoded P2ID script root with procref.p2id::main for compile-time resolution - Default to full fill when both input and inflight amounts are zero - Replace magic address 4000 with named P2ID_RECIPIENT_STORAGE constant - Remove step numbering from comments, fix memory layout docs * refactor: address PR review comments on pswap MASM - Rename P2ID_RECIPIENT_STORAGE to P2ID_RECIPIENT_SUFFIX/PREFIX - Add METADATA_HEADER word layout docs with source reference - Document attachment_scheme parameter in set_word_attachment call - Add stack views after condition checks * refactor: rename pswap storage accessors, add offered_asset_amount helper, simplify build_p2id_payback_note Rename requested_key/requested_value/requested_amount to requested_asset_key/requested_asset_value/requested_asset_amount for clarity. Extract offered_asset_amount() helper on PswapNote to deduplicate offered asset extraction. Simplify build_p2id_payback_note to take fill_amount: u64 instead of aux_word: Word, constructing the aux word internally. * refactor: address PR review comments on pswap Rust module - Use NoteTag::default() instead of NoteTag::new(0) - Change swap_count from u64 to u16 with safe try_into conversion - Rename p2id_tag to payback_note_tag, p2id_payback_note to payback_note - Rename build_ prefix to create_ for consistency - Rename aux_word to attachment_word - Replace Felt::new with Felt::from where possible - Rename inputs to note_storage in TryFrom impl - Make create_payback_note and create_remainder_pswap_note private - Add offered_faucet_id helper to replace unreachable!() * refactor: use builder pattern for PswapNote and PswapNoteStorage - Remove PswapNote::create, add public build() with validation on builder - Add bon::Builder to PswapNoteStorage, remove new() and from_parts() - Remove payback_note_tag field, compute from creator_account_id on the fly - Fix clippy warnings (useless conversions, needless borrows) - Update all unit and integration tests to use builder pattern * refactor: clean up pswap tests and fix clippy warnings - Compare full faucet_id instead of just prefix in build validation - Rename swap_note to pswap_note in integration tests for consistency - Extract PswapNoteStorage builder into separate let bindings - Replace manual current_swap_count counter with enumerate index - Fix clippy useless_conversion and needless_borrow warnings * refactor: rename inputs to storage_items in pswap_note_storage_try_from test * chore: fix Cargo.toml dependency ordering and add CHANGELOG entry for PSWAP * refactor: address PR review comments — use FungibleAsset types, improve safety and docs - PswapNoteStorage: store FungibleAsset instead of raw key/value Words - PswapNote: store offered_asset as FungibleAsset instead of NoteAssets - execute(): take Option<FungibleAsset> for input/inflight assets - Split execute into execute() and execute_full_fill_network() for clarity - create_tag(): accept &FungibleAsset instead of &Asset - Eliminate all Felt::new usage (use Felt::from, Felt::try_from, + ONE) - Add swap_count overflow protection via checked_add - Add script root verification in TryFrom<&Note> - Add MASM fill validation (fill_amount <= requested) - Rename MASM constants: _INPUT -> _ITEM, SWAPP_ -> PSWAP_ - Make calculate_output_amount private, add precision doc comment - Add doc comments on all public accessors and methods - Add network account full fill test - Document network transaction compatibility throughout * refactor: pass creator ID via stack to is_consumer_creator and use account_id::is_equal * refactor: optimize requested asset storage to 4 felts and add NUM_STORAGE_ITEMS constant Store requested asset as individual felts [enable_callbacks, faucet_suffix, faucet_prefix, amount] instead of full ASSET_KEY + ASSET_VALUE (8 felts). Use create_fungible_asset to reconstruct when needed. Reduces note storage items from 18 to 14. Add NUM_STORAGE_ITEMS constant and execute_pswap doc comment. * refactor: use p2id::new, pass inputs via stack, unify serial derivation - Replace manual recipient building with p2id::new in create_p2id_note - Pass all inputs via stack, store intermediates in @Locals - Remove build_p2id_recipient_hash, P2ID_RECIPIENT_*, P2ID_NOTE_IDX - Payback serial: serial[0] + 1, remainder serial: serial[3] + 1 - Update Rust side to match new serial derivation, remove Hasher import * refactor: pass all inputs via stack to create_remainder_note, eliminate global memory reads - create_remainder_note takes 8 stack inputs, stores to @Locals(6) - Extract offered faucet info from ASSET_KEY in execute_pswap - Use create_fungible_asset for consumer receive_asset and remainder asset - Remove OFFERED_ASSET_KEY, PSWAP_NOTE_IDX constants * feat: add P2ID reconstruction test, overfill docs, and stack comments - Add test verifying Alice can reconstruct and consume P2ID payback note from her PSWAP data after Bob's partial fill - Document why overfill is not allowed (likely unintentional error) - Add missing stack comment on partial fill check * fix: use Rust-predicted note for aux data in P2ID reconstruction test The test framework doesn't preserve word attachment content in executed transaction outputs. Use the Rust-predicted note to read the fill amount from aux data, which mirrors production behavior where aux is visible. * refactor: rename input to account fill, extract load_offered_asset proc - Rename AMT_REQUESTED_IN to AMT_REQUESTED_ACCT_FILL and related variables - Extract load_offered_asset proc centralizing asset loading and validation - Remove OFFERED_ASSET_WORD global memory constant * refactor: extract test helpers and reduce verbosity in pswap unit tests * refactor: guard zero-amount receive_asset in MASM, add NUM_STORAGE_ITEMS alias on PswapNote * refactor: rename ERR_PSWAP_WRONG_NUMBER_OF_INPUTS to ERR_PSWAP_WRONG_NUMBER_OF_STORAGE_ITEMS * refactor: replace global memory with @Locals in execute_pswap and pass fill amounts via stack * refactor(pswap): apply PR review nits from #2636 - Drop duplicated u64 stdlib docs inside calculate_tokens_offered_for_requested - Replace `swap push.MAX_U32 mul add` with `swap mul.MAX_U32 add` - Fold redundant empty-input guard in PswapNote::execute into the match arm * feat(pswap): make payback note type configurable via PswapNoteStorage Adds a `payback_note_type: NoteType` field on `PswapNoteStorage` (default `Private`) so the payback note produced on fill can be private even when the pswap note itself is public. Private payback notes are cheaper in fees and bandwidth and don't lose information — the fill amount is already recorded in the executed transaction's output. Stored in the previously-reserved storage slot [6]. MASM now loads the type from that slot when building the payback p2id note instead of using the active note's own type. Remainder pswap note continues to inherit the parent pswap's note type (unchanged). Addresses PhilippGackstatter's review comment on PR #2636. * perf(pswap): compress note storage layout from 14 to 10 items Drops four reserved/zero padding slots ([7] and [9-11] in the old layout) that served no purpose. The compressed layout is contiguous: [0-3] requested asset (callbacks, faucet suffix/prefix, amount) [4] pswap_tag [5] payback_note_tag [6] payback_note_type [7] swap_count [8-9] creator account id (prefix, suffix) Saves 4 felts per pswap note on-chain, reducing fees and bandwidth for every pswap created. No information is lost. Addresses PR #2636 review comment from PhilippGackstatter on compressing the storage layout. * test(pswap): cover combined input+inflight partial and full fill paths Adds two unit tests against `PswapNote::execute` for the input+inflight code path flagged in PR #2636 review: - partial fill: account fill 10 + inflight 20 of a 50-requested pswap. Asserts payback carries 30 of the requested asset and a remainder pswap is produced with 20 requested / 40 offered remaining. - full fill: account fill 30 + inflight 20 of a 50-requested pswap. Asserts payback carries 50 of the requested asset and no remainder. Also factors out a `dummy_consumer_id()` helper alongside `dummy_creator_id()` to keep the new tests readable. * refactor(pswap): rename input/inflight to account_fill/note_fill Addresses the naming feedback on PR #2636. The old `input` / `inflight` terminology was ambiguous — "input" could mean the total fill amount, the note_args input word, or the account-side fill, while "inflight" conflated the transport mechanism with its role. The new names are explicit about where the fill comes from: - offered_out -> payout_amount - input / input_amount -> fill_amount (when referring to the total fill amount) or account_fill_amount (when referring to the native-account portion specifically) - inflight_amount -> note_fill_amount - input_asset -> account_fill_asset - inflight_asset -> note_fill_asset Touches the public `PswapNote::execute` parameters, all internal Rust locals and doc comments, the MASM stack annotations / doc blocks / local-var labels, and the integration test callers. * refactor(pswap): fetch active note_type inline, drop global NOTE_TYPE Upstream PR #2738 added `miden::protocol::note::metadata_into_note_type` which does the bit-correct extraction against the new 1-bit NoteType encoding. The pswap script no longer needs its own `extract_note_type` proc or the `NOTE_TYPE` global slot. Changes: - Drop the `extract_note_type` proc and the `NOTE_TYPE_MASK` / `NOTE_TYPE` consts. - The payback note already reads its type from `PAYBACK_NOTE_TYPE_ITEM` in storage (no active-note fetch needed). - The remainder branch inlines `active_note::get_metadata dropw exec.note::metadata_into_note_type` where the type is actually used, instead of eagerly computing it once and stashing it in global memory. - Update `pswap_note_alice_reconstructs_and_consumes_p2id` to set `payback_note_type(NoteType::Public)` so the reconstructed P2ID payback matches — the previous test implicitly relied on the payback inheriting the pswap's Public type (broken since the bit-encoding change). This also fixes the previously-failing `pswap_note_alice_reconstructs_and_consumes_p2id` test, which was hitting the broken extract_note_type mask (always returning 0 under the new encoding). * refactor(pswap): switch storage consts to STORAGE_PTR base pattern Matches the convention used in p2id.masm: declare a single `STORAGE_PTR` base and derive every note-storage item offset relative to it. This makes the storage layout easier to read and relocate. Also use `push.STORAGE_PTR` at the load site instead of a bare `0`. * docs(pswap): add #! doc block to @note_script main procedure Closes PR #2636 review comment: `pub proc main` lacked a proper `#!` doc block with `Inputs`/`Outputs`/storage layout/panic conditions. Adds one modelled after the canonical style in p2id.masm / swap.masm. * refactor(pswap): scrub last inflight/input references in tests Rename `pswap_note_inflight_cross_swap_test` to `pswap_note_note_fill_cross_swap_test` and update two stale comments that still said "input" / "inflight" to match the new account_fill / note_fill terminology used throughout the module. * style(pswap): readability nits across pswap.masm Minor cleanups surfaced during review, no behavior changes: - Sort imports alphabetically by path. - Reshape storage layout doc block to `[i..j] : N felts` + `Where:` form and drop the duplicate listing in the main proc docstring. - Drop the stale "Memory Addresses" meta-block. - Name local slots via top-of-file consts (e.g. CALC_FILL_AMOUNT, P2ID_NOTE_IDX, REMAINDER_AMT_OFFERED, EXEC_AMT_OFFERED, ...) so loc_store/loc_load sites read like variables rather than loc.N. - Expand the rationale for the FACTOR=1e5 choice (precision vs u64 overflow headroom) and switch the literal to decimal. - Rename boolean stack-comment markers from operator syntax (`amt > 0`, `requested > offered`, `requested == fill_amount`) to readable predicates (`has_account_fill`, `has_note_fill`, `has_account_fill_payout`, `requested_exceeds_offered`, `is_full_fill`). - Drop obvious/redundant comments (error-const headers, `gt pops…` primitive explanation, proc-start stack echoes, validate/calculate rationale moved into the execute_pswap docstring). - Split `push.NUM_STORAGE_ITEMS.0` into two pushes using the STORAGE_PTR constant to match the `bridge_out.masm` pattern. - Add converged stack markers (`# => [...]`) after every if/else `end` and before every proc return. * test(pswap): dedupe integration tests via helpers + rstest - Add shared helpers at the top of the test file: `build_pswap_note`, `note_args`, `assert_fungible_asset`, `assert_vault_added_removed`, `assert_vault_single_added`. Removes the repeated 13-line pswap-note setup and the `if let Asset::Fungible(f) = …` / vault-delta boilerplate. - Consolidate variant-heavy tests with `#[rstest]`: - `pswap_fill_test` merges full/private/partial/network fill cases into one parameterized async test (4 cases). - `pswap_multiple_partial_fills_test` replaces the inner for-loop with 9 `#[case]`s, one per fill amount. - `pswap_partial_fill_ratio_test` merges the non-exact-ratio and hand-listed fuzz suites into one rstest with 27 regression cases, delegating to a new `run_partial_fill_ratio_case` helper. - `pswap_chained_partial_fills_test` parameterizes the outer chain loop with 10 `#[case]`s; the stateful per-chain inner fill loop is preserved intact. - Add `pswap_partial_fill_ratio_fuzz` as a seeded-random coverage sibling: two `#[case]`s (seed 42, seed 1337) × 30 random `(offered, requested, fill)` triples each, drawn from `SmallRng`. Failure message includes seed, iter, and the failing triple so any regression is reproducible from the rstest case name alone. The regression `#[case]` block stays as a permanent edge-case suite. - Leave `alice_reconstructs`, `cross_swap`, `creator_reclaim`, `invalid_input`, `compare`, and `parse_inputs` as standalone tests but rewrite their bodies to use the shared helpers. Net: 1325 → 956 non-helper lines (-369), test count 14 → 58. * style(pswap): apply nightly rustfmt * fix(pswap): guard user-provided fill sum against u64 overflow Switch `execute_pswap`'s `account_fill + note_fill` check to `u64::overflowing_add` + `ERR_PSWAP_FILL_SUM_OVERFLOW`, so a malicious consumer cannot pick operands whose felt sum wraps past u64 and spuriously satisfies the `lte requested` guard. Also document why the matching sums/subs in `create_p2id_note` and `create_remainder_note` cannot over/underflow, and drop a stray trailing space on `FACTOR`. * fix(pswap): correctness + layout cleanups from PR review theme 2 - create_p2id_note: drop three stray zero pads that shoved the real `note_idx` to stack depth 11 in the `has_account_fill` branch, so `move_asset_to_note` was reading a pad zero as the note index. The bug was masked in every existing test because `note_idx == 0`; add a SPAWN-note regression test that forces P2ID to `note_idx == 1`. - load_offered_asset: bump `@locals(2)` to `@locals(8)` — we store two words (8 felts), not two felts. - Drop `pswap_tag` from note storage (10 -> 9 items). The remainder path now lifts the tag out of the active note's metadata via `dup.2 movdn.4` + `metadata_into_note_type`, since the asset pair is unchanged across remainder creation. `PswapNoteStorage` loses the field, `with_pswap_tag`, and the getter; storage offsets and `NUM_STORAGE_ITEMS` shift accordingly. - Document why `create_p2id_note`'s add (P2ID attachment), why `create_remainder_note`'s sub (amt_offered - amt_payout), and why `create_p2id_note` itself are reachable but safe (invariants come from the caller, not local checks). - Correct the `payback_note_type` storage-layout docstring (only P2ID payback uses it; remainder inherits from active-note metadata). - Note in a comment why `procref.main` cannot replace the runtime `active_note::get_script_root` call in `create_remainder_note` (compile-time call-graph cycle via main -> execute_pswap -> create_remainder_note -> procref.main). * fix(pswap): align attachment layout + drop dead swap_count slot - Attachment layout: MASM now emits `[fill, 0, 0, 0]` (Word[0] = fill) for both the P2ID and remainder attachments, matching the Rust-side `Word::from([fill, 0, 0, 0])` convention. Previously the two sides disagreed (Rust wrote Word[0], MASM wrote Word[3]), and the alice-reconstructs test was silently compensating by reading `aux_word[3]` against a Rust prediction it never compared to. - alice-reconstructs test: read from the executed transaction's output (was already), read `aux_word[0]`, and add a direct Rust <-> MASM attachment-word parity `assert_eq!` so a future drift actually fails. - Extend alice-reconstructs to also reconstruct the remainder PSWAP from her original pswap data + the remainder's on-chain attachment, verifying recipient + attachment parity against the executed output. - Drop `pswap_count` from note storage (9 -> 8 items). The field was never read or written by pswap.masm — a dead pass-through slot. The Rust side was silently bumping it on remainder creation, so the Rust-predicted remainder recipient diverged from MASM's (caught only by extending the alice test). Remove the field, getter, builder arg, storage offsets, and all callers; shift creator ID to slots [6-7]. * refactor(pswap): masm perf + style cleanups from PR review theme 4 - Replace three `push.0 gt` (~16 cycles) zero-checks with `neq.0` (~3 cycles) at the has_account_fill, has_note_fill, and has_account_fill_payout branches. - Use `dup.1 dup.1 neq` instead of `lt` for the `is_partial` check; the `lte requested` assertion above guarantees `total_in <= total_requested`, so `!=` is equivalent and cheaper. - Flip the faucet-ID limb order at the create_remainder_note boundary from prefix-then-suffix to suffix-then-prefix (standard miden convention), updating the caller in execute_pswap, the proc docstring, local-slot comment, and the store block. - Drop the redundant `movdn.2` / `movup.2` pair in create_remainder_note — `remaining_requested` is already on top, so `mem_store` can take it directly. - Drop the `(4)` suffix on NOTE_ATTACHMENT / METADATA_HEADER stack comments; uppercase identifiers already imply a full word. - Remove `exec.sys::truncate_stack` and its stale `use` import from execute_pswap. Tracing the proc end-to-end (including every syscall wrapper, call frame, and proc-local store) shows the stack is already balanced at exit, and all 59 pswap integration tests pass without it. * refactor(pswap/tests): PR review theme 5 test quality cleanups - `pswap_note_invalid_input_test` asserts the specific error kind via `assert_transaction_executor_error!(result, ERR_PSWAP_FILL_EXCEEDS_REQUESTED)` instead of a bare `.is_err()`. A future bug that fails the tx for the wrong reason will now surface. - Drop the `rng: &mut RandomCoin` parameter from `build_pswap_note` — the helper draws its serial number from `builder.rng_mut()` directly. Remove all the `let mut rng = RandomCoin::new(Word::default())` declarations at call sites that only existed to feed this param. - Rename local helper `note_args` -> `pswap_args` so it reads clearly next to the (unrelated) `TransactionContextBuilder::extend_note_args` method it gets passed into. - Switch `assert_vault_added_removed` / `assert_vault_single_added` / `assert_fungible_asset_eq` helpers to take `FungibleAsset` instead of `(AccountId, u64)` tuples, and update every call site. - `pswap_note_note_fill_cross_swap_test`: replace the flag-and-for-loop "which note contains which asset" check with `iter().any()` against two locally-built `Asset::Fungible(FungibleAsset::new(...))` values. - `pswap_note_alice_reconstructs_and_consumes_p2id`: propagate the `prove_next_block()` error via `?` instead of swallowing it with `let _ =`. Still outstanding from theme 5: - realistic large amounts (20 * 10^18) in the happy-path tests - an integration test where both account_fill and note_fill are non-zero on the same note (the Rust unit tests cover the predictor path, but the integration-level cross-flow needs a cleaner two-note setup than a simple adaption of the existing cross-swap test). - skipping `Note -> PswapNote::try_from` roundtrips by having `build_pswap_note` return the `PswapNote` directly — a broader signature churn that is orthogonal to the items above. * test(pswap): finish theme 5 — realistic amounts, combined fill, skip try_from - `pswap_fill_test`: scale every amount by `AMOUNT_SCALE = 10^12` so the happy path exercises 12-decimal token values instead of single digits. The scale cap is chosen so `requested * FACTOR` stays under `u64::MAX`, matching the current MASM assumption. - `build_pswap_note` now returns `(PswapNote, Note)` instead of just the `Note`, so call sites can execute the PswapNote directly and skip the `PswapNote::try_from(¬e)?` round-trip the reviewer flagged. Every caller updated. - New `pswap_note_combined_account_fill_and_note_fill_test`: exercises a PSWAP fill where the consumer supplies BOTH `account_fill` and `note_fill` on the same note in the same transaction. Alice offers 100 USDC for 50 ETH, Bob offers 30 ETH for 60 USDC, Charlie consumes both — Alice's pswap uses combined fill (20 ETH from Charlie's vault + 30 ETH sourced from inflight via Bob's pswap payout), Bob's pswap uses pure note_fill. Asserts output notes, recipient parity, and Charlie's vault delta (-20 ETH / +40 USDC; the note_fill legs flow through inflight and never touch his vault). * refactor(pswap): rename execute_full_fill_network -> execute_full_fill The method has nothing network-specific about it — a "network transaction" is just one where the kernel defaults note_args to `[0, 0, 0, 0]` and the MASM script falls back to a full fill, which any caller can trigger by calling this method directly. Rename to reflect that, update the sole caller in the pswap_fill_test, and rewrite the doc comment to describe the behavior (full fill producing only the payback note, no remainder) rather than the network use case. Also scrub stale doc references left over from theme 3's `swap_count` removal: - drop the "swap count overflows u16::MAX" bullet from `execute`'s errors section - drop "swap count overflow" from `execute_full_fill`'s errors section (the whole section is gone — no error paths remain) - update the PswapNote struct doc: the remainder carries "an updated serial number", not "an incremented swap count" - update `create_remainder_pswap_note`'s doc for the same reason * refactor(pswap): drop FACTOR, use u128 math for payout calculation `calculate_tokens_offered_for_requested` previously used a fixed-point 1e5 precision factor with two separate branches (offered >= requested vs requested > offered) plus a full-fill early return, to approximate `offered * fill / requested` without overflowing u64 intermediates. That design had three problems: - Four `u64::wrapping_mul` calls silently produced wrong results for inputs where `x * FACTOR >= 2^64`, capping each operand at about 1.84e14 — roughly 0.000184 whole tokens for any 18-decimal asset. - The two-branch structure was fragile and harder to reason about than a single linear formula. - The FACTOR-scaled intermediate introduced rounding error in `ratio` that then got amplified by `fill`. Replace the whole proc with one linear path: product = u64::widening_mul(offered, fill_amount) # -> u128 quot = u128::div(product, requested_u128) # -> u128 assert q.hi_limbs == 0 # payout fits in u64 return q.lo_limbs combined back into a felt miden-core-lib 0.22 already exposes `u128::div` (advice-provider assisted, same host-compute-and-verify pattern as `u64::div`), so no custom division implementation is needed. Properties of the new version: - Exact integer precision (one floor division at the end, no FACTOR rounding). - Each operand can go up to `FungibleAsset::MAX ≈ 2^63`, so 18-decimal tokens now work at realistic volumes (~9.2 billion whole tokens per swap) — a ~50,000x improvement in dynamic range. - No branching on the offered/requested relationship. No full-fill early return. One generic formula that also handles the `fill_amount == requested` case trivially. - All wrapping_mul references gone, closing the last deferred theme-1 item from the PR review. Also mirror the same change on the Rust side: `calculate_output_amount` now just does `(offered as u128) * (fill as u128) / (requested as u128)` and `try_from`s back to u64, matching MASM exactly. Scrub the old PRECISION_FACTOR constant, the two-branch Rust logic, and the FACTOR doc block. The 8 in-crate unit tests and all 60 pswap script tests (including the 27-case hand-picked ratio regression suite and two seeded 30-iteration fuzzers) still pass. * test(pswap): second pass on PR review follow-ups - `calculate_output_amount` doc: fix a mismatched paren and the empty blank line between doc and fn signature that triggered clippy's `empty_line_after_doc_comments` (caught by CI after the last push). - `pswap_note_alice_reconstructs_and_consumes_p2id`: build the `PswapNote` via the builder, call `pswap.clone().into()` to get the protocol `Note`, and keep `pswap` in scope for `.execute(...)`. Drops the `PswapNote::try_from(&pswap_note)?` roundtrip the reviewer flagged on this exact test as a nit. - `pswap_chained_partial_fills_test`: same pattern — replace the manual `Note::new(NoteAssets / NoteMetadata / NoteRecipient)` construction with `PswapNote::builder().serial_number(current_serial)...build()?` + `pswap.clone().into()`. The raw-Note construction was only there to inject a specific serial per chain position, which the builder already supports. Also drops the dependent `PswapNote::try_from` roundtrip. - `pswap_fill_test`: bump `AMOUNT_SCALE` from 10^12 to 10^18 so the happy-path test exercises the MASM u128 calculation at realistic 18-decimal token magnitudes. Base values adjusted (offered=8, requested=4, fills=3/4) so every amount stays under AssetAmount::MAX ≈ 9.22 × 10^18. Previously the 10^12 scale was chosen to stay under the old FACTOR=1e5 cap; now that the u128 rewrite lifted that cap, the test can stress the calculation at the actual wei-equivalent scale the reviewer asked for. - New `pswap_attachment_layout_matches_masm_test`: dedicated regression test for the shared P2ID and remainder attachment-word layout. Does a partial fill, then explicitly asserts the executed transaction's P2ID attachment equals `[fill_amount, 0, 0, 0]` and the remainder attachment equals `[amt_payout, 0, 0, 0]`, and cross-checks both against the Rust-predicted attachments. Fires if either MASM or Rust drifts the load-bearing felt off `Word[0]`. - Apply the "blank line after every `# => [...]` stack comment" convention across pswap.masm where it was missing — one reviewer tends to leave this as a nit so sweep the whole file for consistency rather than wait for the comments. * style(pswap/tests): apply rustfmt * refactor(pswap): expose `PswapNote::create_args` as public helper Add `PswapNote::create_args(account_fill: u64, note_fill: u64) -> Result<Word, NoteError>` so downstream consumers building PSWAP `NOTE_ARGS` words don't need to know the `[account_fill, note_fill, 0, 0]` layout by hand. Returns a `Result` instead of panicking so the underlying `Felt::try_from` conversion errors propagate cleanly through the standard `NoteError` path — although for any amount that fits in `FungibleAsset::MAX_AMOUNT` this cannot actually fail, the conversion is surfaced explicitly rather than hidden behind an `.expect(...)` in a public API. Drop the local `pswap_args` test helper — every call site now uses `PswapNote::create_args(...)?` directly, matching the reviewer's suggestion that the layout helper live on `PswapNote` rather than being reimplemented in each consumer. All 61 pswap script tests pass with the new signature (twelve call sites updated to propagate the Result via `?`). * Update crates/miden-standards/asm/standards/notes/pswap.masm Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * Update crates/miden-standards/asm/standards/notes/pswap.masm Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * Update crates/miden-standards/asm/standards/notes/pswap.masm Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * Update crates/miden-standards/asm/standards/notes/pswap.masm Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * Update crates/miden-standards/asm/standards/notes/pswap.masm Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * Update crates/miden-standards/asm/standards/notes/pswap.masm Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * Update crates/miden-standards/asm/standards/notes/pswap.masm Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * fix(pswap): CI green — unused imports + duplicated exec Two fixes triggered by `make check-features` / `clippy -D warnings` and the full pswap test suite: - `tests/scripts/pswap.rs`: drop `NoteAssets`, `NoteMetadata`, `NoteRecipient`, `NoteStorage` from the `miden_protocol::note` import list. Leftovers from the theme-5 chained-fills refactor that switched from low-level `Note::new(NoteAssets, NoteMetadata, NoteRecipient)` construction to `PswapNote::builder()`. CI compiles with `-D warnings` so the unused-imports warning was fatal. - `asm/standards/notes/pswap.masm`: remove a duplicate `exec.calculate_tokens_offered_for_requested` line that had crept into the note-fill payout path in one of the upstream edits, plus a duplicate stack comment above the account-fill payout block. With the duplicate `exec`, the second call read garbage off a stack that only held the first call's payout — specifically a zero divisor, which surfaced as `u128::div` "division by zero" and failed 57/61 pswap tests. All 61 pswap script tests pass after the fix, and `cargo check --all-targets --all-features` on miden-testing is clean. * refactoring changes * fix(pswap): address PR review — storage, attachments, asset validation - Derive payback note tag from creator ID in MASM via note_tag::create_account_target instead of storing in note storage, reducing storage from 8 to 7 items. - Add dedicated attachment constructors (payback_attachment, remainder_attachment) on PswapNote for clarity and reuse. - Validate computed amounts as valid fungible asset amounts: - calculate_output_amount now returns Result and checks via AssetAmount. - MASM assert_valid_asset_amount proc checks u64 values against FUNGIBLE_ASSET_MAX_AMOUNT before felt conversion to prevent silent wrapping, applied to both fill sum and payout quotient. - Fix P2ID comment nit per reviewer suggestion. - Remove unused NoteTag import from p2id test. * style: apply nightly rustfmt * fix(pswap): simplify assert_valid_asset_amount and rename procedure - Replace manual hi/lo branch logic with exec.u64::lte - Rename calculate_tokens_offered_for_requested to calculate_output_amount (matches Rust) - Simplify tag derivation in create_p2id_note: swap+dup+swap+movup.2 → dup.1+movdn.2 * Update crates/miden-standards/asm/standards/notes/pswap.masm Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * Update crates/miden-standards/asm/standards/notes/pswap.masm Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * Update crates/miden-standards/asm/standards/notes/pswap.masm Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com> * test(pswap): add rstest cases for fill-sum overflow and max-asset-amount validation Converts pswap_note_invalid_input_test to rstest with three named cases: - fill_exceeds_requested (existing) - fill_sum_u64_overflow: both fills at 2^63, sum overflows u64 → ERR_PSWAP_FILL_SUM_OVERFLOW - fill_sum_exceeds_max_asset_amount: both fills at MAX_AMOUNT, sum > MAX_AMOUNT → ERR_PSWAP_NOT_VALID_ASSET_AMOUNT --------- Co-authored-by: Vaibhav Jindal <vaibhavjindal@Vaibhavs-MacBook-Pro.local> Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com>
* chore: remove attachment parameters from SWAP and MINT * chore: add changelog * chore: remove outdated comment
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
12 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.
This is a tracking PR for v0.15.0 release.