refactor!: fine-grained mutability instead of boolean ReadOnly()

Author: ARR4N

core/vm/contracts.libevm.go

@@ -88,6 +88,32 @@ func (t CallType) OpCode() OpCode {
 	return INVALID
 }
 
+// StateMutability describes the available state access.
+type StateMutability uint
+
+const (
+	unknownStateMutability StateMutability = iota
+	MutableState
+	// ReadOnlyState is equivalent to Solidity's "view".
+	ReadOnlyState
+	// Pure is a Solidity concept disallowing all access, read or write, to
+	// state.
+	Pure
+)
+
+// String returns a human-readable representation of the StateMutability.
+func (m StateMutability) String() string {
+	switch m {
+	case MutableState:
+		return "mutable"
+	case ReadOnlyState:
+		return "read-only"
+	case Pure:
+		return "no state access"
+	}
+	return fmt.Sprintf("unknown %T(%[1]d)", m)
+}
+
 // 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) {
@@ -141,21 +167,24 @@ func (p statefulPrecompile) Run([]byte) ([]byte, error) {
 type PrecompileEnvironment interface {
 	ChainConfig() *params.ChainConfig
 	Rules() params.Rules
-	// StateDB will be non-nil i.f.f !ReadOnly().
+	// StateDB will be non-nil i.f.f StateMutability() returns [MutableState].
 	StateDB() StateDB
-	// ReadOnlyState will be non-nil i.f.f. SetPure() has not been called.
+	// ReadOnlyState will be non-nil i.f.f. StateMutability() does not return
+	// [Pure].
 	ReadOnlyState() libevm.StateReader
 
-	// SetReadOnly ensures that all future calls to ReadOnly() will return true.
-	// Is can be used as a guard against accidental writes when a read-only
-	// function is invoked with EVM call() instead of staticcall().
-	SetReadOnly()
-	// SetPure ensures that all future calls to ReadOnly() will return true and
-	// that calls to ReadOnlyState() will return nil.
-	// TODO(arr4n) DO NOT MERGE: change this and SetReadOnly to return new
-	// environments.
-	SetPure()
-	ReadOnly() bool
+	// StateMutability can infer [MutableState] vs [ReadOnlyState] based on EVM
+	// context, but [Pure] is a Solidity concept that is enforced by user code.
+	StateMutability() StateMutability
+	// AsReadOnly returns a copy of the current environment for which
+	// StateMutability() returns [ReadOnlyState]. It can be used as a guard
+	// against accidental writes when a read-only function is invoked with EVM
+	// call() instead of staticcall().
+	AsReadOnly() PrecompileEnvironment
+	// AsPure returns a copy of the current environment that has no access to
+	// state; i.e. StateMutability() returns [Pure]. All calls to both StateDB()
+	// and ReadOnlyState() will return nil.
+	AsPure() PrecompileEnvironment
 
 	IncomingCallType() CallType
 	Addresses() *libevm.AddressContext

core/vm/contracts.libevm_test.go

@@ -108,7 +108,7 @@ type statefulPrecompileOutput struct {
 	Addresses               *libevm.AddressContext
 	StateValue              common.Hash
 	ValueReceived           *uint256.Int
-	ReadOnly                bool
+	Mutability              vm.StateMutability
 	BlockNumber, Difficulty *big.Int
 	BlockTime               uint64
 	Input                   []byte
@@ -149,8 +149,8 @@ func TestNewStatefulPrecompile(t *testing.T) {
 	gasCost := rng.Uint64n(gasLimit)
 
 	run := func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) ([]byte, uint64, error) {
-		if got, want := env.StateDB() != nil, !env.ReadOnly(); got != want {
-			return nil, 0, fmt.Errorf("PrecompileEnvironment().StateDB() must be non-nil i.f.f. not read-only; got non-nil? %t; want %t", got, want)
+		if got, want := env.StateDB() != nil, env.StateMutability() == vm.MutableState; got != want {
+			return nil, 0, fmt.Errorf("PrecompileEnvironment().StateDB() must be non-nil i.f.f. state is mutable; got non-nil? %t; want %t", got, want)
 		}
 		hdr, err := env.BlockHeader()
 		if err != nil {
@@ -162,7 +162,7 @@ func TestNewStatefulPrecompile(t *testing.T) {
 			Addresses:        env.Addresses(),
 			StateValue:       env.ReadOnlyState().GetState(precompile, slot),
 			ValueReceived:    env.Value(),
-			ReadOnly:         env.ReadOnly(),
+			Mutability:       env.StateMutability(),
 			BlockNumber:      env.BlockNumber(),
 			BlockTime:        env.BlockTime(),
 			Difficulty:       hdr.Difficulty,
@@ -216,8 +216,8 @@ func TestNewStatefulPrecompile(t *testing.T) {
 		wantTransferValue *uint256.Int
 		// Note that this only covers evm.readOnly being true because of the
 		// precompile's call. See TestInheritReadOnly for alternate case.
-		wantReadOnly bool
-		wantCallType vm.CallType
+		wantMutability vm.StateMutability
+		wantCallType   vm.CallType
 	}{
 		{
 			name: "EVM.Call()",
@@ -229,7 +229,7 @@ func TestNewStatefulPrecompile(t *testing.T) {
 				Caller: caller,
 				Self:   precompile,
 			},
-			wantReadOnly:      false,
+			wantMutability:    vm.MutableState,
 			wantTransferValue: transferValue,
 			wantCallType:      vm.Call,
 		},
@@ -243,7 +243,7 @@ func TestNewStatefulPrecompile(t *testing.T) {
 				Caller: caller,
 				Self:   caller,
 			},
-			wantReadOnly:      false,
+			wantMutability:    vm.MutableState,
 			wantTransferValue: transferValue,
 			wantCallType:      vm.CallCode,
 		},
@@ -257,7 +257,7 @@ func TestNewStatefulPrecompile(t *testing.T) {
 				Caller: eoa, // inherited from caller
 				Self:   caller,
 			},
-			wantReadOnly:      false,
+			wantMutability:    vm.MutableState,
 			wantTransferValue: uint256.NewInt(0),
 			wantCallType:      vm.DelegateCall,
 		},
@@ -271,7 +271,7 @@ func TestNewStatefulPrecompile(t *testing.T) {
 				Caller: caller,
 				Self:   precompile,
 			},
-			wantReadOnly:      true,
+			wantMutability:    vm.ReadOnlyState,
 			wantTransferValue: uint256.NewInt(0),
 			wantCallType:      vm.StaticCall,
 		},
@@ -284,7 +284,7 @@ func TestNewStatefulPrecompile(t *testing.T) {
 				Addresses:        tt.wantAddresses,
 				StateValue:       stateValue,
 				ValueReceived:    tt.wantTransferValue,
-				ReadOnly:         tt.wantReadOnly,
+				Mutability:       tt.wantMutability,
 				BlockNumber:      header.Number,
 				BlockTime:        header.Time,
 				Difficulty:       header.Difficulty,
@@ -334,7 +334,7 @@ func TestInheritReadOnly(t *testing.T) {
 		PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{
 			precompile: vm.NewStatefulPrecompile(
 				func(env vm.PrecompileEnvironment, input []byte) ([]byte, error) {
-					if env.ReadOnly() {
+					if env.StateMutability() != vm.MutableState {
 						return []byte{ifReadOnly}, nil
 					}
 					return []byte{ifNotReadOnly}, nil
@@ -560,9 +560,9 @@ func TestPrecompileMakeCall(t *testing.T) {
 			}),
 			dest: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte) (ret []byte, err error) {
 				out := &statefulPrecompileOutput{
-					Addresses: env.Addresses(),
-					ReadOnly:  env.ReadOnly(),
-					Input:     input, // expected to be callData
+					Addresses:  env.Addresses(),
+					Mutability: env.StateMutability(),
+					Input:      input, // expected to be callData
 				}
 				return out.Bytes(), nil
 			}),
@@ -591,7 +591,8 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: sut,
 					Self:   dest,
 				},
-				Input: precompileCallData,
+				Input:      precompileCallData,
+				Mutability: vm.MutableState,
 			},
 		},
 		{
@@ -603,7 +604,8 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: caller, // overridden by CallOption
 					Self:   dest,
 				},
-				Input: precompileCallData,
+				Input:      precompileCallData,
+				Mutability: vm.MutableState,
 			},
 		},
 		{
@@ -614,7 +616,8 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: caller, // SUT runs as its own caller because of CALLCODE
 					Self:   dest,
 				},
-				Input: precompileCallData,
+				Input:      precompileCallData,
+				Mutability: vm.MutableState,
 			},
 		},
 		{
@@ -626,7 +629,8 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: caller, // CallOption is a NOOP
 					Self:   dest,
 				},
-				Input: precompileCallData,
+				Input:      precompileCallData,
+				Mutability: vm.MutableState,
 			},
 		},
 		{
@@ -637,7 +641,8 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: caller, // as with CALLCODE
 					Self:   dest,
 				},
