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

Check EIP-3607 at transaction validation #1084

Merged

Conversation

librelois
Copy link
Contributor

The EIP-3607 check should be done on the transaction validation (here) andon transaction execution, otherwise a transaction from a contract key will be included in the mempool and pollute the mempool forever.

@librelois librelois requested a review from sorpaas as a code owner June 21, 2023 14:28
Copy link
Contributor

@tgmichel tgmichel left a comment

Choose a reason for hiding this comment

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

lgtm, just missing a ts test for this.

An alternative to this would be instantiate CheckEvmTransaction with is_empty result, and make this check be part of validate_in_pool_for. The added benefit of that approach is we do this check when following pallet-ethereum or pallet-evm direct code paths, so sharing behaviour and error handling is ideal.

// Do not allow transactions for which `tx.sender` has any code deployed.
//
// This check should be done on the transaction validation (here) **and**
// on trnasaction execution, otherwise a contract tx will be included in
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// on trnasaction execution, otherwise a contract tx will be included in
// on transaction execution, otherwise a contract tx will be included in

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants