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: optional nonce check #1195

Merged
merged 1 commit into from Mar 14, 2024
Merged

Conversation

Wodann
Copy link
Contributor

@Wodann Wodann commented Mar 13, 2024

For Hardhat we need the ability to disable nonce checks. This PR follows the existing pattern for similar features and adds it to the dev feature flag.

@Wodann
Copy link
Contributor Author

Wodann commented Mar 13, 2024

The failing test is also present on main

@rakita
Copy link
Member

rakita commented Mar 14, 2024

This pattern is going to be removed in future.

Would create a handler register that overrides the validation handler and sets Env nonce to the expected value:

pub fn validate_tx_against_state<SPEC: Spec, EXT, DB: Database>(
context: &mut Context<EXT, DB>,
) -> Result<(), EVMError<DB::Error>> {
// load acc
let tx_caller = context.evm.env.tx.caller;
let (caller_account, _) = context
.evm
.inner
.journaled_state
.load_account(tx_caller, &mut context.evm.inner.db)?;
context
.evm
.inner
.env
.validate_tx_against_state::<SPEC>(caller_account)
.map_err(EVMError::Transaction)?;
Ok(())
}

In the future you can expect most of those features to be removed, as the same result can be done with handler register.

But as handler register are still new, would be okay to include this now. Up to you, how would you want to proceed?

@Wodann
Copy link
Contributor Author

Wodann commented Mar 14, 2024

As we need this for a bug fix in Hardhat, I'd appreciate if we can use this approach for now.

@rakita
Copy link
Member

rakita commented Mar 14, 2024

btw you would get the same result if you set tx.nonce = None

@rakita
Copy link
Member

rakita commented Mar 14, 2024

approving this as temporary fix.

@rakita rakita merged commit fe841be into bluealloy:main Mar 14, 2024
21 of 25 checks passed
@github-actions github-actions bot mentioned this pull request Mar 14, 2024
@Wodann
Copy link
Contributor Author

Wodann commented Mar 14, 2024

btw you would get the same result if you set tx.nonce = None

That's good to know. That's just as easy a fix, actually. Thanks!

@Wodann
Copy link
Contributor Author

Wodann commented Mar 14, 2024

The ability to set the tx.nonce to zero is just as good as the changes that I made here, so feel free to revert the commit.

Apologies; I should've realised.

@Wodann Wodann deleted the feat/disable-nonce-check branch March 14, 2024 21:35
rakita added a commit that referenced this pull request Mar 19, 2024
rakita added a commit that referenced this pull request Mar 19, 2024
* Revert "feat: optional nonce check (#1195)"

This reverts commit fe841be.

* typo
fubuloubu pushed a commit to ApeWorX/revm that referenced this pull request Apr 11, 2024
fubuloubu pushed a commit to ApeWorX/revm that referenced this pull request Apr 11, 2024
* Revert "feat: optional nonce check (bluealloy#1195)"

This reverts commit fe841be.

* typo
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.

None yet

2 participants