[WIP] EIP-2330 EXTSLOAD

[dominicletz]

This is a draft pull request to reference in EIP-2330: EXTSLOAD opcode

https://eips.ethereum.org/EIPS/eip-2330

This is my first pull on aleth and changing the code was a bliss. Let me know if I did something obvious wrong here. Also I would like to include a test, how do you gus sync this with the remote test repo? Anything I can do there apart from local testing.

Thanks

[dominicletz]

The linux-clang6 test failed while gcc and xcode90 went through. In the clang6 build it falied in a network test:

663/749 Test #668: network/net/evictionWithOldNodeAnswering ..............................................................................***Failed    0.50 sec

For which I don't see how it could be related to my change. Is this test itself unstable?

[gumb0]

Hi @dominicletz, thanks for interest in implementing your proposal in aleth!

Here are some comments:

1. we have currently two versions of the interpreter, you've added your implementation to aleth-interpreter, but I would recommend to implement in LegacyVM, because it's somewhat simpler and allows to activate the EIP without fork definition/EVMC complications.
   It's in https://github.com/ethereum/aleth/blob/master/libevm/LegacyVM.cpp and the code is mostly similar to aleth-interpreter.
2. You didn't define opcode's cost. This should be added to https://github.com/ethereum/aleth/blob/master/libevm/Instruction.cpp
3. The way you defined it it's activated at genesis (so this breaks Frontier and all further fork rules). You should throw "unknown instruction" error if EIP is not activated. See for example how CHAINID does it.
4. It would be best to support activation of this EIP similar to for example Move LegacyVM EIP-1380 implementation from Berlin to individual activation #5809
   Then we could merge it and you would be able to create the state tests for it using testeth.
5. Tests repo is a submodule here at tests/jsontests, new consensus tests should go to subdirs there, but maybe they shouldn't be included in this PR.
6. Some network/net tests are unstable, yes, ignore them for now.

[gumb0]

Suggested change

	- Added: [#XXXX](https://github.com/ethereum/aleth/pull/XXXX) EIP XXXX: EXTSLOAD opcode.
	- Added: [#5805](https://github.com/ethereum/aleth/pull/5805) EIP 2330: EXTSLOAD opcode.

[dominicletz]

Thanks, this is great feedback, I'll make those changes.

Regarding LegacyVM.cpp vs interpreter VM.cpp I would like to understand more about their differences and future. Is there an origin story about them I can read up on?

[gumb0]

Not much of a story - aleth-interpreter is the same code adapted to conform to EVMC API. We keep both for now, because EVMC does not yet support all of the features we need in EVM (like tracing and individual EIP activation)