refactor(ntx-builder): replace the account actors with a scheduler - #2585
Open
SantiagoPittella wants to merge 1 commit into
Open
refactor(ntx-builder): replace the account actors with a scheduler#2585SantiagoPittella wants to merge 1 commit into
SantiagoPittella wants to merge 1 commit into
Conversation
SantiagoPittella
force-pushed
the
santiagopittella-ntx-scheduler-swap
branch
from
September 7, 2026 14:18
c33fb3c to
ccbc958
Compare
SantiagoPittella
force-pushed
the
santiagopittella-ntx-scheduler-swap
branch
from
September 8, 2026 13:33
ccbc958 to
831b6f9
Compare
SantiagoPittella
force-pushed
the
santiagopittella-ntx-scheduler-swap
branch
from
September 9, 2026 21:00
831b6f9 to
c5b29a1
Compare
Collaborator
There was a problem hiding this comment.
I'm not a fan of attempt as a base name.
Perhaps something like NetworkTransactionContext and the current free standing functions could be methods?
Comment on lines
+32
to
+34
| /// Chain tip at submission. With the transaction expiration delta this bounds how long the | ||
| /// account stays blocked when the submission never lands. | ||
| submitted_at: BlockNumber, |
Collaborator
There was a problem hiding this comment.
Would it make more sense to store expires_at given that's what actually triggers?
Comment on lines
+53
to
+57
| /// The spawned attempt tasks. | ||
| tasks: JoinSet<AttemptOutcome>, | ||
|
|
||
| /// Accounts with a running attempt, keyed by task id. These occupy the attempt slots. | ||
| running: HashMap<Id, AccountId>, |
Collaborator
There was a problem hiding this comment.
Perhaps we should make the utils Tasks generic and use that here?
Comment on lines
+64
to
+69
| /// Maximum number of attempts computed concurrently. | ||
| max_concurrent_txs: usize, | ||
|
|
||
| /// Number of blocks after which a submitted transaction expires. An in-flight entry older than | ||
| /// this is dropped, which releases the account for a new attempt. | ||
| tx_expiration_delta: NonZeroU16, |
Collaborator
There was a problem hiding this comment.
Consider moving to a dedicated Config struct.
Comment on lines
+294
to
+298
| pub async fn shutdown(&mut self) { | ||
| self.tasks.shutdown().await; | ||
| self.running.clear(); | ||
| self.in_flight.clear(); | ||
| } |
Collaborator
There was a problem hiding this comment.
When is this called? I'm surprised this isn't shutdown(self), then no need to keep this re-useable afterwards.
Comment on lines
+181
to
+192
| loop { | ||
| match self.tasks.join_next_with_id().await { | ||
| Some(Ok((id, outcome))) => { | ||
| self.running.remove(&id); | ||
| return Ok(outcome); | ||
| }, | ||
| Some(Err(err)) => { | ||
| let account_id = self.running.remove(&err.id()); | ||
| // Cancelled tasks were aborted on shutdown. | ||
| if err.is_cancelled() { | ||
| continue; | ||
| } |
Collaborator
There was a problem hiding this comment.
This loop and cancellation check feel a bit weird?
Comment on lines
+208
to
+212
| /// A slot is refilled immediately after an attempt that made progress, so a completed | ||
| /// transaction does not leave capacity idle until the next block. An attempt that found no | ||
| /// viable work, or that could not run at all, does not trigger a refill: the state that | ||
| /// selected its account has not changed, so an immediate re-dispatch could pick the same | ||
| /// account again. |
Collaborator
There was a problem hiding this comment.
This is referencing stuff outside of this functions scope.
Comment on lines
+263
to
+264
| // Applied before the failures so a note that is both corrected and penalized keeps the | ||
| // backoff block the penalty computes, which is the later of the two. |
Collaborator
There was a problem hiding this comment.
I don't quite understand why notes need to be corrected?
SantiagoPittella
force-pushed
the
santiagopittella-ntx-scheduler-swap
branch
from
September 10, 2026 16:47
c5b29a1 to
14c5a4e
Compare
SantiagoPittella
force-pushed
the
santiagopittella-ntx-scheduler-swap
branch
from
September 11, 2026 21:27
14c5a4e to
d5adee5
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
Now the loop spawns a task per account, up to
--max-concurrent-txsat a time. The task picks notes, executes, proves, submits, and returns what happened, also this loops does the writting.The scheduler doesn't hold anything in memory per account except which transactions it has submitted and not yet seen committed.
Changelog