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
96 changes: 89 additions & 7 deletions core/vm/contracts.libevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,41 +6,60 @@ 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 forceReadOnly and all other methods MUST set it to
// inheritReadOnly; i.e. equivalent to the boolean they each pass to
// EVMInterpreter.Run().
readWrite rwInheritance
}

type rwInheritance uint8

const (
inheritReadOnly rwInheritance = iota + 1
forceReadOnly
)

// 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 +88,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 {
if args.readWrite == inheritReadOnly {
if args.evm.interpreter.readOnly { //nolint:gosimple // Clearer code coverage for difficult-to-test branch
return true
}
return false
}
// Even though args.readWrite may be some value other than forceReadOnly,
// that would be an invalid use of the API so we default to read-only as the
// safest failure mode.
return true
}

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)
// Note that this only covers evm.readWrite being set to forceReadOnly,
// via StaticCall(). See TestInheritReadOnly for alternate case.
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 TestInheritReadOnly(t *testing.T) {
// The regular test of stateful precompiles only checks the read-only state
// when called directly via vm.EVM.*Call*() methods. That approach will not
// result in a read-only state via inheritance, which occurs when already in
// a read-only environment there is a non-static call to a precompile.
//
// Test strategy:
//
// 1. Create a precompile that echoes 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 [forceReadOnly] isn't being used 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, inheritReadOnly}
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, inheritReadOnly}
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, inheritReadOnly}
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, forceReadOnly}
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