fix(agglayer): enforce NoteType::Public for B2AGG bridge-out notes#2988
Merged
Conversation
4ce0b10 to
9016ace
Compare
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>
9016ace to
2569f44
Compare
…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>
bobbinth
reviewed
May 27, 2026
Contributor
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left a couple of comments inline.
Contributor
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Looks good to me, though my agglayer-related knowledge is small.
…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>
mmagician
approved these changes
May 27, 2026
Collaborator
mmagician
left a comment
There was a problem hiding this comment.
LGTM modulo the comment verbosity
Co-authored-by: Marti <marti@miden.team>
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>
bobbinth
approved these changes
May 27, 2026
Contributor
bobbinth
left a comment
There was a problem hiding this comment.
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.
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
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2984.
Problem
B2AGG (bridge-out) notes must be
NoteType::Publicso 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 inNoteMetadata, not in the recipient commitment, so an attacker can assemble a note with an identical recipient, attachment, and asset butNoteType::Private. To consensus this is indistinguishable from a compliant B2AGG note: the bridge picks it up,bridge_outruns end-to-end, and the LET frontier is updated. But off-chain, only theNoteIdis 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'ssender_id_suffix_and_note_typefelt.Note on the implementation: the fix suggested in the issue uses
miden::protocol::note::metadata_into_note_typeand a re-exportedNOTE_TYPE_PUBLIC, but those were added onnext(PR #2738) and do not exist on theagglayerbranch, so the helper above is implemented against agglayer's existing primitives (active_note::get_metadata).Follow-ups
is_note_publicprocedure (miden::standards::note::metadata). It is useful well beyond the bridge and should be cherry-picked back to thenextbranch, where it can sit alongside / build onmetadata_into_note_type(feat: addmetadata_into_note_typeprocedure to note.masm #2738).