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

EIP-2677: Cap initcode (draft) #2677

Merged
merged 7 commits into from
Aug 29, 2020
Merged

Conversation

holiman
Copy link
Contributor

@holiman holiman commented May 28, 2020

Propose a cap on initcode.

:shipit:

holiman and others added 2 commits May 28, 2020 13:34
Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>
@axic
Copy link
Member

axic commented Aug 28, 2020

@holiman merged in master and added a discussion URL so this can be merged.

@axic axic requested a review from MicahZoltu August 28, 2020 20:03
@axic axic changed the title EIP draft initcode cap EIP-2677: Cap initcode (draft) Aug 28, 2020
Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

Good enough to merge as a draft, but I wouldn't be willing to merge this to Last Call as it is (even ignoring the incomplete sections). I recommend reviewing the feedback provided and making adjustments before submitting to last call.

Also consider adjusting the tense of the writing here. I very much prefer to write EIPs as technical documents that target a reader base of future specification readers, rather than current/past readers. This means instead of saying "there has been a size limit of ..." you might say "prior to this EIP there was a size limit of ..." (as a simple example).

EIPs are assertions, not proposals. This means the EIP shouldn't say stuff like "we propose you do X", it should instead say "do X". whether someone follows the EIP or not is a totally separate conversation that should not be part of the EIP itself, and once someone has decided to follow the EIP everything in it should be an assertion unless it is intentionally left ambiguous (e.g., SHOULD or MAY).


## Simple Summary

Enforce a maximum size limit (`max_initcode_size`) of `49152` (`0xc000`) for `initcode`.
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
Enforce a maximum size limit (`max_initcode_size`) of `49152` (`0xc000`) for `initcode`.
Set a maximum size limit of 49152 for `initcode`.

Comment on lines +20 to +25
Since [EIP-170](https://eips.ethereum.org/EIPS/eip-170) was implemented, there has been a size limit of `24576` (`0x6000`) on contract code. We propose to also limit the size of executable code to `2x` the above limit, i.e. `49152` (`0xc000`).

This also leads to two nice properties:

- instruction offset in code fits 16-bit value,
- code size fits 16-bit value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider moving all of this to the motivation section. Abstract should be a human-friendly and terse version of the specification, while the motivation section is where you state why this is a good idea, how these changes make things better, etc.

Comment on lines +40 to +48
## Specification

There are three situations where this is applicable:

* `CREATE`,
* `CREATE2`,
* creation using a transaction with empty receiver.

In all these (and future) cases, the EVM should fail with Out Of Gas error if the code has a length more than `max_initcode_size`.
Copy link
Contributor

Choose a reason for hiding this comment

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

A user should be able to implement an EIP from the specification section alone. Currently, this specification section is not self contained as it appears to be referring to information in other sections (e.g., the this reference in the first sentence).

Also it should make assertions, not suggestions, especially for consensus code. "should fail" will result in total consensus failure if some clients fail and some clients don't fail. My guess is that you meant MUST fail here.

Also, the specification section should not be discussing hypothetical future cases here. It may be worth mentioning in the security considerations or backward compatibility section, but discussion about the behavior of future hypothetical cases are not really relevant for the specification section of a technical document.

@MicahZoltu MicahZoltu merged commit bc04fa0 into ethereum:master Aug 29, 2020
tkstanczak pushed a commit to tkstanczak/EIPs that referenced this pull request Nov 7, 2020
* EIP draft initcode cap

* Update EIPS/eip-draft_cap_initcode.md

Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>

* EIP-2677: linter fixes, minor changes, update with EIP number

* Small typographic changes

* Add discussion URL

* Fix EIP header

Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>
Co-authored-by: Alex Beregszaszi <alex@rtfs.hu>
Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request Mar 6, 2021
* EIP draft initcode cap

* Update EIPS/eip-draft_cap_initcode.md

Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>

* EIP-2677: linter fixes, minor changes, update with EIP number

* Small typographic changes

* Add discussion URL

* Fix EIP header

Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>
Co-authored-by: Alex Beregszaszi <alex@rtfs.hu>
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.

5 participants