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

feat(rpc): improve error message for nonce too low #10511

Closed
wants to merge 4 commits into from

Conversation

i-sangh
Copy link

@i-sangh i-sangh commented Aug 25, 2024

ref #10464. closes #10498.

The current error wasn't very helpful: nonce too low

So ideally it should now be:

nonce too low: next nonce 72, tx nonce 71

crates/rpc/rpc-eth-types/src/error.rs Outdated Show resolved Hide resolved
@emhane
Copy link
Member

emhane commented Aug 25, 2024

make sure to run lint with nightly tool chain, otherwise a bunch of semicolons are added that we don't want, try command make lint

More descriptive line for error nonce too low

Co-authored-by: Emilia Hane <emiliaha95@gmail.com>
@emhane emhane enabled auto-merge August 25, 2024 16:20
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

one more suggestion

@@ -494,7 +503,7 @@ impl From<reth_primitives::InvalidTransactionError> for RpcInvalidTransactionErr
// txpool (e.g. `eth_sendRawTransaction`) to their corresponding RPC
match err {
InvalidTransactionError::InsufficientFunds { .. } => Self::InsufficientFunds,
InvalidTransactionError::NonceNotConsistent => Self::NonceTooLow,
InvalidTransactionError::NonceNotConsistent => Self::NonceTooLow { tx: 0, state: 0},
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we can also add these fields to the NonceNotConsistent error variant

// Checks for nonce
if transaction.nonce() < account.nonce {
return TransactionValidationOutcome::Invalid(
transaction,
InvalidTransactionError::NonceNotConsistent.into(),
)
}

Comment on lines 240 to 242
EVMError::Transaction(_) => EthApiError::InvalidTransaction(
RpcInvalidTransactionError::NonceTooLow { tx: 0, state: 0 }
),
Copy link
Collaborator

Choose a reason for hiding this comment

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

this doesn't look right, why this change?

@DaniPopes DaniPopes changed the title Improved the rpc error for nonce two low #10464 feat(rpc): improve error message for nonce too low Aug 26, 2024
@i-sangh
Copy link
Author

i-sangh commented Aug 28, 2024

error.rs & eth.rs

@i-sangh i-sangh closed this Aug 28, 2024
@mattsse
Copy link
Collaborator

mattsse commented Aug 28, 2024

why was this closed @i-sangh ? this was on track and almost ready

@i-sangh
Copy link
Author

i-sangh commented Aug 28, 2024 via email

@emnul
Copy link
Contributor

emnul commented Sep 4, 2024

Happy to create a new branch and continue work on on this one @mattsse

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.

Improve rpc error for nonce too low
4 participants