Skip to content
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.

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

feat: read-only stateful precompiles #20

Merged
merged 8 commits into from
Sep 14, 2024
88 changes: 81 additions & 7 deletions core/vm/contracts.libevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,41 +6,52 @@ import (
"github.com/holiman/uint256"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/libevm"
"github.com/ethereum/go-ethereum/params"
)

// 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. As {Delegate,Static}Call don't
// accept a value, they MUST set the respective field to nil.
// parameters, prepended with the receiver name and appended with additional
// values. As {Delegate,Static}Call don't accept a value, they MUST set the
// respective field to nil.
//
// Instantiation can be achieved by merely copying the parameter names, in
// order, which is trivially achieved with AST manipulation:
//
// func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas uint64, value *uint256.Int) ... {
// ...
// args := &evmCallArgs{evm, caller, addr, input, gas, value}
// ...
// args := &evmCallArgs{evm, caller, addr, input, gas, value, false}
type evmCallArgs struct {
evm *EVM
evm *EVM
// args:start
caller ContractRef
addr common.Address
input []byte
gas uint64
value *uint256.Int
// args:end

// 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. StaticCall() MUST set
// this to true and all other methods MUST set it to false; i.e. the same
// boolean as they each pass to EVMInterpreter.Run().
forceReadOnly bool
}

// run runs the [PrecompiledContract], differentiating between stateful and
// regular types.
func (args *evmCallArgs) run(p PrecompiledContract, input []byte) (ret []byte, err error) {
if p, ok := p.(statefulPrecompile); ok {
return p.run(args.evm.StateDB, &args.evm.chainRules, args.caller.Address(), args.addr, input)
return p.run(args, input)
}
return p.Run(input)
}

// PrecompiledStatefulRun is the stateful equivalent of the Run() method of a
// [PrecompiledContract].
type PrecompiledStatefulRun func(_ StateDB, _ *params.Rules, caller, self common.Address, input []byte) ([]byte, error)
type PrecompiledStatefulRun func(env PrecompileEnvironment, input []byte) ([]byte, error)

// NewStatefulPrecompile constructs a new PrecompiledContract that can be used
// via an [EVM] instance but MUST NOT be called directly; a direct call to Run()
Expand Down Expand Up @@ -69,6 +80,69 @@ func (p statefulPrecompile) Run([]byte) ([]byte, error) {
panic(fmt.Sprintf("BUG: call to %T.Run(); MUST call %T", p, p.run))
}

