Skip to content

fix(ntx-builder): actors self healing after error when submitting#2371

Open
SantiagoPittella wants to merge 2 commits into
nextfrom
santiagopittella-fix-ntx-builder-self-healing
Open

fix(ntx-builder): actors self healing after error when submitting#2371
SantiagoPittella wants to merge 2 commits into
nextfrom
santiagopittella-fix-ntx-builder-self-healing

Conversation

@SantiagoPittella

Copy link
Copy Markdown
Collaborator

Summary

Why

The network transaction builder's actor held a single in-memory copy of its account for its whole lifetime, refreshed from the DB only at spawn and on transaction expiry. When a submission was rejected (e.g. AccountCommitmentMismatch: the declared initial account commitment no longer matches the committed chain), the actor kept the stale in-memory account and re-executed against it, re-declaring the same stale commitment and failing repeatedly until an unrelated expiry or a respawn forced a reload. This showed up as repeated SubmitProvenTx rejections for a network account whose committed state had already moved on.

How

On a submission rejection (NtxError::Submission), reload the account from the DB before the next selection, mirroring the existing expiry-reload path. Re-execution stays gated on the next committed block (the actor returns to NoViableNotes), so it re-syncs without tight-looping if the local DB is momentarily behind.

Changelog

[[entry]]
scope       = "ntx-builder"
impact      = "fixed"
description = "Reload account state from the database after a rejected submission so the actor stops re-submitting against a stale account commitment."

@SantiagoPittella
SantiagoPittella requested review from Mirko-von-Leipzig and igamigo and removed request for Mirko-von-Leipzig July 23, 2026 15:49

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

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.

Is this driving the monitor errors on devnet?

And is this a problem on testnet?

@SantiagoPittella

Copy link
Copy Markdown
Collaborator Author

Is this driving the monitor errors on devnet?

Yes, I cannot check with the value in the database for the account commitment on the monitor, but the in-memory account is not getting updating after a failed transaction submission (the only cases where we re-fetch from the database the accoutn is when spawning the actor and when a submitted transaction expires). This changes adds a re-fetch when submission fails too.

And is this a problem on testnet?

Yes, this applies there too. I plan to cherry pick the commit there

@igamigo igamigo left a comment

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.

LGTM! I left some questions mostly out of curiosity, so feel free to disregard.
I'm not entirely sure merging this to next first is preferable to merging it to main (I think you run some risks of still having conflicts when merging next back to main later on, but not super sure about this).

Comment on lines +665 to +666
// A rejected submission (e.g. an account-commitment mismatch) means our in-memory
// account has diverged from the committed chain. Reload it from the DB below so the

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.

Do we know how it could diverge? The NTX should currently always get committed blocks, no?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In the DB we have the account version of the streamed committed blocks, but in the actor we have an in-memory version of the account that gets updated applying in place each transaction's account patch. The divergence could happen if the account thinks that a submitted transaction was applied and the coordinator didn't receive that update yet or the account was updated by someone else

Comment on lines 574 to 575
/// Any error reaching this method is therefore terminal for the candidate: the batch's notes
/// are marked failed and the actor moves on.

@igamigo igamigo Jul 23, 2026

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.

nit: Maybe the new behavior could be specified here. If we also want to keep this function non-mutating, this could return an error so it can be reacted to from the outside

Comment thread bin/ntx-builder/src/actor/mod.rs
@SantiagoPittella

Copy link
Copy Markdown
Collaborator Author

I'm not entirely sure merging this to next first is preferable to merging it to main (I think you run some risks of still having conflicts when merging next back to main later on, but not super sure about this).

Conflicts will arise for sure no matter to which branch is merged first because the actors impls are a bit different in each. But those shouldn't be much work considering that this is a pretty small change. Though in any case the conflcits can be solved in the cherry-pick

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.

3 participants