Skip to content

fix(agglayer): enforce NoteType::Public for B2AGG bridge-out notes#2988

Merged
partylikeits1983 merged 6 commits into
agglayerfrom
ajl-claude/b2agg-enforce-public-note
May 27, 2026
Merged

fix(agglayer): enforce NoteType::Public for B2AGG bridge-out notes#2988
partylikeits1983 merged 6 commits into
agglayerfrom
ajl-claude/b2agg-enforce-public-note

Conversation

@partylikeits1983
Copy link
Copy Markdown
Contributor

@partylikeits1983 partylikeits1983 commented May 26, 2026

Closes #2984.

Problem

B2AGG (bridge-out) notes must be NoteType::Public so AggLayer's off-chain indexer (aggkit) can recover the leaf pre-image and mirror the on-chain Local Exit Tree (LET). The note type lives in NoteMetadata, not in the recipient commitment, so an attacker can assemble a note with an identical recipient, attachment, and asset but NoteType::Private. To consensus this is indistinguishable from a compliant B2AGG note: the bridge picks it up, bridge_out runs end-to-end, and the LET frontier is updated. But off-chain, only the NoteId is published for a private note, so aggkit can never reconstruct the just-appended leaf. From that leaf on, aggkit's LET mirror permanently diverges from the bridge account's storage, bricking the bridge-out exit path for everyone.

Fix

Enforce the public note type in the bridge (non-reclaim) branch of B2AGG.masm, before the note storage is trusted.

The public/private check is factored into a reusable procedure miden::standards::note::metadata::is_note_public, which reads the active note's metadata and returns whether it is public. The note type is the low byte of the metadata header's sender_id_suffix_and_note_type felt.

