Merge pull request #56 from shemnon/eof/extcall-fixes

Eof/extcall fixes

core/state_transition.go

@@ -512,15 +512,14 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) {
 		vmerr error // vm errors do not effect consensus and are therefore not assigned to err
 	)
 	if contractCreation {
-		ret, _, st.gasRemaining, vmerr = st.evm.Create(sender, msg.Data, st.gasRemaining, value)
+		ret, _, st.gasRemaining, vmerr = st.evm.Create(sender, msg.Data, st.gasRemaining, value, rules.IsPrague)
 		// Special case for EOF, if the initcode or deployed code is
 		// invalid, the tx is considered valid (so update nonce), but
-		// is to be treated as an exceptional abort (so burn all gas).
+		// gas for initcode execution is not consumed.
+		// Only intrinsic creation transaction costs are charged.
 		if errors.Is(vmerr, vm.ErrInvalidEOFInitcode) {
-			st.gasRemaining = 0
 			st.state.SetNonce(msg.From, st.state.GetNonce(sender.Address())+1)
 		}
-		fmt.Println(vmerr)
 	} else {
 		ret, st.gasRemaining, vmerr = st.evm.Call(sender, st.to(), msg.Data, st.gasRemaining, value)
 	}

core/vm/eips.go

@@ -924,22 +924,27 @@ func opReturnContract(pc *uint64, interpreter *EVMInterpreter, scope *ScopeConte
 	}
 	ret := scope.Memory.GetPtr(offset.Uint64(), size.Uint64())
 	containerCode := scope.Contract.Container.ContainerCode[idx]
