Flexible Minting Policies for Token Standards#2559
Flexible Minting Policies for Token Standards#2559PhilippGackstatter merged 39 commits into0xMiden:nextfrom
Conversation
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! Not a full review yet - but I left some comments inline. The main comment so far is about splitting the component into a "policy manager" component and separate policy components (probably just a single policy component for now).
| #! | ||
| #! Invocation: exec | ||
| proc assert_valid_policy_root | ||
| dupw exec.word::eqz |
There was a problem hiding this comment.
We could use word::testz procedure here to avoid the dupw.
| exec.active_account::get_map_item | ||
| # => [ALLOWED_FLAG, MINT_POLICY_ROOT] | ||
|
|
||
| dupw exec.word::eqz not |
| const MINT_POLICY_PROC_ROOT_SLOT=word("miden::standards::mint_policies::proc_root") | ||
| const MINT_POLICY_PARAMS_SLOT=word("miden::standards::mint_policies::params") | ||
| const MINT_POLICY_ALLOWED_ROOTS_SLOT=word("miden::standards::mint_policies::allowed_roots") | ||
| const MINT_POLICY_PROC_ROOT_PTR=0 |
There was a problem hiding this comment.
Would be good to add some comments explaining the purpose and expected formats of the storage slots. For example, it is not immediately clear what "allowed roots" means.
crates/miden-standards/asm/standards/mint_policies/owner_controlled.masm
Show resolved
Hide resolved
|
All done! Please review: @bobbinth |
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Looks good to me!
I left a few comments related to naming and structure, e.g. whether it should be MintPolicies or MintPolicyManager.
| # See the `BasicFungibleFaucet` Rust type's documentation for more details. | ||
|
|
||
| pub use ::miden::standards::faucets::basic_fungible::distribute | ||
| pub use ::miden::standards::faucets::basic_fungible::mint |
There was a problem hiding this comment.
That's a very welcome change 🙂
Since this does both minting (like miden::protocol::faucet::mint), but also creating a note, maybe we should consider mint_to_note as the name to reflect the note creation side effect in the name.
There was a problem hiding this comment.
@PhilippGackstatter I think the intent of mint_to_note is good but instead what about having mint_and_create_note which semantically sounds better?
There was a problem hiding this comment.
That's a bit more explicit and also a good choice. Another one might be mint_and_send (saw it in OpenZeppelin/miden-confidential-contracts#79 (comment)) which is a bit more concise but less explicit (does not mention note).
There was a problem hiding this comment.
I like mint_and_send - though, not a very strong preference.
And, in a future PR, we could also add a regular mint procedure which mints an asset and adds it to the account's vault.
| pub proc distribute | ||
| pub proc mint | ||
| exec.faucets::distribute |
There was a problem hiding this comment.
The faucets::distribute should probably also be renamed to mint, right?
| #! - note sender is not owner. | ||
| #! | ||
| #! Invocation: dynexec | ||
| pub proc mint_policy_owner_only |
There was a problem hiding this comment.
Nit: It might be sufficient to name this owner_only, since the module is called mint_policies already, so the module-qualified name would be mint_policies::owner_only which reads naturally.
| # Active policy root slot. | ||
| # Layout: [proc_root_w0, proc_root_w1, proc_root_w2, proc_root_w3] | ||
| const MINT_POLICY_PROC_ROOT_SLOT=word("miden::standards::mint_policies::proc_root") |
There was a problem hiding this comment.
| # Active policy root slot. | |
| # Layout: [proc_root_w0, proc_root_w1, proc_root_w2, proc_root_w3] | |
| const MINT_POLICY_PROC_ROOT_SLOT=word("miden::standards::mint_policies::proc_root") | |
| # Active policy root slot. | |
| # Layout: [PROC_ROOT] | |
| const MINT_POLICY_PROC_ROOT_SLOT=word("miden::standards::mint_policies::proc_root") |
Nit: Avoid specifying the individual word elements.
| exec.word::testz not | ||
| assert.err=ERR_MINT_POLICY_ROOT_NOT_ALLOWED | ||
| # => [ALLOWED_FLAG, MINT_POLICY_ROOT] | ||
|
|
||
| dropw | ||
| # => [MINT_POLICY_ROOT] |
There was a problem hiding this comment.
Nit: We can avoid the dropw by simplifying to word::eqz not assert (or assertz directly).
| #[derive(Debug, Clone, Copy)] | ||
| pub struct MintPolicies { | ||
| initial_policy_root: Word, | ||
| } |
There was a problem hiding this comment.
Should this be called MintPolicyManager and the mint_policies_library be renamed to mint_policy_manager_library along with crates/miden-standards/asm/account_components/faucets/mint_policies.masm?
I'd also suggest renaming MintPolicyConfig -> MintPolicy so we can "instantiate a mint policy manager with a mint policy".
| let allowed_roots = | ||
| if mint_policies.initial_policy_root == MintPolicies::owner_only_policy_root() { | ||
| StorageMap::with_entries([( | ||
| StorageMapKey::from_raw(MintPolicies::owner_only_policy_root()), | ||
| Word::from([1u32, 0, 0, 0]), | ||
| )]) | ||
| } else { | ||
| StorageMap::with_entries([ | ||
| ( | ||
| StorageMapKey::from_raw(MintPolicies::owner_only_policy_root()), | ||
| Word::from([1u32, 0, 0, 0]), | ||
| ), | ||
| ( | ||
| StorageMapKey::from_raw(mint_policies.initial_policy_root), | ||
| Word::from([1u32, 0, 0, 0]), | ||
| ), | ||
| ]) | ||
| } |
There was a problem hiding this comment.
Nit: Consider introducing a constant or variable that represents the "allowed flag" (word [1,0,0,0]).
| let mut rng = RpoRandomCoin::new([Felt::from(rng_seed); 4].into()); | ||
| let note = NoteBuilder::new(sender_account_id, &mut rng) | ||
| .note_type(NoteType::Private) | ||
| .tag(NoteTag::default().into()) | ||
| .serial_number(Word::from([rng_seed, rng_seed + 1, rng_seed + 2, rng_seed + 3])) | ||
| .code(note_script_code) | ||
| .build()?; |
There was a problem hiding this comment.
| let mut rng = RpoRandomCoin::new([Felt::from(rng_seed); 4].into()); | |
| let note = NoteBuilder::new(sender_account_id, &mut rng) | |
| .note_type(NoteType::Private) | |
| .tag(NoteTag::default().into()) | |
| .serial_number(Word::from([rng_seed, rng_seed + 1, rng_seed + 2, rng_seed + 3])) | |
| .code(note_script_code) | |
| .build()?; | |
| let mut rng = RpoRandomCoin::new([Felt::from(rng_seed); 4].into()); | |
| let note = NoteBuilder::new(sender_account_id, &mut rng) | |
| .note_type(NoteType::Private) | |
| .code(note_script_code) | |
| .build()?; |
Nit: The serial number is generated by the RNG internally and the note tag should already be set to its default by the builder.
The NoteBuilder can also compile a string itself so no need to use CodeBuilder above.
|
|
||
| let tx_context = mock_chain | ||
| .build_tx_context(faucet_id, &[], &[note])? | ||
| .add_note_script(note_script) |
There was a problem hiding this comment.
| .add_note_script(note_script) |
Nit: Haven't checked but this is likely unnecessary.
| /// Tests owner minting on a network faucet built from two separate components: | ||
| /// `NetworkFungibleFaucet` and `MintPolicies`. | ||
| #[tokio::test] | ||
| async fn test_network_faucet_owner_can_mint_with_separate_mint_policies_component() |
There was a problem hiding this comment.
Is this test effectively testing additional things than the other tests that use a network fungible faucet with MintPolicyConfig::OwnerOnly? I think it doesn't and if so, I'd remove it.
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! Not a full review still, but I left some comments inline.
crates/miden-standards/asm/standards/mint_policies/policy_manager.masm
Outdated
Show resolved
Hide resolved
| #! Enforces active mint policy by dynamic execution. | ||
| #! | ||
| #! Inputs: [amount, tag, note_type, RECIPIENT, pad(9)] | ||
| #! Outputs: [amount, tag, note_type, RECIPIENT, pad(9)] |
There was a problem hiding this comment.
I think the motivation here is that we create a note right after calling this procedure, and so, duplicating all note inputs would be a bit wasteful. I don't really have a strong preference here.
But it does make me think that this procedure is specifically related to sending notes. So, maybe the name here should reflect this? Something like enforce_mint_and_send_policy?
| #! - active policy predicate fails. | ||
| #! | ||
| #! Invocation: exec | ||
| pub proc enforce_mint_policy |
There was a problem hiding this comment.
I actually don't mind enforce_ prefix, but also don't have anything against the execute_ prefix.
| .storage_mode(AccountStorageMode::Network) | ||
| .with_auth_component(auth_component) | ||
| .with_component(NetworkFungibleFaucet::new(symbol, decimals, max_supply, owner_account_id)?) | ||
| .with_component(MintPolicies::new(mint_policy_config)) |
There was a problem hiding this comment.
Once we have the Ownable2Step component, we'd need to add it here too, right?
crates/miden-standards/asm/standards/mint_policies/owner_controlled.masm
Show resolved
Hide resolved
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Looks good to me!
I'll approve since most of my comments apply to follow-ups.
| pub use ::miden::standards::mint_policies::owner_controlled::owner_only | ||
| pub use ::miden::standards::mint_policies::policy_manager::set_mint_policy | ||
| pub use ::miden::standards::mint_policies::policy_manager::get_mint_policy |
There was a problem hiding this comment.
Generally agreed that it seems better to have a base MintPolicyManager component that can be extended in plug & play fashion with additional mint policy components.
Not a strong opinion whether we should do it in this PR or later.
crates/miden-standards/asm/standards/mint_policies/policy_manager.masm
Outdated
Show resolved
Hide resolved
crates/miden-standards/asm/standards/mint_policies/policy_manager.masm
Outdated
Show resolved
Hide resolved
crates/miden-standards/asm/standards/mint_policies/policy_manager.masm
Outdated
Show resolved
Hide resolved
| #! | ||
| #! Invocation: exec | ||
| @locals(4) | ||
| pub proc execute_mint_policy |
There was a problem hiding this comment.
This procedure validates the mint policy, but get_mint_policy does not. Depending on the perspective, both are valid approaches:
- We can say that since
set_mint_policyvalidates the procedure when setting, so the stored mint policy is valid. - The only exception to the above is that the account deployer could set an invalid root, and so we could also say that we should always validate the mint policy on retrieval.
So far, I think we have usually used the first approach, though this was mostly for component-internal data. I'm not sure how much damage a malicious deployer could do by setting an invalid mint policy root, probably not much other than making their own account unusable, so I'd go with approach 1.
This makes me wish we could let every account component have some on-chain validation of its storage at account creation time so we could always go with the first approach and provide stronger guarantees.
There was a problem hiding this comment.
I think this is a good point! I agree that we would have stronger guarantees with on-chain validation, but it also adds more complexity for account creation, and this also needs an execution order design and rules between the other account components. I'm not exactly sure which one is the right direction.
| let policy_authority_slot = StorageSlot::with_value( | ||
| AuthControlled::policy_authority_slot().clone(), | ||
| Word::from([0u32, 0, 0, 0]), | ||
| ); |
There was a problem hiding this comment.
I think we should introduce an enum for the policy authority, because it's otherwise hard to understand which zero represents the actual config and what that zero represents. Something like:
#[repr(u8)]
pub enum MintPolicyAuthority {
AuthControlled = 0,
OwnerControlled = 1,
}
impl From<MintPolicyAuthority> for StorageSlot { ... }
impl AuthControlled {
pub fn mint_policy_authority(&self) -> MintPolicyAuthority {
MintPolicyAuthority::AuthControlled
}
}And then here we'd have:
let policy_authority_slot = StorageSlot::from(auth_controlled.mint_policy_authority);| static ACTIVE_MINT_POLICY_PROC_ROOT_SLOT_NAME: LazyLock<StorageSlotName> = LazyLock::new(|| { | ||
| StorageSlotName::new("miden::standards::mint_policy_manager::active_policy_proc_root") | ||
| .expect("storage slot name should be valid") | ||
| }); | ||
| static ALLOWED_MINT_POLICY_PROC_ROOTS_SLOT_NAME: LazyLock<StorageSlotName> = LazyLock::new(|| { | ||
| StorageSlotName::new("miden::standards::mint_policy_manager::allowed_policy_proc_roots") | ||
| .expect("storage slot name should be valid") | ||
| }); | ||
| static POLICY_AUTHORITY_SLOT_NAME: LazyLock<StorageSlotName> = LazyLock::new(|| { | ||
| StorageSlotName::new("miden::standards::mint_policy_manager::policy_authority") | ||
| .expect("storage slot name should be valid") | ||
| }); |
There was a problem hiding this comment.
These definitions are duplicated in crates/miden-standards/src/account/mint_policies/auth_controlled.rs, would be great to deduplicate, but also fine to do as part of the MintPolicyManager refactor.
The "miden::standards::mint_policy_manager::policy_authority" slot could be defined on the previously proposed MintPolicyAuthority enum.
|
All done! The corresponding issues for the follow-up PRs are opened! @bobbinth @PhilippGackstatter |
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
There was a problem hiding this comment.
Looks good! Thank you! I left just two small comments inline.
I think we should also create an issue with all the remaining follow-ups. These include:
- Extracting policy manager into its own component.
- Potentially collapsing basic and network faucets into a single component.
- Adding ability to update policy allowlist (though, this probably would be a separate issue which would be blocked on code upgrade functionality).
- Anything else?
| static MINT_POLICY_OWNER_CONTROLLED_LIBRARY: LazyLock<Library> = LazyLock::new(|| { | ||
| let bytes = include_bytes!(concat!( | ||
| env!("OUT_DIR"), | ||
| "/assets/account_components/mint_policies/owner_controlled.masl" | ||
| )); | ||
| Library::read_from_bytes(bytes) | ||
| .expect("Shipped Mint Policy Owner Controlled library is well-formed") | ||
| }); | ||
|
|
||
| // Initialize the Mint Policy Auth Controlled library only once. | ||
| static MINT_POLICY_AUTH_CONTROLLED_LIBRARY: LazyLock<Library> = LazyLock::new(|| { | ||
| let bytes = include_bytes!(concat!( | ||
| env!("OUT_DIR"), | ||
| "/assets/account_components/mint_policies/auth_controlled.masl" | ||
| )); | ||
| Library::read_from_bytes(bytes) | ||
| .expect("Shipped Mint Policy Auth Controlled library is well-formed") | ||
| }); |
There was a problem hiding this comment.
I've started moving these static variables into relevant modules (e.g., see here). We can do this in a follow-up though.
There was a problem hiding this comment.
I've left it in the follow-up as the static variables would change.
| #[repr(u8)] | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub enum MintPolicyAuthority { | ||
| AuthControlled = 0, | ||
| OwnerControlled = 1, | ||
| } |
There was a problem hiding this comment.
Would be great to add doc comments both to the enum itself as well as to the variants explaining their meaning and purpose.
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Looks good to me!
| pub(super) fn policy_authority_slot_name() -> &'static StorageSlotName { | ||
| &POLICY_AUTHORITY_SLOT_NAME | ||
| } |
There was a problem hiding this comment.
Might be good to expose this as a pub fn, so
impl MintPolicyAuthority {
pub fn slot() -> &'static StorageSlotName { ... }
}* 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>
This PR corresponds to the discussion for Miden Token Standards, overall, it implements flexible and modular minting policies: OpenZeppelin/miden-confidential-contracts#39
Design specification related to this document: OpenZeppelin/miden-confidential-contracts#39 (comment)
Please see additional design discussion here: OpenZeppelin/miden-confidential-contracts#39 (reply in thread)
[ ] Left to-do: I would like to rename this procedure as
pub proc mintas we have discussed with @bobbinth but I would like to double-check is there any other thoughts on this?