// A PrecompileEnvironment provides information about the context in which a
// precompiled contract is being run.
type PrecompileEnvironment interface {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: in general is it preferable to define interfaces like this (no param getters) or to have a struct like:

Suggested change
type PrecompileEnvironment interface {
type PrecompileEnvironment struct {
Rules params.Rules
ReadOnly bool
StateDB StateDB
ReadOnlyState libevm.StateReader
Addresses *libevm.AddressContext
}

Copy link
Collaborator Author

@ARR4N ARR4N Sep 14, 2024

Choose a reason for hiding this comment

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

Leaving this unresolved to reply to it later because it's a nuanced answer.

Rules() params.Rules
ReadOnly() bool
// StateDB will be non-nil i.f.f !ReadOnly().
StateDB() StateDB
// ReadOnlyState will always be non-nil.
ReadOnlyState() libevm.StateReader
Addresses() *libevm.AddressContext
}

//
// ****** SECURITY ******
//
// If you are updating PrecompileEnvironment to provide the ability to call back
// into another contract, you MUST revisit the evmCallArgs.forceReadOnly flag.
//
// It is possible that forceReadOnly is true but evm.interpreter.readOnly is
// false. This is safe for now, but may not be if recursive calling *from* a
// precompile is enabled.
//
// ****** SECURITY ******

var _ PrecompileEnvironment = (*evmCallArgs)(nil)

func (args *evmCallArgs) Rules() params.Rules { return args.evm.chainRules }

func (args *evmCallArgs) ReadOnly() bool {
// Not using `return a || b` as this verbose pattern allows better
// inspection of code coverage.
switch {
case args.evm.interpreter.readOnly: // already in a read-only context
return true
case args.forceReadOnly: // precompile called via StaticCall
return true
default:
return false
}
}

func (args *evmCallArgs) StateDB() StateDB {
if args.ReadOnly() {
return nil
}
return args.evm.StateDB
}

func (args *evmCallArgs) ReadOnlyState() libevm.StateReader {
// Even though we're actually returning a full state database, the user
// would have to actively circumvent the returned interface to use it. At
// that point they're off-piste and it's not our problem.
return args.evm.StateDB
}

func (args *evmCallArgs) Addresses() *libevm.AddressContext {
return &libevm.AddressContext{
Origin: args.evm.TxContext.Origin,
Caller: args.caller.Address(),
Self: 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
194 changes: 182 additions & 12 deletions core/vm/contracts.libevm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package vm_test

import (
"fmt"
"math/big"
"testing"

"github.com/holiman/uint256"
Expand Down Expand Up @@ -86,18 +87,25 @@ func TestNewStatefulPrecompile(t *testing.T) {
const gasLimit = 1e6
gasCost := rng.Uint64n(gasLimit)

makeOutput := func(caller, self common.Address, input []byte, stateVal common.Hash) []byte {
makeOutput := func(caller, self common.Address, input []byte, stateVal common.Hash, readOnly bool) []byte {
return []byte(fmt.Sprintf(
"Caller: %v Precompile: %v State: %v Input: %#x",
caller, self, stateVal, input,
"Caller: %v Precompile: %v State: %v Read-only: %t, Input: %#x",
caller, self, stateVal, readOnly, input,
))
}
run := func(env vm.PrecompileEnvironment, input []byte) ([]byte, error) {
if got, want := env.StateDB() != nil, !env.ReadOnly(); got != want {
return nil, fmt.Errorf("PrecompileEnvironment().StateDB() must be non-nil i.f.f. not read-only; got non-nil? %t; want %t", got, want)
}

addrs := env.Addresses()
val := env.ReadOnlyState().GetState(precompile, slot)
return makeOutput(addrs.Caller, addrs.Self, input, val, env.ReadOnly()), nil
}
hooks := &hookstest.Stub{
PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{
precompile: vm.NewStatefulPrecompile(
func(state vm.StateDB, _ *params.Rules, caller, self common.Address, input []byte) ([]byte, error) {
return makeOutput(caller, self, input, state.GetState(precompile, slot)), nil
},
run,
func(b []byte) uint64 {
return gasCost
},
Expand All @@ -112,13 +120,175 @@ func TestNewStatefulPrecompile(t *testing.T) {

state, evm := ethtest.NewZeroEVM(t)
state.SetState(precompile, slot, value)
wantReturnData := makeOutput(caller, precompile, input, value)
wantGasLeft := gasLimit - gasCost

gotReturnData, gotGasLeft, err := evm.Call(vm.AccountRef(caller), precompile, input, gasLimit, uint256.NewInt(0))
require.NoError(t, err)
assert.Equal(t, wantReturnData, gotReturnData)
assert.Equal(t, wantGasLeft, gotGasLeft)
tests := []struct {
name string
call func() ([]byte, uint64, error)
wantReadOnly bool
}{
{
name: "EVM.Call()",
call: func() ([]byte, uint64, error) {
return evm.Call(vm.AccountRef(caller), precompile, input, gasLimit, uint256.NewInt(0))
},
wantReadOnly: false,
},
{
name: "EVM.CallCode()",
call: func() ([]byte, uint64, error) {
return evm.CallCode(vm.AccountRef(caller), precompile, input, gasLimit, uint256.NewInt(0))
},
wantReadOnly: false,
},
{
name: "EVM.DelegateCall()",
call: func() ([]byte, uint64, error) {
return evm.DelegateCall(vm.AccountRef(caller), precompile, input, gasLimit)
},
wantReadOnly: false,
},
{
name: "EVM.StaticCall()",
call: func() ([]byte, uint64, error) {
return evm.StaticCall(vm.AccountRef(caller), precompile, input, gasLimit)
},
wantReadOnly: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
wantReturnData := makeOutput(caller, precompile, input, value, tt.wantReadOnly)
wantGasLeft := gasLimit - gasCost

gotReturnData, gotGasLeft, err := tt.call()
require.NoError(t, err)
assert.Equal(t, string(wantReturnData), string(gotReturnData))
assert.Equal(t, wantGasLeft, gotGasLeft)
})
}
}

func TestPrecompileFromReadOnlyState(t *testing.T) {
// The regular test of stateful precompiles only checks the read-only state
// when called directly via vm.EVM.*Call*() methods. This will hit the
// [evmCallArgs.forceReadOnly] branch of [PrecompileEnvironment.ReadOnly],
// but not the [EVMInterpreter.readOnly] branch. The latter occurs when we
// are already in a read-only environment and there is a non-static call to
// a precompile.
//
// Test strategy:
//
// 1. Create a precompile that reflects its read-only status in the return
// data. We MUST NOT assert inside the precompile as we need proof that
// the precompile was actually called.
//
// 2. Create a bytecode contract that calls the precompile with CALL and
// propagates the return data. Using CALL (i.e. not STATICCALL) means
// that we know for certain that evmCallArgs.forceReadOnly isn't being
// set to true and, instead, the read-only state is being read from
// evm.interpreter.readOnly.
//
// 3. Assert that the returned input is as expected for the read-only state.

// (1)

var precompile common.Address
const precompileAddr = 255
precompile[common.AddressLength-1] = precompileAddr

const (
ifReadOnly = iota + 1 // see contract bytecode for rationale
ifNotReadOnly
)
hooks := &hookstest.Stub{
PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{
precompile: vm.NewStatefulPrecompile(
func(env vm.PrecompileEnvironment, input []byte) ([]byte, error) {
if env.ReadOnly() {
return []byte{ifReadOnly}, nil
}
return []byte{ifNotReadOnly}, nil
},
func([]byte) uint64 { return 0 },
),
},
}
hookstest.Register(t, params.Extras[*hookstest.Stub, *hookstest.Stub]{
NewRules: func(_ *params.ChainConfig, r *params.Rules, _ *hookstest.Stub, blockNum *big.Int, isMerge bool, timestamp uint64) *hookstest.Stub {
r.IsCancun = true // enable PUSH0
return hooks
},
})

// (2)

// See CALL signature: https://www.evm.codes/#f1?fork=cancun
const p0 = vm.PUSH0
contract := []vm.OpCode{
vm.PUSH1, 1, // retSize (bytes)
p0, // retOffset
p0, // argSize
p0, // argOffset
p0, // value
vm.PUSH1, precompileAddr,
p0, // gas
vm.CALL,
// It's ok to ignore the return status. If the CALL failed then we'll
// return []byte{0} next, and both non-failure return buffers are
// non-zero because of the `iota + 1`.
vm.PUSH1, 1, // size (byte)
p0,
vm.RETURN,
}

state, evm := ethtest.NewZeroEVM(t)
rng := ethtest.NewPseudoRand(42)
contractAddr := rng.Address()
state.CreateAccount(contractAddr)
state.SetCode(contractAddr, contractCode(contract))

// (3)

caller := vm.AccountRef(rng.Address())
tests := []struct {
name string
call func() ([]byte, uint64, error)
want byte
}{
{
name: "EVM.Call()",
call: func() ([]byte, uint64, error) {
return evm.Call(caller, contractAddr, []byte{}, 1e6, uint256.NewInt(0))
},
want: ifNotReadOnly,
},
{
name: "EVM.StaticCall()",
call: func() ([]byte, uint64, error) {
return evm.StaticCall(vm.AccountRef(rng.Address()), contractAddr, []byte{}, 1e6)
},
want: ifReadOnly,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, _, err := tt.call()
require.NoError(t, err)
require.Equalf(t, []byte{tt.want}, got, "want %d if read-only, otherwise %d", ifReadOnly, ifNotReadOnly)
})
}
}

// contractCode converts a slice of op codes into a byte buffer for storage as
// contract code.
func contractCode(ops []vm.OpCode) []byte {
ret := make([]byte, len(ops))
for i, o := range ops {
ret[i] = byte(o)
}
return ret
}

func TestCanCreateContract(t *testing.T) {
Expand Down
8 changes: 4 additions & 4 deletions core/vm/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas
}

if isPrecompile {
args := &evmCallArgs{evm, caller, addr, input, gas, value}
args := &evmCallArgs{evm, caller, addr, input, gas, value, false /*forceReadOnly*/}
ret, gas, err = args.RunPrecompiledContract(p, input, gas)
} else {
// Initialise a new contract and set the code that is to be used by the EVM.
Expand Down Expand Up @@ -292,7 +292,7 @@ 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, caller, addr, input, gas, value}
args := &evmCallArgs{evm, caller, addr, input, gas, value, false /*forceReadOnly*/}
ret, gas, err = args.RunPrecompiledContract(p, input, gas)
} else {
addrCopy := addr
Expand Down Expand Up @@ -338,7 +338,7 @@ 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, caller, addr, input, gas, nil}
args := &evmCallArgs{evm, caller, addr, input, gas, nil, false /*forceReadOnly*/}
ret, gas, err = args.RunPrecompiledContract(p, input, gas)
} else {
addrCopy := addr
Expand Down Expand Up @@ -388,7 +388,7 @@ func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte
}

if p, isPrecompile := evm.precompile(addr); isPrecompile {
args := &evmCallArgs{evm, caller, addr, input, gas, nil}
args := &evmCallArgs{evm, caller, addr, input, gas, nil, true /*forceReadOnly*/}
ARR4N marked this conversation as resolved.
Show resolved Hide resolved
ret, gas, err = args.RunPrecompiledContract(p, input, gas)
} else {
// At this point, we use a copy of address. If we don't, the go compiler will
Expand Down