refactor(ntx-builder): extract transaction selection out of the account actor - #2583
Open
SantiagoPittella wants to merge 1 commit into
Open
Conversation
SantiagoPittella
force-pushed
the
santiagopittella-ntx-extract-selection
branch
from
September 7, 2026 14:18
83603ed to
8f5a83e
Compare
SantiagoPittella
force-pushed
the
santiagopittella-ntx-extract-selection
branch
from
September 8, 2026 13:33
8f5a83e to
73c6b60
Compare
Mirko-von-Leipzig
approved these changes
Sep 9, 2026
Mirko-von-Leipzig
left a comment
Mirko-von-Leipzig
left a comment
Collaborator
There was a problem hiding this comment.
I have questions, but since this is an extraction of already existing code it shouldn't hold up the refactor.
Comment on lines
+30
to
+32
| /// Maximum number of `FEE_SPONSORSHIP` notes attached to a single feature note. A feature note with | ||
| /// more pending sponsorships than this keeps a subset of this size. | ||
| const MAX_SPONSORSHIPS_PER_NOTE: usize = 3; |
Collaborator
There was a problem hiding this comment.
Would it simplify matters if we restrict sponsorship to a single note which uses the native token for now? As in, always select the sponsorship with the maximum amount of native token.
Comment on lines
+56
to
+58
| /// Notes dropped because the account does not allowlist their script root. They can never be | ||
| /// consumed by this account, so the caller must penalize them. | ||
| pub rejected: Vec<(Nullifier, NoteError)>, |
Collaborator
There was a problem hiding this comment.
We should be able to reject these at ingress time already I think?
Comment on lines
+114
to
+115
| // A bundle must leave room for its feature note within the per-tx note budget. | ||
| let max_sponsorships = MAX_SPONSORSHIPS_PER_NOTE.min(max_notes - 1); |
Collaborator
There was a problem hiding this comment.
How does this work? I thought each feature note gets a sponsorship note?
Comment on lines
+145
to
+146
| // Notes just dropped by the allowlist re-enter eligibility through backoff, so ask for a | ||
| // re-check on the next block rather than reporting the account as having no pending work. |
Collaborator
There was a problem hiding this comment.
Is this because the allowlist is a storage item? That's.. really annoying.
SantiagoPittella
force-pushed
the
santiagopittella-ntx-extract-selection
branch
from
September 9, 2026 21:00
73c6b60 to
9616274
Compare
SantiagoPittella
force-pushed
the
santiagopittella-ntx-extract-selection
branch
from
September 10, 2026 16:47
9616274 to
92c5e86
Compare
SantiagoPittella
force-pushed
the
santiagopittella-ntx-extract-selection
branch
from
September 11, 2026 21:07
92c5e86 to
aa7ec8a
Compare
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
2 participants
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.
Summary
Moves selection, the failure-attribution helpers and the transaction arguments builder out of
AccountActorintoselection.rs, and movescandidate.rs,allowlist.rsandexecute.rsout ofactor/select_candidatebecomes a free function that reports the notes an account's allowlist rejects rather than writing them. The actor does that write, as it already does for every other note failure, so behaviour is unchanged.Changelog