Skip to content

add b2agg ntx tag check #2189

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
partylikeits1983 wants to merge 2 commits into
base: next
Choose a base branch
from
Open

add b2agg ntx tag check #2189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1ee91e0
feat: add ntx tag check on b2agg consumption
partylikeits1983 Dec 16, 2025
2215d33
refactor: keep account id on stack
partylikeits1983 Dec 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 37 additions & 4 deletions crates/miden-lib/asm/agglayer/note_scripts/B2AGG.masm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use.agglayer::bridge_out -> agglayer
use.miden::account_id
use.miden::active_account
use.miden::active_note
use.miden::note

# CONSTANTS
# =================================================================================================
Expand All @@ -14,11 +15,34 @@ const.ERR_B2AGG_WRONG_NUMBER_OF_ASSETS="B2AGG script requires exactly 1 note ass

const.ERR_B2AGG_WRONG_NUMBER_OF_INPUTS="B2AGG script expects exactly 6 note inputs"

const.ACTIVE_ACCOUNT_TAG_DOES_NOT_MATCH_TARGET="B2AGG note tag does not match active account ntx tag"

# Verifies that the consuming account is the bridge contract by comparing note tags.
# Panics if the B2AGG note's tag doesn't match the consuming account's network tag.
#!
#! Inputs: [account_id_prefix, account_id_suffix]
#! Outputs: []
proc assert_account_id_is_target
Comment on lines +20 to +25
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter Dec 17, 2025

Choose a reason for hiding this comment

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

I know very little about the agglayer design, so this is just a question. At first glance, this looks a lot like a target account ID check in a P2ID note, except we're using the note tag instead of note inputs. Why can we not use note inputs instead?
The way I understand it is that we want a B2AGG note to be only consumed by a specific, canonical network faucet instead of any faucet that has a matching account interface. If that's true, then its account ID should be known and we should be able to use note inputs instead?

Copy link
Contributor Author

@partylikeits1983 partylikeits1983 Dec 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, we could add it to the note inputs, but then we'd have to increase the size of the note inputs of the B2AGG note

Copy link
Contributor

@PhilippGackstatter PhilippGackstatter Dec 17, 2025

Choose a reason for hiding this comment

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

I think with #2109 in mind, it will be easier to rely on note inputs instead of the format we'll end up using for network account targets.

Copy link
Contributor

@mmagician mmagician Dec 17, 2025

Choose a reason for hiding this comment

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

but then we'd have to increase the size of the note inputs of the B2AGG note

I think that would be preferable indeed to include it in note inputs

Copy link
Contributor Author

@partylikeits1983 partylikeits1983 Dec 17, 2025

Choose a reason for hiding this comment

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

sounds like a plan, will refactor.

Copy link
Contributor

@PhilippGackstatter PhilippGackstatter Dec 18, 2025

Choose a reason for hiding this comment

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

Thought a bit about your comment on this topic in the call. I think it could be fine to check the target account ID via the note attachment, in the future.
We'll only have NoteAttachment::Raw and NoteAttachment::Commitment after #2109 and I personally would like to avoid adding a NetworkAccount variant. Primarily because network notes and txs have very little support at the protocol level and they already work fine. Instead of enshrining more network transaction things at the protocol level, it'd be amazing to make network notes a miden standard instead of a miden protocol feature, to keep the protocol lean. The few things we do have in the protocol can be done in other ways now that we have named storage slots (detecting network accounts) and once we have user-extensible note attachments (detecting network account targets). Specifically, the last part, relevant here, can be specified as a miden standard.

So, checking via the note tag/attachment is possible but would make the agglayer more dependent on the network transaction "standard", i.e. the layout of how target account IDs are encoded in a note attachment. That's probably fine, because the agglayer already strongly assumes network transactions, afaict, but it's an additional thing to keep in mind.

On the other hand, having the target account ID in the note inputs sidesteps the network transaction specifics, as it becomes simply part of the "agglayer standard". This comes at the cost of being less efficient and encoding the account ID twice.

So, to summarize, I think checking via the tag/attachment is fine and probably a good idea to avoid that inefficiency. Also, the network transactions as a standard instead of a protocol feature is only an idea at this point that needs to be discussed more.

Copy link
Contributor

@bobbinth bobbinth Dec 26, 2025

Choose a reason for hiding this comment

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

I think the question is whether we'd want to make note attachments accessible programmatically. My initial thinking was that no - but now I'm wondering if maybe we should do this. So, then this boils down to:

  1. If note attachment is not accessible programmatically, the target account ID would need to go into note inputs.
  2. Otherwise, we could check note attachments and that's maybe a better option (and this pattern would probably be used in most network transactions).

