feat(standards): add Blocklistable component for per-account freezing by onurinanc · Pull Request #2820 · 0xMiden/protocol

onurinanc · 2026-04-23T13:14:21Z

Closes the issue:

Add ability to freeze specific accounts: Blocklistable #2819
Add ability to freeze specific accounts: Blocklistable OpenZeppelin/miden-confidential-contracts#103

bobbinth

Looks good! Thank you! Not a full review yet, but I left some comments inline which could change the approach in this PR.

bobbinth

Thank you! Looks good! Not a full review yet, but I left a few comments inline about some of the structural aspects.

bobbinth · 2026-05-11T07:42:25Z

+pub enum TransferPolicy {
+    /// Active policy = [`TransferAllowAll::root`] (the callback predicate accepts unconditionally).
+    #[default]
+    AllowAll,
+    /// Active policy = [`TransferIfNotBlocklisted::root`]. The policy component installs the
+    /// `blocked_users` storage map alongside its predicate procedure (see
+    /// [`crate::account::faucets::Blocklist`] for the storage namespace).
+    IfNotBlocklisted,
+    /// Active policy = the provided root. The corresponding component(s) must be installed by
+    /// the caller separately; resolving this variant into built-in components yields an empty
+    /// list.
+    Custom(Word),
+}


Maybe not for this PR, but I wonder if we should change this into a struct - e.g., something like:

pub struct TransferPolicy(Vec<AccountComponent>); impl TransferPolicy { pub fn from_component(component: AccountComponent) -> Self { ... } pub fn allow_all() -> Self { ... } pub fn basic_blocklist() -> Self { ... } }

This would also apply to other similar enums.

PhilippGackstatter

Looks good to me!

I think the primary things for this PR are:

Potentially removing a level of indirection for the asset callbacks, as described by @bobbinth.
Potentially collapsing TransferIfNotBlocklisted and Blocklist into one.
Avoiding panics in TokenPolicyManager.

The outcome of #2862 should probably not hold this PR up.

PhilippGackstatter · 2026-05-12T08:42:57Z

+
+# The slot where the per-faucet map of blocked users is stored.
+# Map entries: [0, 0, account_id_suffix, account_id_prefix] => [is_blocked, 0, 0, 0]
+const BLOCKED_USERS_SLOT = word("miden::standards::faucets::restrictions::blocklist::blocked_users")


Why does this fall under restrictions rather than policies::transfer?

PhilippGackstatter · 2026-05-12T11:11:01Z

+/// exposed on a production faucet. Tests may invoke them via `exec.` directly when the test
+/// transaction runs with the faucet as the native account.
+#[derive(Debug, Clone, Copy, Default)]
+pub struct TransferIfNotBlocklisted;


Not necessarily for this PR, but I think we should probably support the "reconstruction" of account components from an Account. The unit structs we have now do not support this, because here, for example, we assume the block list is initially empty (which is fine). But to support account inspection, and component upgrades, it'd be easier if we could support such reconstruction. This I think applies to:

TransferIfNotBlocklisted

Blocklist

RoleBasedAccessControl

For now I think we can just open an issue and track this.

PhilippGackstatter · 2026-05-12T11:12:30Z