Note on the implementation: the fix suggested in the issue uses miden::protocol::note::metadata_into_note_type and a re-exported NOTE_TYPE_PUBLIC, but those were added on next (PR #2738) and do not exist on the agglayer branch, so the helper above is implemented against agglayer's existing primitives (active_note::get_metadata).

Follow-ups

  • This PR adds a new reusable is_note_public procedure (miden::standards::note::metadata). It is useful well beyond the bridge and should be cherry-picked back to the next branch, where it can sit alongside / build on metadata_into_note_type (feat: add metadata_into_note_type procedure to note.masm #2738).
  • Probably warrants a wider discussion: should all network notes be required to be public and reclaimable? The observability argument here generalizes - any note a network account consumes whose pre-image is not public produces state that off-chain indexers and clients cannot reconstruct, and a non-reclaimable network note that is never consumed strands its assets. Enforcing public + reclaimable at the network-note level would make this a protocol-wide invariant rather than a per-script check.

@partylikeits1983 partylikeits1983 force-pushed the ajl-claude/b2agg-enforce-public-note branch 2 times, most recently from 4ce0b10 to 9016ace Compare May 26, 2026 22:16
The B2AGG note type lives in NoteMetadata, not in the recipient
commitment, so a recipient-identical note marked NoteType::Private was
accepted on-chain. Its leaf would be folded into the Local Exit Tree
while aggkit could never recover the pre-image, permanently desyncing
the LET mirror and bricking the bridge-out path.

Enforce the public note type in the bridge branch of B2AGG.masm before
the note storage is trusted. The public/private check is factored into a
reusable miden::standards::note::metadata::is_note_public procedure that
returns whether the active note is public. Add a parameterized test
covering the recipient-identical private note alongside the existing
invalid-destination case.

Closes #2984

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@partylikeits1983 partylikeits1983 force-pushed the ajl-claude/b2agg-enforce-public-note branch from 9016ace to 2569f44 Compare May 26, 2026 22:21
…blic comments

Add the missing CHANGELOG.md entry for the B2AGG public-note enforcement so
the changelog CI check passes, and tidy the is_note_public doc/inline comments
(drop a redundant parenthetical, fix the 'mask off' wording, and split the
note-type extraction into one op per line to match the repo's masm style).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! I left a couple of comments inline.

Comment thread crates/miden-standards/asm/standards/note/metadata.masm Outdated
Comment thread crates/miden-agglayer/asm/note_scripts/B2AGG.masm Outdated
Copy link
Copy Markdown
Contributor

@PhilippGackstatter PhilippGackstatter left a comment

Choose a reason for hiding this comment

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

Looks good to me, though my agglayer-related knowledge is small.

Comment thread crates/miden-standards/asm/standards/note/metadata.masm Outdated
Comment thread crates/miden-standards/asm/standards/note/metadata.masm Outdated
Comment thread crates/miden-testing/src/kernel_tests/tx/test_active_note.rs
…ote::is_public/is_private

Address review feedback on #2988:
- Move the public-note check out of the B2AGG note script and into
  bridge::bridge_out::bridge_out, where it belongs (per Bobbin).
- Move the is_public logic into the protocol active_note module as
  active_note::is_public, backed by a new reusable
  note::extract_note_type_from_metadata primitive (per Bobbin & Philipp).
- Add active_note::is_private for completeness (per Philipp), and drop the
  redundant expected_is_public test column by deriving the expected flags
  from NoteType.
- Remove the standalone standards/note/metadata.masm module.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@mmagician mmagician left a comment

Choose a reason for hiding this comment

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

LGTM modulo the comment verbosity

Comment thread crates/miden-agglayer/asm/agglayer/bridge/bridge_out.masm Outdated
Comment thread crates/miden-agglayer/asm/agglayer/bridge/bridge_out.masm Outdated
Comment thread crates/miden-testing/src/kernel_tests/tx/test_active_note.rs Outdated
partylikeits1983 and others added 2 commits May 27, 2026 15:41
Address review nits on #2988: reduce the bridge_out public-note doc
comment to a single behavior-level line (dropping indexer mechanism and
metadata-header implementation detail), and remove the change-narrating
comment in test_active_note_is_public_and_is_private.

Co-Authored-By: Claude (Opus) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! I left one question inline - but I suspect this is because this is going into the agglayer branch. We should make sure to update the constants when we merge this back into next.

Comment thread crates/miden-protocol/asm/protocol/active_note.masm Outdated
Move NOTE_TYPE_PUBLIC / NOTE_TYPE_PRIVATE out of active_note.masm and into
the note shared utils module (miden::protocol::util::note), alongside
MAX_NOTE_STORAGE_ITEMS and ATTACHMENT_KIND_*, then import them rather than
redefining locally. Addresses review feedback on duplicating protocol-level
data.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@partylikeits1983 partylikeits1983 added this pull request to the merge queue May 27, 2026
Merged via the queue into agglayer with commit d67d0d0 May 27, 2026
15 checks passed
@partylikeits1983 partylikeits1983 deleted the ajl-claude/b2agg-enforce-public-note branch May 27, 2026 22:59
partylikeits1983 added a commit that referenced this pull request May 28, 2026
Cherry-pick the reusable protocol-level building blocks from #2988,
which was merged to the agglayer branch as an agglayer-specific
B2AGG bridge-out hardening:

  - active_note::is_public
  - active_note::is_private

Both procs read the active note's metadata and decode its note
type via the existing note::metadata_into_note_type primitive
(from #2738), then compare against NOTE_TYPE_PUBLIC /
NOTE_TYPE_PRIVATE from miden::protocol::util::note. A parameterized
kernel test covers the Public and Private cases.

The agglayer-only files from #2988 (bridge_out.masm, B2AGG.masm,
tests/agglayer/bridge_out.rs) and the constant-value flip the PR
applied on agglayer's NoteType encoding are intentionally not
brought along: next already encodes Private=0, Public=1, and the
existing metadata_into_note_type is tailored to next's metadata
bit layout.

Co-Authored-By: Claude (Opus) <noreply@anthropic.com>
partylikeits1983 added a commit that referenced this pull request May 28, 2026
…2988)

Cherry-pick the bridge-side half of #2988 to next. The B2AGG note
type lives in NoteMetadata, not in the recipient commitment, so an
attacker could submit a note with an identical recipient,
attachment, and asset but NoteType::Private. consensus would accept
it, the leaf would be folded into the on-chain Local Exit Tree, but
aggkit could never recover the pre-image off-chain — the LET mirror
would permanently desync and the bridge-out path would brick.

Add an active_note::is_public assert at the top of bridge_out,
extend the bridge_out and b2agg.masm doc comments, and parameterize
the existing destination-network-is-Miden test into
test_bridge_out_rejects_invalid_b2agg_note covering the
recipient-identical private-note case too.

Co-Authored-By: Claude (Opus) <noreply@anthropic.com>
partylikeits1983 added a commit that referenced this pull request May 28, 2026
…2988)

Cherry-pick the bridge-side half of #2988 to next. The B2AGG note
type lives in NoteMetadata, not in the recipient commitment, so an
attacker could submit a note with an identical recipient,
attachment, and asset but NoteType::Private. consensus would accept
it, the leaf would be folded into the on-chain Local Exit Tree, but
aggkit could never recover the pre-image off-chain — the LET mirror
would permanently desync and the bridge-out path would brick.

Add an active_note::is_public assert at the top of bridge_out,
extend the bridge_out and b2agg.masm doc comments, and parameterize
the existing destination-network-is-Miden test into
test_bridge_out_rejects_invalid_b2agg_note covering the
recipient-identical private-note case too.

Co-Authored-By: Claude (Opus) <noreply@anthropic.com>
partylikeits1983 added a commit that referenced this pull request May 28, 2026
…2988)

