Some clarifications for BEGINDATA. #3328
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):
|
MrChico
reviewed
Mar 5, 2021
| ## Rationale | ||
| The byte `0xb6` was chosen to align with [EIP-615](./eip-615.md). | ||
| The choice to `STOP` if `BEGINDATA` is encountered is somewhat arbitrary. An alternative would be to be to abort the execution with an out-of-gas error. | ||
|
|
||
| ## Backwards Compatibility | ||
| The proposal will not change any existing contracts unless their current behaviour relies upon the usage of unused opcodes. | ||
|
|
||
| Since contracts have been using data from the very start, in a sense all of them use unused opcodes, | ||
| but they would have to use data in a way that it is skipped during execution and jumped over. | ||
| The Solidity compiler never generated such code. It has to be evaluated whether contracts created by other means |
There was a problem hiding this comment.
There are some examples of this at https://github.com/wolflo/meta-jumpers, although I believe the ones in that repo were designed specifically to exhibit this behavior, and does not exactly warrant being regarded as "real world examples"
MrChico
reviewed
Mar 5, 2021
|
Besides the nit this looks good to me |
chriseth
force-pushed
the
clarifyBegindata
branch
from
7a7d25d to
f57cfed
Compare
|
Incomporated review comments and squashed. |
|
@MrChico if the PR looks good to you, can you merge please so the bot can complete the merge? Thanks. |
MrChico
approved these changes
Mar 5, 2021
Arachnid
pushed a commit
to Arachnid/EIPs
that referenced
this pull request
Mar 6, 2021
Hi, I'm a bot! This change was automatically merged because: - It only modifies existing Draft, Review, or Last Call EIP(s) - The PR was approved or written by at least one author of each modified EIP - The build is passing
phi-line
pushed a commit
to phi-line/EIPs
that referenced
this pull request
Apr 29, 2021
Hi, I'm a bot! This change was automatically merged because: - It only modifies existing Draft, Review, or Last Call EIP(s) - The PR was approved or written by at least one author of each modified EIP - The build is passing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Kindly asking the author @MrChico for review.