+impl OwnerOnlyBlocklistAdmin {
+    /// The name of the component.
+    pub const NAME: &'static str =
+        "miden::standards::components::faucets::restrictions::blocklist_owner";
+


I would make the component struct name and the NAME consistent, e.g.:

Suggested change

impl OwnerOnlyBlocklistAdmin {

/// The name of the component.

pub const NAME: &'static str =

"miden::standards::components::faucets::restrictions::blocklist_owner";

impl OwnerOnlyBlocklistAdmin {

/// The name of the component.

pub const NAME: &'static str =

"miden::standards::components::faucets::restrictions::owner_only_blocklist_admin";

PhilippGackstatter · 2026-05-12T11:15:37Z

+///   word `[1, 0, 0, 0]`; the zero word (including the default for unset entries) means not
+///   blocked.
+#[derive(Debug, Clone, Copy)]
+pub struct Blocklist;


Maybe I'm missing it, but what is the advantage of having Blocklist and TransferIfNotBlocklisted? Seems like these could be merged.

PhilippGackstatter · 2026-05-12T12:31:38Z


 mod basic_fungible;
 mod network_fungible;
+pub mod restrictions;


Suggested change

pub mod restrictions;

mod restrictions;

We usually don't make these public.

PhilippGackstatter · 2026-05-12T13:04:14Z

+    /// Every kind must have exactly one [`PolicyRegistration::Active`] entry; otherwise the
+    /// internal storage-slot construction panics.
    fn into_iter(self) -> Self::IntoIter {


We should avoid these panics by construction.

We could use a bon::builder to make sure the active policies are set:

#[bon::bon] impl TokenPolicyManager { #[builder] pub fn new( #[builder(field)] allowed_mint_policies: BTreeMap<Word, PolicyConfig>, #[builder(field)] allowed_burn_policies: BTreeMap<Word, PolicyConfig>, #[builder(field)] allowed_send_policies: BTreeMap<Word, PolicyConfig>, #[builder(field)] allowed_receive_policies: BTreeMap<Word, PolicyConfig>, authority: PolicyAuthority, active_mint_policy: MintPolicyConfig, active_burn_policy: BurnPolicyConfig, active_send_policy: TransferPolicy, active_receive_policy: TransferPolicy, ) -> TokenPolicyManager { // convert and insert active policies into allowed Self { authority, mint_policies: allowed_mint_policies.into_values().collect(), burn_policies: allowed_burn_policies.into_values().collect(), send_policies: allowed_send_policies.into_values().collect(), receive_policies: allowed_receive_policies.into_values().collect(), } } } impl<S: token_policy_manager_builder::State> TokenPolicyManagerBuilder<S> { /// TODO pub fn allowed_mint_policy(mut self, config: MintPolicyConfig) -> Self { // TODO: self.allowed_mint_policies.insert(config.root, config); self } } // Usage fn test() { TokenPolicyManager::builder() .authority(PolicyAuthority::OwnerControlled) .active_mint_policy(MintPolicyConfig::OwnerOnly) .allowed_mint_policy(MintPolicyConfig::AllowAll) // ... .build(); }

The builder ensures an active policy for each category is set.

The BTreeMap ensures deduplication by root (maybe it makes sense to do it differently, or use the map also within the token policy manager - this is just a high-level idea).

PhilippGackstatter · 2026-05-12T13:09:39Z

+        for kind in [mint_policies, burn_policies, send_policies, receive_policies] {
+            for entry in kind {


Nit: kind and entry make this a bit hard to read. Consider:

let policy_iter = mint_policies .into_iter() .chain(burn_policies) .chain(send_policies) .chain(receive_policies); for policy_config in policy_iter { ... }

If the fields internally were all BTreeMap<Word, PolicyConfig>, they could be merged here for natural deduplication. Then, iteration wouldn't have to check for duplication.

I think overall this would be nicer, but we should still be able to determine that a given policy category (mint, burn, send, receive) has only one active policy set. Maybe the way to define the struct could be:

pub struct TokenPolicyManager { authority: PolicyAuthority, active_mint_policy_root: Word, active_burn_policy_root: Word, active_send_policy_root: Word, active_receive_policy_root: Word, policies: BTreeMap<Word, PolicyConfig>, }

PhilippGackstatter · 2026-05-12T13:13:23Z

+/// Builds the `allowed_*_policies` storage map for a given kind. Includes both Active and
+/// Reserved entries so runtime `set_*_policy` validation accepts swaps to either.
+fn build_allowed_map(entries: &[PolicyConfig], kind: &str) -> StorageMap {
+    let allowed_flag = Word::from([1u32, 0, 0, 0]);
+    let map_entries: Vec<_> = entries
+        .iter()
+        .map(|e| (StorageMapKey::from_raw(e.root), allowed_flag))
+        .collect();
+    StorageMap::with_entries(map_entries)
+        .unwrap_or_else(|_| panic!("allowed {kind} policy roots should have unique keys"))
+}


Same here, if the fields were BTreeMap this would more naturally give us unique entries.

Nit: Change this to an expect.

PhilippGackstatter · 2026-05-12T13:13:55Z

 pub mod burn;
 mod manager;
 pub mod mint;
+pub mod transfer;

 pub use burn::{BurnAllowAll, BurnOwnerOnly, BurnPolicyConfig};
-pub use manager::TokenPolicyManager;
+pub use manager::{PolicyConfig, TokenPolicyManager};
 pub use mint::{MintAllowAll, MintOwnerOnly, MintPolicyConfig};
+pub use transfer::{TransferAllowAll, TransferIfNotBlocklisted, TransferPolicy};


Since we pub use all the items of these modules, I think we can make the modules themselves private, right?

bobbinth

Looks good! Thank you! I left some more comments inline - but most should be pretty easy to address.

bobbinth · 2026-05-13T00:14:44Z

+        // Only register the asset-callback slots when at least one of the send / receive
+        // policies actually performs enforcement. Beyond saving slots and dispatch overhead
+        // for no-op `AllowAll`, this also keeps the minted asset value word free of
+        // `AssetCallbackFlag::Enabled` — `protocol::faucet::create_fungible_asset` reads the
+        // callback slots and stamps the flag on every minted asset when they are populated.
+        //
+        // With the flattened policy dispatch, the protocol callback slots hold the active
+        // policy proc root directly (no manager-side wrapper), so we initialize them to the
+        // active send / receive policy roots.
+        let needs_callbacks = self.send_policies.iter().any(|p| p.requires_callbacks)
+            || self.receive_policies.iter().any(|p| p.requires_callbacks);


"Allow all" would be the only policy that doesn't require a callback, right? If so, I wonder if we could get rid of the requires_callback field and just do a match against the "allow all" procedure root.

More generally, I wonder if "allow all" should be treated somehow specially. Basically, if "allow all" is the active policy - do we even need to populate call back policy slot?

bobbinth · 2026-05-13T00:18:35Z

+        for kind in [mint_policies, burn_policies, send_policies, receive_policies] {
+            for entry in kind {


I think overall this would be nicer, but we should still be able to determine that a given policy category (mint, burn, send, receive) has only one active policy set. Maybe the way to define the struct could be:

pub struct TokenPolicyManager { authority: PolicyAuthority, active_mint_policy_root: Word, active_burn_policy_root: Word, active_send_policy_root: Word, active_receive_policy_root: Word, policies: BTreeMap<Word, PolicyConfig>, }

bobbinth

Looks good! Thank you! I left some more comments inline - but once these are addressed, we should be good to merge.

I think we also have a couple of follow-ups coming out of this PR. Could you create an issue capturing these?

bobbinth · 2026-05-13T19:54:59Z

+    pub fn with_blocked_accounts<I>(blocked_accounts: I) -> Self
+    where
+        I: IntoIterator<Item = AccountId>,
+    {
+        Self {
+            blocked_accounts: blocked_accounts.into_iter().collect(),
+        }
+    }


How should we deal with duplicates here? We could either silently remove them, or return an error.

Or, we could also could also change the internal type into BTreeSet (and this would silently de-duplicate).

bobbinth · 2026-05-13T19:59:58Z

+pub(crate) fn account_id_to_map_key(id: AccountId) -> StorageMapKey {
+    use miden_protocol::Felt;
+    let key = Word::from([Felt::ZERO, Felt::ZERO, id.suffix(), id.prefix().as_felt()]);
+    StorageMapKey::from_raw(key)
+}


Do we not already have a helper function to do this? cc @PhilippGackstatter in case he knows.

If not, I'd probably as impl From<AccountId> for StorageMapKey and put this code in the account ID module.

bobbinth · 2026-05-13T20:01:39Z

+        let entries: Vec<_> = self
+            .blocked_accounts
+            .iter()
+            .map(|id| (account_id_to_map_key(*id), blocked_word))
+            .collect();
+        StorageMap::with_entries(entries).expect("initial blocked accounts should have unique IDs")


nit: we should be able to avoid allocating entries here and just pass the iterator the StorageMap::with_entries().

bobbinth · 2026-05-13T20:11:45Z

+        let blocked_accounts_slot = StorageSlot::with_map(
+            BlocklistStorage::blocked_accounts_slot().clone(),
+            storage.build_storage_map(),
+        );


Ideally, we'd be able to do something like:

let slots = storage.into_slots();

And then, line 103 below would be something like:

AccountComponent::new( BasicBlocklist::code().clone(), storage.into_slots(), metadata, )

bobbinth · 2026-05-13T21:22:01Z

+        if registration == PolicyRegistration::Active {
+            assert!(
+                self.active_mint_policy_root == Word::default(),
+                "token policy manager: more than one active mint policy registered",
+            );
+            self.active_mint_policy_root = root;


I think it would be better if this was an error rather than panic - but let's leave this for a follow-up PR.

This also applies to other similar methods.

bobbinth · 2026-05-13T21:24:52Z

+    /// Returns the active send policy procedure root. Panics if no active send policy has
+    /// been registered.
+    pub fn active_send_policy(&self) -> Word {
+        assert!(
+            self.active_send_policy_root != Word::default(),
+            "token policy manager: no active send policy registered",
+        );
+        self.active_send_policy_root
+    }


Here, and in other similar methods, instead of panicking we should return Option<Word>.

onurinanc · 2026-05-14T15:15:52Z

Addressed the review suggestions, and opened an issue to track the follow-up items:

Consider changing TransferPolicy from enum to struct #2929

onurinanc added 2 commits April 23, 2026 14:21

rename blocklist -> blocklistable

8ce07e2

changelog

4f14440

onurinanc requested review from PhilippGackstatter and bobbinth April 23, 2026 13:31

bobbinth reviewed Apr 26, 2026

View reviewed changes

Merge branch 'next' into blocklist

6bb920b

onurinanc mentioned this pull request Apr 28, 2026

Pausable #2793

Open

mmagician added pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority standards Related to standard note scripts or account components labels Apr 29, 2026

mmagician requested a review from Fumuran April 29, 2026 08:40

Merge branch 'next' into blocklist

b8cff31

onurinanc mentioned this pull request Apr 30, 2026

Add transfer policy to TokenPolicyManager #2855

Open

onurinanc added 5 commits May 4, 2026 10:22

Merge remote-tracking branch 'origin/next' into blocklist

08b33bd

merge blocklistable/mint-burn-extraction

904ce67

add transfer policy to TokenPolicyManager

e173156

Merge remote-tracking branch 'origin/blocklist' into blocklist

45be7a4

add issue reproduction tests

0b532b1

onurinanc mentioned this pull request May 4, 2026

Cannot use mint_and_send on a faucet with AssetCallbacks added #2864

Closed

onurinanc added 6 commits May 8, 2026 08:18

introduce send and receive policies

75e240f

add owner controlled restrictions

887f80b

add restrictions::blocklist

3317637

Merge branch 'next' into blocklist

40f216e

clippy

da3c7e3

Merge remote-tracking branch 'origin/blocklist' into blocklist

4c135ec

onurinanc requested a review from bobbinth May 8, 2026 12:41

bobbinth reviewed May 11, 2026

View reviewed changes

Merge branch 'next' into blocklist

baf5e6a

bobbinth reviewed May 12, 2026

View reviewed changes

Comment thread crates/miden-standards/asm/standards/faucets/policies/transfer/blocklist/owner_controlled.masm

bobbinth reviewed May 12, 2026

View reviewed changes

Comment thread crates/miden-standards/asm/account_components/faucets/policies/transfer/if_not_blocklisted.masm Outdated

Merge branch 'next' into blocklist

b73d24b

bobbinth mentioned this pull request May 12, 2026

Investigate separation between internal and external account component procedures #2862

Open

onurinanc added 6 commits May 12, 2026 12:05

Merge branch 'next' into blocklist

dcf2d96

Merge branch 'next' into blocklist

3c4889b

flatten callbacks

fe6130b

merge next

018d483

Merge remote-tracking branch 'origin/blocklist' into blocklist

92297b6

fix documentation

e196ed2

PhilippGackstatter reviewed May 12, 2026

View reviewed changes

onurinanc and others added 3 commits May 12, 2026 15:20

lint

f423d23

fix agglayer callbacks

d7a1baf

chore: remove unused files

e892a30

bobbinth reviewed May 13, 2026

View reviewed changes

onurinanc added 7 commits May 13, 2026 10:46

Merge branch 'next' into blocklist

e350a64

Merge branch 'next' into blocklist

fda0c9a

apply review fixes

8e72ac2

Merge remote-tracking branch 'origin/blocklist' into blocklist

8536136

fix merge

f83854f

merge next

6662a21

remore require_callback

8302d57

onurinanc requested review from PhilippGackstatter and bobbinth May 13, 2026 14:16

bobbinth approved these changes May 13, 2026

View reviewed changes

onurinanc added 3 commits May 14, 2026 10:52

Merge branch 'next' into blocklist

21a60aa

apply review fixes

924f84b

Merge remote-tracking branch 'origin/blocklist' into blocklist

b96e482

onurinanc mentioned this pull request May 14, 2026

Consider changing TransferPolicy from enum to struct #2929

Open

fix documentation

353dedf

onurinanc requested a review from bobbinth May 14, 2026 15:30

		for kind in [mint_policies, burn_policies, send_policies, receive_policies] {
		for entry in kind {

Conversation

onurinanc commented Apr 23, 2026

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

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!

bobbinth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bobbinth 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!

Choose a reason for hiding this comment

Uh oh!

onurinanc commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone