feat(standards): add flexible Burn Policies for Regulated Fungible Tokens by onurinanc · Pull Request #2664 · 0xMiden/protocol

onurinanc · 2026-03-23T13:52:48Z

Related Issue: #2638, OpenZeppelin/miden-confidential-contracts#88

We currently have MintPolicies in place (without having the MintPolicyManager as a separate account component), and in this PR we introduce BurnPolicies while preserving the same overall structure.

Follow-up PR: Similar to MintPolicyManager for the mint policies, we will include BurnPolicyManager in the follow-up PR when introducing role-based access control.

PhilippGackstatter

Looks good!

I have a few main comments:

I think the burn policy should not return the asset it takes as input, since it can't do anything useful with it, but correct me if I'm wrong.
There is a lot of duplication:
- We should be able to deduplicate the greatest amount of Rust code, i.e. OwnerControlled and AuthControlled.
- I didn't find a good opportunity to deduplicate the mint and burn MASM policy managers, but if there is one, we should probably do it.
May be worth double-checking that all procedures in the mint and burn policies modules that have Invocation: dynexec or Invocation: exec do not have pad comments, so we don't accidentally start overwriting these in the future (since the pad elements aren't actually garbage).

PhilippGackstatter · 2026-03-25T14:07:16Z

It looks like this is basically a copy of the mint policies OwnerControlled and similarly for AuthControlled, with the main difference being that it uses a different slot name? If there is no meaningful difference between the two, I wouldn't duplicate these but try to come up with a structure that accounts for the shared code. Maybe something like this:

// We should be able to make this type purely a private helper type now. pub(super) struct OwnerControlled { initial_policy_root: Word, } pub struct MintOwnerControlled(OwnerControlled); pub struct BurnOwnerControlled(OwnerControlled); impl MintOwnerControlled { pub const NAME: &'static str = "miden::standards::components::mint_policies::owner_controlled"; } impl BurnOwnerControlled { pub const NAME: &'static str = "miden::standards::components::burn_policies::owner_controlled"; } impl From<MintOwnerControlled> for AccountComponent { fn from(mint_owner_controlled: MintOwnerControlled) -> Self { // The encoding logic lives in OwnerControlled, and we just pass different slot names // here. let storage_slots = mint_owner_controlled.0.to_storage_slots( MintOwnerControlled::allowed_policy_proc_roots_slot(), MintOwnerControlled::active_policy_proc_root_slot(), ); todo!() } } impl From<BurnOwnerControlled> for AccountComponent { fn from(burn_owner_controlled: BurnOwnerControlled) -> Self { let storage_slots = mint_owner_controlled.0.to_storage_slots( BurnOwnerControlled::allowed_policy_proc_roots_slot(), BurnOwnerControlled::active_policy_proc_root_slot(), ); todo!() } }

This is just meant to illustrate the overall structure. Iiuc, the same applies to AuthControlled.

For the *PolicyAuthority, it might be fine to leave this duplicated since it's fairly little code.

Yes, but I do think that this deduplication makes the things more complicated. I'm not sure this is the ideal way to do it, I've tried to deduplicate but it doesn't seem good. I would prefer to address this again after we add RoleBasedAccessControlled for mint and burn policies. What do you think?

PhilippGackstatter · 2026-03-25T14:17:41Z

+pub proc allow_all
+    # Dummy predicate, no checks yet.
+    push.0 drop


Nit: "No checks yet" sounds like we'll add some meaningful checks in the future, but this policy allows everything, so is this accurate? If we change it here, let's also change it in the mint policy one.

PhilippGackstatter · 2026-03-25T14:20:26Z

+#! Inputs:  [ASSET_KEY, ASSET_VALUE, pad(16)]
+#! Outputs: [ASSET_KEY, ASSET_VALUE, pad(16)]
+#!
+#! Panics if:
+#! - note sender is not owner.
+#!
+#! Invocation: dynexec
+pub proc owner_only


I didn't notice this in the last PR, but since this procedure is exec-uted, we should remove the pad comments because there will actually be meaningful values on the stack that we shouldn't drop.

Applies to all burn and mint policies.

PhilippGackstatter · 2026-03-25T14:43:51Z

+    # => [ASSET_KEY, ASSET_VALUE, pad(16)]
+
+    dupw loc_storew_le.BURN_POLICY_ASSET_KEY_PTR dropw
+    dupw.1 loc_storew_le.BURN_POLICY_ASSET_VALUE_PTR dropw


Question: Why is the burn policy expected to return the asset if we require it to be identical? This means we could pass a copy of the asset to the policy instead and have it not return anything.

Also, loading the asset from the active note works, but incurs a lot of extra work (piping data from the advice provider through memory), and it would be easy to pass the asset instead, since this procedure is called from a trusted context.

So, ideally we can have this procedure be:

#! Executes active burn policy for the provided asset by dynamic execution. #! #! Inputs: [ASSET_KEY, ASSET_VALUE] #! Outputs: [] #! #! Panics if: #! - burn policy root is invalid. #! - active burn policy validation fails. #! - active burn policy mutates the asset key or value. #! #! Invocation: exec @locals(4) pub proc execute_burn_policy exec.get_burn_policy_root # => [BURN_POLICY_ROOT, ASSET_KEY, ASSET_VALUE] exec.assert_existing_policy_root # => [BURN_POLICY_ROOT, ASSET_KEY, ASSET_VALUE] exec.assert_allowed_policy_root # => [BURN_POLICY_ROOT, ASSET_KEY, ASSET_VALUE] loc_storew_le.BURN_POLICY_PROC_ROOT_PTR dropw # => [ASSET_KEY, ASSET_VALUE] locaddr.BURN_POLICY_PROC_ROOT_PTR # => [policy_root_ptr, ASSET_KEY, ASSET_VALUE] dynexec # => [] end

Note also that I've removed the pad comments, because this procedure is executed and should not drop any additional elements from the stack. We have 3-4 more procedures in this module and in the mint policy manager that have Invocation: exec but have pad comments. To avoid future mistakes due to these pad comments, it'd be good to remove these.

onurinanc · 2026-03-27T11:52:28Z

Hi @PhilippGackstatter, could you review this when you have a chance?

I’m wondering whether the deduplication related to the comment: #2664 (comment) is actually better now, or if it ended up making things more complex. Right now, it feels like the storage slots don’t have a clear structure anymore.

PhilippGackstatter

Looks good to me!

I agree the deduplication brought only minor benefits, but maybe this will change again with role-based access control? In any case, I left a few small comments, feel free to apply what you think makes sense.

PhilippGackstatter · 2026-04-01T14:14:13Z

+        Self(AuthControlled { initial_policy_root })
+    }
+
+    /// Creates a new [`BurnAuthControlled`] component with `allow_all` policy as default.


Suggested change

/// Creates a new [`BurnAuthControlled`] component with `allow_all` policy as default.

/// Creates a new [`BurnAuthControlled`] component with `allow_all` policy.

PhilippGackstatter · 2026-04-01T14:23:23Z