-				Input: precompileCallData,
+				Input:      precompileCallData,
+				Mutability: vm.MutableState,
 			},
 		},
 		{
@@ -649,7 +654,8 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: caller, // CallOption is a NOOP
 					Self:   dest,
 				},
-				Input: precompileCallData,
+				Input:      precompileCallData,
+				Mutability: vm.MutableState,
 			},
 		},
 		{
@@ -665,7 +671,7 @@ func TestPrecompileMakeCall(t *testing.T) {
 				// (non-static) CALL, the read-only state is inherited. Yes,
 				// this is _another_ way to get a read-only state, different to
 				// the other tests.
-				ReadOnly: true,
+				Mutability: vm.ReadOnlyState,
 			},
 		},
 		{
@@ -677,8 +683,8 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: caller, // overridden by CallOption
 					Self:   dest,
 				},
-				Input:    precompileCallData,
-				ReadOnly: true,
+				Input:      precompileCallData,
+				Mutability: vm.ReadOnlyState,
 			},
 		},
 	}

core/vm/environment.libevm.go

@@ -17,6 +17,7 @@
 package vm
 
 import (
+	"errors"
 	"fmt"
 	"math/big"
 
@@ -39,6 +40,12 @@ type environment struct {
 	view, pure bool
 }
 
+func (e *environment) copy() *environment {
+	// This deliberately does not use field names so that a change in the fields
+	// will break this code and force it to be reviewed.
+	return &environment{e.evm, e.self, e.callType, e.view, e.pure}
+}
+
 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()) }
