Hacker News new | ask | show | jobs
by fjl 1898 days ago
The fix is in https://github.com/openethereum/openethereum/pull/364

Actually, it's this commit of the PR: https://github.com/openethereum/openethereum/pull/364/commit...

2 comments

Looks like some really subtle and (as far as I can tell) undocumented aspect of Ethereum internals tripped them up.
They didn't properly implement EIP-2929 (https://eips.ethereum.org/EIPS/eip-2929). I looked at the first section of the code diff in the PR and it matched up with stuff already documented in the EIP, so I wouldn't assume the issue was because of undocumented stuff. Multiple other Ethereum implementations managed to implement the EIP-2929 spec correctly.
386 files changed? This has to include more than just the fix, right?
Looks like they've extracted it to a separate hotfix PR here:

https://github.com/openethereum/openethereum/pull/366

Shouldn't at least a unit test be added before accepting a pull request?

We're talking about 15% of a 300 billion dollar currency.

I have written tests for smaller things than that.

If it's true that Ethereum blocks have any kind of checksum of the expected results, then if the patch has a mistake, I'd think the failure mode would just be that they continue to fail processing the failed block, or they get further but fail to process a more recent block. In that case, then I think it would make sense here to rush out a patch that's been manually checked, and then go back to add some tests.
The right way to fix this is to have an integration test that passes on all other implementations but fails on OpenEthereum, and push this fix afterwards.

People are storing and trading billions of dollars on Ethereum, so I think both automated and manual checking is important. Also an integration test for all branches should be able to find these errors.