From 9328628ee564d083f0c29bede2ebd9f041959c56 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Thu, 26 Sep 2024 12:22:47 +0100
Subject: [PATCH 01/15] feat: `vm.PrecompiledStatefulContract` can make `CALL`s

---
 core/vm/contracts.libevm.go | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index a9312027703..f46206b65a7 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -122,19 +122,9 @@ type PrecompileEnvironment interface {
 	BlockHeader() (types.Header, error)
 	BlockNumber() *big.Int
 	BlockTime() uint64
-}
 
-//
-// ****** 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 ******
+	Call(common.Address, []byte, uint64, *uint256.Int) ([]byte, uint64, error)
+}
 
 var _ PrecompileEnvironment = (*evmCallArgs)(nil)
 
@@ -168,11 +158,13 @@ func (args *evmCallArgs) ReadOnlyState() libevm.StateReader {
 	return args.evm.StateDB
 }
 
+func (args *evmCallArgs) self() common.Address { return args.addr }
+
 func (args *evmCallArgs) Addresses() *libevm.AddressContext {
 	return &libevm.AddressContext{
 		Origin: args.evm.TxContext.Origin,
 		Caller: args.caller.Address(),
-		Self:   args.addr,
+		Self:   args.self(),
 	}
 }
 
@@ -194,6 +186,24 @@ func (args *evmCallArgs) BlockNumber() *big.Int {
 
 func (args *evmCallArgs) BlockTime() uint64 { return args.evm.Context.Time }
 
+func (args *evmCallArgs) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int) ([]byte, uint64, error) {
+	in := args.evm.interpreter
+
+	// The precompile run didn't increment the depth so this is necessary even
+	// though Call() will eventually result in it being done again.
+	in.evm.depth++
+	defer func() { in.evm.depth-- }()
+
+	// This will happen if the precompile was invoked via StaticCall() from a
+	// non read-only state.
+	if args.readWrite == forceReadOnly && !in.readOnly {
+		in.readOnly = true
+		defer func() { in.readOnly = false }()
+	}
+
+	return args.evm.Call(AccountRef(args.self()), addr, input, gas, value)
+}
+
 var (
 	// These lock in the assumptions made when implementing [evmCallArgs]. If
 	// these break then the struct fields SHOULD be changed to match these

From 2abf1f2bac7eaa17615c92fefaf2a344348f9fd2 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Thu, 26 Sep 2024 13:10:10 +0100
Subject: [PATCH 02/15] fix: caller propagation

---
 core/vm/contracts.libevm.go | 21 ++++++++++++++++++++-
 core/vm/evm.go              |  8 ++++----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index f46206b65a7..6a3cce19f22 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -57,6 +57,11 @@ type evmCallArgs struct {
 	// inheritReadOnly; i.e. equivalent to the boolean they each pass to
 	// EVMInterpreter.Run().
 	readWrite rwInheritance
+
+	// If a precompile issues its own Call() then caller semantics are dependent
+	// on whether the precompile was delegate-called or not. DelegateCall() MUST
+	// set this to delegated and all other methods MUST set it to notDelegated.
+	delegation delegation
 }
 
 type rwInheritance uint8
@@ -66,6 +71,13 @@ const (
 	forceReadOnly
 )
 
+type delegation uint8
+
+const (
+	delegated delegation = iota + 1
+	notDelegated
+)
+
 // run runs the [PrecompiledContract], differentiating between stateful and
 // regular types.
 func (args *evmCallArgs) run(p PrecompiledContract, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) {
@@ -201,7 +213,14 @@ func (args *evmCallArgs) Call(addr common.Address, input []byte, gas uint64, val
 		defer func() { in.readOnly = false }()
 	}
 
-	return args.evm.Call(AccountRef(args.self()), addr, input, gas, value)
+	// This is equivalent to the `contract` variables created by evm.*Call*()
+	// methods to pass to [EVMInterpreter.Run], which are then propagated by the
+	// *CALL* opcodes as the caller.
+	precompile := NewContract(args.caller, AccountRef(args.self()), args.value, args.gas)
+	if args.delegation == delegated {
+		precompile = precompile.AsDelegate()
+	}
+	return args.evm.Call(precompile, addr, input, gas, value)
 }
 
 var (
diff --git a/core/vm/evm.go b/core/vm/evm.go
index 48c1112f18e..872a24077cf 100644
--- a/core/vm/evm.go
+++ b/core/vm/evm.go
@@ -230,7 +230,7 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas
 	}
 
 	if isPrecompile {
-		args := &evmCallArgs{evm, caller, addr, input, gas, value, inheritReadOnly}
+		args := &evmCallArgs{evm, caller, addr, input, gas, value, inheritReadOnly, notDelegated}
 		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.
@@ -294,7 +294,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, inheritReadOnly}
+		args := &evmCallArgs{evm, caller, addr, input, gas, value, inheritReadOnly, notDelegated}
 		ret, gas, err = args.RunPrecompiledContract(p, input, gas)
 	} else {
 		addrCopy := addr
@@ -340,7 +340,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, inheritReadOnly}
+		args := &evmCallArgs{evm, caller, addr, input, gas, nil, inheritReadOnly, delegated}
 		ret, gas, err = args.RunPrecompiledContract(p, input, gas)
 	} else {
 		addrCopy := addr
@@ -390,7 +390,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, forceReadOnly}
+		args := &evmCallArgs{evm, caller, addr, input, gas, nil, forceReadOnly, notDelegated}
 		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

From c63b031cdf2d3d427aebfbcb43bfd8579dd12cae Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Thu, 26 Sep 2024 16:55:38 +0100
Subject: [PATCH 03/15] feat: precompile can override default caller in
 `Call()`

---
 core/vm/contracts.libevm.go | 21 ++++++++++++++++++---
 core/vm/options.libevm.go   | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 core/vm/options.libevm.go

diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index 6a3cce19f22..44ee754d49c 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -135,7 +135,10 @@ type PrecompileEnvironment interface {
 	BlockNumber() *big.Int
 	BlockTime() uint64
 
-	Call(common.Address, []byte, uint64, *uint256.Int) ([]byte, uint64, error)
+	// Call is equivalent to [EVM.Call], with the caller defaulting to the
+	// precompile receiving the environment, or to its own caller if invoked via
+	// a delegated call.
+	Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, _ ...CallOption) (ret []byte, gasRemaining uint64, _ error)
 }
 
 var _ PrecompileEnvironment = (*evmCallArgs)(nil)
@@ -198,7 +201,7 @@ func (args *evmCallArgs) BlockNumber() *big.Int {
 
 func (args *evmCallArgs) BlockTime() uint64 { return args.evm.Context.Time }
 
-func (args *evmCallArgs) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int) ([]byte, uint64, error) {
+func (args *evmCallArgs) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) ([]byte, uint64, error) {
 	in := args.evm.interpreter
 
 	// The precompile run didn't increment the depth so this is necessary even
@@ -220,7 +223,19 @@ func (args *evmCallArgs) Call(addr common.Address, input []byte, gas uint64, val
 	if args.delegation == delegated {
 		precompile = precompile.AsDelegate()
 	}
-	return args.evm.Call(precompile, addr, input, gas, value)
+	var caller ContractRef = precompile
+
+	for _, o := range opts {
+		switch o := o.(type) {
+		case withCallerOpt:
+			caller = o.ContractRef
+		case nil:
+		default:
+			return nil, gas, fmt.Errorf("unsupported option %T", o)
+		}
+	}
+
+	return args.evm.Call(caller, addr, input, gas, value)
 }
 
 var (
diff --git a/core/vm/options.libevm.go b/core/vm/options.libevm.go
new file mode 100644
index 00000000000..4f0337130a3
--- /dev/null
+++ b/core/vm/options.libevm.go
@@ -0,0 +1,33 @@
+// Copyright 2024 the libevm authors.
+//
+// The libevm additions to go-ethereum are free software: you can redistribute
+// them and/or modify them under the terms of the GNU Lesser General Public License
+// as published by the Free Software Foundation, either version 3 of the License,
+// or (at your option) any later version.
+//
+// The libevm additions are distributed in the hope that they will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
+// General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see
+// <http://www.gnu.org/licenses/>.
+
+package vm
+
+// A CallOption modifies the default behaviour of a contract call.
+type CallOption interface {
+	libevmCallOption() // noop to only allow internally defined options
+}
+
+// WithCaller overrides the default caller.
+func WithCaller(c ContractRef) CallOption {
+	return withCallerOpt{c}
+}
+
+type withCallerOpt struct {
+	ContractRef
+}
+
+func (withCallerOpt) libevmCallOption() {}

From 584fa5f1662204a18717526547f846eb2a55b8b2 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Thu, 26 Sep 2024 20:06:50 +0100
Subject: [PATCH 04/15] refactor: `WithUNSAFEForceDelegate()` replaces
 `WithCaller()`

---
 core/vm/contracts.libevm.go | 15 ++++++++-------
 core/vm/options.libevm.go   | 17 ++++++++++-------
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index 44ee754d49c..97281ead7f1 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -220,22 +220,23 @@ func (args *evmCallArgs) Call(addr common.Address, input []byte, gas uint64, val
 	// methods to pass to [EVMInterpreter.Run], which are then propagated by the
 	// *CALL* opcodes as the caller.
 	precompile := NewContract(args.caller, AccountRef(args.self()), args.value, args.gas)
-	if args.delegation == delegated {
-		precompile = precompile.AsDelegate()
-	}
-	var caller ContractRef = precompile
 
+	asDelegate := args.delegation == delegated // precompile was DELEGATECALLed
 	for _, o := range opts {
 		switch o := o.(type) {
-		case withCallerOpt:
-			caller = o.ContractRef
+		case callOptForceDelegate:
+			// See documentation of [WithUNSAFEForceDelegate].
+			asDelegate = true
 		case nil:
 		default:
 			return nil, gas, fmt.Errorf("unsupported option %T", o)
 		}
 	}
+	if asDelegate {
+		precompile = precompile.AsDelegate()
+	}
 
-	return args.evm.Call(caller, addr, input, gas, value)
+	return args.evm.Call(precompile, addr, input, gas, value)
 }
 
 var (
diff --git a/core/vm/options.libevm.go b/core/vm/options.libevm.go
index 4f0337130a3..9dfce763984 100644
--- a/core/vm/options.libevm.go
+++ b/core/vm/options.libevm.go
@@ -21,13 +21,16 @@ type CallOption interface {
 	libevmCallOption() // noop to only allow internally defined options
 }
 
-// WithCaller overrides the default caller.
-func WithCaller(c ContractRef) CallOption {
-	return withCallerOpt{c}
+// WithUNSAFEForceDelegate results in precompiles making contract calls acting
+// as if they themselves were DELEGATECALLed. This is not safe for regular use
+// as the precompile will act as its own caller even when not expected to.
+//
+// Deprecated: this option MUST NOT be used other than to allow migration to
+// libevm when backwards compatibility is required.
+func WithUNSAFEForceDelegate() CallOption {
+	return callOptForceDelegate{}
 }
 
-type withCallerOpt struct {
-	ContractRef
-}
+type callOptForceDelegate struct{}
 
-func (withCallerOpt) libevmCallOption() {}
+func (callOptForceDelegate) libevmCallOption() {}

From d487b268bbb2b92488a7a37ff5482ce686d51845 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Fri, 27 Sep 2024 19:07:20 +0100
Subject: [PATCH 05/15] refactor: `WithUNSAFECallerAddressProxying()` instead
 of `ForceDelegate`

This matches the pattern used by `ava-labs/coreth` `NativeAssetCall`.
---
 core/vm/contracts.libevm.go | 17 +++++++++--------
 core/vm/options.libevm.go   | 16 +++++++++-------
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index 97281ead7f1..b44b3f31145 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -220,23 +220,24 @@ func (args *evmCallArgs) Call(addr common.Address, input []byte, gas uint64, val
 	// methods to pass to [EVMInterpreter.Run], which are then propagated by the
 	// *CALL* opcodes as the caller.
 	precompile := NewContract(args.caller, AccountRef(args.self()), args.value, args.gas)
+	if args.delegation == delegated {
+		precompile = precompile.AsDelegate()
+	}
+	var caller ContractRef = precompile
 
-	asDelegate := args.delegation == delegated // precompile was DELEGATECALLed
 	for _, o := range opts {
 		switch o := o.(type) {
-		case callOptForceDelegate:
-			// See documentation of [WithUNSAFEForceDelegate].
-			asDelegate = true
+		case callOptUNSAFECallerAddressProxy:
+			// Note that, in addition to being unsafe, this breaks an EVM
+			// assumption that the caller ContractRef is always a *Contract.
+			caller = AccountRef(args.caller.Address())
 		case nil:
 		default:
 			return nil, gas, fmt.Errorf("unsupported option %T", o)
 		}
 	}
-	if asDelegate {
-		precompile = precompile.AsDelegate()
-	}
 
-	return args.evm.Call(precompile, addr, input, gas, value)
+	return args.evm.Call(caller, addr, input, gas, value)
 }
 
 var (
diff --git a/core/vm/options.libevm.go b/core/vm/options.libevm.go
index 9dfce763984..94ecdbb0457 100644
--- a/core/vm/options.libevm.go
+++ b/core/vm/options.libevm.go
@@ -21,16 +21,18 @@ type CallOption interface {
 	libevmCallOption() // noop to only allow internally defined options
 }
 
-// WithUNSAFEForceDelegate results in precompiles making contract calls acting
-// as if they themselves were DELEGATECALLed. This is not safe for regular use
-// as the precompile will act as its own caller even when not expected to.
+// WithUNSAFECallerAddressProxying results in precompiles making contract calls
+// specifying their own caller's address as the caller. This is NOT SAFE for
+// regular use as callers of the precompile may not understand that they are
+// escalating the precompile's privileges.
 //
 // Deprecated: this option MUST NOT be used other than to allow migration to
 // libevm when backwards compatibility is required.
-func WithUNSAFEForceDelegate() CallOption {
-	return callOptForceDelegate{}
+func WithUNSAFECallerAddressProxying() CallOption {
+	return callOptUNSAFECallerAddressProxy{}
 }
 
-type callOptForceDelegate struct{}
+// Deprecated: see [WithUNSAFECallerAddressProxying].
+type callOptUNSAFECallerAddressProxy struct{}
 
-func (callOptForceDelegate) libevmCallOption() {}
+func (callOptUNSAFECallerAddressProxy) libevmCallOption() {}

From 46346f51c78cfcf594d915d65f48c743f79fbcca Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Sat, 28 Sep 2024 19:35:51 +0100
Subject: [PATCH 06/15] refactor: `type callType` replaces `rwInheritance` +
 `delegation` types

---
 core/vm/contracts.libevm.go | 43 ++++++++++++++++---------------------
 core/vm/evm.go              |  8 +++----
 2 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index b44b3f31145..ffd9dad98b8 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -41,7 +41,9 @@ import (
 //	    ...
 //	    args := &evmCallArgs{evm, caller, addr, input, gas, value, false}
 type evmCallArgs struct {
-	evm *EVM
+	evm      *EVM
+	callType callType
+
 	// args:start
 	caller ContractRef
 	addr   common.Address
@@ -56,26 +58,19 @@ type evmCallArgs struct {
 	// 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
 
 	// If a precompile issues its own Call() then caller semantics are dependent
 	// on whether the precompile was delegate-called or not. DelegateCall() MUST
 	// set this to delegated and all other methods MUST set it to notDelegated.
-	delegation delegation
 }
 
-type rwInheritance uint8
-
-const (
-	inheritReadOnly rwInheritance = iota + 1
-	forceReadOnly
-)
-
-type delegation uint8
+type callType uint8
 
 const (
-	delegated delegation = iota + 1
-	notDelegated
+	call callType = iota + 1
+	callCode
+	delegateCall
+	staticCall
 )
 
 // run runs the [PrecompiledContract], differentiating between stateful and
@@ -147,16 +142,16 @@ func (args *evmCallArgs) ChainConfig() *params.ChainConfig { return args.evm.cha
 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
-		}
+	// A switch statement provides clearer code coverage for difficult-to-test
+	// cases.
+	switch {
+	case args.callType == staticCall:
+		return true
+	case args.evm.interpreter.readOnly:
+		return true
+	default:
 		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 {
@@ -209,9 +204,7 @@ func (args *evmCallArgs) Call(addr common.Address, input []byte, gas uint64, val
 	in.evm.depth++
 	defer func() { in.evm.depth-- }()
 
-	// This will happen if the precompile was invoked via StaticCall() from a
-	// non read-only state.
-	if args.readWrite == forceReadOnly && !in.readOnly {
+	if args.callType == staticCall && !in.readOnly {
 		in.readOnly = true
 		defer func() { in.readOnly = false }()
 	}
@@ -220,7 +213,7 @@ func (args *evmCallArgs) Call(addr common.Address, input []byte, gas uint64, val
 	// methods to pass to [EVMInterpreter.Run], which are then propagated by the
 	// *CALL* opcodes as the caller.
 	precompile := NewContract(args.caller, AccountRef(args.self()), args.value, args.gas)
-	if args.delegation == delegated {
+	if args.callType == delegateCall {
 		precompile = precompile.AsDelegate()
 	}
 	var caller ContractRef = precompile
diff --git a/core/vm/evm.go b/core/vm/evm.go
index 310ba33217c..c6c735627fd 100644
--- a/core/vm/evm.go
+++ b/core/vm/evm.go
@@ -230,7 +230,7 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas
 	}
 
 	if isPrecompile {
-		args := &evmCallArgs{evm, caller, addr, input, gas, value, inheritReadOnly, notDelegated}
+		args := &evmCallArgs{evm, call, caller, addr, input, gas, value}
 		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.
@@ -294,7 +294,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, inheritReadOnly, notDelegated}
+		args := &evmCallArgs{evm, callCode, caller, addr, input, gas, value}
 		ret, gas, err = args.RunPrecompiledContract(p, input, gas)
 	} else {
 		addrCopy := addr
@@ -340,7 +340,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, inheritReadOnly, delegated}
+		args := &evmCallArgs{evm, delegateCall, caller, addr, input, gas, nil}
 		ret, gas, err = args.RunPrecompiledContract(p, input, gas)
 	} else {
 		addrCopy := addr
@@ -390,7 +390,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, forceReadOnly, notDelegated}
+		args := &evmCallArgs{evm, staticCall, caller, addr, input, gas, nil}
 		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

From 78c02cea54fe374a5c66cfb972bbda27c5cf05c0 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Sat, 28 Sep 2024 20:18:22 +0100
Subject: [PATCH 07/15] refactor: abstract return-data-proxy contract bytecode

---
 core/vm/contracts.libevm_test.go | 80 ++++++++++++++++++++------------
 1 file changed, 50 insertions(+), 30 deletions(-)

diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go
index b28174b3823..76106daef94 100644
--- a/core/vm/contracts.libevm_test.go
+++ b/core/vm/contracts.libevm_test.go
@@ -265,9 +265,7 @@ func TestInheritReadOnly(t *testing.T) {
 
 	// (1)
 
-	var precompile common.Address
-	const precompileAddr = 255
-	precompile[common.AddressLength-1] = precompileAddr
+	precompile := common.Address{255}
 
 	const (
 		ifReadOnly = iota + 1 // see contract bytecode for rationale
@@ -293,31 +291,13 @@ func TestInheritReadOnly(t *testing.T) {
 	})
 
 	// (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,
-	}
+	contract := makeReturnProxy(t, precompile, vm.CALL)
 
 	state, evm := ethtest.NewZeroEVM(t)
 	rng := ethtest.NewPseudoRand(42)
 	contractAddr := rng.Address()
 	state.CreateAccount(contractAddr)
-	state.SetCode(contractAddr, contractCode(contract))
+	state.SetCode(contractAddr, convertBytes[vm.OpCode, byte](contract))
 
 	// (3)
 
@@ -352,14 +332,54 @@ func TestInheritReadOnly(t *testing.T) {
 	}
 }
 
-// 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)
+// makeReturnProxy returns the bytecode of a contract that will call `dest` with
+// the specified call type and propagated the returned value.
+//
+// The contract does NOT check if the call reverted. In this case, the
+// propagated return value will always be an empty slice. Tests using these
+// proxies MUST use non-empty slices as test values.
+//
+// TODO(arr4n): convert this to arr4n/specops for clarity and to make it easier
+// to generate a revert check.
+func makeReturnProxy(t *testing.T, dest common.Address, call vm.OpCode) []vm.OpCode {
+	t.Helper()
+	const p0 = vm.PUSH0
+	contract := []vm.OpCode{
+		vm.PUSH1, 1, // retSize (bytes)
+		p0, // retOffset
+		p0, // argSize
+		p0, // argOffset
+	}
+
+	// See CALL signature: https://www.evm.codes/#f1?fork=cancun
+	switch call {
+	case vm.CALL, vm.CALLCODE:
+		contract = append(contract, p0) // value
+	case vm.DELEGATECALL, vm.STATICCALL:
+	default:
+		t.Fatalf("Bad test setup: invalid non-CALL-type opcode %s", call)
+	}
+
+	contract = append(contract, vm.PUSH20)
+	contract = append(contract, convertBytes[byte, vm.OpCode](dest[:])...)
+
+	contract = append(contract,
+		p0, // gas
+		call,
+
+		// See function comment re ignored reverts.
+		vm.RETURNDATASIZE, p0, p0, vm.RETURNDATACOPY,
+		vm.RETURNDATASIZE, p0, vm.RETURN,
+	)
+	return contract
+}
+
+func convertBytes[From ~byte, To ~byte](buf []From) []To {
+	out := make([]To, len(buf))
+	for i, b := range buf {
+		out[i] = To(b)
 	}
-	return ret
+	return out
 }
 
 func TestCanCreateContract(t *testing.T) {

From 0982753b532ad1b5e5da4c4d4db9ec407e8e146e Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Sat, 28 Sep 2024 20:51:36 +0100
Subject: [PATCH 08/15] doc: fix comments from `46346f51`

---
 core/vm/contracts.libevm.go | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index ffd9dad98b8..dc314ddaea3 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -30,16 +30,16 @@ import (
 
 // 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 appended with additional
-// values. As {Delegate,Static}Call don't accept a value, they MUST set the
-// respective field to nil.
+// parameters, prepended with the receiver name and call type. 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, false}
+//	    args := &evmCallArgs{evm, callCode, caller, addr, input, gas, value}
 type evmCallArgs struct {
 	evm      *EVM
 	callType callType
@@ -51,17 +51,6 @@ type evmCallArgs struct {
 	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().
-
-	// If a precompile issues its own Call() then caller semantics are dependent
-	// on whether the precompile was delegate-called or not. DelegateCall() MUST
-	// set this to delegated and all other methods MUST set it to notDelegated.
 }
 
 type callType uint8
@@ -146,6 +135,9 @@ func (args *evmCallArgs) ReadOnly() bool {
 	// cases.
 	switch {
 	case args.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 args.evm.interpreter.readOnly:
 		return true

From c616fb04cfe955fc8f9169aa20fc2c3a54e12032 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Sun, 29 Sep 2024 14:23:48 +0100
Subject: [PATCH 09/15] fix: `PrecompileEnvironment.Addresses()` for all call
 types

---
 core/vm/contracts.libevm.go      | 121 ++++++++++---------------------
 core/vm/contracts.libevm_test.go |  66 ++++++++++++-----
 core/vm/environment.libevm.go    |  94 ++++++++++++++++++++++++
 3 files changed, 177 insertions(+), 104 deletions(-)
 create mode 100644 core/vm/environment.libevm.go

diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index dc314ddaea3..cd3890df250 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -66,7 +66,7 @@ const (
 // regular types.
 func (args *evmCallArgs) run(p PrecompiledContract, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) {
 	if p, ok := p.(statefulPrecompile); ok {
-		return p(args, input, suppliedGas)
+		return p(args.env(), input, suppliedGas)
 	}
 	// Gas consumption for regular precompiles was already handled by the native
 	// RunPrecompiledContract(), which called this method.
@@ -125,12 +125,44 @@ type PrecompileEnvironment interface {
 	Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, _ ...CallOption) (ret []byte, gasRemaining uint64, _ error)
 }
 
-var _ PrecompileEnvironment = (*evmCallArgs)(nil)
+func (args *evmCallArgs) env() *environment {
+	env := &environment{
+		evm:      args.evm,
+		readOnly: args.readOnly(),
+		callType: args.callType,
+	}
+
+	var self common.Address
+	switch addr := args.addr; args.callType {
+	case staticCall:
+		args.value = new(uint256.Int)
+		fallthrough
+	case call:
+		self = addr
+
+	case delegateCall:
+		args.value = nil
+		fallthrough
+	case callCode:
+		self = args.caller.Address()
+	}
 
-func (args *evmCallArgs) ChainConfig() *params.ChainConfig { return args.evm.chainConfig }
-func (args *evmCallArgs) Rules() params.Rules              { return args.evm.chainRules }
+	// This is equivalent to the `contract` variables created by evm.*Call*()
+	// methods to pass to [EVMInterpreter.Run].
+	env.self = NewContract(args.caller, AccountRef(self), args.value, args.gas)
+	if args.callType == delegateCall {
+		env.self = env.self.AsDelegate()
+	}
+
+	env.addrs = libevm.AddressContext{
+		Origin: args.evm.Origin,
+		Caller: env.self.CallerAddress,
+		Self:   self,
+	}
+	return env
+}
 
-func (args *evmCallArgs) ReadOnly() bool {
+func (args *evmCallArgs) readOnly() bool {
 	// A switch statement provides clearer code coverage for difficult-to-test
 	// cases.
 	switch {
@@ -146,85 +178,6 @@ func (args *evmCallArgs) ReadOnly() bool {
 	}
 }
 
-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) self() common.Address { return args.addr }
-
-func (args *evmCallArgs) Addresses() *libevm.AddressContext {
-	return &libevm.AddressContext{
-		Origin: args.evm.TxContext.Origin,
-		Caller: args.caller.Address(),
-		Self:   args.self(),
-	}
-}
-
-func (args *evmCallArgs) BlockHeader() (types.Header, error) {
-	hdr := args.evm.Context.Header
-	if hdr == nil {
-		// Although [core.NewEVMBlockContext] sets the field and is in the
-		// typical hot path (e.g. miner), there are other ways to create a
-		// [vm.BlockContext] (e.g. directly in tests) that may result in no
-		// available header.
-		return types.Header{}, fmt.Errorf("nil %T in current %T", hdr, args.evm.Context)
-	}
-	return *hdr, nil
-}
-
-func (args *evmCallArgs) BlockNumber() *big.Int {
-	return new(big.Int).Set(args.evm.Context.BlockNumber)
-}
-
-func (args *evmCallArgs) BlockTime() uint64 { return args.evm.Context.Time }
-
-func (args *evmCallArgs) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) ([]byte, uint64, error) {
-	in := args.evm.interpreter
-
-	// The precompile run didn't increment the depth so this is necessary even
-	// though Call() will eventually result in it being done again.
-	in.evm.depth++
-	defer func() { in.evm.depth-- }()
-
-	if args.callType == staticCall && !in.readOnly {
-		in.readOnly = true
-		defer func() { in.readOnly = false }()
-	}
-
-	// This is equivalent to the `contract` variables created by evm.*Call*()
-	// methods to pass to [EVMInterpreter.Run], which are then propagated by the
-	// *CALL* opcodes as the caller.
-	precompile := NewContract(args.caller, AccountRef(args.self()), args.value, args.gas)
-	if args.callType == delegateCall {
-		precompile = precompile.AsDelegate()
-	}
-	var caller ContractRef = precompile
-
-	for _, o := range opts {
-		switch o := o.(type) {
-		case callOptUNSAFECallerAddressProxy:
-			// Note that, in addition to being unsafe, this breaks an EVM
-			// assumption that the caller ContractRef is always a *Contract.
-			caller = AccountRef(args.caller.Address())
-		case nil:
-		default:
-			return nil, gas, fmt.Errorf("unsupported option %T", o)
-		}
-	}
-
-	return args.evm.Call(caller, addr, input, gas, value)
-}
-
 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
index 76106daef94..58caf3e6f3b 100644
--- a/core/vm/contracts.libevm_test.go
+++ b/core/vm/contracts.libevm_test.go
@@ -100,7 +100,7 @@ func TestPrecompileOverride(t *testing.T) {
 
 type statefulPrecompileOutput struct {
 	ChainID                 *big.Int
-	Caller, Self            common.Address
+	Addresses               *libevm.AddressContext
 	StateValue              common.Hash
 	ReadOnly                bool
 	BlockNumber, Difficulty *big.Int
@@ -116,8 +116,11 @@ func (o statefulPrecompileOutput) String() string {
 		fld := out.Field(i).Interface()
 
 		verb := "%v"
-		if _, ok := fld.([]byte); ok {
+		switch fld.(type) {
+		case []byte:
 			verb = "%#x"
+		case *libevm.AddressContext:
+			verb = "%+v"
 		}
 		lines = append(lines, fmt.Sprintf("%s: "+verb, name, fld))
 	}
@@ -125,8 +128,8 @@ func (o statefulPrecompileOutput) String() string {
 }
 
 func TestNewStatefulPrecompile(t *testing.T) {
+	precompile := common.HexToAddress("60C0DE") // GO CODE
 	rng := ethtest.NewPseudoRand(314159)
-	precompile := rng.Address()
 	slot := rng.Hash()
 
 	const gasLimit = 1e6
@@ -141,11 +144,9 @@ func TestNewStatefulPrecompile(t *testing.T) {
 			return nil, 0, err
 		}
 
-		addrs := env.Addresses()
 		out := &statefulPrecompileOutput{
 			ChainID:     env.ChainConfig().ChainID,
-			Caller:      addrs.Caller,
-			Self:        addrs.Self,
+			Addresses:   env.Addresses(),
 			StateValue:  env.ReadOnlyState().GetState(precompile, slot),
 			ReadOnly:    env.ReadOnly(),
 			BlockNumber: env.BlockNumber(),
@@ -167,11 +168,14 @@ func TestNewStatefulPrecompile(t *testing.T) {
 		Time:       rng.Uint64(),
 		Difficulty: rng.BigUint64(),
 	}
-	caller := rng.Address()
 	input := rng.Bytes(8)
 	value := rng.Hash()
 	chainID := rng.BigUint64()
 
+	caller := common.HexToAddress("CA11E12") // caller of the precompile
+	eoa := common.HexToAddress("E0A")        // caller of the precompile-caller
+	callerContract := vm.NewContract(vm.AccountRef(eoa), vm.AccountRef(caller), uint256.NewInt(0), 1e6)
+
 	state, evm := ethtest.NewZeroEVM(
 		t,
 		ethtest.WithBlockContext(
@@ -182,39 +186,61 @@ func TestNewStatefulPrecompile(t *testing.T) {
 		),
 	)
 	state.SetState(precompile, slot, value)
+	evm.Origin = eoa
 
 	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.
+		name          string
+		call          func() ([]byte, uint64, error)
+		wantAddresses *libevm.AddressContext
+		// Note that this only covers evm.readOnly being true because of the
+		// precompile's call. 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))
+				return evm.Call(callerContract, precompile, input, gasLimit, uint256.NewInt(0))
+			},
+			wantAddresses: &libevm.AddressContext{
+				Origin: eoa,
+				Caller: caller,
+				Self:   precompile,
 			},
 			wantReadOnly: false,
 		},
 		{
 			name: "EVM.CallCode()",
 			call: func() ([]byte, uint64, error) {
-				return evm.CallCode(vm.AccountRef(caller), precompile, input, gasLimit, uint256.NewInt(0))
+				return evm.CallCode(callerContract, precompile, input, gasLimit, uint256.NewInt(0))
+			},
+			wantAddresses: &libevm.AddressContext{
+				Origin: eoa,
+				Caller: caller,
+				Self:   caller,
 			},
 			wantReadOnly: false,
 		},
 		{
 			name: "EVM.DelegateCall()",
 			call: func() ([]byte, uint64, error) {
-				return evm.DelegateCall(vm.AccountRef(caller), precompile, input, gasLimit)
+				return evm.DelegateCall(callerContract, precompile, input, gasLimit)
+			},
+			wantAddresses: &libevm.AddressContext{
+				Origin: eoa,
+				Caller: eoa, // inherited from caller
+				Self:   caller,
 			},
 			wantReadOnly: false,
 		},
 		{
 			name: "EVM.StaticCall()",
 			call: func() ([]byte, uint64, error) {
-				return evm.StaticCall(vm.AccountRef(caller), precompile, input, gasLimit)
+				return evm.StaticCall(callerContract, precompile, input, gasLimit)
+			},
+			wantAddresses: &libevm.AddressContext{
+				Origin: eoa,
+				Caller: caller,
+				Self:   precompile,
 			},
 			wantReadOnly: true,
 		},
@@ -222,22 +248,22 @@ func TestNewStatefulPrecompile(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			wantReturnData := statefulPrecompileOutput{
+			wantOutput := statefulPrecompileOutput{
 				ChainID:     chainID,
-				Caller:      caller,
-				Self:        precompile,
+				Addresses:   tt.wantAddresses,
 				StateValue:  value,
 				ReadOnly:    tt.wantReadOnly,
 				BlockNumber: header.Number,
 				BlockTime:   header.Time,
 				Difficulty:  header.Difficulty,
 				Input:       input,
-			}.String()
+			}
+
 			wantGasLeft := gasLimit - gasCost
 
 			gotReturnData, gotGasLeft, err := tt.call()
 			require.NoError(t, err)
-			assert.Equal(t, wantReturnData, string(gotReturnData))
+			assert.Equal(t, wantOutput.String(), string(gotReturnData))
 			assert.Equal(t, wantGasLeft, gotGasLeft)
 		})
 	}
diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go
new file mode 100644
index 00000000000..2c3a105c7db
--- /dev/null
+++ b/core/vm/environment.libevm.go
@@ -0,0 +1,94 @@
+// Copyright 2024 the libevm authors.
+//
+// The libevm additions to go-ethereum are free software: you can redistribute
+// them and/or modify them under the terms of the GNU Lesser General Public License
+// as published by the Free Software Foundation, either version 3 of the License,
+// or (at your option) any later version.
+//
+// The libevm additions are distributed in the hope that they will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser
+// General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see
+// <http://www.gnu.org/licenses/>.
+
+package vm
+
+import (
+	"fmt"
+	"math/big"
+
+	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethereum/go-ethereum/core/types"
+	"github.com/ethereum/go-ethereum/libevm"
+	"github.com/ethereum/go-ethereum/params"
+	"github.com/holiman/uint256"
+)
+
+var _ PrecompileEnvironment = (*environment)(nil)
+
+type environment struct {
+	evm      *EVM
+	readOnly bool
+	addrs    libevm.AddressContext
+	self     *Contract
+	callType callType
+}
+
+func (e *environment) ChainConfig() *params.ChainConfig  { return e.evm.chainConfig }
+func (e *environment) Rules() params.Rules               { return e.evm.chainRules }
+func (e *environment) ReadOnly() bool                    { return e.readOnly }
+func (e *environment) ReadOnlyState() libevm.StateReader { return e.evm.StateDB }
+func (e *environment) Addresses() *libevm.AddressContext { return &e.addrs }
+func (e *environment) BlockNumber() *big.Int             { return new(big.Int).Set(e.evm.Context.BlockNumber) }
+func (e *environment) BlockTime() uint64                 { return e.evm.Context.Time }
+
+func (e *environment) StateDB() StateDB {
+	if e.ReadOnly() {
+		return nil
+	}
+	return e.evm.StateDB
+}
+
+func (e *environment) BlockHeader() (types.Header, error) {
+	hdr := e.evm.Context.Header
+	if hdr == nil {
+		// Although [core.NewEVMBlockContext] sets the field and is in the
+		// typical hot path (e.g. miner), there are other ways to create a
+		// [vm.BlockContext] (e.g. directly in tests) that may result in no
+		// available header.
+		return types.Header{}, fmt.Errorf("nil %T in current %T", hdr, e.evm.Context)
+	}
+	return *hdr, nil
+}
+
+func (e *environment) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) ([]byte, uint64, error) {
+	in := e.evm.interpreter
+
+	// The precompile run didn't increment the depth so this is necessary even
+	// though Call() will eventually result in it being done again.
+	in.evm.depth++
+	defer func() { in.evm.depth-- }()
+
+	if e.callType == staticCall && !in.readOnly {
+		in.readOnly = true
+		defer func() { in.readOnly = false }()
+	}
+
+	var caller ContractRef = e.self
+	for _, o := range opts {
+		switch o := o.(type) {
+		case callOptUNSAFECallerAddressProxy:
+			// Note that, in addition to being unsafe, this breaks an EVM
+			// assumption that the caller ContractRef is always a *Contract.
+			caller = AccountRef(e.addrs.Caller)
+		case nil:
+		default:
+			return nil, gas, fmt.Errorf("unsupported option %T", o)
+		}
+	}
+
+	return e.evm.Call(caller, addr, input, gas, value)
+}

From 563196afcfa65a1086d9d46ea07ff7f22e33a7e8 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Sun, 29 Sep 2024 14:31:44 +0100
Subject: [PATCH 10/15] chore: readability, linting & mark upstream test flaky

---
 .github/workflows/go.yml      |  2 +-
 core/vm/contracts.libevm.go   | 10 +++++-----
 core/vm/environment.libevm.go |  3 ++-
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml
index 851cb51eb9b..1282b281d63 100644
--- a/.github/workflows/go.yml
+++ b/.github/workflows/go.yml
@@ -18,6 +18,6 @@ jobs:
           go-version: 1.21.4
       - name: Run tests
         run: | # Upstream flakes are race conditions exacerbated by concurrent tests
-          FLAKY_REGEX='go-ethereum/(eth|accounts/keystore|eth/downloader|miner|ethclient/gethclient|eth/catalyst)$';
+          FLAKY_REGEX='go-ethereum/(eth|accounts/keystore|eth/downloader|miner|ethclient|ethclient/gethclient|eth/catalyst)$';
           go list ./... | grep -P "${FLAKY_REGEX}" | xargs -n 1 go test -short;
           go test -short $(go list ./... | grep -Pv "${FLAKY_REGEX}");
diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index cd3890df250..059cc2d6115 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -119,9 +119,9 @@ type PrecompileEnvironment interface {
 	BlockNumber() *big.Int
 	BlockTime() uint64
 
-	// Call is equivalent to [EVM.Call], with the caller defaulting to the
-	// precompile receiving the environment, or to its own caller if invoked via
-	// a delegated call.
+	// Call is equivalent to [EVM.Call] except that the `caller` argument is
+	// automatically determined according to the type of call that invoked the
+	// precompile.
 	Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, _ ...CallOption) (ret []byte, gasRemaining uint64, _ error)
 }
 
@@ -133,12 +133,12 @@ func (args *evmCallArgs) env() *environment {
 	}
 
 	var self common.Address
-	switch addr := args.addr; args.callType {
+	switch args.callType {
 	case staticCall:
 		args.value = new(uint256.Int)
 		fallthrough
 	case call:
-		self = addr
+		self = args.addr
 
 	case delegateCall:
 		args.value = nil
diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go
index 2c3a105c7db..0f037c19854 100644
--- a/core/vm/environment.libevm.go
+++ b/core/vm/environment.libevm.go
@@ -20,11 +20,12 @@ import (
 	"fmt"
 	"math/big"
 
+	"github.com/holiman/uint256"
+
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/libevm"
 	"github.com/ethereum/go-ethereum/params"
-	"github.com/holiman/uint256"
 )
 
 var _ PrecompileEnvironment = (*environment)(nil)

From f711f21acd5234b9f3ef053aa0277971d86fb908 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Sun, 29 Sep 2024 15:02:23 +0100
Subject: [PATCH 11/15] test: `PrecompileEnvironment.Call()`

---
 core/vm/contracts.libevm_test.go | 107 ++++++++++++++++++++++++++++++-
 1 file changed, 106 insertions(+), 1 deletion(-)

diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go
index 58caf3e6f3b..e2ff72730c9 100644
--- a/core/vm/contracts.libevm_test.go
+++ b/core/vm/contracts.libevm_test.go
@@ -127,6 +127,10 @@ func (o statefulPrecompileOutput) String() string {
 	return strings.Join(lines, "\n")
 }
 
+func (o statefulPrecompileOutput) Bytes() []byte {
+	return []byte(o.String())
+}
+
 func TestNewStatefulPrecompile(t *testing.T) {
 	precompile := common.HexToAddress("60C0DE") // GO CODE
 	rng := ethtest.NewPseudoRand(314159)
@@ -154,7 +158,7 @@ func TestNewStatefulPrecompile(t *testing.T) {
 			Difficulty:  hdr.Difficulty,
 			Input:       input,
 		}
-		return []byte(out.String()), suppliedGas - gasCost, nil
+		return out.Bytes(), suppliedGas - gasCost, nil
 	}
 	hooks := &hookstest.Stub{
 		PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{
@@ -491,3 +495,104 @@ func TestActivePrecompilesOverride(t *testing.T) {
 
 	require.Equal(t, precompiles, vm.ActivePrecompiles(newRules()), "vm.ActivePrecompiles() returns overridden addresses")
 }
+
+func TestPrecompileMakeCall(t *testing.T) {
+	// Each test runs as follows:
+	// 1. `eoa` makes a call to a bytecode contract, `caller`
+	// 2. `caller` calls `sut`, the precompile under test, via all *CALL* op codes
+	// 3. `sut` makes a Call() to `dest`, which reflects env data for testing
+	eoa := common.HexToAddress("E0A")
+	caller := common.HexToAddress("CA11E12")
+	sut := common.HexToAddress("7E57ED")
+	dest := common.HexToAddress("DE57")
+
+	hooks := &hookstest.Stub{
+		PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{
+			sut: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) {
+				// We are ultimately testing env.Call(), hence why this is the SUT.
+				return env.Call(dest, input, suppliedGas, uint256.NewInt(0))
+			}),
+			dest: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) {
+				out := &statefulPrecompileOutput{
+					Addresses: env.Addresses(),
+					ReadOnly:  env.ReadOnly(),
+				}
+				return out.Bytes(), suppliedGas, nil
+			}),
+		},
+	}
+	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
+		},
+	})
+
+	tests := []struct {
+		incomingCallType vm.OpCode
+		// Unlike TestNewStatefulPrecompile, which tests the AddressContext of
+		// the precompile itself, these test the AddressContext of a contract
+		// called by the precompile.
+		want statefulPrecompileOutput
+	}{
+		{
+			incomingCallType: vm.CALL,
+			want: statefulPrecompileOutput{
+				Addresses: &libevm.AddressContext{
+					Origin: eoa,
+					Caller: sut,
+					Self:   dest,
+				},
+			},
+		},
+		{
+			incomingCallType: vm.CALLCODE,
+			want: statefulPrecompileOutput{
+				Addresses: &libevm.AddressContext{
+					Origin: eoa,
+					Caller: caller, // SUT runs as its own caller because of CALLCODE
+					Self:   dest,
+				},
+			},
+		},
+		{
+			incomingCallType: vm.DELEGATECALL,
+			want: statefulPrecompileOutput{
+				Addresses: &libevm.AddressContext{
+					Origin: eoa,
+					Caller: caller, // as with CALLCODE
+					Self:   dest,
+				},
+			},
+		},
+		{
+			incomingCallType: vm.STATICCALL,
+			want: statefulPrecompileOutput{
+				Addresses: &libevm.AddressContext{
+					Origin: eoa,
+					Caller: sut,
+					Self:   dest,
+				},
+				// This demonstrates that even though the precompile makes a
+				// (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,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("via %s", tt.incomingCallType), func(t *testing.T) {
+			state, evm := ethtest.NewZeroEVM(t)
+			evm.Origin = eoa
+			state.CreateAccount(caller)
+			proxy := makeReturnProxy(t, sut, tt.incomingCallType)
+			state.SetCode(caller, convertBytes[vm.OpCode, byte](proxy))
+
+			got, _, err := evm.Call(vm.AccountRef(eoa), caller, nil, 1e6, uint256.NewInt(0))
+			require.NoError(t, err)
+			require.Equal(t, tt.want.String(), string(got))
+		})
+	}
+}

From a34b1bd9a21ca0c64abf3202660085de93a1d498 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Sun, 29 Sep 2024 16:47:40 +0100
Subject: [PATCH 12/15] refactor: improved {read,maintain}ability

---
 core/vm/contracts.libevm.go   | 45 ++++++++++++++++-------------------
 core/vm/environment.libevm.go | 45 +++++++++++++++++++++++++----------
 2 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index 059cc2d6115..ba96f6c1c7f 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -31,15 +31,15 @@ import (
 // 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 MUST set the respective
-// field to nil.
+// {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) Call(caller ContractRef, addr common.Address, input []byte, gas uint64, value *uint256.Int) ... {
 //	    ...
-//	    args := &evmCallArgs{evm, callCode, caller, addr, input, gas, value}
+//	    args := &evmCallArgs{evm, staticCall, caller, addr, input, gas, nil /*value*/}
 type evmCallArgs struct {
 	evm      *EVM
 	callType callType
@@ -103,8 +103,9 @@ func (p statefulPrecompile) Run([]byte) ([]byte, error) {
 	panic(fmt.Sprintf("BUG: call to %T.Run(); MUST call %T itself", p, p))
 }
 
-// A PrecompileEnvironment provides information about the context in which a
-// precompiled contract is being run.
+// A PrecompileEnvironment provides (a) information about the context in which a
+// precompiled contract is being run; and (b) a means of calling other
+// contracts.
 type PrecompileEnvironment interface {
 	ChainConfig() *params.ChainConfig
 	Rules() params.Rules
@@ -120,46 +121,42 @@ type PrecompileEnvironment interface {
 	BlockTime() uint64
 
 	// Call is equivalent to [EVM.Call] except that the `caller` argument is
-	// automatically determined according to the type of call that invoked the
-	// precompile.
+	// removed and automatically determined according to the type of call that
+	// invoked the precompile.
 	Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, _ ...CallOption) (ret []byte, gasRemaining uint64, _ error)
 }
 
 func (args *evmCallArgs) env() *environment {
-	env := &environment{
-		evm:      args.evm,
-		readOnly: args.readOnly(),
-		callType: args.callType,
-	}
-
-	var self common.Address
+	var (
+		self  common.Address
+		value = args.value
+	)
 	switch args.callType {
 	case staticCall:
-		args.value = new(uint256.Int)
+		value = new(uint256.Int)
 		fallthrough
 	case call:
 		self = args.addr
 
 	case delegateCall:
-		args.value = nil
+		value = nil
 		fallthrough
 	case callCode:
 		self = args.caller.Address()
 	}
 
 	// This is equivalent to the `contract` variables created by evm.*Call*()
-	// methods to pass to [EVMInterpreter.Run].
-	env.self = NewContract(args.caller, AccountRef(self), args.value, args.gas)
+	// methods, for non precompiles, to pass to [EVMInterpreter.Run].
+	contract := NewContract(args.caller, AccountRef(self), value, args.gas)
 	if args.callType == delegateCall {
-		env.self = env.self.AsDelegate()
+		contract = contract.AsDelegate()
 	}
 
-	env.addrs = libevm.AddressContext{
-		Origin: args.evm.Origin,
-		Caller: env.self.CallerAddress,
-		Self:   self,
+	return &environment{
+		evm:           args.evm,
+		self:          contract,
+		forceReadOnly: args.readOnly(),
 	}
-	return env
 }
 
 func (args *evmCallArgs) readOnly() bool {
diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go
index 0f037c19854..b81d56758d5 100644
--- a/core/vm/environment.libevm.go
+++ b/core/vm/environment.libevm.go
@@ -31,21 +31,26 @@ import (
 var _ PrecompileEnvironment = (*environment)(nil)
 
 type environment struct {
-	evm      *EVM
-	readOnly bool
-	addrs    libevm.AddressContext
-	self     *Contract
-	callType callType
+	evm           *EVM
+	self          *Contract
+	forceReadOnly bool
 }
 
 func (e *environment) ChainConfig() *params.ChainConfig  { return e.evm.chainConfig }
 func (e *environment) Rules() params.Rules               { return e.evm.chainRules }
-func (e *environment) ReadOnly() bool                    { return e.readOnly }
+func (e *environment) ReadOnly() bool                    { return e.forceReadOnly || e.evm.interpreter.readOnly }
 func (e *environment) ReadOnlyState() libevm.StateReader { return e.evm.StateDB }
-func (e *environment) Addresses() *libevm.AddressContext { return &e.addrs }
 func (e *environment) BlockNumber() *big.Int             { return new(big.Int).Set(e.evm.Context.BlockNumber) }
 func (e *environment) BlockTime() uint64                 { return e.evm.Context.Time }
 
+func (e *environment) Addresses() *libevm.AddressContext {
+	return &libevm.AddressContext{
+		Origin: e.evm.Origin,
+		Caller: e.self.CallerAddress,
+		Self:   e.self.Address(),
+	}
+}
+
 func (e *environment) StateDB() StateDB {
 	if e.ReadOnly() {
 		return nil
@@ -66,14 +71,18 @@ func (e *environment) BlockHeader() (types.Header, error) {
 }
 
 func (e *environment) Call(addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) ([]byte, uint64, error) {
+	return e.callContract(call, addr, input, gas, value, opts...)
+}
+
+func (e *environment) callContract(typ callType, addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) ([]byte, uint64, error) {
+	// Depth and read-only setting are handled by [EVMInterpreter.Run], which
+	// isn't used for precompiles, so we need to do it ourselves.
 	in := e.evm.interpreter
 
-	// The precompile run didn't increment the depth so this is necessary even
-	// though Call() will eventually result in it being done again.
 	in.evm.depth++
 	defer func() { in.evm.depth-- }()
 
-	if e.callType == staticCall && !in.readOnly {
+	if e.forceReadOnly && !in.readOnly { // i.e. the precompile was StaticCall()ed
 		in.readOnly = true
 		defer func() { in.readOnly = false }()
 	}
@@ -84,12 +93,24 @@ func (e *environment) Call(addr common.Address, input []byte, gas uint64, value
 		case callOptUNSAFECallerAddressProxy:
 			// Note that, in addition to being unsafe, this breaks an EVM
 			// assumption that the caller ContractRef is always a *Contract.
-			caller = AccountRef(e.addrs.Caller)
+			caller = AccountRef(e.self.CallerAddress)
 		case nil:
 		default:
 			return nil, gas, fmt.Errorf("unsupported option %T", o)
 		}
 	}
 
-	return e.evm.Call(caller, addr, input, gas, value)
+	switch typ {
+	case call:
+		return e.evm.Call(caller, addr, input, gas, value)
+	case callCode, delegateCall, staticCall:
+		// TODO(arr4n): these cases should be very similar to CALL, hence the
+		// early abstraction, to signal to future maintainers. If implementing
+		// them, there's likely no need to honour the
+		// [callOptUNSAFECallerAddressProxy] because it's purely for backwards
+		// compatibility.
+		fallthrough
+	default:
+		return nil, gas, fmt.Errorf("unimplemented precompile call type %v", typ)
+	}
 }

From ff90c181f4bf23725ffcd7a86ac904a3a50e7599 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Sun, 29 Sep 2024 16:49:36 +0100
Subject: [PATCH 13/15] doc: fix `evmCallArgs` example

---
 core/vm/contracts.libevm.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/vm/contracts.libevm.go b/core/vm/contracts.libevm.go
index ba96f6c1c7f..78b634ad4bf 100644
--- a/core/vm/contracts.libevm.go
+++ b/core/vm/contracts.libevm.go
@@ -37,7 +37,7 @@ import (
 // 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) ... {
+//	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 {

From 2b83ed3400fc3da18baf65ab4eda1bb6991b7c28 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Sun, 29 Sep 2024 17:04:15 +0100
Subject: [PATCH 14/15] test: `PrecompileEnvironment.Call()` input data

---
 core/vm/contracts.libevm_test.go | 22 +++++++++++++++++-----
 core/vm/environment.libevm.go    |  3 ++-
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/core/vm/contracts.libevm_test.go b/core/vm/contracts.libevm_test.go
index e2ff72730c9..f1cecca30cf 100644
--- a/core/vm/contracts.libevm_test.go
+++ b/core/vm/contracts.libevm_test.go
@@ -497,25 +497,33 @@ func TestActivePrecompilesOverride(t *testing.T) {
 }
 
 func TestPrecompileMakeCall(t *testing.T) {
-	// Each test runs as follows:
-	// 1. `eoa` makes a call to a bytecode contract, `caller`
-	// 2. `caller` calls `sut`, the precompile under test, via all *CALL* op codes
-	// 3. `sut` makes a Call() to `dest`, which reflects env data for testing
+	// There is one test per *CALL* op code:
+	//
+	// 1. `eoa` makes a call to a bytecode contract, `caller`;
+	// 2. `caller` calls `sut`, the precompile under test, via the test's *CALL* op code;
+	// 3. `sut` makes a Call() to `dest`, which reflects env data for testing.
+	//
+	// This acts as a full integration test of a precompile being invoked before
+	// making an "outbound" call.
 	eoa := common.HexToAddress("E0A")
 	caller := common.HexToAddress("CA11E12")
 	sut := common.HexToAddress("7E57ED")
 	dest := common.HexToAddress("DE57")
 
+	rng := ethtest.NewPseudoRand(142857)
+	callData := rng.Bytes(8)
+
 	hooks := &hookstest.Stub{
 		PrecompileOverrides: map[common.Address]libevm.PrecompiledContract{
 			sut: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) {
 				// We are ultimately testing env.Call(), hence why this is the SUT.
-				return env.Call(dest, input, suppliedGas, uint256.NewInt(0))
+				return env.Call(dest, callData, suppliedGas, uint256.NewInt(0))
 			}),
 			dest: vm.NewStatefulPrecompile(func(env vm.PrecompileEnvironment, input []byte, suppliedGas uint64) (ret []byte, remainingGas uint64, err error) {
 				out := &statefulPrecompileOutput{
 					Addresses: env.Addresses(),
 					ReadOnly:  env.ReadOnly(),
+					Input:     input, // expected to be callData
 				}
 				return out.Bytes(), suppliedGas, nil
 			}),
@@ -543,6 +551,7 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: sut,
 					Self:   dest,
 				},
+				Input: callData,
 			},
 		},
 		{
@@ -553,6 +562,7 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: caller, // SUT runs as its own caller because of CALLCODE
 					Self:   dest,
 				},
+				Input: callData,
 			},
 		},
 		{
@@ -563,6 +573,7 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: caller, // as with CALLCODE
 					Self:   dest,
 				},
+				Input: callData,
 			},
 		},
 		{
@@ -573,6 +584,7 @@ func TestPrecompileMakeCall(t *testing.T) {
 					Caller: sut,
 					Self:   dest,
 				},
+				Input: callData,
 				// This demonstrates that even though the precompile makes a
 				// (non-static) CALL, the read-only state is inherited. Yes,
 				// this is _another_ way to get a read-only state, different to
diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go
index b81d56758d5..babfae63c5e 100644
--- a/core/vm/environment.libevm.go
+++ b/core/vm/environment.libevm.go
@@ -76,7 +76,8 @@ 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) ([]byte, uint64, error) {
 	// Depth and read-only setting are handled by [EVMInterpreter.Run], which
-	// isn't used for precompiles, so we need to do it ourselves.
+	// isn't used for precompiles, so we need to do it ourselves to maintain the
+	// expected invariants.
 	in := e.evm.interpreter
 
 	in.evm.depth++

From 10147dc25889bf409dd5d7e0ae3aec527a7998e6 Mon Sep 17 00:00:00 2001
From: Arran Schlosberg <me@arranschlosberg.com>
Date: Mon, 30 Sep 2024 10:10:57 +0100
Subject: [PATCH 15/15] fix: write protection for non-zero call value

---
 core/vm/environment.libevm.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/core/vm/environment.libevm.go b/core/vm/environment.libevm.go
index babfae63c5e..75a10c0d857 100644
--- a/core/vm/environment.libevm.go
+++ b/core/vm/environment.libevm.go
@@ -103,6 +103,9 @@ func (e *environment) callContract(typ callType, addr common.Address, input []by
 
 	switch typ {
 	case call:
+		if in.readOnly && !value.IsZero() {
+			return nil, gas, ErrWriteProtection
+		}
 		return e.evm.Call(caller, addr, input, gas, value)
 	case callCode, delegateCall, staticCall:
 		// TODO(arr4n): these cases should be very similar to CALL, hence the