+}
+
+impl OwnerControlled {
+    pub(crate) fn mint_initial_storage_slots(


I agree the deduplication didn't bring many benefits and since this and mint_initial_storage_slots do not share any logic, I don't think we need to have these here. I think we can move these as private helper functions to MintOwnerControlled and BurnOwnerControlled.

PhilippGackstatter · 2026-04-01T14:25:35Z

+impl AuthControlled {
+    /// Builds the three storage slots for an auth-controlled policy manager component.
+    ///
+    /// `active_slot` / `allowed_slot` are storage **names**. `allow_all_procedure_root` is the
+    /// MAST root of the built-in `allow_all` procedure (map key when seeding `allowed_slot`), not
+    /// a slot.
+    pub(crate) fn initial_storage_slots(
+        &self,


If this is the only shared function, we can also consider making it a pub(crate) helper function that both use and remove the AuthControlled type. We can also leave it as is if we think we'll add more shared functionality in the future (as part of the role-based access control work, for example).

PhilippGackstatter · 2026-04-01T14:35:14Z

+pub use ::miden::standards::burn_policies::auth_controlled::allow_all
+pub use ::miden::standards::burn_policies::owner_controlled::owner_only
+pub use ::miden::standards::burn_policies::policy_manager::set_burn_policy
+pub use ::miden::standards::burn_policies::policy_manager::get_burn_policy


It looks odd that we export auth_controlled::allow_all from owner_controlled. I guess the reason is because we want to make allow_all available for use in OwnerControlled, so the owner can switch between allow_all or owner_only without having to change components? If so, would be good to add a comment here because this doesn't seem obvious.

I think the approach makes sense, but it made me realize that allow_all is generic and doesn't really have anything to do with "auth controlled". So I think we could export this instead as:

pub use ::miden::standards::burn_policies::allow_all

Which we should be able to do by defining that procedure in crates/miden-standards/asm/standards/burn_policies/mod.masm. And the auth controlled component would happen to use that.

The same probably applies to the mint policy one to keep things consistent, even if we don't re-export it from mint_policies::owner_controlled.

PhilippGackstatter · 2026-04-01T14:40:39Z

+static ACTIVE_BURN_POLICY_PROC_ROOT_SLOT_NAME: LazyLock<StorageSlotName> = LazyLock::new(|| {
+    StorageSlotName::new("miden::standards::burn_policy_manager::active_policy_proc_root")
+        .expect("storage slot name should be valid")
+});
+static ALLOWED_BURN_POLICY_PROC_ROOTS_SLOT_NAME: LazyLock<StorageSlotName> = LazyLock::new(|| {
+    StorageSlotName::new("miden::standards::burn_policy_manager::allowed_policy_proc_roots")
+        .expect("storage slot name should be valid")
+});


Nit: These are re-defined in auth and owner controlled, could we define these just once as private helpers?

PhilippGackstatter · 2026-04-01T14:45:11Z

+    /// Returns the storage slot schema for policy authority mode.
+    pub fn policy_authority_slot_schema() -> (StorageSlotName, StorageSlotSchema) {
+        (
+            Self::policy_authority_slot().clone(),
+            StorageSlotSchema::value(
+                "Policy authority mode (AuthControlled = tx auth, OwnerControlled = external owner)",
+                [
+                    FeltSchema::u8("policy_authority"),
+                    FeltSchema::new_void(),
+                    FeltSchema::new_void(),
+                    FeltSchema::new_void(),
+                ],
+            ),


Nit: If we move the storage slots to a shared location, it would also be nice to only define the schema once next to the slots.

PhilippGackstatter · 2026-04-01T15:06:16Z

+fn create_set_burn_policy_note_script(policy_root: Word) -> String {
+    format!(
+        r#"
+        use miden::standards::burn_policies::policy_manager->policy_manager


Suggested change

use miden::standards::burn_policies::policy_manager->policy_manager

use miden::standards::burn_policies::policy_manager

Nit: This seems redundant

PhilippGackstatter · 2026-04-01T15:06:47Z

+            repeat.12 push.0 end
+            push.{policy_root}


Suggested change

repeat.12 push.0 end

push.{policy_root}

padw padw padw

push.{policy_root}

Nit

PhilippGackstatter · 2026-04-01T15:17:14Z

One other scenario I was wondering about is adding a new policy component and use its exported policy. To use it, we'd also have to update the map of allowed procedure policy roots, but I think we don't have an API for this? This may be something to add in the future. If that's correct, would be great to open an issue to track that unless it will be covered in future PRs already. Thanks!

PhilippGackstatter

Looks good to me!

I'd remove the OwnerControlled type now that it doesn't have any functionality, and after that I think we can merge.

PhilippGackstatter · 2026-04-13T13:48:28Z

 #[derive(Debug, Clone, Copy)]
 pub(crate) struct OwnerControlled {
    pub(crate) initial_policy_root: Word,
 }


Since we now ended up with only sharing the struct layout, but no functionality, I'd remove this type and inline the initial_policy_root: Word.

PhilippGackstatter · 2026-04-16T09:13:41Z

+        .with_component(MintOwnerControlled::owner_only())
+        .with_component(BurnOwnerControlled::allow_all())


Judging by this comment (#2724 (comment)), this may have to be BurnOwnerControlled::owner_only(). Maybe @mmagician can double check.

onurinanc added 2 commits March 23, 2026 14:44

add burn policies

0d0bab2

changelog

5f83f68

onurinanc mentioned this pull request Mar 24, 2026

Add role-based access control to miden-standards #2685

Open

bobbinth requested review from PhilippGackstatter, bobbinth and mmagician and removed request for mmagician March 25, 2026 06:18

PhilippGackstatter reviewed Mar 25, 2026

View reviewed changes

onurinanc added 3 commits March 26, 2026 11:35

merge next

dd98e91

fix comments

384d1a0

refactor OwnerControlled and AuthControlled

c372e8c

onurinanc requested a review from PhilippGackstatter March 26, 2026 18:02

onurinanc mentioned this pull request Mar 27, 2026

Add BurnPolicies similar to MintPolicies OpenZeppelin/miden-confidential-contracts#88

Open

Merge branch 'next' into burn-policies

75b2bd9

PhilippGackstatter approved these changes Apr 1, 2026

View reviewed changes

onurinanc added 2 commits April 2, 2026 09:26

Merge branch 'next' into burn-policies

191004a

review fix

993f49f

onurinanc mentioned this pull request Apr 2, 2026

Update mint policy allowlist after account code upgrades #2616

Open

onurinanc added 2 commits April 2, 2026 17:33

move slots to a shared location

da85160

Merge remote-tracking branch 'origin/burn-policies' into burn-policies

a2f20cd

onurinanc mentioned this pull request Apr 2, 2026

Extract MintPolicyManager into its own component #2615

Open

onurinanc requested a review from PhilippGackstatter April 2, 2026 15:43

PhilippGackstatter approved these changes Apr 13, 2026

View reviewed changes

PhilippGackstatter reviewed Apr 16, 2026

View reviewed changes

	/// Creates a new [`BurnAuthControlled`] component with `allow_all` policy as default.
	/// Creates a new [`BurnAuthControlled`] component with `allow_all` policy.

	use miden::standards::burn_policies::policy_manager->policy_manager
	use miden::standards::burn_policies::policy_manager

		.with_component(MintOwnerControlled::owner_only())
		.with_component(BurnOwnerControlled::allow_all())

Conversation

onurinanc commented Mar 23, 2026

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

onurinanc commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PhilippGackstatter commented Apr 1, 2026

Uh oh!

PhilippGackstatter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

onurinanc commented Mar 27, 2026 •

edited

Loading