Conversation
🦋 Changeset detectedLatest commit: 3c4b99c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
2-reviewers
A-integration
The current behavior of the sequencer will return the error message `nonce too low` even in the case when the nonce it too high. This should be followed up with changes that fix that problem. Need to double check that it will not break any integrations.
Previously the error message `nonce too low` would be returned even when the user submitted a transaction with a nonce that was too high
tynes
force-pushed
the
fix/nonce-rpc-checks
branch
from
cc76511 to
a04b004
Compare
Codecov Report
@@ Coverage Diff @@
## develop #1882 +/- ##
========================================
Coverage 71.99% 71.99%
========================================
Files 70 70
Lines 2321 2321
Branches 346 346
========================================
Hits 1671 1671
Misses 650 650
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
|
Did we get an explicit user request for this? What experience do users usually get when they send a transaction with too high of a nonce on L1? I want to make sure that the experience is pretty much identical on L2 and I'd be worried about adding a new error that doesn't exist on L1. |
|
When users send a tx with too high of a nonce on L1, it can be accepted into the mempool still. Since we do not have a mempool, we cannot accept transactions with too high of a nonce. This was to improve the error messaging back to the user since the error message would be incorrect. This is true that this adds a new error message that doesn't exist on L1 |
|
I see. Do we have user reports of this being an issue? |
No user reports but it does improve the error message for both the user and our internal logging of error messages |
| if nonce > tx.Nonce() { | ||
| return ErrNonceTooLow | ||
| } else if nonce < tx.Nonce() { | ||
| return errors.New("nonce too high") |
There was a problem hiding this comment.
we can use the concrete ErrNonceTooHigh error:
Line 34 in 8d67991
@smartcontracts this is also relevant to #1816, which will now bubble up the low-level execution errors. We have the option of either mapping |
|
This is already implemented; we split it across two different PRs and merged them both last week. |
## Overview
Couple of changes:
1. Adds a `release-perf` build profile that has `codegen-units = 1` set
and link time optimizations turned on.
- This profile is used by default when building application images.
1. Enables CPU-specific optimizations in the generic application
dockerfile's build routine.
1. Enables the `asm-keccak` feature in `alloy-primitives` within
`kona-node`'s dependency graph.
5 participants
Description
The current behavior of the sequencer will return the
error message
nonce too loweven in the case when thenonce it too high.
This PR changes that functionality so that there are 2 explicit error messages
for when users send transactions with an incorrect nonce.
When the nonce is too low, the user can expect to get the message back:
invalid transaction: nonce too lowWhen the nonce is too high, the user can expect to get the message back:
invalid transaction: nonce too highThere isn't a concept of
nonce too highin the L1 mempool due to the asyncnature of the p2p network, but since there is not a p2p network that gossips
transactions, this feature is added to give a better error message back to the users.