Skip to content

remove evmCallArgs and replace with env #213

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

remove evmCallArgs and replace with env #213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7dbf994
refactor: set `EVM.readOnly` and `depth` before running stateful prec…
ARR4N Aug 5, 2025
f8824ab
Merge branch 'main' into arr4n/increment-call-depth-early
ARR4N Aug 7, 2025
b0f91b4
refactor: abstract `CallType.readOnly`
ARR4N Aug 7, 2025
141237d
remove evmCallArgs and replace with env
ceyonur Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/vm/contracts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,15 +168,15 @@ func activePrecompiles(rules params.Rules) []common.Address {
// - the returned bytes,
// - the _remaining_ gas,
// - any error that occurred
func (args *evmCallArgs) RunPrecompiledContract(p PrecompiledContract, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) {
func (env *environment) RunPrecompiledContract(p PrecompiledContract, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) {
gasCost := p.RequiredGas(input)
if suppliedGas < gasCost {
return nil, 0, ErrOutOfGas
}
suppliedGas -= gasCost
args.gasRemaining = suppliedGas
output, err := args.run(p, input)
return output, args.gasRemaining, err
env.gasRemaining = suppliedGas
output, err := env.run(p, input)
return output, env.GasRemaining(), err
}

// ECRECOVER implemented as a native contract.
Expand Down
94 changes: 26 additions & 68 deletions core/vm/contracts.libevm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,31 +57,6 @@ func ActivePrecompiles(rules params.Rules) []common.Address {
return active
}

// evmCallArgs mirrors the parameters of the [EVM] methods Call(), CallCode(),
// DelegateCall() and StaticCall(). Its fields are identical to those of the
// parameters, prepended with the receiver name and call type. As
// {Delegate,Static}Call don't accept a value, they MAY set the respective field
// to nil as it will be ignored.
//
// Instantiation can be achieved by merely copying the parameter names, in
// order, which is trivially achieved with AST manipulation:
//
// 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 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

evm *EVM
callType CallType

// args:start
caller ContractRef
addr common.Address
input []byte
gasRemaining uint64
value *uint256.Int
// args:end
}

// A CallType refers to a *CALL* [OpCode] / respective method on [EVM].
type CallType OpCode

Expand All @@ -101,6 +76,12 @@ func (t CallType) isValid() bool {
}
}

// readOnly returns whether the CallType induces a read-only state if not
// already in one.
func (t CallType) readOnly() bool {
return t == StaticCall
}

