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

Require MAX_INITCODE_SIZE limit to be satisfied during validation #125

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

pdobacz
Copy link
Member

@pdobacz pdobacz commented Jun 6, 2024

Pushing in form of a PR after discussion during EOF implementers call 49.

To reiterate the reasoning behind:

Synopsis of the problem:

  • subcontainer have size limit of 64K implied by the header format
  • but this doesn't apply to "top-level" containers!
  • we have EIP-3860 MAX_INITCODE_SIZE of 48K - but not part of EOF validation!
  • EVM implementations doing validate_eof and handling the containers/headers must make assumptions about the maximum size of the container to expect and handle
    • is it MAX_INITCODE_SIZE?
    • is it 64K, same as subcontainer?
    • is it whatever can be maxed out with sections, i.e. max code sections, then max of max container sections and max data size?
    • is it some yet other value?
  • for example, evmone had a subtle bug, where large top-level containers had "overflown" data_section_offset persisted in the header, because offsets were expected to not exceed 64K

Proposal

Introduce a validation rule of max top-level container size of MAX_INITCODE_SIZE 48K (or twice that constant, for practical reasons like solidity testing methods). Whenever either the bytestring being the top-level container or the declared (in the header, as discovered during header parsing) size of the top-level container exceed that, validation fails.

@pdobacz pdobacz self-assigned this Jun 6, 2024
@chfast
Copy link
Member

chfast commented Jun 6, 2024

I agree.

@pdobacz
Copy link
Member Author

pdobacz commented Jun 10, 2024

Just got an answer from Solidity, that limiting to MAX_INITCODE_SIZE is not an issue from their POV

@pdobacz
Copy link
Member Author

pdobacz commented Jun 13, 2024

Opening an EIPs PR for this and #128, as well as putting adding EEST tests on my list.

@pdobacz pdobacz merged commit d0f35ea into main Jun 13, 2024
2 checks passed
@pdobacz pdobacz deleted the max-size-in-validation branch June 13, 2024 14:31
pdobacz added a commit to pdobacz/EIPs that referenced this pull request Jun 13, 2024
pdobacz added a commit to ipsilon/execution-spec-tests that referenced this pull request Jun 14, 2024
pdobacz added a commit to pdobacz/EIPs that referenced this pull request Jun 17, 2024
pdobacz added a commit to ipsilon/execution-spec-tests that referenced this pull request Jun 17, 2024
pdobacz added a commit to pdobacz/EIPs that referenced this pull request Jun 17, 2024
pdobacz added a commit to ipsilon/execution-spec-tests that referenced this pull request Jun 20, 2024
pdobacz added a commit to ipsilon/execution-spec-tests that referenced this pull request Jun 20, 2024
pdobacz added a commit to ipsilon/execution-spec-tests that referenced this pull request Jun 24, 2024
pdobacz added a commit to ipsilon/execution-spec-tests that referenced this pull request Jun 27, 2024
marioevz added a commit to ethereum/execution-spec-tests that referenced this pull request Jun 27, 2024
* new(tests): EOF - EIP-3540 container size

Tests ipsilon/eof#125

* fix(tests): Adjust to container size limit

* Update tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_container_size.py

* Update tests/prague/eip7692_eof_v1/eip7480_data_section/test_code_validation.py

* fix(tests): import

---------

Co-authored-by: Mario Vega <[email protected]>
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.

2 participants