investigate hive/ethereum/consensus error message Forks can't be assigned out of order

[jangko]

Forks can't be assigned out of order config.nim:330

This error explains why many of Istanbul test cases failing.

TODO: add test cases for --customnetwork parser and probaly rewrite of crappy customnetwork parser.

Both 0 or high(BlockNumber) is not ideal default number for fork block number in customnetwork.

[jlokier]

Can you clarify, is the Istanbul line struck out because it doesn't explain the test cases failing after all?

0 isn't a good value, but I think high(BlockNumber) is an ok value, because it's a sentinel that means "never". An explicit bool could be added, but that would complicate logic elsewhere. That said, perhaps there should only be one function in the system that maps block number to fork, so it shouldn't matter. Anyway, I'd like to do what Geth does, and use EIP feature flags instead of fork number in places that currnetly check fork.

Regard the regression tag: I treat true regressions as high priority if they are new issues, because it's usually the best time to fix them and often indicates other bugs added recently have not been detected. A regression is something that used to work and stopped. Usually it means we can search backward for what caused the issue to stop working. git bisect can be used but I tend to bisect by hand (e.g. I did this for the C stack overflow targets, to find the commit when they stopped working).

Was there a time when this fork-assignment feature worked?

[jangko]

Can you clarify, is the Istanbul line struck out because it doesn't explain the test cases failing after all?

Because of this regression, every fork will fail in graphql/ethereum/consensus test, not only Istanbul. Or to be precise, every single test case is likely never executed because the --customnetwork parser terminate the process.

Was there a time when this fork-assignment feature worked?

68e70eb

[jangko]

Anyway, I'd like to do what Geth does, and use EIP feature flags instead of fork number in places that currnetly check fork.

I agree, the current fork system is confusing and not only the EVM have forks, the block chain itself also have different forks. some of them are overlapping, and it's not good from maintenance perspective.

[jangko]

I'm closing this now. after #645, we can run hive tests again.
about the --customnetwork parser, that will be handled when we deal with #640

[jangko]

turn out, two quick fix is not enough. the custom network parser really need to be fixed thoroughly.