refactor(ntx-builder): extract transaction selection out of the account actor - #2583
SantiagoPittella wants to merge 1 commit into
Conversation
83603ed to
8f5a83e
Compare
8f5a83e to
73c6b60
Compare
Mirko-von-Leipzig
left a comment
Mirko-von-Leipzig
left a comment
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.
| /// 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; |
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.
There was a problem hiding this comment.
Probably but just a little, I don't thik it changes much things
| /// 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)>, |
There was a problem hiding this comment.
We should be able to reject these at ingress time already I think?
There was a problem hiding this comment.
Not sure, since the account might not exist in our database yet (though to solve this we could first check accounts and then notes). Also the allowlist is mutable so it may be allowed the next selection time since it is checked against the last state.
| // 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); |
There was a problem hiding this comment.
How does this work? I thought each feature note gets a sponsorship note?
There was a problem hiding this comment.
It can have multiple sponsorship notes
| // 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. |
There was a problem hiding this comment.
Is this because the allowlist is a storage item? That's.. really annoying.
73c6b60 to
9616274
Compare
9616274 to
92c5e86
Compare
92c5e86 to
aa7ec8a
Compare
aa7ec8a to
0058e0b
Compare
0058e0b to
7e19bc3
Compare
fb5cc6e to
d83a2ba
Compare
d83a2ba to
42ad9b2
Compare
This branch has not been deployed
2 participants
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