If we decide to go with option 1, we can refactor this PR now. If we go with option 2, we should wait until #2109 is implemented.

I am leaning slightly toward option 2.

# => [account_id_prefix, account_id_suffix]

exec.note::build_note_tag_for_network_account
# => [active_account_tag]

exec.active_note::get_metadata
# => [METADATA, active_account_tag]

drop swap drop swap drop
# => [b2agg_tag, active_account_tag]

eq assert.err=ACTIVE_ACCOUNT_TAG_DOES_NOT_MATCH_TARGET
# => []
end

#! Bridge-to-AggLayer (B2AGG) note script: bridges assets from Miden to an AggLayer-connected chain.
#!
#! This note can be consumed in two ways:
#! - If the consuming account is the sender (reclaim): the note's assets are added back to the consuming account.
#! - If the consuming account is the Agglayer Bridge: the note's assets are moved to a BURN note,
#! - If the consuming account is the Agglayer Bridge: the note's assets are moved to a BURN note,
#! and the note details are hashed into a leaf and appended to the Local Exit Tree.
#! global exit root (GER) merkle tree structure.
#!
Expand All @@ -44,19 +68,28 @@ begin

# Check if reclaim
exec.active_account::get_id
# => [account_id_prefix, account_id_suffix, pad(16)]
# => [id_prefix, id_suffix, pad(16)]

dup.1 dup.1
# => [id_prefix, id_suffix, id_prefix, id_suffix, pad(16)]

exec.active_note::get_sender
# => [sender_id_prefix, sender_id_suffix, account_id_prefix, account_id_suffix, pad(16)]
# => [sender_id_prefix, sender_id_suffix, id_prefix, id_suffix, id_prefix, id_suffix, pad(16)]

exec.account_id::is_equal
# => [reclaim, pad(16)]
# => [reclaim, id_prefix, id_suffix, pad(16)]

# B2AGG note is being reclaimed; adding note assets to account
if.true
drop drop
# => [pad(16)]

exec.active_note::add_assets_to_account
# => [pad(16)]
else
exec.assert_account_id_is_target
# => [pad(16)]

# Store note inputs -> mem[8..14]
push.8 exec.active_note::get_inputs
# => [num_inputs, dest_ptr, pad(16)]
Expand Down
13 changes: 10 additions & 3 deletions crates/miden-testing/tests/agglayer/bridge_out.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ async fn test_bridge_out_consumes_b2agg_note() -> anyhow::Result<()> {
let storage_slots = vec![StorageSlot::with_empty_map(storage_slot_name)];
let bridge_component = bridge_out_component(storage_slots);
let account_builder = Account::builder(builder.rng_mut().random())
.storage_mode(AccountStorageMode::Public)
.storage_mode(AccountStorageMode::Network)
.with_component(bridge_component);
let mut bridge_account =
builder.add_account_from_builder(Auth::IncrNonce, account_builder, AccountState::Exists)?;
Expand All @@ -70,7 +70,7 @@ async fn test_bridge_out_consumes_b2agg_note() -> anyhow::Result<()> {

let amount = Felt::new(100);
let bridge_asset: Asset = FungibleAsset::new(faucet.id(), amount.into()).unwrap().into();
let tag = NoteTag::for_local_use_case(0, 0).unwrap();
let tag = NoteTag::from_account_id(bridge_account.id());
let aux = Felt::new(0);
let note_execution_hint = NoteExecutionHint::always();
let note_type = NoteType::Public; // Use Public note type for network transaction
Expand Down Expand Up @@ -217,6 +217,13 @@ async fn test_b2agg_note_reclaim_scenario() -> anyhow::Result<()> {
AccountStorageMode::Private,
);

let bridge_account_id = AccountId::dummy(
[1; 15],
AccountIdVersion::Version0,
AccountType::RegularAccountImmutableCode,
AccountStorageMode::Network,
);

// Create a network faucet to provide assets for the B2AGG note
let faucet =
builder.add_existing_network_faucet("AGG", 1000, faucet_owner_account_id, Some(100))?;
Expand All @@ -229,7 +236,7 @@ async fn test_b2agg_note_reclaim_scenario() -> anyhow::Result<()> {

let amount = Felt::new(50);
let bridge_asset: Asset = FungibleAsset::new(faucet.id(), amount.into()).unwrap().into();
let tag = NoteTag::for_local_use_case(0, 0).unwrap();
let tag = NoteTag::from_account_id(bridge_account_id);
let aux = Felt::new(0);
let note_execution_hint = NoteExecutionHint::always();
let note_type = NoteType::Public;
Expand Down