// String returns a human-readable representation of the CallType.
func (t CallType) String() string {
if t.isValid() {
Expand All @@ -119,16 +100,28 @@ func (t CallType) OpCode() OpCode {

// run runs the [PrecompiledContract], differentiating between stateful and
// regular types, updating `args.gasRemaining` in the stateful case.
func (args *evmCallArgs) run(p PrecompiledContract, input []byte) (ret []byte, err error) {
switch p := p.(type) {
default:
func (env *environment) run(p PrecompiledContract, input []byte) (ret []byte, err error) {
sp, ok := p.(statefulPrecompile)
if !ok {
return p.Run(input)
case statefulPrecompile:
env := args.env()
ret, err := p(env, input)
args.gasRemaining = env.Gas()
return ret, err
}

// Depth and read-only setting are handled by [EVMInterpreter.Run],
// which isn't used for precompiles, so we need to do it ourselves to
// maintain the expected invariants.
in := env.evm.interpreter

in.evm.depth++
defer func() { in.evm.depth-- }()

if env.readOnlyArg && !in.readOnly {
in.readOnly = true
defer func() { in.readOnly = false }()
}

ret, err = sp(env, input)
env.gasRemaining = env.Gas()
return ret, err
}

// PrecompiledStatefulContract is the stateful equivalent of a
Expand Down Expand Up @@ -196,41 +189,6 @@ type PrecompileEnvironment interface {
Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, _ ...CallOption) (ret []byte, _ error)
}

func (args *evmCallArgs) env() *environment {
var (
self common.Address
value = args.value
)
switch args.callType {
case StaticCall:
value = new(uint256.Int)
fallthrough
case Call:
self = args.addr

case DelegateCall:
value = nil
fallthrough
case CallCode:
self = args.caller.Address()
}

// This is equivalent to the `contract` variables created by evm.*Call*()
// methods, for non precompiles, to pass to [EVMInterpreter.Run].
contract := NewContract(args.caller, AccountRef(self), value, args.gasRemaining)
if args.callType == DelegateCall {
contract = contract.AsDelegate()
}

return &environment{
evm: args.evm,
self: contract,
callType: args.callType,
rawCaller: args.caller.Address(),
rawSelf: args.addr,
}
}

var (
// These lock in the assumptions made when implementing [evmCallArgs]. If
// these break then the struct fields SHOULD be changed to match these
Expand Down
38 changes: 8 additions & 30 deletions core/vm/environment.libevm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,19 @@ import (
var _ PrecompileEnvironment = (*environment)(nil)

type environment struct {
evm *EVM
self *Contract
callType CallType
evm *EVM
self *Contract
callType CallType
readOnlyArg bool // readOnlyArg is different than interpreter.readOnly
gasRemaining uint64

rawSelf, rawCaller common.Address
}

func (e *environment) Gas() uint64 { return e.self.Gas }
func (e *environment) UseGas(gas uint64) bool { return e.self.UseGas(gas) }
func (e *environment) Value() *uint256.Int { return new(uint256.Int).Set(e.self.Value()) }
func (e *environment) GasRemaining() uint64 { return e.gasRemaining }

func (e *environment) ChainConfig() *params.ChainConfig { return e.evm.chainConfig }
func (e *environment) Rules() params.Rules { return e.evm.chainRules }
Expand All @@ -63,19 +66,7 @@ func (e *environment) refundGas(add uint64) error {
}

func (e *environment) ReadOnly() bool {
// A switch statement provides clearer code coverage for difficult-to-test
// cases.
switch {
case e.callType == StaticCall:
// evm.interpreter.readOnly is only set to true via a call to
// EVMInterpreter.Run() so, if a precompile is called directly with
// StaticCall(), then readOnly might not be set yet.
return true
case e.evm.interpreter.readOnly:
return true
default:
return false
}
return e.evm.interpreter.readOnly
}

func (e *environment) Addresses() *libevm.AddressContext {
Expand Down Expand Up @@ -116,19 +107,6 @@ func (e *environment) Call(addr common.Address, input []byte, gas uint64, value
}

func (e *environment) callContract(typ CallType, addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) (retData []byte, retErr error) {
// Depth and read-only setting are handled by [EVMInterpreter.Run], which
// isn't used for precompiles, so we need to do it ourselves to maintain the
// expected invariants.
in := e.evm.interpreter

in.evm.depth++
defer func() { in.evm.depth-- }()

if e.ReadOnly() && !in.readOnly { // i.e. the precompile was StaticCall()ed
in.readOnly = true
defer func() { in.readOnly = false }()
}

var caller ContractRef = e.self
if options.As[callConfig](opts...).unsafeCallerAddressProxying {
// Note that, in addition to being unsafe, this breaks an EVM
Expand All @@ -141,7 +119,7 @@ func (e *environment) callContract(typ CallType, addr common.Address, input []by
}
}

if in.readOnly && value != nil && !value.IsZero() {
if e.ReadOnly() && value != nil && !value.IsZero() {
return nil, ErrWriteProtection
}
if !e.UseGas(gas) {
Expand Down
57 changes: 46 additions & 11 deletions core/vm/evm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,8 +235,17 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas
}

if isPrecompile {
args := &evmCallArgs{evm, Call, caller, addr, input, gas, value}
ret, gas, err = args.RunPrecompiledContract(p, input, gas)
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)
Comment on lines +238 to +248
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

} else {
// Initialise a new contract and set the code that is to be used by the EVM.
// The contract is a scoped environment for this execution context only.
Expand Down Expand Up @@ -287,7 +296,7 @@ func (evm *EVM) CallCode(caller ContractRef, addr common.Address, input []byte,
if !evm.Context.CanTransfer(evm.StateDB, caller.Address(), value) {
return nil, gas, ErrInsufficientBalance
}
var snapshot = evm.StateDB.Snapshot()
snapshot := evm.StateDB.Snapshot()

// Invoke tracer hooks that signal entering/exiting a call frame
if evm.Config.Tracer != nil {
Expand All @@ -299,8 +308,17 @@ func (evm *EVM) CallCode(caller ContractRef, addr common.Address, input []byte,

// It is allowed to call precompiles, even via delegatecall
if p, isPrecompile := evm.precompile(addr); isPrecompile {
args := &evmCallArgs{evm, CallCode, caller, addr, input, gas, value}
ret, gas, err = args.RunPrecompiledContract(p, input, gas)
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)
} else {
addrCopy := addr
// Initialise a new contract and set the code that is to be used by the EVM.
Expand Down Expand Up @@ -329,7 +347,7 @@ func (evm *EVM) DelegateCall(caller ContractRef, addr common.Address, input []by
if evm.depth > int(params.CallCreateDepth) {
return nil, gas, ErrDepth
}
var snapshot = evm.StateDB.Snapshot()
snapshot := evm.StateDB.Snapshot()

// Invoke tracer hooks that signal entering/exiting a call frame
if evm.Config.Tracer != nil {
Expand All @@ -345,8 +363,16 @@ func (evm *EVM) DelegateCall(caller ContractRef, addr common.Address, input []by

// It is allowed to call precompiles, even via delegatecall
if p, isPrecompile := evm.precompile(addr); isPrecompile {
args := &evmCallArgs{evm, DelegateCall, caller, addr, input, gas, nil}
ret, gas, err = args.RunPrecompiledContract(p, input, gas)
env := &environment{
evm: evm,
callType: DelegateCall,
rawSelf: addr,
rawCaller: caller.Address(),
gasRemaining: gas,
self: NewContract(caller, AccountRef(caller.Address()), nil, gas).AsDelegate(),
readOnlyArg: false,
}
ret, gas, err = env.RunPrecompiledContract(p, input, gas)
} else {
addrCopy := addr
// Initialise a new contract and make initialise the delegate values
Expand Down Expand Up @@ -378,7 +404,7 @@ func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte
// after all empty accounts were deleted, so this is not required. However, if we omit this,
// then certain tests start failing; stRevertTest/RevertPrecompiledTouchExactOOG.json.
// We could change this, but for now it's left for legacy reasons
var snapshot = evm.StateDB.Snapshot()
snapshot := evm.StateDB.Snapshot()

// We do an AddBalance of zero here, just in order to trigger a touch.
// This doesn't matter on Mainnet, where all empties are gone at the time of Byzantium,
Expand All @@ -395,8 +421,17 @@ func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte
}

if p, isPrecompile := evm.precompile(addr); isPrecompile {
args := &evmCallArgs{evm, StaticCall, caller, addr, input, gas, nil}
ret, gas, err = args.RunPrecompiledContract(p, input, gas)
addrCopy := addr
env := &environment{
evm: evm,
callType: StaticCall,
rawSelf: addrCopy,
rawCaller: caller.Address(),
gasRemaining: gas,
self: NewContract(caller, AccountRef(addrCopy), new(uint256.Int), gas),
readOnlyArg: true,
}
ret, gas, err = env.RunPrecompiledContract(p, input, gas)
} else {
// At this point, we use a copy of address. If we don't, the go compiler will
// leak the 'contract' to the outer scope, and make allocation for 'contract'
Expand Down