Cherry-pick the bridge-side half of #2988 to next. The B2AGG note
type lives in NoteMetadata, not in the recipient commitment, so an
attacker could submit a note with an identical recipient,
attachment, and asset but NoteType::Private. consensus would accept
it, the leaf would be folded into the on-chain Local Exit Tree, but
aggkit could never recover the pre-image off-chain — the LET mirror
would permanently desync and the bridge-out path would brick.

Add an active_note::is_public assert at the top of bridge_out,
extend the bridge_out and b2agg.masm doc comments, and parameterize
the existing destination-network-is-Miden test into
test_bridge_out_rejects_invalid_b2agg_note covering the
recipient-identical private-note case too.

Co-Authored-By: Claude (Opus) <noreply@anthropic.com>
partylikeits1983 added a commit that referenced this pull request May 28, 2026
…2988)

Cherry-pick the bridge-side half of #2988 to next. The B2AGG note
type lives in NoteMetadata, not in the recipient commitment, so an
attacker could submit a note with an identical recipient,
attachment, and asset but NoteType::Private. consensus would accept
it, the leaf would be folded into the on-chain Local Exit Tree, but
aggkit could never recover the pre-image off-chain — the LET mirror
would permanently desync and the bridge-out path would brick.

Add an active_note::is_public assert at the top of bridge_out,
extend the bridge_out and b2agg.masm doc comments, and parameterize
the existing destination-network-is-Miden test into
test_bridge_out_rejects_invalid_b2agg_note covering the
recipient-identical private-note case too.

Co-Authored-By: Claude (Opus) <noreply@anthropic.com>
mmagician added a commit that referenced this pull request May 29, 2026
…2988) (#3005)

* feat(protocol): add active_note::is_public / is_private (#2988)

Cherry-pick the reusable protocol-level building blocks from #2988,
which was merged to the agglayer branch as an agglayer-specific
B2AGG bridge-out hardening:

  - active_note::is_public
  - active_note::is_private

Both procs read the active note's metadata and decode its note
type via the existing note::metadata_into_note_type primitive
(from #2738), then compare against NOTE_TYPE_PUBLIC /
NOTE_TYPE_PRIVATE from miden::protocol::util::note. A parameterized
kernel test covers the Public and Private cases.

The agglayer-only files from #2988 (bridge_out.masm, B2AGG.masm,
tests/agglayer/bridge_out.rs) and the constant-value flip the PR
applied on agglayer's NoteType encoding are intentionally not
brought along: next already encodes Private=0, Public=1, and the
existing metadata_into_note_type is tailored to next's metadata
bit layout.

Co-Authored-By: Claude (Opus) <noreply@anthropic.com>

* fix(agglayer): enforce NoteType::Public for B2AGG bridge-out notes (#2988)

Cherry-pick the bridge-side half of #2988 to next. The B2AGG note
type lives in NoteMetadata, not in the recipient commitment, so an
attacker could submit a note with an identical recipient,
attachment, and asset but NoteType::Private. consensus would accept
it, the leaf would be folded into the on-chain Local Exit Tree, but
aggkit could never recover the pre-image off-chain — the LET mirror
would permanently desync and the bridge-out path would brick.

Add an active_note::is_public assert at the top of bridge_out,
extend the bridge_out and b2agg.masm doc comments, and parameterize
the existing destination-network-is-Miden test into
test_bridge_out_rejects_invalid_b2agg_note covering the
recipient-identical private-note case too.

Co-Authored-By: Claude (Opus) <noreply@anthropic.com>

* Apply suggestions from code review

Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>

* chore: fmt files

---------

Co-authored-by: Claude (Opus) <noreply@anthropic.com>
Co-authored-by: Marti <marti@miden.team>
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
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.

5 participants