feat: read-only stateful precompiles (#20)

ARR4N · web-flow · commit 58f38836c0b1 · 2024-09-14T10:45:18.000+01:00
* feat: read-only argument to stateful precompiles

* refactor: group `PrecompiledStatefulRun()` args except input

* test: read-only status of all `EVM.*Call*()` methods

* feat: `PrecompileEnvironment.ReadOnlyStateDB()`

* doc: warning about allowing recursive calling

* test: precompile call from within read-only env

* refactor: introduce `{inherit,force}ReadOnly` consts

* fix: `nolint` verbose if statement
diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
@@ -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()
@@ -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 {
+	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
diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go
@@ -2,6 +2,7 @@ package vm_test
 
 import (
 	"fmt"
+	"math/big"
 	"testing"
 
 	"github.com/holiman/uint256"
@@ -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
 				},
@@ -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) {
diff --git a/core/vm/evm.go b/core/vm/evm.go
@@ -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.
@@ -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
@@ -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
@@ -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
