Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

block proposal and approval must ensure nonce continuity #341

Closed
jchappelow opened this issue Oct 5, 2023 · 1 comment · Fixed by #338
Closed

block proposal and approval must ensure nonce continuity #341

jchappelow opened this issue Oct 5, 2023 · 1 comment · Fixed by #338
Assignees

Comments

@jchappelow
Copy link
Member

jchappelow commented Oct 5, 2023

Presently:

  • block preparation (in PrepareProposal) can incorrectly submit a block including transactions with nonces that are not continuous with the previous best committed nonce (not just order within the block).
  • block approval (in ProcessProposal) does not ensure there are no nonce gaps.

Right now if you submit a transaction with an invalid nonce, the transaction still makes it to DeliverTx (it is mined in a block)! Although it fails execution when it tries to Spend and finds the incorrect nonce, the transaction with the bad nonce is still mined. This cannot happen. The account store failed to spend (did not increment the nonce), yet the blockchain included a transaction with a higher nonce into a block.

New mempool checks will prevent some of these issues, but they must also be there at the level of our consensus rules to ensure incoming blocks from peer nodes follow the rules.

In other words with well behaved nodes, when called from DeliverTx, there should be no way that Spend should error on account of nonce.

EDIT: I think that sentry/non-validator nodes do not need to be concerned with this, and they should just execute, but I'm not positive.

@jchappelow
Copy link
Member Author

jchappelow commented Oct 5, 2023

With the way we have checkSpend blocking updateAccount where the nonce gets incremented in the account store (state), this actually does mean that nonces within a block also have to be ordered. PrepareProposal ensures that in #334, but ProcessProposal has to as well. So, it's more than just avoiding gaps compared to previous block.

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 a pull request may close this issue.

2 participants