Skip to content

fix(tx): make NoteConsumptionChecker bundle-aware - #3801

Open
Fumuran wants to merge 3 commits into
nextfrom
fumuran-claude/bundle-aware-note-checker
Open

fix(tx): make NoteConsumptionChecker bundle-aware#3801
Fumuran wants to merge 3 commits into
nextfrom
fumuran-claude/bundle-aware-note-checker

Conversation

@Fumuran

@Fumuran Fumuran commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #3710.

A feature note and the FEE_SPONSORSHIP notes bound to it are consumable only together. An
uncovered feature note fails in the epilogue, which lands the batch in
find_largest_executable_combination;
that search grew its candidate set one note at a time, and neither half of a bound pair executes
alone. Intact pairs were therefore dropped alongside the note that actually failed - with F1
uncovered, [F0, S0, F1] reported nothing successful rather than {F0, S0}.

The fix makes the bundle, not the note, the unit of the search.

Changes

  • NoteBundle::group groups the inputs by the binding that already exists on chain, since a
    FEE_SPONSORSHIP note names its feature note by NoteId. No caller has to supply the grouping,
    so the ntx builder and clients are fixed together. A sponsorship whose feature note is absent
    forms its own bundle and so fails alone rather than taking others down with it.
  • find_largest_executable_combination appends whole bundles, and its for size in 1..=n loop
    gives way to a round loop that stops when a pass adds nothing - which also fixes the loop bound
    having been computed before remaining_notes started shrinking.
  • FailedNote::error becomes Arc<TransactionExecutorError> so one error can be shared across a
    rejected bundle; new and error keep their signatures, so this stays source-compatible. New
    bundled_with() marks the notes dropped as collateral, which callers need in order to tell those
    apart from genuine failures.
  • New FeeSponsorshipNote::sponsored_feature_note_id puts the binding rule in one public place the
    node can call for its own checks.

The elimination path is untouched: it is only reached on note-script failures, where it already
converges on bundles, and leaving it alone avoids inventing an error for collaterally-dropped notes
there. No MASM, commitment or account ID changes.

Open questions

  1. Two further asks accumulated on the issue that this PR leaves alone: domain-based ordering of
    order-sensitive config notes, and a NoteConsumptionStatus variant for "consumable had the fee
    been covered". Both look like separate issues to me rather than v0.17 blockers on this PR, but
    worth confirming before this is marked ready.

Comment thread crates/miden-standards/src/note/fee_sponsorship.rs Outdated
Comment thread CHANGELOG.md Outdated
Comment thread crates/miden-testing/tests/scripts/fee_collection.rs Outdated
Comment thread crates/miden-tx/src/executor/notes_checker.rs Outdated
Comment thread crates/miden-tx/src/executor/notes_checker.rs Outdated
Comment thread crates/miden-tx/src/executor/notes_checker.rs Outdated
Comment thread crates/miden-tx/src/executor/notes_checker.rs Outdated
A feature note and the FEE_SPONSORSHIP notes bound to it are consumable
only together: fee collection rejects a sponsorship whose feature note is
absent, and a feature note whose fee is uncovered. Because fee collection
runs in the auth procedure, an uncovered note fails in the epilogue, which
routed the batch into find_largest_executable_combination.

That search grew its candidate set one note at a time from the empty set,
so it only ever reached sets containing a consumable subset with one note
fewer. A bound pair has none, so intact pairs were dropped alongside the
note that actually failed.

Group input notes into bundles that must be consumed together, derived
from the note ID a sponsorship names in its storage, and make the bundle
the unit of the search. Notes of a rejected bundle that were not blamed
for the failure are reported with FailedNote::bundled_with set, so callers
can tell a collateral drop from a genuine failure.

Closes #3710

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSDwt99bX6hr5TEfaff1Uf
Key each note by the index of the note heading its bundle - its feature
note's for a sponsorship bound to one, its own otherwise - so a single
map replaces the staging vector, the bundle index and the second pass.

Keying on the input index rather than the note ID keeps bundles in the
caller's order, which the search relies on to probe candidates in the
order the caller submitted them.

A sponsorship naming another sponsorship no longer needs its own branch:
the named note either heads a bundle, in which case joining it is right,
or was itself bundled elsewhere, in which case nothing heads that key and
the note is left on its own. Grouping is unchanged in every case.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSDwt99bX6hr5TEfaff1Uf
Drop FeeSponsorshipNote::sponsored_feature_note_id and use the existing
TryFrom<&Note> in NoteBundle::group instead, which already checks the
script root. It also validates the rest of the note's shape, so a note
carrying the sponsorship script but the wrong asset count or attachments
now heads its own bundle rather than being bundled as a sponsorship it
could never be consumed as. miden-standards is left untouched.

Trim the NoteBundle, group and search doc comments to the essence, and
point the changelog entry at the PR rather than the issue.

The v0.17.0-pre.1 release renamed the section this entry had been added
to, so it moves to a new unreleased section rather than a dated one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GSDwt99bX6hr5TEfaff1Uf
@Fumuran
Fumuran force-pushed the fumuran-claude/bundle-aware-note-checker branch from 445f243 to 1fd5e49 Compare September 7, 2026 21:31
@Fumuran
Fumuran marked this pull request as ready for review September 7, 2026 21:35
error: TransactionExecutorError,
/// The error the failing execution produced.
///
/// Shared rather than owned because a whole bundle of notes is tested at once, and every note

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.

"shared rather than owned" reads like unnecessary impl details in public documentation