-	deployedCode := append(containerCode, ret...)
+	if len(containerCode) == 0 {
-	if len(deployedCode) == 0 {
 		return nil, errors.New("nonexistant subcontainer")
 	}
 	// Validate the subcontainer
 	var c Container
-	if err := c.UnmarshalBinary(deployedCode, true); err != nil {
+	if err := c.UnmarshalSubContainer(containerCode, false); err != nil {
-		return nil, err
-	}
-	if err := c.ValidateCode(interpreter.tableEOF, true); err != nil {
 		return nil, err
 	}
+
+	// append the auxdata
+	c.Data = append(c.Data, ret...)
 	if len(c.Data) < c.DataSize {
-		return nil, errors.New("invalid subcontainer")
+		return nil, errors.New("incomplete aux data")
 	}
 	c.DataSize = len(c.Data)
+
+	// probably unneeded as subcontainers are deeply validated
+	if err := c.ValidateCode(interpreter.tableEOF, false); err != nil {
+		return nil, err
+	}
+
 	// Restore context
 	retCtx := scope.ReturnStack.Pop()
 	scope.CodeSection = retCtx.Section
@@ -1055,13 +1060,20 @@ func opExtCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]
 	if interpreter.readOnly && !value.IsZero() {
 		return nil, ErrWriteProtection
 	}
-	if !value.IsZero() {
+
-		gas += params.CallStipend
+	var (
+		ret       []byte
+		returnGas uint64
+		err       error
+	)
+	if interpreter.evm.callGasTemp == 0 {
+		// zero temp call gas indicates a min retained gas error
+		ret, returnGas, err = nil, 0, ErrExecutionReverted
+	} else {
+		ret, returnGas, err = interpreter.evm.Call(scope.Contract, toAddr, args, gas, &value)
 	}
 
-	ret, returnGas, err := interpreter.evm.Call(scope.Contract, toAddr, args, gas, &value)
+	if errors.Is(err, ErrExecutionReverted) || errors.Is(err, ErrInsufficientBalance) || errors.Is(err, ErrDepth) {
-
-	if err == ErrExecutionReverted {
 		temp.SetOne()
 	} else if err != nil {
 		temp.SetUint64(2)
@@ -1102,11 +1114,14 @@ func opExtDelegateCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeCont
 		err = ErrExecutionReverted
 		ret = nil
 		returnGas = gas
+	} else if interpreter.evm.callGasTemp == 0 {
+		// zero temp call gas indicates a min retained gas error
+		ret, returnGas, err = nil, 0, ErrExecutionReverted
 	} else {
 		ret, returnGas, err = interpreter.evm.DelegateCall(scope.Contract, toAddr, args, gas, true)
 	}
 
-	if err == ErrExecutionReverted {
+	if err == ErrExecutionReverted || err == ErrDepth {
 		temp.SetOne()
 	} else if err != nil {
 		temp.SetUint64(2)
@@ -1136,8 +1151,19 @@ func opExtStaticCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContex
 	// Get arguments from the memory.
 	args := scope.Memory.GetPtr(inOffset.Uint64(), inSize.Uint64())
 
-	ret, returnGas, err := interpreter.evm.StaticCall(scope.Contract, toAddr, args, gas)
+	var (
-	if err == ErrExecutionReverted {
+		ret       []byte
+		returnGas uint64
+		err       error
+	)
+	if interpreter.evm.callGasTemp == 0 {
+		// zero temp call gas indicates a min retained gas error
+		ret, returnGas, err = nil, 0, ErrExecutionReverted
+	} else {
+		ret, returnGas, err = interpreter.evm.StaticCall(scope.Contract, toAddr, args, gas)
+	}
+
+	if err == ErrExecutionReverted || err == ErrDepth {
 		temp.SetOne()
 	} else if err != nil {
 		temp.SetUint64(2)

core/vm/eof.go

@@ -145,10 +145,15 @@ func (c *Container) MarshalBinary() []byte {
 
 // UnmarshalBinary decodes an EOF container.
 func (c *Container) UnmarshalBinary(b []byte, isInitcode bool) error {
-	return c.unmarshalSubContainer(b, isInitcode, true)
+	return c.unmarshalContainer(b, isInitcode, true)
 }
 
-func (c *Container) unmarshalSubContainer(b []byte, isInitcode bool, topLevel bool) error {
+// UnmarshalSubContainer decodes an EOF container that is container in another container
+func (c *Container) UnmarshalSubContainer(b []byte, isInitcode bool) error {
+	return c.unmarshalContainer(b, isInitcode, false)
+}
+
+func (c *Container) unmarshalContainer(b []byte, isInitcode bool, topLevel bool) error {
 	if !hasEOFMagic(b) {
 		return fmt.Errorf("%w: want %x", ErrInvalidMagic, eofMagic)
 	}
@@ -240,7 +245,7 @@ func (c *Container) unmarshalSubContainer(b []byte, isInitcode bool, topLevel bo
 		return fmt.Errorf("%w: have %d, want %d", ErrInvalidContainerSize, len(b), expectedSize)
 	}
 	// Only check that the expected size is not exceed on non-initcode
-	if !isInitcode && len(b) > expectedSize {
+	if (!topLevel || !isInitcode) && len(b) > expectedSize {
 		return fmt.Errorf("%w: have %d, want %d", ErrInvalidContainerSize, len(b), expectedSize)
 	}
 
@@ -294,7 +299,7 @@ func (c *Container) unmarshalSubContainer(b []byte, isInitcode bool, topLevel bo
 			}
 			c := new(Container)
 			end := min(idx+size, len(b))
-			if err := c.unmarshalSubContainer(b[idx:end], isInitcode, false); err != nil {
+			if err := c.unmarshalContainer(b[idx:end], isInitcode, false); err != nil {
 				if topLevel {
 					return fmt.Errorf("%w in sub container %d", err, i)
 				}

core/vm/errors.go

@@ -42,7 +42,7 @@ var (
 	ErrInvalidEOFInitcode       = errors.New("invalid eof initcode")
 	ErrNonceUintOverflow        = errors.New("nonce uint64 overflow")
 	ErrInvalidNumberOfOutputs   = errors.New("invalid number of outputs")
-	ErrInvalidNonReturningFlag  = errors.New("Invalid non-returning flag, bad RETF")
+	ErrInvalidNonReturningFlag  = errors.New("invalid non-returning flag, bad RETF")
 
 	// errStopToken is an internal token indicating interpreter loop termination,
 	// never returned to outside callers.

core/vm/evm.go

@@ -457,7 +457,7 @@ func (c *codeAndHash) Hash() common.Hash {
 }
 
 // create creates a new contract using code as deployment code.
-func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64, value *uint256.Int, address common.Address, typ OpCode, input []byte, fromEOF bool) (ret []byte, createAddress common.Address, leftOverGas uint64, err error) {
+func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64, value *uint256.Int, address common.Address, typ OpCode, input []byte, allowEOF bool) (ret []byte, createAddress common.Address, leftOverGas uint64, err error) {
 	if evm.Config.Tracer != nil {
 		evm.captureBegin(evm.depth, typ, caller.Address(), address, codeAndHash.code, gas, value.ToBig())
 		defer func(startGas uint64) {
@@ -482,11 +482,8 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64,
 
 	// Validate initcode per EOF rules. If caller is EOF and initcode is legacy, fail.
 	isInitcodeEOF := hasEOFMagic(codeAndHash.code)
-	if evm.chainRules.IsPrague {
+	if isInitcodeEOF {
-		if isInitcodeEOF {
+		if allowEOF {
-			if !fromEOF {
-				return nil, common.Address{}, gas, fmt.Errorf("%w: %v", ErrInvalidEOFInitcode, ErrLegacyCode)
-			}
 			// If the initcode is EOF, verify it is well-formed.
 			var c Container
 			if err := c.UnmarshalBinary(codeAndHash.code, isInitcodeEOF); err != nil {
@@ -496,7 +493,7 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64,
 				return nil, common.Address{}, gas, fmt.Errorf("%w: %v", ErrInvalidEOFInitcode, err)
 			}
 			contract.Container = &c
-		} else if fromEOF {
+		} else {
 			// Don't allow EOF contract to execute legacy initcode.
 			return nil, common.Address{}, gas, ErrLegacyCode
 		}
@@ -573,7 +570,7 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64,
 
 	// Reject code starting with 0xEF if EIP-3541 is enabled.
 	if err == nil && len(ret) >= 1 && HasEOFByte(ret) {
-		if evm.chainRules.IsShanghai {
+		if evm.chainRules.IsPrague && isInitcodeEOF {
 			// Don't reject EOF contracts after Shanghai
 		} else if evm.chainRules.IsLondon {
 			err = ErrInvalidCode
@@ -619,9 +616,9 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64,
 }
 
 // Create creates a new contract using code as deployment code.
-func (evm *EVM) Create(caller ContractRef, code []byte, gas uint64, value *uint256.Int) (ret []byte, contractAddr common.Address, leftOverGas uint64, err error) {
+func (evm *EVM) Create(caller ContractRef, code []byte, gas uint64, value *uint256.Int, allowEOF bool) (ret []byte, contractAddr common.Address, leftOverGas uint64, err error) {
 	contractAddr = crypto.CreateAddress(caller.Address(), evm.StateDB.GetNonce(caller.Address()))
-	return evm.create(caller, &codeAndHash{code: code}, gas, value, contractAddr, CREATE, nil, false)
+	return evm.create(caller, &codeAndHash{code: code}, gas, value, contractAddr, CREATE, nil, allowEOF)
 }
 
 // Create2 creates a new contract using code as deployment code.

core/vm/gas.go

@@ -17,6 +17,7 @@
 package vm
 
 import (
+	"github.com/ethereum/go-ethereum/params"
 	"github.com/holiman/uint256"
 )
 
@@ -53,3 +54,32 @@ func callGas(isEip150 bool, availableGas, base uint64, callCost *uint256.Int) (u
 
 	return callCost.Uint64(), nil
 }
+
+// extCallGas returns the actual gas cost for ext*call operations.
+//
+// EOF v1 includes EIP-150 rules (all but 1/64) with a floor of MIN_RETAINED_GAS (5000)
+// and a minimum returned value of MIN_CALLE_GASS (2300).
+// There is also no call gas, so all available gas is used.
+//
+// If the minimum retained gas constraint is violated, zero gas and no error is returned
+func extCallGas(availableGas, base uint64) (uint64, error) {
+	if availableGas < base {
+		return 0, ErrOutOfGas
+	}
+	availableGas = availableGas - base
+	if availableGas < params.ExtCallMinRetainedGas {
+		return 0, nil
+	}
+
+	retainedGas := availableGas / 64
+	if retainedGas < params.ExtCallMinRetainedGas {
+		retainedGas = params.ExtCallMinRetainedGas
+	}
+	gas := availableGas - retainedGas
+
+	if gas < params.ExtCallMinCalleeGas {
+		return 0, nil
+	} else {
+		return gas, nil
+	}
+}

core/vm/gas_table.go

@@ -23,7 +23,6 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/math"
 	"github.com/ethereum/go-ethereum/params"
-	"github.com/holiman/uint256"
 )
 
 // memoryGasCost calculates the quadratic gas for memory expansion. It does so
@@ -484,37 +483,32 @@ func gasStaticCall(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memo
 func gasExtCall(evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) {
 	var (
 		gas            uint64
-		transfersValue = !stack.Back(2).IsZero()
+		transfersValue = !stack.Back(3).IsZero()
-		address        = common.Address(stack.Back(1).Bytes20())
+		address        = common.Address(stack.Back(0).Bytes20())
+		overflow       bool
 	)
-	if evm.chainRules.IsEIP158 {
+	if transfersValue {
-		if transfersValue && evm.StateDB.Empty(address) {
+		if evm.StateDB.Empty(address) {
 			gas += params.CallNewAccountGas
 		}
-	} else if !evm.StateDB.Exist(address) {
+		if evm.chainRules.IsEIP4762 {
-		gas += params.CallNewAccountGas
+			gas, overflow = math.SafeAdd(gas, evm.AccessEvents.ValueTransferGas(contract.Address(), address))
-	}
+			if overflow {
-	if transfersValue && !evm.chainRules.IsEIP4762 {
+				return 0, ErrGasUintOverflow
-		gas += params.CallValueTransferGas
+			}
+		} else {
+			gas += params.CallValueTransferGas
+		}
 	}
 	memoryGas, err := memoryGasCost(mem, memorySize)
 	if err != nil {
 		return 0, err
 	}
-	var overflow bool
 	if gas, overflow = math.SafeAdd(gas, memoryGas); overflow {
 		return 0, ErrGasUintOverflow
 	}
-	if evm.chainRules.IsEIP4762 {
-		if transfersValue {
-			gas, overflow = math.SafeAdd(gas, evm.AccessEvents.ValueTransferGas(contract.Address(), address))
-			if overflow {
-				return 0, ErrGasUintOverflow
-			}
-		}
-	}
 
-	evm.callGasTemp, err = callGas(true, contract.Gas, gas, new(uint256.Int).SetUint64(contract.Gas))
+	evm.callGasTemp, err = extCallGas(contract.Gas, gas)
 	if err != nil {
 		return 0, err
 	}
@@ -531,7 +525,7 @@ func gasExtDelegateCall(evm *EVM, contract *Contract, stack *Stack, mem *Memory,
 	if err != nil {
 		return 0, err
 	}
-	evm.callGasTemp, err = callGas(true, contract.Gas, gas, new(uint256.Int).SetUint64(contract.Gas))
+	evm.callGasTemp, err = extCallGas(contract.Gas, gas)
 	if err != nil {
 		return 0, err
 	}
@@ -547,7 +541,7 @@ func gasExtStaticCall(evm *EVM, contract *Contract, stack *Stack, mem *Memory, m
 	if err != nil {
 		return 0, err
 	}
-	evm.callGasTemp, err = callGas(true, contract.Gas, gas, new(uint256.Int).SetUint64(contract.Gas))
+	evm.callGasTemp, err = extCallGas(contract.Gas, gas)
 	if err != nil {
 		return 0, err
 	}

core/vm/instructions.go

@@ -715,7 +715,7 @@ func opCreate(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]b
 
 	scope.Contract.UseGas(gas, interpreter.evm.Config.Tracer, tracing.GasChangeCallContractCreation)
 
-	res, addr, returnGas, suberr := interpreter.evm.Create(scope.Contract, input, gas, &value)
+	res, addr, returnGas, suberr := interpreter.evm.Create(scope.Contract, input, gas, &value, false)
 	// Push item on the stack based on the returned error. If the ruleset is
 	// homestead we must check for CodeStoreOutOfGasError (homestead only
 	// rule) and treat as an error, if the ruleset is frontier we must

core/vm/runtime/runtime.go

@@ -185,6 +185,7 @@ func Create(input []byte, cfg *Config) ([]byte, common.Address, uint64, error) {
 		input,
 		cfg.GasLimit,
 		uint256.MustFromBig(cfg.Value),
+		false,
 	)
 	return code, address, leftOverGas, err
 }

core/vm/validate.go

@@ -155,9 +155,6 @@ func validateCode(code []byte, section int, container *Container, jt *JumpTable,
 			if ct := container.ContainerSections[arg]; len(ct.Data) != ct.DataSize {
 				return nil, fmt.Errorf("%w: container %d, have %d, claimed %d, pos %d", ErrEOFCreateWithTruncatedSection, arg, len(ct.Data), ct.DataSize, i)
 			}
-			if _, ok := visitedSubcontainers[arg]; ok {
-				return nil, fmt.Errorf("section already referenced, arg :%d", arg)
-			}
 			// We need to store per subcontainer how it was referenced
 			if v, ok := visitedSubcontainers[arg]; ok && v != RefByEOFCreate {
 				return nil, fmt.Errorf("section already referenced, arg :%d", arg)

params/protocol_params.go

@@ -89,6 +89,8 @@ const (
 	CreateNGasEip4762     uint64 = 1000  // Once per CREATEn operations post-verkle
 	SelfdestructRefundGas uint64 = 24000 // Refunded following a selfdestruct operation.
 	MemoryGas             uint64 = 3     // Times the address of the (highest referenced byte in memory + 1). NOTE: referencing happens on read, write and in instructions such as RETURN and CALL.
+	ExtCallMinRetainedGas uint64 = 5000  // For EXT*CALL this is the minimum gas that the EIp158 1/64th rule must retain
+	ExtCallMinCalleeGas   uint64 = 2300  // For EXT*CALL this is the minimum gas that must be passed to the callee, ignoring 63/64
 
 	TxDataNonZeroGasFrontier  uint64 = 68   // Per byte of data attached to a transaction that is not equal to zero. NOTE: Not payable on data of calls between transactions.
 	TxDataNonZeroGasEIP2028   uint64 = 16   // Per byte of non zero data attached to a transaction after EIP 2028 (part in Istanbul)

tests/state_test_util.go

@@ -259,10 +259,7 @@ func (t *StateTest) RunNoVerify(subtest StateSubtest, vmconfig vm.Config, snapsh
 	vmconfig.ExtraEips = eips
 
 	block := t.genesis(config).ToBlock()
-	genesisAlloc := t.json.Pre
+	st = MakePreState(rawdb.NewMemoryDatabase(), t.json.Pre, snapshotter, scheme)
-	genesisAlloc[params.BeaconRootsAddress] = types.Account{Nonce: 1, Code: params.BeaconRootsCode}
-	//genesisAlloc[params.HistoryStorageAddress] = types.Account{Nonce: 1, Code: params.HistoryStorageCode}
-	st = MakePreState(rawdb.NewMemoryDatabase(), genesisAlloc, snapshotter, scheme)
 
 	var baseFee *big.Int
 	if config.IsLondon(new(big.Int)) {
@@ -319,10 +316,6 @@ func (t *StateTest) RunNoVerify(subtest StateSubtest, vmconfig vm.Config, snapsh
 	if config.IsCancun(new(big.Int), block.Time()) && t.json.Env.ExcessBlobGas != nil {
 		context.BlobBaseFee = eip4844.CalcBlobFee(*t.json.Env.ExcessBlobGas)
 	}
-	{
-		evm := vm.NewEVM(context, vm.TxContext{}, st.StateDB, config, vmconfig)
-		core.ProcessBeaconBlockRoot(common.HexToHash("0x00"), evm, st.StateDB)
-	}
 
 	evm := vm.NewEVM(context, txContext, st.StateDB, config, vmconfig)