search

ethcatherders / hard-fork-checklist

A markdown document containing pertinent stake holder information in preparation for hard forks
19 stars 6 forks source link

Data Gathering for Post-Mortem of Constantinople Hard Fork Delay #1

Open chrishobcroft opened 5 years ago

chrishobcroft commented 5 years ago

This thread is designed to capture information, thoughts, ideas for topics to discuss in a post-mortem of the decision to delay the Constantinople hard fork, which are unable to be addressed in this moment.

Thank you for your contributions and patience.

veox commented 5 years ago

Q: Why was the possibility of reentrancy discovered so late?

A: Via gitter, by @holgerd77:

(...) I think one part of the story might be the not-so-long-yet availability of Constantinople dev tools. We released the EthereumJS Constantinople VM 22 Nov 2018, it took Truffle some time to integrate and the first Constantinople-ready Ganache version came out just 6 days ago. Since ChainSecure uses this first beta to test the vulnerability my assumption is that these releases might have triggered experimentation, will investigate this further.

One outcome of this might be to take dev tool readyness stronger into account when planning a hardfork date, and make release of the 2-3 most used tools (and not just the VM as some base layer) a precondition for some date settlement.

These are, obviously, "candidate Q" and "candidate A".

Tip: to get markdown if you don't have Owner access to this repo, go to the gitter message, click the message menu icon ..., and select "Quote".

tayvano commented 5 years ago

Some bullet points I've been collecting:

Things that could / should be considered when writing / choosing / merging EIPs:

Potentially things clients should do:

Misc

CPSTL commented 5 years ago

I made this during the emergency call, "The Decision Process For The Constantipole Ethereum Hard Fork / Network Upgrade Postponement":

https://docs.google.com/document/d/1gr0hHAxobzpLYDXZnZFk6qPuv8_to3XLNQ9TvXMCWxw/edit?usp=sharing

I added ideas and suggest we all deconstruct the process and suggest ideas for future decision making ✍️

tayvano commented 5 years ago

user consideritwon on reddit:

Secondly, on how this slipped through for so long. Is there any way automated testing can be improved to catch this sort of thing or is it something that needs to be manually discovered? Any lessons learned?

tayvano commented 5 years ago

Vitalik on reddit:

All of the really nasty security issues that we had have been around the interactions between different components. The quadratic DoS attacks combined EVM memory and the call stack frame or reverts and the call stack frame, this potential threat arose because of interactions between the default gas in send, SSTORE gas costs and re-entrancy issues. So if you have N protocol features, there are N2 ways they could potentially break.

I would say my personal takeaway from this is to be much more explicit about writing down invariants (properties guaranteed by the protocol) that we rely on so we can check against them when changing things.

benjamincburns commented 5 years ago

Q: Why was the possibility of reentrancy discovered so late?

A: Via gitter, by @holgerd77:

(...) I think one part of the story might be the not-so-long-yet availability of Constantinople dev tools. We released the EthereumJS Constantinople VM 22 Nov 2018, it took Truffle some time to integrate and the first Constantinople-ready Ganache version came out just 6 days ago. Since ChainSecure uses this first beta to test the vulnerability my assumption is that these releases might have triggered experimentation, will investigate this further. One outcome of this might be to take dev tool readyness stronger into account when planning a hardfork date, and make release of the 2-3 most used tools (and not just the VM as some base layer) a precondition for some date settlement.

These are, obviously, "candidate Q" and "candidate A".

Tip: to get markdown if you don't have Owner access to this repo, go to the gitter message, click the message menu icon ..., and select "Quote".

Adding my reply from gitter:

I share the concern (I'm a member of the Truffle team and oversee the development of ganache-core and ganache-cli). Who should I be coordinating with to make sure we're supporting things appropriately here? also if there's a formal (or informal) retrospective on this, I'd be happy to participate

veox commented 5 years ago

Regarding automated testing: this time, it was not discovered by fuzzing.

lithp commented 5 years ago

Regarding automated testing: this time, it was not discovered by fuzzing.

Is this the kind of thing that fuzzing is looking for? The problem is that an invariant is being broken which nobody had previously written down.

holgerd77 commented 5 years ago

Had some exchange on this with Ralph who is the organizer of our local Vienna Ethereum meetup and helped to discover this: he told me that he actually knew about this for months (sigh) and just thought that ChainSecurity would have already reported this.

Instead of blaming anyone here (Ralph: surely not, ChainSecurity: there might be some questions) I think we can go one level higher and state that there are definitely some communication issues here, since the information was there in the community, it was just not possible to harness.

Ideas to address this could be some bug bounty program, or at least some "Community call to submit every suspicion of a bug/fork inconsistency/weird behavior" blog post somewhere in a fork preparation process.

ralph-pichler commented 5 years ago

I wouldn't say "knew", more like "suspected". I thought surely somebody else already thought of this at some point (since it seemed obvious) and that surely it wouldn't work for some reason. Anyway I wanted to test this on Ropsten first (as I already submitted a bounty in the past and back then it turned out to be an issue with testrpc instead), but that was a side project that just ended up getting delayed and delayed again for various reasons. It was only during a recent discussion with chainsecurity that I realized this really might be a serious problem. That being said it might have been better to already report this immediately instead of trying to scan for vulnerable contracts first.

tayvano commented 5 years ago

That being said it might have been better to already report this immediately instead of trying to scan for vulnerable contracts first.

This cannot be emphasized enough. While I understand the desire to fully verify a problem/bug before reporting, doing something is better than nothing. That something could be bringing it to the attention of developers, other security researchers, or a variety of other people in this space. Another option would be to post on the EIP itself.

What types of things could be done to help ensure people like Ralph put the information out there, even if its not a confirmed bug? Brainstorming here....

5chdn commented 5 years ago

Ideas to address this could be some bug bounty program, or at least some "Community call to submit every suspicion of a bug/fork inconsistency/weird behavior" blog post somewhere in a fork preparation process.

Both the Ethereum Foundation and Parity Technologies have a bug bounty program. I'm rather surprised this is not known.

5chdn commented 5 years ago

https://twitter.com/SDLerner/status/1041852437311705088

https://twitter.com/SDLerner/status/1085347016534904832

benjamincburns commented 5 years ago

https://twitter.com/SDLerner/status/1041852437311705088

https://twitter.com/SDLerner/status/1085347016534904832

I'm guessing there's some diffusion of responsibility at play here.

tayvano commented 5 years ago

Another suggestion: a hardfork drinking game bingo card should be made for all upcoming forks. Hudson has an example card template that can be used.