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-1013: Finalize all Constantinople hard-fork EIPs #1642

Closed
wants to merge 5 commits into from

Conversation

5chdn
Copy link
Contributor

@5chdn 5chdn commented Dec 9, 2018

  • move status to final
  • add block number for mainnet

I'm not sure if meta EIPs need a last call here? They are just informational, aren't they?

@eip-automerger
Copy link

eip-automerger commented Dec 9, 2018

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):

  • Trying to change EIP 1013 state from Draft to Final
  • EIP 1014 is in state Accepted, not Draft or Last Call
  • EIP 1052 is in state Accepted, not Draft or Last Call
  • EIP 1234 is in state Accepted, not Draft or Last Call
  • EIP 1283 is in state Accepted, not Draft or Last Call
  • EIP 145 is in state Accepted, not Draft or Last Call
  • EIP 1013 requires approval from one of (@nicksavers)

- Block >= TBD on the Ethereum mainnet
- Block >= 4,230,000 on the Ropsten testnet
- `Block >= 7_080_000` on the Ethereum mainnet
- `Block >= 4_230_000` on the Ropsten testnet
- Included EIPs:
Copy link
Member

Choose a reason for hiding this comment

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

I think this PR should also mark all these included EIPs as Final.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@5chdn
Copy link
Contributor Author

5chdn commented Jan 21, 2019

Can we get this merged soon? ref #1716

@5chdn 5chdn changed the title eip-1013: finalize constantinople hard-fork meta EIP-1013: Finalize all Constantinople hard-fork EIPs Jan 21, 2019
@fulldecent
Copy link
Contributor

There is a bug in 1052 and it should not be included in Constantinople in its current form.

I submitted it for the Ethereum Bug Bounty. And I didn't read the details of whether I am allowed to fully disclose details while it is being considered for bug bounty, so I won't say anything else here yet.

@5chdn
Copy link
Contributor Author

5chdn commented Jan 22, 2019

There is a bug in 1052 and it should not be included in Constantinople in its current form.

Whatever the outcome is, Constantinople is final as it is active on Ropsten, Kovan, Rinkeby, Sokol and xDAI already. If we need to disable it, we will need a new upgrade. This does not affect EIP-1052 or 1013.

@holiman
Copy link
Contributor

holiman commented Jan 24, 2019

@fulldecent we answered, that submission is not an issue, it's a false positive. Feel free to share.

@fulldecent
Copy link
Contributor

@holiman Thank you for the quick response!


The EIP-1052 issue

Here is my issue with EIP-1052 (the version as of 80b8f80).

I believe this test case:

"The EXTCODEHASH of an precompiled contract is either c5d246... or 0."

is ambiguous and could lead to multiple, conflicting implementations. That of course would lead to a network split and cost billions of dollars.

The solution is to provide more explicit test cases, one that returns c5d246... and one that returns 0.


Disclosure timeline:

2019-01-21 -- I identified issue and disclosed to EF under the Ethereum Bounty Program since it could lead to a network split and that is a security issue.
2019-01-21 -- The author of the EIP replied privately and defended that no change is necessary
2019-01-22 -- Another number of EF replied privately and defended that the current EIP is clear as specified and that no change is necessary.
2019-01-24 -- A member of EF invited me to publish this
2019-01-26 -- I am posting this publicly.

Practically speaking, any impact this issue report could have is limited by the fact that EF controls every/nearly every implementation of the client and the upgrade path of all deployed clients.

I do recognize that the value of using the full two-week EIP process and fixing standards is more important for the actual decentralized future and setting a good example, rather than worrying about what might happen the day after this current fork.

@5chdn 5chdn closed this Feb 17, 2019
@5chdn 5chdn reopened this Feb 17, 2019
@5chdn 5chdn closed this Feb 17, 2019
@5chdn 5chdn deleted the patch-3 branch February 17, 2019 19:09
@axic
Copy link
Member

axic commented Feb 18, 2019

@5chdn why did you close this? I think this one should be merged.

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.

6 participants