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-2315: updated testcases + clarifications #2656

Merged
merged 1 commit into from
May 19, 2020

Conversation

holiman
Copy link
Contributor

@holiman holiman commented May 19, 2020

This PR contains the updated testcases with the recent changes. Also some clarifications and I re-added cost of BEGINSUB. All opcodes should have a defined cost, IMO.

@holiman holiman requested a review from gcolvin May 19, 2020 13:35
@eip-automerger eip-automerger merged commit 77cab3e into ethereum:master May 19, 2020
@holiman holiman mentioned this pull request May 20, 2020


```
Error: at pc=0, op=BEGINSUB: invalid subroutine entry
Copy link

Choose a reason for hiding this comment

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

Do we want an OOG message or another like invalid subroutine entry ?
If we want to use this new message maybe we need to update this part https://github.com/ethereum/EIPs/blob/master/EIPS/eip-2315.md#beginsub ?

@holiman
Copy link
Contributor Author

holiman commented May 22, 2020 via email

@matkt
Copy link

matkt commented May 22, 2020

The actual error message is not part of consensus

yes indeed it's just to have a consistency :)

@holiman
Copy link
Contributor Author

holiman commented May 23, 2020

Well, I think saying "it goes OOG", means that it will "consensus-wise" behave as an OOG, which is a well-understood failure mode at this time. Whether the human-readable strings that a client spits out says that or something else is not something that should be defined in the EIP, imo.

If we want more consistency, it would be more suited for another eip (not core), to standardize stuff that are non-consensus-critical.

But I guess it's un-intuitive that the EIP says OOG error but the EIP-examples says something else. SO may be I should clarify in the EIP that the testcases/examples are based on geth and the message itself is an implementation detail.

pizzarob pushed a commit to pizzarob/EIPs that referenced this pull request Jun 12, 2020
Hi, I'm a bot! This change was automatically merged because:

 - It only modifies existing Draft or Last Call EIP(s)
 - The PR was approved or written by at least one author of each modified EIP
 - The build is passing
tkstanczak pushed a commit to tkstanczak/EIPs that referenced this pull request Nov 7, 2020
Hi, I'm a bot! This change was automatically merged because:

 - It only modifies existing Draft or Last Call EIP(s)
 - The PR was approved or written by at least one author of each modified EIP
 - The build is passing
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 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants