remove evmCallArgs and replace with env #213
Conversation
| addrCopy := addr | ||
| env := &environment{ | ||
| evm: evm, | ||
| callType: Call, | ||
| rawSelf: addrCopy, | ||
| rawCaller: caller.Address(), | ||
| gasRemaining: gas, | ||
| self: NewContract(caller, AccountRef(addrCopy), value, gas), | ||
| readOnlyArg: false, | ||
| } | ||
| ret, gas, err = env.RunPrecompiledContract(p, input, gas) |
There was a problem hiding this comment.
Extensive changes like this are exactly what we've avoided in libevm, not just for their length but for their complexity in fixing during geth syncs (which happens to introduce significant refactors to this file, ahead of where we are). While there are benefits in having the code that replicates behaviour near said behaviour (like you've done), there's a trade-off with maintainability. What we really care about is equivalence and the only way to enforce that is with tests (which we have, and can always extend if necessary).
While directly populating environment like this will pre-compute all of the variables, it places the burden on the sync PR when there is already enough complexity there. Whoever has to update contracts.go next will have to consider the exact rules of the 4 call types (as will their reviewer) instead of just mechanically propagating arguments to a place where their treatment is outlined, unambiguously, as code.
| // func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte, gas uint64) ... { | ||
| // ... | ||
| // args := &evmCallArgs{evm, staticCall, caller, addr, input, gas, nil /*value*/} | ||
| type evmCallArgs struct { |
There was a problem hiding this comment.
I'd like to keep the environment, which may change in implementation, separate from where it connects with the upstream code. The interconnect between the pure geth and the libevm worlds is the riskiest point and this intermediate type simplifies it.
Why this should be merged
How this works
How this was tested