Skip to content

feat(agglayer): correct script allowlists#3335

Merged
mmagician merged 6 commits into
release/v0.16.0-alphafrom
igamigo-agglayer-allowlists
Jul 17, 2026
Merged

feat(agglayer): correct script allowlists#3335
mmagician merged 6 commits into
release/v0.16.0-alphafrom
igamigo-agglayer-allowlists

Conversation

@igamigo

@igamigo igamigo commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Discovered by @juan518munoz here: the agglayer integration tests were not working on the client because the network accounts were not allowing the script that the NTX builder invariably uses.
This PR adds:

  • Allowlists for NetworkAccount
  • NetworkAccount validates that the expiration script root is allowed by it. Right now the NTX builder always attaches this script, so it makes sense to make it part of the standard
  • NetworkAccount::builder() which returns an account builder seeded with the correct components
  • Changes NetworkAccount::try_from to pick up allowlists with the expiration script (this is used by the node to pick up network accounts so it should avoid picking up unusable network accounts)
  • Tests for checking that network agglayer accounts are indeed network accounts

These changes are somewhat opinionated as all we really needed was to add the correct script to the allowlists, so we can discard whatever we think is unnecessary (and also, maybe there were more specific plans for this). In short, it makes NetworkAccount a more specific of a standard.

This will require a new release

@igamigo
igamigo marked this pull request as ready for review July 17, 2026 00:22
@igamigo
igamigo marked this pull request as draft July 17, 2026 00:45
@igamigo
igamigo marked this pull request as ready for review July 17, 2026 00:48

@bobbinth bobbinth left a comment

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.

Looks good! Thank you for fixing this!

@PhilippGackstatter PhilippGackstatter left a comment

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.

Looks good!

Minor nit (feel free to ignore): Looking forward, we will need to integrate parts or all of the fee manager into AuthNetworkAccount in the future, meaning it will require more parameters to build this component (fee policy, fee asset ID). I think it might end up being nicer for ergonomics if we keep a builder-style approach for building AuthNetworkAccount rather than passing all of these parameters through NetworkAccount::builder. I believe this change was mainly about including the ExpirationTransactionScript root into the allowlist by default? Since AuthNetworkAccount itself reflects the requirements of the network tx standard, we could also add this script by default when building AuthNetworkAccount.

Comment thread crates/miden-standards/src/account/auth/network_account/network_account.rs Outdated

@mmagician mmagician left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for providing such a comprehensive fix! ✅

Avoids the implicit From conversion so call sites in NetworkAccount::new
stay explicit about which allowlist failed to decode.
@mmagician
mmagician enabled auto-merge July 17, 2026 10:07
@mmagician
mmagician merged commit 77a3092 into release/v0.16.0-alpha Jul 17, 2026
16 checks passed
@mmagician
mmagician deleted the igamigo-agglayer-allowlists branch July 17, 2026 10:08
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.

6 participants