Comment on lines +160 to +162
///
/// Neither half of such a group executes on its own, so probing its notes individually always
/// fails and the search for an executable set has to treat the bundle as its smallest unit.

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.

the latter part explains behavior under a specific failure case, I'd skip this in documentation

impl NoteBundle {
/// Groups `notes` into bundles that must be consumed together.
///
/// A FEE_SPONSORSHIP note joins the bundle of the feature note it names; one whose feature note

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.

personal preference (also noticed this in some of other PRs of my own): Claude uses "it names" quite a lot, which I find vague.
I'd be more specific, and depending on the context, use one of:

  • it commits to
  • it specifies in storage / attachment / etc.

Comment on lines +171 to +172
/// A FEE_SPONSORSHIP note joins the bundle of the feature note it names; one whose feature note
/// is absent forms a bundle of its own, so that it fails alone rather than dropping the notes

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.

"one whose feature note is absent forms a bundle of its own" -> an unpaired sponsorship note forms a bundle of its own

///
/// A FEE_SPONSORSHIP note joins the bundle of the feature note it names; one whose feature note
/// is absent forms a bundle of its own, so that it fails alone rather than dropping the notes
/// it would otherwise have been grouped with. Every other note forms a bundle of its own.

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.

Suggested change
/// it would otherwise have been grouped with. Every other note forms a bundle of its own.
/// it would otherwise have been grouped with. Every other note type forms a bundle of its own.

Comment on lines +174 to +176
///
/// The note heading a bundle, the one the rest of it is bound to, is always first, and bundles
/// keep the ordering of `notes`.

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.

Does the concept of a "heading note" matter for any downstream consumer?

Comment on lines +181 to +183
// Key every note by the index of the note heading its bundle: its feature note's for a
// sponsorship bound to one, its own otherwise. Keying by index rather than by note ID keeps
// the bundles in the caller's order.

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.

reads very cryptic, I have a hard time understanding this comment

Comment on lines +188 to +190
match FeeSponsorshipNote::try_from(&note)
.ok()
.and_then(|sponsorship| note_indices.get(&sponsorship.feature_note_id()).copied())

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.

does this not risk adding the same feature note to two bundles, if for some reason two sponsorship notes are bound to the same feature note?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If I understood you correctly, no, that will not happen. If there is two sponsorship notes bound to one feature note, they will be placed in the map entry i -> [feature, sponsorship_1, sponsorship_2], where i is the index of the feature note which will be placed in this entry at the 0th index.

for (idx, note) in notes.into_iter().enumerate() {
// A sponsorship is only bundled when the note it names is actually an input; otherwise
// it can only be reclaimed, which is something it has to attempt on its own.
match FeeSponsorshipNote::try_from(&note)

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.

we always try to reconstruct a fee sponsorship note first, but what if the first note is a feature note instead?
IIUC, it would be added to a bundle of its own, which would be a mistake, since it should be bundled with the sponsorship note

But I'm not sure I understood the loop correctly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's a bit complicated indeed, even after simplification it's still quite confusing.

What I'm trying to do here is to get a map with entries i -> [feature_note, sponsor_note_1, sponsor_note_2, ...]. So we will end up with an ordered entries which value holds both feature note and its sponsorship notes. i here is the index of the feature note in the original vector of notes — it is used only to preserve the order.

So here are the cases:

  • Provided note is a feature note. In that case we end up in the None branch and we add this feature note in the map under its index. The value there could be empty or already contain some sponsorship notes: i -> [feature_note] or i -> [feature_note, sponsorship_note].
  • Provided note is a sponsorship note. If so, we check that the feature note it sponsors is in the set and get its (feature) index to add this sponsorship note in the map: i -> [sponsorship_note] or i -> [feature_note, sponsorship_note]. If there is no feature note that this sponsorship one sponsors, this note will be added as kind of its own feature note: j -> [sponsorship_note], where j is the index of this orphan sponsorship note in the original set.

So if the input vec looks like so [feature_0, sponsor_0, sponsor_1, feature_1, sponsor_2, feature_2, sponsor_3], in the end we potentially can have entries such as:
0 -> [feature_0, sponsor_0, sponsor_1] <- feature with two sponsors; index 0 since feature_0 has index 0
3 -> [feature_1, sponsor_2] <- feature with one sponsor; index 3 since feature_1 has index 3
5 -> [feature_2] <- feature without sponsors
6 -> [sponsor_3] <- sponsor without feature; index 6 since sponsor_3 has index 6

@partylikeits1983 partylikeits1983 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.

Direction looks good. I left two comments about cases where the checker drops notes that could still be consumed. Could you address those cases and then add regression tests?

.ok()
.and_then(|sponsorship| note_indices.get(&sponsorship.feature_note_id()).copied())
{
Some(head_idx) => bundles.entry(head_idx).or_default().push(note),

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.

If the account does not charge a fee to consume a note, that note does not need a sponsorhip note. Here, adding a sponsorship note with the wrong fee asset makes this checker reject the note too. The previous note consumption checker rejected only the sponsorship. Can we keep the valid note and add a test for this?

for (idx, note) in notes.into_iter().enumerate() {
// A sponsorship is only bundled when the note it names is actually an input; otherwise
// it can only be reclaimed, which is something it has to attempt on its own.
match FeeSponsorshipNote::try_from(&note)

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.

FeeSponsorshipNote::try_from rejects sponsorships with attachments, even though they can be consumed with the notes they sponsor. This means the checker doesn't group them together. When another note's fee isn't covered, the checker rejects the valid pair too. Can we recognize these pairs and add a test?

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.

Make NoteConsumptionChecker bundle-aware

4 participants