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

Add maxInitCodeSize txpool test #924

Closed
Rjected opened this issue Jan 18, 2023 · 6 comments · Fixed by #1062
Closed

Add maxInitCodeSize txpool test #924

Rjected opened this issue Jan 18, 2023 · 6 comments · Fixed by #1062
Assignees
Labels
A-tx-pool Related to the transaction mempool C-enhancement New feature or request C-test A change that impacts how or what we test

Comments

@Rjected
Copy link
Member

Rjected commented Jan 18, 2023

Describe the feature

An edge case in geth's txpool implementation was recently found, and fixed in go-ethereum#26504. We should create a test for this case.

The intended behavior of the txpool is to reject transactions where the transaction is a contract creation and the tx.data exceeds maxInitCodeSize.

The maxInitCodeSize and maxCodeSize, a related constant, are listed in geth here:

	MaxCodeSize     = 24576           // Maximum bytecode to permit for a contract
	MaxInitCodeSize = 2 * MaxCodeSize // Maximum initcode to permit in a creation transaction and create instructions

Additional context

No response

@Rjected Rjected added C-enhancement New feature or request S-needs-triage This issue needs to be labelled labels Jan 18, 2023
@Rjected Rjected added A-tx-pool Related to the transaction mempool C-test A change that impacts how or what we test and removed S-needs-triage This issue needs to be labelled labels Jan 18, 2023
@jinsankim
Copy link
Contributor

I'm interested in taking this one.

@mattsse
Copy link
Collaborator

mattsse commented Jan 19, 2023

great,

there's a function that performs these checks:

/// Additional checks for a new transaction.
///
/// This will enforce all additional rules in the context of this pool, such as:
/// - Spam protection: reject new non-local transaction from a sender that exhausted its slot
/// capacity.
/// - Gas limit: reject transactions if they exceed a block's maximum gas.
fn ensure_valid(

@onbjerg
Copy link
Member

onbjerg commented Jan 21, 2023

We need to make sure that the max_init_size is configurable for other networks

@onbjerg onbjerg moved this from Todo to In Progress in Reth Tracker Jan 21, 2023
@mattsse
Copy link
Collaborator

mattsse commented Jan 21, 2023

good point, this should be part of the TransactionValidator impl trait, never the less we can use this as standalone function

@jinsankim
Copy link
Contributor

jinsankim commented Jan 23, 2023

Do I have to implement to check if initcode size is exceeded?
When I read the issue first, I thought this issue is just writing a test case. Of course, I'm still interested in it. I'm just like to get confirm whether I understand correctly.

@mattsse
Copy link
Collaborator

mattsse commented Jan 24, 2023

we'll likely move this check into the TransactionValidator trait, which is intended for protocol specific validation.

since maxInitCodeSize will always be checked, it should be a standalone function that is basically just pub fn ensure_max_init_code_size(code: Bytes, max)-> Result

so this should be pretty straight forward. It should go into the validate.rs file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tx-pool Related to the transaction mempool C-enhancement New feature or request C-test A change that impacts how or what we test
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants