Refactor(account) introducing account id key#2495
Merged
PhilippGackstatter merged 16 commits into0xMiden:nextfrom Mar 6, 2026
Merged
Refactor(account) introducing account id key#2495PhilippGackstatter merged 16 commits into0xMiden:nextfrom
PhilippGackstatter merged 16 commits into0xMiden:nextfrom
Conversation
Contributor
|
Hi @swaploard, thanks for the PR! Some larger PRs were merged and now would be a good time to merge this PR. Can you merge next into this one? |
Contributor
Author
|
Sure
Sure, I will do that. |
Contributor
Author
|
@PhilippGackstatter, You can review it now. |
bobbinth
reviewed
Mar 4, 2026
Contributor
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you. I left some comments inline - most of them are small nits.
Comment on lines
+11
to
+12
| /// The account ID encoded as a key for use in AccountTree and | ||
| /// advice maps in `TransactionAdviceInputs`. |
Contributor
There was a problem hiding this comment.
nit: comment wrapping (we usually don't wrap comments before 100 chars).
| } | ||
|
|
||
| // SMT WORD REPRESENTATION | ||
| // ================================================================================================ |
Contributor
There was a problem hiding this comment.
nit: inside impls, we usually use ----- for separators (instead of ======).
Comment on lines
+52
to
+53
| // LEAF INDEX | ||
| // ================================================================================================ |
Comment on lines
+61
to
+62
| // ADVICE MAP KEY | ||
| // ================================================================================================ |
Comment on lines
+47
to
+50
| pub fn from_word(word: Word) -> AccountId { | ||
| AccountId::try_from_elements(word[KEY_SUFFIX_IDX], word[KEY_PREFIX_IDX]) | ||
| .expect("account tree should only contain valid IDs") | ||
| } |
Contributor
There was a problem hiding this comment.
I think this should be try_from_word() and return an error if the conversion failed.
Comment on lines
+64
to
+67
| /// Returns advice map key: | ||
| /// | ||
| /// `[suffix, prefix, 0, 0]` | ||
| pub fn to_advice_map_word(&self) -> Word { |
Contributor
There was a problem hiding this comment.
Do we actually want to have different ways of encoding the ID into a word? Maybe we can align these to make sure we always use [0, 0, suffix, prefix] representation?
Contributor
Author
There was a problem hiding this comment.
We do not need this.
Comment on lines
32
to
34
| pub fn account_id_to_smt_index(account_id: AccountId) -> LeafIndex<SMT_DEPTH> { | ||
| account_id_to_smt_key(account_id).into() | ||
| AccountIdKey::from(account_id).as_word().into() | ||
| } |
Contributor
There was a problem hiding this comment.
Do we still need this function?
Contributor
Author
There was a problem hiding this comment.
No we dont need this
| pub fn leaf(&self) -> SmtLeaf { | ||
| if self.commitment == Word::empty() { | ||
| let leaf_idx = LeafIndex::from(account_id_to_smt_key(self.id)); | ||
| let leaf_idx = LeafIndex::from(AccountIdKey::from(self.id).as_word()); |
Contributor
There was a problem hiding this comment.
Could this not be something like:
let leaf_idx = AccountIdKey::from(self.id).to_left_index();|
|
||
| // Read the account header elements from the advice map. | ||
| let account_id_key = TransactionAdviceInputs::account_id_map_key(account_id); | ||
| let account_id_key = AccountIdKey::from(account_id).as_word(); |
Contributor
There was a problem hiding this comment.
nit: I'd probably do just:
let account_id_key = AccountIdKey::from(account_id)And then below, would do:
.get(&account_id_key.as_word())
3 tasks
PhilippGackstatter
approved these changes
Mar 6, 2026
Contributor
PhilippGackstatter
left a comment
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Looks good to me, thanks! I'll push some small fixes to prevent potential panics in AccountTree code.
afa7789
pushed a commit
to afa7789/miden-base
that referenced
this pull request
Mar 9, 2026
* Introducing a dedicated AccountIdKey type to unify and centralize all AccountId * changelog for introduce AccountIdKey type * refactor: clean up comments and improve code readability in AccountIdKey and tests * refactor: update AccountIdKey conversion method and clean up imports * refactor: reorganize AccountIdKey indices and clean up related code * Apply suggestions from code review * Update crates/miden-protocol/src/block/account_tree/account_id_key.rs * Update crates/miden-protocol/src/block/account_tree/account_id_key.rs * feat: validate account ID in `AccountTree::new` --------- Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com> Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
afa7789
pushed a commit
to afa7789/miden-base
that referenced
this pull request
Mar 9, 2026
* Introducing a dedicated AccountIdKey type to unify and centralize all AccountId * changelog for introduce AccountIdKey type * refactor: clean up comments and improve code readability in AccountIdKey and tests * refactor: update AccountIdKey conversion method and clean up imports * refactor: reorganize AccountIdKey indices and clean up related code * Apply suggestions from code review * Update crates/miden-protocol/src/block/account_tree/account_id_key.rs * Update crates/miden-protocol/src/block/account_tree/account_id_key.rs * feat: validate account ID in `AccountTree::new` --------- Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com> Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
mmagician
added a commit
that referenced
this pull request
Mar 19, 2026
* chore: refactor tx kernel from `ASSET` to `ASSET_KEY` and `ASSET_VALUE` (#2396) * chore: refactor `miden::protocol` from `ASSET` to `ASSET_KEY` and `ASSET_VALUE` (#2410) * feat: adapt the layout of `Asset`s to the double word representation (#2437) * feat: migrate to miden VM 0.21 and miden crypto 0.22 (#2508) * feat: optimize layouts and procedures for little-endian (#2512) * chore: use `get_balance` helper in account_delta * chore: Add `TryFrom<Word> for AssetVaultKey` * feat: refactor `asset.masm` * feat: add `fungible_asset.masm` * feat: refactor `asset_vault.masm` * feat: refactor `faucet.masm` * feat: refactor `account.masm` * feat: refactor `account_delta.masm` * feat: refactor `epilogue.masm` * feat: refactor `output_note.masm` * feat: refactor `prologue.masm` * chore: increase `NOTE_MEM_SIZE` to 3072 * chore: adapt `NoteAssets` commitment * feat: refactor `note.masm` * chore: refactor `api.masm` * chore: regenerate kernel proc hashes * chore: add changelog * fix: faucet::mint output docs * chore: update memory.rs input/output note memory layouts * fix: duplicate num assets in memory.rs table * feat: move `build_asset_vault_key` to shared utils * feat: refactor `faucet::mint` * feat: refactor `faucet::burn` * chore: refactor `create_non_fungible_asset` for uniformity * feat: refactor `native_account::remove_asset` * chore: move `mock::util` lib to miden-standards * feat: refactor `move_asset_to_note` * feat: add asset key to SWAP storage * feat: refactor `native_account::add_asset` * chore: refactor `receive_asset` * feat: refactor `output_note::add_asset` * chore: deduplicate epilogue asset preservation test * chore: remove re-export of vault key builder procedures * chore: regenerate kernel procedure hashes * chore: add changelog * fix: doc link to mock util lib * chore: improve send_note_body impl and test * fix: replace leftover `ASSET`s with `ASSET_VALUE` * chore: update protocol library docs * fix: rename leftover `ASSET` to `ASSET_VALUE` * chore: remove unused error * chore: regenerate tx kernel errors * chore: improve note assets commitment computation * fix: input notes memory assertions * chore: add renamed procedures to changelog * fix: incorrect stack and doc comment * fix: p2id::new test * feat: validate new asset layouts * chore: update asset procedure calls in faucet * feat: add create_fungible_asset_unchecked * chore: change has_non_fungible_asset to take asset key * feat: include full faucet ID limbs in asset delta * chore: remove into `Word` conversions for assets * feat: Implement strongly typed asset vault key * chore: temporarily remove asset vault key hash * chore: remove asset from word conversion * feat: update `Asset` docs * chore: remove unused (non-)fungible asset builders * feat: refactor asset serialization and tests * chore: add validation in get_asset and get_initial_asset * chore: adapt miden-standards to changed asset def * chore: adapt miden-tx to changed asset def * feat: return native asset ID and fee amount as tx output * chore: update create_non_fungible_asset faucet/asset APIs * chore: adapt tests to new asset layouts * feat: validate asset ID prefix in non-fungible assets * chore: drop trailing "asset" from `asset::validate_asset` * chore: rewrite asset validation tests * chore: merge validate_value in `fungible_asset` * chore: remove unused build_asset_vault_key procedures * chore: remove `LexicographicWord` wrapper in non-fungible asset delta * chore: update asset::create_non_fungible_asset signature in docs * fix: test name, vault key display impl, remove unused errors * fix: intra doc links * chore: add changelog * Initial plan * Address review nits: add load_asset_key_and_value utility, use explicit ASSET_PTR constants, rename variables, add test Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> * Fix load_asset_key_and_value procedure and usages in prologue and epilogue Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> * Run cargo fmt to fix formatting issues Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> * Rename asset_value back to asset in tx_event.rs Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> * Run cargo fmt to fix formatting after variable rename Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> * chore: simplify fungible_asset merge and add split * chore: remove outdated edge case handling in vault * chore: convert u64s from BE to LE * feat: migrate rpo256 -> poseidon2 * chore: upgrade miden VM & crypto in Cargo.toml * chore: refactor memory.masm from BE to LE * chore: update broken imports in `miden-protocol` * chore: introduce `FromNum` and `TryFromNum` traits * chore: FieldElement import, as_int, and read_many_iter in `protocol` * chore: replace imports in miden-protocol-macros * chore: update imports in miden-standards * chore: update imports in miden-tx * chore: deactivate agglayer in workspace and miden-testing * chore: update imports in miden-testing * chore: migrate account ID and asset-related MASM modules to LE * chore: migrate account, delta and note-related MASM modules to LE * chore: migrate prologue & epilogue to LE * chore: update empty SMT root * chore: migrate tx and api.masm modules to LE * chore: reverse tx stack inputs and outputs * chore: migrate miden::protocol from BE to LE * chore: swap order of link map operands on adv stack * chore: swap prefix/suffix in asset vault test * chore: use more resilient way to write test_transaction_epilogue * chore: migrate tests in miden-testing from BE to LE * chore: update schema type IDs for falcon512_poseidon2::pub_key * chore: migrate miden::standards::{access, auth} from BE to LE * chore: migrate miden::standards::faucets from BE to LE * chore: migrate miden::standards::data_structures from BE to LE * chore: migrate miden::standards::notes from BE to LE * chore: migrate miden::standards::{wallets, attachments} from BE to LE * fix: mmr authentication path for latest block * chore: use falcon512_poseidon2::encode_signature for sig prep * chore: update protocol library docs * chore: remove @BigEndian from type defs * fix: avoid using undeclared stack * feat: reexport asset::mem_load * chore: migrate singlesig_acl from be to le * fix: multisig stack layouts after migration * chore: update Cargo.lock and regenerate kernel proc hashes * chore: cargo update to latest crypto and VM patch releases * fix: unused imports after miden-field upgrade * chore: use `Felt::{from, try_from}` and remove `(Try)FromNum` * chore: remove temp mmr fix and adapt find_half_key_value call * chore: regenerate kernel proc hashes * chore: make toml * chore: add changelog entry * fix: outdated doc links * fix: make format * chore: deactivate miden-agglayer in check-features.sh * chore: allow bincode in deny.toml * fix: deny.toml entries and upgrade toml to 1.0 * fix: transaction context builder doc test * feat: Add `Asset::as_elements` * chore: use `asset::mem_load` in swap note * fix: update agglayer::bridge::bridge_out to new asset layout * chore: remove `Ord for Asset` impl * chore: re-add error when asset-to-be-removed is not present * chore: remove whitespace in docs * chore: `cargo update` * chore: use seed digest 2 and 3 as account ID suffix and prefix * chore: remove `AccountId::try_from<[Felt; 2]>`; add `try_from_elements` * chore: consistently use `RATE0, RATE1, CAPACITY` for hasher state * chore: optimize account_id::validate for little-endian order * chore: optimize account ID seed validation * chore: reverse tx summary stack layout * feat: optimize `account::is_slot_id_lt` for little-endian layout * chore: regenerate kernel proc hashes * chore: add changelog * chore: rename `Falcon512Rpo` to `Falcon512Poseidon2` * chore: rename `_ADDRESS` to `_PTR` in swap * chore: rename `asset::{mem_store, mem_load}` to store/load * chore: use imports instead of absolute paths * chore: reword non-fungible asset key collision resistance * chore: add `AssetId::is_empty` * chore: remove unnecessary imports in `AssetVault` * chore: spell out asset key layout in has_non_fungible_asset * chore: validate account ID type in AssetVaultKey::new * fix: typo in Asset docs * chore: prefix `get/into_faucet_id` with `key` * chore: prefix `get/into_asset_id` with `key` * chore: suffix `is_(non_)fungible_asset` with `key` * chore: address review comments * chore: update outdated tx kernel input/output docs * chore: LE layout for upcoming fpi account ID; fix stack comments * chore: replace `type Word` with built-in `word` * fix: asset::mem_store using `_be` instructions * chore: update `add_input_notes` docs * chore: Add `fungible_asset::to_amount` * chore: replace `swap.3` with `movdn.3` * chore: rename `get_balance_from_fungible_asset` to `value_into_amount` * chore: use more explicit `poseidon2::copy_digest` * chore: ensure asset vault key validity for fungible keys --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mmagician <8402446+mmagician@users.noreply.github.com> Co-authored-by: Bobbin Threadbare <bobbinth@protonmail.com> * fix: remove unused MASM imports (#2543) * feat: Enable warnings_as_errors in assembler * fix: remove unused imports in miden-protocol * fix: remove unused imports in miden-standards * fix: enable `miden-crypto/std` in `testing` feature to fix MSRV check (#2544) The migration to miden-crypto 0.22 replaced `winter_rand_utils::rand_value` with `miden_crypto::rand::test_utils::rand_value`, which is gated behind `#[cfg(any(test, feature = "std"))]`. Since the workspace dependency uses `default-features = false`, crates depending on `miden-protocol/testing` without `std` failed to compile, breaking the release-dry-run MSRV check. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: enable CI on merge queue trigger (#2540) * chore: enable CI on merge queue trigger * Update .github/workflows/build-docs.yml Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * feat: enable warning as errors for `CodeBuilder` (#2558) * refactor: enforce defining supported types on metadata (#2554) * feat: enforce maximum serialized size for output notes (#2205) * feat: introduce `AccountIdKey` (#2495) * Introducing a dedicated AccountIdKey type to unify and centralize all AccountId * changelog for introduce AccountIdKey type * refactor: clean up comments and improve code readability in AccountIdKey and tests * refactor: update AccountIdKey conversion method and clean up imports * refactor: reorganize AccountIdKey indices and clean up related code * Apply suggestions from code review * Update crates/miden-protocol/src/block/account_tree/account_id_key.rs * Update crates/miden-protocol/src/block/account_tree/account_id_key.rs * feat: validate account ID in `AccountTree::new` --------- Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com> Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com> * fix: make format and remove unused import (#2564) * feat: make `NoteMetadataHeader` public (#2561) * chore: ProvenBlock constructor with validation (#2553) * refactor: include fee in TransactionId computation * refactor: remove `Ord` and `PartialOrd` from `StorageSlot` (#2549) * chore: Replace SMT leaf conversion function (#2271) * feat: add ability to submit user batches for the MockChain (#2565) * chore: Explicitly use `get_native_account_active_storage_slots_ptr` in `account::set_item` and `account::set_map_item` * chore: fix typos * feat: integrate PSM contracts to AuthMultisig (#2527) * chore: remove `ProvenTransactionBuilder` in favor of `ProvenTransaction::new` (#2567) * feat: implement `Ownable2Step` (#2292) * feat: implement two-step ownership management for account components - Add `ownable2step.masm` to provide two-step ownership functionality, requiring explicit acceptance of ownership transfers. - Create Rust module for `Ownable2Step` struct to manage ownership state and storage layout. - Introduce error handling for ownership operations in `standards.rs`. - Develop comprehensive tests for ownership transfer, acceptance, and renouncement scenarios in `ownable2step.rs`. * feat: add Ownable2Step account component for two-step ownership transfer * fix: correct ownership word layout and update transfer ownership logic in ownable2step * Refactor ownership management to use Ownable2Step pattern - Updated the access control mechanism to implement a two-step ownership transfer pattern in the ownable2step module. - Modified the storage layout to accommodate the new ownership structure, reversing the order of owner and nominated owner fields. - Refactored the network fungible faucet to utilize the new Ownable2Step for ownership management. - Adjusted related tests to validate the new ownership transfer logic and ensure correct behavior when transferring and accepting ownership. - Updated error handling to reflect changes in ownership management, including new error messages for ownership transfer scenarios. * refactor: update ownership word layout and adjust related logic in Ownable2Step * refactor: rename owner and nominated_owner to get_owner and get_nominated_owner for consistency * refactor: streamline ownership management in tests by utilizing Ownable2Step according to philip * fix: `make format` --------- Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com> Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com> * feat: prefix account components with miden::standards namespace (#2400) * refactor: rename output note structs (#2569) * feat: add `create_fungible_key` proc for fungible asset vault keys (#2575) * feat(asm): add create_fungible_key proc for fungible asset vault keys * chore: re-export create_fungible_key from `protocol::asset` * chore: add changelog --------- Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com> * feat: migrate miden-agglayer to VM 0.21 and crypto 0.22 (#2546) * feat: migrate miden-agglayer to VM 0.21 and crypto 0.22 Re-enable the miden-agglayer crate in the workspace and apply all necessary API and convention changes from the VM 0.21 migration: Rust changes: - Remove FieldElement trait imports (removed from miden-core) - Felt::as_int() -> Felt::as_canonical_u64() - Program import moved to miden_core::program::Program - bytes_to_packed_u32_felts -> bytes_to_packed_u32_elements (miden_core::utils) - AccountId::try_from([Felt;2]) -> AccountId::try_from_elements(suffix, prefix) - Serializable/Deserializable from miden_assembly::serde (was ::utils) MASM changes: - rpo256 -> poseidon2 (hash function migration) - asset::mem_store/mem_load -> asset::store/load (procedure renames) - u64::overflowing_mul -> u64::widening_mul - u128_sub_no_underflow rewritten for LE convention (u64 procedures now use [lo, hi] input/output order) - verify_u128_to_native_amount_conversion updated for LE result order Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: migrate agglayer tests and fix LE convention issues Re-enable agglayer tests in miden-testing and fix all compilation and runtime errors from the VM 0.21 migration: Test compilation fixes: - FieldElement import removal, as_int -> as_canonical_u64 - AuthScheme::Falcon512Rpo -> Falcon512Poseidon2 - AdviceInputs path: miden_processor::advice::AdviceInputs - FastProcessor::new_debug -> new().with_advice().with_debugging() - StackInputs::new(vec![]) -> new(&[]) - bytes_to_packed_u32_felts -> bytes_to_packed_u32_elements - AccountId::try_from -> try_from_elements MASM runtime fixes: - eth_address.masm: fix u32split LE output order in build_felt verification (movup.4 -> movup.3 for lo/hi comparison) - bridge_out.masm: fix create_burn_note note_idx corruption - loc_loadw_be overwrites top 4 stack elements including both copies from dup; save note_idx to local instead (pre-existing bug that only manifested with multiple output notes) - bridge_out.masm: fix num_leaves storage LE ordering - push new_leaf_count to stack top for Word[0] storage, use mem_storew_le instead of mem_storew_be for loading - bridge_config.masm: update GER hash from Rpo256 to Poseidon2 - canonical_zeros: remove .rev() from build.rs generation, swap push order for LE memory layout - Word element ordering fixes for bridge admin, GER manager, faucet registry keys, and conversion metadata Test expectation fixes: - Rpo256 -> Poseidon2 for GER hash computation - Removed word reversal in root/proof reading (LE convention) - Fixed expected storage value ordering - mem_storew_be -> mem_storew_le in test MASM code All 39 agglayer tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * style: apply rustfmt formatting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: address PR review comments - Extract `create_id_key(id: AccountId) -> Word` helper and reuse for bridge_admin, ger_manager, bridge_account_id, and faucet_registry_key - Replace `felts_to_bytes` with re-export of `packed_u32_elements_to_bytes` from miden-core (identical implementation) - Remove stale comment in config_note.rs - Use `Felt::ONE` instead of `Felt::new(1)` in config_bridge test - Add TODO comments referencing issue #2548 for getter helpers - Simplify note_idx handling: use `padw` before `loc_loadw_le` instead of saving to a local variable; revert @Locals(15) to @Locals(14) - Switch `loc_storew_be`/`loc_loadw_be` to `_le` variants in create_burn_note - Add stack state comments before third overflowing_sub in u128_sub_no_underflow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: padding is 15 * remove utils re-export, faucet_registry_key * chore: drop 4 words * fix: pad before call invoc.; explicit stack comments * fix: unnecesary dup and incorrect movup * style: codify multi-element naming convention for MASM stack comments Add convention to masm_doc_comment_fmt.md: - `value` = single felt - `value(N)` = N felts (non-word) - `VALUE` = word (4 felts, no (4) suffix needed) Apply throughout agglayer MASM: drop redundant (4) from ASSET_KEY, ASSET_VALUE, SERIAL_NUM, SCRIPT_ROOT, RECIPIENT, NOTE_ATTACHMENT, PROC_MAST_ROOT, OLD_VALUE, VALUE, U256_LO, U256_HI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: update comments on poseidon inputs * Update crates/miden-protocol/masm_doc_comment_fmt.md Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com> * chore: test for high x3 * fix: correctly rearrange lo/hi for sub * chore: more explicit sub comments * chore: revert changes to faucet config slot * chore: revert prefix<>suffix storage ordering * fix: store flag values at word[0] for LE convention Change GER_KNOWN_FLAG and IS_FAUCET_REGISTERED_FLAG to be stored at word[0] instead of word[3]. Under LE, `push.0.0.0.FLAG` puts FLAG at stack top = word[0], matching the documented layout [1, 0, 0, 0]. Previously `push.FLAG.0.0.0` placed FLAG at stack bottom = word[3], resulting in Word([0, 0, 0, 1]) which contradicted the docs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: use felts directly instead of extracting u64 * chore: fix build * Apply suggestion from @PhilippGackstatter Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com> * chore: use AccountIdKey * refactor: push explicit zeros, dont assume padding is 0 * refactor: simplify assert_faucet_registered by asserting first * chore: re-add miden-agglayer to feature checks Now that miden-agglayer has been migrated, add it back to the check-features.sh loop. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor: explicit pointers for storing prefix/suffix * refactor: swap to_account_id output to LE convention [suffix, prefix] All other bridge/faucet MASM procedures use [suffix, prefix] (suffix on top) for account IDs on the stack. Align to_account_id with this convention by adding a swap at the end, and update all callers: - faucet/mod.masm: update get_destination_account_id_data, claim, and build_p2id_output_note to handle the new stack order - solidity_miden_address_conversion test: swap stack index expectations Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude (Opus) <noreply@anthropic.com> Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com> Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com> Co-authored-by: Bobbin Threadbare <bobbinth@protonmail.com> * feat: make `Ownable2Step` an `AccountComponent` (#2572) * feat: add `from_parts_unchecked()` method for `InputNoteCommitment` (#2588) * feat: support bool types on schemas (#2591) * fix: move recompute logic to `OutputNoteBuilder::add_asset` to avoid repeated hashing (#2577) * fix: move recompute logic to OutputNoteBuilder::add_asset to avoid rehashing * changelog and lint * chore: add `NoteAssets::into_vec` --------- Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com> Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com> * feat: expose `AccountComponentMetadata` through public method (#2596) * fix: `TokenSymbol::try_from(Felt)` underflow (#2568) * feat: implement asset callback support in tx kernel (#2571) * feat: implement `on_before_asset_added_to_note` callback (#2595) * feat: add support for callbacks in non-fungible assets * chore: add test for block list note callback * feat: implement `on_before_asset_added_to_note` callback * chore: add test to make sure inputs are received correctly * chore: deduplicate test setup code * chore: add changelog * fix: non-fungible asset delta not including asset metadata * chore: deduplicate callback invocation procedures * chore: deduplicate blocked account test * chore: deduplicate note callback test * chore: test empty callback proc root slot * chore: Introduce `end_foreign_callback_context` * feat: validate asset metadata (#2609) * feat: flexible Minting Policies for Token Standards (#2559) * refactor: move storage schema component into a separate file (#2603) * feat: add `Package` support in `MockChainBuilder` & `NoteScript` (#2502) * feat: introduce `SwapNoteStorage` (#2592) * feat: add callback docs (#2604) * feat: add hooks to enable TX debugging (#2574) * Migrate to Miden VM `v0.22.0-alpha.1` (#2625) * refactor: update RandomCoin, use position of LeafIndex * chore: update deny file * chore: use version from crates.io, update deny file * fix: swap FAUCET_ID_SUFFIX/PREFIX constants in CONFIG_AGG_BRIDGE The constants FAUCET_ID_SUFFIX and FAUCET_ID_PREFIX were swapped relative to the actual storage layout in config_note.rs (suffix at index 5, prefix at index 6). This caused register_faucet to store with a mismatched key, so lookups returned empty. Also fix the misleading doc comment in config_note.rs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * lint * fix: use Ownable2Step and OwnerControlled components for agglayer faucet The NetworkFungibleFaucet's mint_and_send now requires Ownable2Step for ownership verification and OwnerControlled for mint policy management. Migrate the agglayer faucet from a custom owner_config storage slot to these standard components: - Remove bridge_account_id from AggLayerFaucet (ownership is now handled by the Ownable2Step component) - Add Ownable2Step and OwnerControlled as companion components in the faucet account builder - Use Ownable2Step::try_from_storage to extract the owner in bridge_account_id() helper Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: include Ownable2Step and OwnerControlled in faucet code commitment The build.rs computes the faucet code commitment for validation checks. After adding Ownable2Step and OwnerControlled as companion components, the code commitment must include their procedures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use mem_storew_le for MINT note storage in bridge_in The MINT note script reads storage with mem_loadw_le (LE convention), so the bridge must store the P2ID script root, serial number, and attachment data using mem_storew_le to match. Using mem_storew_be caused the P2ID script root hash to be stored with reversed elements, making it unresolvable by the kernel. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: bridge_in endianness + clippy warnings - Use mem_storew_le for MINT note storage in bridge_in.masm to match the MINT note script's mem_loadw_le reads (VM 0.21 byte-endianness). - Add MintNote::script() to bridge_in TX3 context. - Fix clippy useless_conversion warnings in bridge.rs. - Formatting fix in faucet.rs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: add `FixedWidthString<N>` utility (#2633) * Update crates/miden-agglayer/src/bridge.rs * chore: fix padding comments * chore: add owner controlled slot names to faucet --------- Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Bobbin Threadbare <bobbinth@protonmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: igamigo <ignacio.amigo@lambdaclass.com> Co-authored-by: Forostovec <ilonaforostovec22@gmail.com> Co-authored-by: Nikhil Patil <nikhil876706@gmail.com> Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com> Co-authored-by: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com> Co-authored-by: Serge Radinovich <47865535+sergerad@users.noreply.github.com> Co-authored-by: Percy Dikec <112529374+PercyDikec@users.noreply.github.com> Co-authored-by: onurinanc <e191322@metu.edu.tr> Co-authored-by: Himess <95512809+Himess@users.noreply.github.com> Co-authored-by: Arthur Abeilice <afa7789@gmail.com> Co-authored-by: Poulav <bpoulav@gmail.com> Co-authored-by: juan518munoz <62400508+juan518munoz@users.noreply.github.com> Co-authored-by: Tomas Fabrizio Orsi <tomas.orsi@lambdaclass.com> Co-authored-by: djole <djolertrk@gmail.com> Co-authored-by: Andrey Khmuro <andrey@polygon.technology>
afa7789
pushed a commit
to afa7789/miden-base
that referenced
this pull request
Apr 21, 2026
* Introducing a dedicated AccountIdKey type to unify and centralize all AccountId * changelog for introduce AccountIdKey type * refactor: clean up comments and improve code readability in AccountIdKey and tests * refactor: update AccountIdKey conversion method and clean up imports * refactor: reorganize AccountIdKey indices and clean up related code * Apply suggestions from code review * Update crates/miden-protocol/src/block/account_tree/account_id_key.rs * Update crates/miden-protocol/src/block/account_tree/account_id_key.rs * feat: validate account ID in `AccountTree::new` --------- Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com> Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
onurinanc
added a commit
to onurinanc/miden-base
that referenced
this pull request
Apr 22, 2026
* Refactor code structure for improved readability and maintainability
* fix: fixing after rebase.
* refactor: improve formatting and readability in various files
* Refactor metadata handling in fungible faucets
- Updated the `create_basic_fungible_faucet` and `create_network_fungible_faucet` functions to use boolean flags for description, logo URI, and external link mutability instead of integer flags.
- Modified the `Info` struct to replace the previous flag system with separate boolean fields for initialized and mutable states.
- Adjusted storage layout documentation to reflect changes in metadata configuration.
- Updated tests to align with the new boolean flag system for mutability and initialization.
- Ensured backward compatibility by updating mock chain builder and test cases to use the new structure.
* refactor: update token metadata references in faucet tests to use FungibleTokenMetadata
* chore: lint
* Refactor token metadata encoding to use 7-bytes-per-felt, length-prefixed format
- Updated `TokenName`, `Description`, `LogoURI`, and `ExternalLink` structs to support new encoding scheme.
- Adjusted maximum byte lengths and encoding/decoding methods to accommodate the new format.
- Modified metadata module to reflect changes in field sizes and encoding conventions.
- Updated tests to validate new field sizes and encoding behavior.
- Changed mock chain builder and associated tests to handle updated metadata structure.
* refactor: replace RPO256 with Poseidon2 for hashing in fungible metadata and update related tests
* refactor: format BasicFungibleFaucet initialization for improved readability
* fix: adapt metadata MASM to little-endian word ordering after upstream migration
The upstream LE migration (PR #2512) changed get_item to return [word[0], ..., word[3]]
(word[0] on top) instead of [word[3], ..., word[0]]. This commit:
- Reverses element extraction logic in all getters (get_token_metadata, get_max_supply,
get_decimals, get_token_symbol, get_token_supply, mutability checks)
- Fixes mutability flag extraction in all optional setters
- Fixes max_supply replacement logic in optional_set_max_supply
- Changes loc_storew_be/loc_loadw_be to loc_storew_le/loc_loadw_le to preserve
natural word element ordering in memory for hash computations
- Updates test hasher from Rpo256 to Poseidon2 to match MASM poseidon2 module
- Fixes get_owner stack depth (get_item returns +2 net elements with 2-felt slot IDs)
- Updates all inline MASM test assertions to match LE stack ordering
* chore: adding type as entry instead of separate variables
* refactor: enforce defining supported types on metadata (#2554)
* feat: enforce maximum serialized size for output notes (#2205)
* feat: introduce `AccountIdKey` (#2495)
* Introducing a dedicated AccountIdKey type to unify and centralize all AccountId
* changelog for introduce AccountIdKey type
* refactor: clean up comments and improve code readability in AccountIdKey and tests
* refactor: update AccountIdKey conversion method and clean up imports
* refactor: reorganize AccountIdKey indices and clean up related code
* Apply suggestions from code review
* Update crates/miden-protocol/src/block/account_tree/account_id_key.rs
* Update crates/miden-protocol/src/block/account_tree/account_id_key.rs
* feat: validate account ID in `AccountTree::new`
---------
Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com>
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
* fix: make format and remove unused import (#2564)
* feat: make `NoteMetadataHeader` public (#2561)
* chore: ProvenBlock constructor with validation (#2553)
* refactor: include fee in TransactionId computation
* refactor: remove `Ord` and `PartialOrd` from `StorageSlot` (#2549)
* chore: Replace SMT leaf conversion function (#2271)
* feat: add ability to submit user batches for the MockChain (#2565)
* chore: Explicitly use `get_native_account_active_storage_slots_ptr` in `account::set_item` and `account::set_map_item`
* chore: fix typos
* feat: integrate PSM contracts to AuthMultisig (#2527)
* chore: remove `ProvenTransactionBuilder` in favor of `ProvenTransaction::new` (#2567)
* feat: implement `Ownable2Step` (#2292)
* feat: implement two-step ownership management for account components
- Add `ownable2step.masm` to provide two-step ownership functionality, requiring explicit acceptance of ownership transfers.
- Create Rust module for `Ownable2Step` struct to manage ownership state and storage layout.
- Introduce error handling for ownership operations in `standards.rs`.
- Develop comprehensive tests for ownership transfer, acceptance, and renouncement scenarios in `ownable2step.rs`.
* feat: add Ownable2Step account component for two-step ownership transfer
* fix: correct ownership word layout and update transfer ownership logic in ownable2step
* Refactor ownership management to use Ownable2Step pattern
- Updated the access control mechanism to implement a two-step ownership transfer pattern in the ownable2step module.
- Modified the storage layout to accommodate the new ownership structure, reversing the order of owner and nominated owner fields.
- Refactored the network fungible faucet to utilize the new Ownable2Step for ownership management.
- Adjusted related tests to validate the new ownership transfer logic and ensure correct behavior when transferring and accepting ownership.
- Updated error handling to reflect changes in ownership management, including new error messages for ownership transfer scenarios.
* refactor: update ownership word layout and adjust related logic in Ownable2Step
* refactor: rename owner and nominated_owner to get_owner and get_nominated_owner for consistency
* refactor: streamline ownership management in tests by utilizing Ownable2Step according to philip
* fix: `make format`
---------
Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com>
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
* feat: add token metadata support to NetworkFungibleFaucet
* feat: enforce maximum serialized size for output notes (#2205)
* feat: implement `Ownable2Step` (#2292)
* feat: implement two-step ownership management for account components
- Add `ownable2step.masm` to provide two-step ownership functionality, requiring explicit acceptance of ownership transfers.
- Create Rust module for `Ownable2Step` struct to manage ownership state and storage layout.
- Introduce error handling for ownership operations in `standards.rs`.
- Develop comprehensive tests for ownership transfer, acceptance, and renouncement scenarios in `ownable2step.rs`.
* feat: add Ownable2Step account component for two-step ownership transfer
* fix: correct ownership word layout and update transfer ownership logic in ownable2step
* Refactor ownership management to use Ownable2Step pattern
- Updated the access control mechanism to implement a two-step ownership transfer pattern in the ownable2step module.
- Modified the storage layout to accommodate the new ownership structure, reversing the order of owner and nominated owner fields.
- Refactored the network fungible faucet to utilize the new Ownable2Step for ownership management.
- Adjusted related tests to validate the new ownership transfer logic and ensure correct behavior when transferring and accepting ownership.
- Updated error handling to reflect changes in ownership management, including new error messages for ownership transfer scenarios.
* refactor: update ownership word layout and adjust related logic in Ownable2Step
* refactor: rename owner and nominated_owner to get_owner and get_nominated_owner for consistency
* refactor: streamline ownership management in tests by utilizing Ownable2Step according to philip
* fix: `make format`
---------
Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com>
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
* feat: prefix account components with miden::standards namespace (#2400)
* Refactor code structure for improved readability and maintainability
* Refactor metadata handling in fungible faucets
- Updated the `create_basic_fungible_faucet` and `create_network_fungible_faucet` functions to use boolean flags for description, logo URI, and external link mutability instead of integer flags.
- Modified the `Info` struct to replace the previous flag system with separate boolean fields for initialized and mutable states.
- Adjusted storage layout documentation to reflect changes in metadata configuration.
- Updated tests to align with the new boolean flag system for mutability and initialization.
- Ensured backward compatibility by updating mock chain builder and test cases to use the new structure.
* refactor: update token metadata references in faucet tests to use FungibleTokenMetadata
* chore: adding type as entry instead of separate variables
* feat: add owner support to TokenMetadata and update related components
* fix: correct output order in get_owner function
* feat: enhance ownership handling in fungible faucets and metadata components
* fix: update documentation for metadata_slot and description commitment tests
* Update crates/miden-standards/asm/standards/access/ownable.masm
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
* refactor: improve comments and assertions in distribute function for minting constraints
* Refactor fungible metadata procedures: rename optional setters to standard setters and remove unused commitment functions
* refactor: standardize TokenMetadata usage across faucets and tests
* chore: Refactor code structure for improved readability and maintainability
* feat: add TokenMetadata and AccountSchemaCommitment components for enhanced account metadata management, split from mod.
* chore: lint
* refactor: replace TokenName::new("") with TokenName::default() for consistency
* refactor: update metadata field limits and improve code consistency
* chore: lint
* remove duplicates!
* refactor: remove unused scale_down_amount procedure
* refactor: account metadata handling and remove owner dependency in favor of ownable2steps.
- Updated account metadata module to clarify ownership management, removing direct references to the owner in metadata.
- Simplified `TokenMetadata` struct by eliminating owner-related fields and methods, focusing on name and optional fields.
- Adjusted storage layout and accessors to reflect changes in metadata structure.
- Removed owner-related tests and mock implementations from testing suite.
- Introduced Rpo256 hashing for field data to enhance integrity checks in metadata operations.
* refactor: simplify network fungible faucet creation by consolidating metadata parameters
* refactor: clone token symbol in FungibleTokenMetadata creation to avoid ownership issues
* refactor: update comments for clarity and fixed hash handling in token metadata tests
* refactor: streamline fungible token metadata handling in tests and simplify faucet creation
* Refactor FungibleTokenMetadata creation to use builder pattern
- Introduced FungibleTokenMetadataBuilder to simplify the creation of FungibleTokenMetadata instances.
- Updated all instances of FungibleTokenMetadata creation across the codebase to utilize the new builder pattern.
- Enhanced readability and maintainability by allowing optional fields to be set in a chainable manner.
- Adjusted tests and mock implementations to accommodate the new builder approach.
* refactor: update faucet initialization to use FungibleTokenMetadataBuilder
* refactor: format imports and streamline FungibleTokenMetadataBuilder usage in tests
* refactor: rename mutability flags for clarity and consistency in token metadata handling
* docs: update comments to clarify procedures in BasicFungibleFaucet, fixing lint/doc
* feat: integrate AuthControlled mint policy into BasicFungibleFaucet setup
* feat: changed metadata structure and fixed-width string utility
- moved `TokenMetadata` and `TokenName` structs to manage token metadata including name, description, logo URI, and external link.
- Implemented `FixedWidthString` for encoding fixed-width UTF-8 strings into storage words.
- Added utility functions for reading and writing token metadata to account storage.
- Created a new module for string utilities and organized existing modules.
* chore: ci/cd fix
* chore: change for rust doc
* chore: fmt
* refactor: update fungible token metadata methods to be on pair with comments and discussion on github.
* chore: lint wrong import
* refactor: update fungible token metadata structure and error handling
* reset files in crates agglayer to be exactly the same the next.
* chore: reset files in crates agglayer to be exactly the same the next.
* chore: MASM - rename + refactor fungible_faucet
* refactor: core metadata restructure
* chore: faucets cleanup
* refactor: clean up imports and update token metadata handling
* refactor: simplify FungibleTokenMetadataBuilder usage and improve readability
* fix: update layout sync reference for fungible faucet metadata
* refactor: update imports for FungibleTokenMetadataBuilder and clean up faucet references
* refactor: update function calls to use fungible_faucet metadata
* sync agglayer with upstream/next
* chore: remove agglayer changes.
* refactor: update versioning and error handling in token metadata and faucet modules
* refactor: reorganize token metadata handling and maintain backward compatibility on Agglayer
* refactor: update slot name for token metadata to align with FungibleTokenMetadata
* refactor: update token metadata handling and error messages for fungible tokens
* refactor: enhance fungible token metadata handling and validation logic
* refactor: change new_validated + builder to accept u64 for supply fields
Move max_supply validation and Felt conversion into new_validated so the
check lives in exactly one place. Builder token_supply field is now u64
for API consistency, removing the Felt::new() with SAFETY comment.
* fix: propagate TokenName errors in network_faucet_metadata helper
Replace silent fallback to empty TokenName with ? operator so test
failures surface the actual error instead of hiding it.
* refactor: replace Felt::new assertions with try_from storage roundtrip
Use FungibleTokenMetadata::try_from(storage) to verify stored values
instead of raw Felt::new comparisons on individual storage word elements.
* test: remove duplicate MASM name readback tests
Remove basic_faucet_name_readable_from_masm and
network_faucet_name_readable_from_masm which are functionally
equivalent to get_name_from_masm.
* test: remove name_32_bytes_accepted duplicate test
Already covered by basic_faucet_with_max_name_and_full_description
which builds an account with a 32-byte name and verifies storage
round-trip.
* test: move name_33_bytes_rejected + description_7_words to unit tests
Delete name_33_bytes_rejected (already covered by token_name_too_long)
and description_7_words_full_capacity from integration tests. Add
description_7_words_full_capacity as a pure unit test in miden-standards
using into_storage_slots() instead of building a full Account.
* refactor: convert is_field_mutable_checks to rstest parametrized test
Replace manual Vec of cases with rstest #[case] attributes using
build_faucet_metadata() + with_*_mutable() for conciseness. No content
fields needed since these tests only check the flag bit.
* refactor: simplify field setter test scripts
Remove .add_note_script() from owner-succeeds test since notes already
have code via .code(). Simplify non-owner-fails test by removing hash
push and advice_map since the auth check fires before data is touched.
* test: use multi-word token name in build_pol_faucet_metadata
Change token name from "POL" (3 bytes, single word chunk) to "Polygon
Token" (13 bytes) so both name_chunk_0 and name_chunk_1 are non-zero,
exercising both storage slots.
* test: remove unnecessary push.0 between getter calls
The push.0 padding between successive call-convention getter invocations
was unnecessary — call already pads the stack to 16 elements. Verified
the test passes without them.
* refactor: remove unused import and redundant note script compilation
* refactor: remove unused note script compilation from max supply tests
* refactor: simplify hash handling in test_field_setter_owner_succeeds
* refactor: simplify note script code in set_max_supply_mutable_non_owner_fails test
* refactor: update `miden-vm` to v0.22.1 and apply compatibility fixes (#2742)
* chore: increment crate versions to v0.14.3 (#2744)
* refactor(agglayer): lowercase note script filenames in miden-agglayer (#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>
* refactor: change `TokenSymbol` to `Symbol` and add new `TokenSymbol` and `RoleSymbol` types (#2690)
* feat(asset): add AssetAmount wrapper type (#2721)
* 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>
* fix: validate leafType on CLAIM note processing (#2730)
* chore: bump rand to 0.9.3 and 0.10.1 (#2756)
* feat: add `metadata_into_note_type` procedure to note.masm (#2738)
* 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>
* refactor: remove unnecessary asm/ tree copy from standards and agglayer 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 (#2741)
* 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>
* chore: make metadata procedure names consistent and update swap tag construction (#2758)
* chore: make note procedure names consistent
* chore: add changelog
* chore: update swap tag construction
* feat: document reentrancy of auth procedures (#2784)
* feat: Add cycle counts to successful and failed notes (#2772)
* Add cycle counts to successful and failed notes
* Changelog
* Cycle count order comment
* Make fields private
* fix(bench): enable concurrent feature for miden-tx in bench-transaction (#2790)
* Improve deserialization error handling in AccountCode (#2788)
* fix: improve deserialization error handling in AccountCode
* chore: update changelog
* refactor: removed AccountCode::from_bytes()
* Enable `concurrent` feature for `std` context (#2791)
* refactor: always enable concurrent on std in miden-tx
* chore: delete miden-protocol-macros crate
* chore: update changelog
* PSwap Contract (#2636)
* 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 unused SWAP and MINT attachments (#2789)
* chore: remove attachment parameters from SWAP and MINT
* chore: add changelog
* chore: remove outdated comment
---------
Co-authored-by: igamigo <ignacio.amigo@lambdaclass.com>
Co-authored-by: Forostovec <ilonaforostovec22@gmail.com>
Co-authored-by: Nikhil Patil <nikhil876706@gmail.com>
Co-authored-by: Bobbin Threadbare <43513081+bobbinth@users.noreply.github.com>
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
Co-authored-by: Santiago Pittella <87827390+SantiagoPittella@users.noreply.github.com>
Co-authored-by: Serge Radinovich <47865535+sergerad@users.noreply.github.com>
Co-authored-by: Bobbin Threadbare <bobbinth@protonmail.com>
Co-authored-by: Percy Dikec <112529374+PercyDikec@users.noreply.github.com>
Co-authored-by: onurinanc <e191322@metu.edu.tr>
Co-authored-by: Himess <95512809+Himess@users.noreply.github.com>
Co-authored-by: djole <djolertrk@gmail.com>
Co-authored-by: Alexander John Lee <77119221+partylikeits1983@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Giwa Victor <132751591+giwaov@users.noreply.github.com>
Co-authored-by: Marti <marti@miden.team>
Co-authored-by: nad <100929843+addnad@users.noreply.github.com>
Co-authored-by: VaibhavJindal <vaibhavjindal29@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Al-Kindi-0 <82364884+Al-Kindi-0@users.noreply.github.com>
Co-authored-by: Vaibhav Jindal <vaibhavjindal@Vaibhavs-MacBook-Pro.local>
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
3 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 PR addresses issue (#2443) by introducing a dedicated AccountIdKey type to unify and centralize all AccountId → SMT and advice-map key conversions across the account tree and transaction layers. Previously, this logic was implemented via multiple free/helper functions, leading to duplication and scattered layout assumptions. This change removes those functions and replaces them with a single, strongly-typed abstraction that encapsulates the canonical word representations, leaf index conversion, and advice map key format, improving type safety, maintainability, and clarity.