Skip to content

fix(standards): reject faucets whose policies lack their required storage slots - #3527

Open
onurinanc wants to merge 5 commits into
nextfrom
fix-ownable2step
Open

fix(standards): reject faucets whose policies lack their required storage slots#3527
onurinanc wants to merge 5 commits into
nextfrom
fix-ownable2step

Conversation

@onurinanc

Copy link
Copy Markdown
Collaborator

Closes: #3526

Comment on lines +560 to +570
let account = NetworkAccount::builder(init_seed, note_allowlist, fee_policy_manager)
.expect("MintNote + BurnNote allowlist is non-empty")
.with_component(faucet)
.with_components(access_control)
.with_components(token_policy_manager)
.with_component(Pausable::unpaused())
.with_component(PausableManager)
.build()
.map_err(NonFungibleFaucetError::AccountCreationFailed)
.map_err(NonFungibleFaucetError::AccountCreationFailed)?;

verify_policy_dependencies(&required_slots, account.storage())?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A preliminary question: should this check be actually happening in the AccountBuilder? This would be a more general solution, but probably a bit more difficult to implement (or at least to design).

If we do decide to how this route, the way to implement this is to specify dependencies at the AccountComponentMetadata level. Then, the builder would be able to read dependencies of all components and verify that they are all satisfied.

I think the main complexity here would come from figuring out how to specify dependencies. The approach taken in this PR is to use just slot names. Maybe this is enough, but maybe we want to list actual components as dependencies too.

@onurinanc onurinanc Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good idea. I've implemented this in this commit: c606f32.

However, I'm not sure about adding some part in the AccountComponentMetadata. So, it would be good if you can review @PhilippGackstatter. I'm also fine with rebasing this PR to the commit: d9944c0 delegating this part of the issue as it is more related to the miden-protocol

@onurinanc
onurinanc requested a review from bobbinth August 10, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MintPolicy::owner_only() omits required Ownable2Step, which can permanently brick owner-only minting

2 participants