@@ -58,24 +65,35 @@ func (e *environment) refundGas(add uint64) error {
 	return nil
 }
 
-func (e *environment) SetReadOnly() { e.view = true }
-func (e *environment) SetPure()     { e.pure = true }
+func (e *environment) AsReadOnly() PrecompileEnvironment {
+	cp := e.copy()
+	cp.view = true
+	return cp
+}
+
+func (e *environment) AsPure() PrecompileEnvironment {
+	cp := e.copy()
+	cp.pure = true
+	return cp
+}
 
-func (e *environment) ReadOnly() bool {
+func (e *environment) StateMutability() StateMutability {
 	// 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
+		return ReadOnlyState
 	case e.evm.interpreter.readOnly:
-		return true
-	case e.view || e.pure:
-		return true
+		return ReadOnlyState
+	case e.view:
+		return ReadOnlyState
+	case e.pure:
+		return Pure
 	default:
-		return false
+		return MutableState
 	}
 }
 
@@ -87,7 +105,7 @@ func (e *environment) ReadOnlyState() libevm.StateReader {
 }
 
 func (e *environment) StateDB() StateDB {
-	if e.ReadOnly() {
+	if e.StateMutability() != MutableState {
 		return nil
 	}
 	return e.evm.StateDB
@@ -117,7 +135,13 @@ func (e *environment) Call(addr common.Address, input []byte, gas uint64, value
 	return e.callContract(Call, addr, input, gas, value, opts...)
 }
 
+var errPureFunctionMakeCall = errors.New("contract call from pure function")
+
 func (e *environment) callContract(typ CallType, addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) (retData []byte, retErr error) {
+	if e.StateMutability() == Pure {
+		return nil, errPureFunctionMakeCall
+	}
+
 	// 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.
@@ -126,7 +150,7 @@ func (e *environment) callContract(typ CallType, addr common.Address, input []by
 	in.evm.depth++
 	defer func() { in.evm.depth-- }()
 
-	if e.ReadOnly() && !in.readOnly { // i.e. the precompile was StaticCall()ed
+	if e.StateMutability() != MutableState && !in.readOnly { // i.e. the precompile was StaticCall()ed
 		in.readOnly = true
 		defer func() { in.readOnly = false }()
 	}

libevm/precompilegen/precompile.go.tmpl

@@ -78,9 +78,9 @@ func (p precompile) run(env vm.PrecompileEnvironment, input []byte) ([]byte, err
 		}
 	case "payable":
 	case "pure":
-		env.SetPure()
+		env = env.AsPure()
 	case "view":
-		env.SetReadOnly()
+		env = env.AsReadOnly()
 	default:
 		// If this happens then `precompilegen` needs to be extended because the
         // Solidity ABI spec changed.