feat: Have ExecutionManager pass data upwards

.changeset/mean-comics-rush.md

@@ -0,0 +1,6 @@
+---
+"@eth-optimism/l2geth": patch
+"@eth-optimism/contracts": patch
+---
+
+Add ExecutionManager return data & RLP encoding

l2geth/core/vm/evm.go

@@ -133,13 +133,10 @@ type Context struct {
 	Difficulty  *big.Int       // Provides information for DIFFICULTY
 
 	// OVM_ADDITION
-	EthCallSender         *common.Address
-	OriginalTargetAddress *common.Address
-	OriginalTargetResult  []byte
-	OriginalTargetReached bool
-	OvmExecutionManager   dump.OvmDumpAccount
-	OvmStateManager       dump.OvmDumpAccount
-	OvmSafetyChecker      dump.OvmDumpAccount
+	EthCallSender       *common.Address
+	OvmExecutionManager dump.OvmDumpAccount
+	OvmStateManager     dump.OvmDumpAccount
+	OvmSafetyChecker    dump.OvmDumpAccount
 }
 
 // EVM is the Ethereum Virtual Machine base object and provides
@@ -251,34 +248,6 @@ func (evm *EVM) Interpreter() Interpreter {
 // the necessary steps to create accounts and reverses the state in case of an
 // execution error or failed value transfer.
 func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas uint64, value *big.Int) (ret []byte, leftOverGas uint64, err error) {
-	var isTarget = false
-	if UsingOVM {
-		// OVM_ENABLED
-		if evm.depth == 0 {
-			// We're inside a new transaction, so make sure to wipe these variables beforehand.
-			evm.Context.OriginalTargetAddress = nil
-			evm.Context.OriginalTargetResult = []byte("00")
-			evm.Context.OriginalTargetReached = false
-		}
-
-		if caller.Address() == evm.Context.OvmExecutionManager.Address &&
-			!bytes.HasPrefix(addr.Bytes(), deadPrefix) &&
-			!bytes.HasPrefix(addr.Bytes(), zeroPrefix) &&
-			!bytes.HasPrefix(addr.Bytes(), fortyTwoPrefix) &&
-			evm.Context.OriginalTargetAddress == nil {
-			// Whew. Okay, so: we consider ourselves to be at a "target" as long as we were called
-			// by the execution manager, and we're not a precompile or "dead" address.
-			evm.Context.OriginalTargetAddress = &addr
-			evm.Context.OriginalTargetReached = true
-			isTarget = true
-		}
-		// Handle eth_call
-		if evm.Context.EthCallSender != nil && (caller.Address() == common.Address{}) {
-			evm.Context.OriginalTargetReached = true
-			isTarget = true
-		}
-	}
-
 	if evm.vmConfig.NoRecursion && evm.depth > 0 {
 		return nil, gas, nil
 	}
@@ -357,28 +326,17 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas
 	if UsingOVM {
 		// OVM_ENABLED
 
-		if isTarget {
-			// If this was our target contract, store the result so that it can be later re-inserted
-			// into the user-facing return data (as seen below).
-			evm.Context.OriginalTargetResult = ret
-		}
-
 		if evm.depth == 0 {
 			// We're back at the root-level message call, so we'll need to modify the return data
 			// sent to us by the OVM_ExecutionManager to instead be the intended return data.
 
-			if !evm.Context.OriginalTargetReached {
-				// If we didn't get to the target contract, then our execution somehow failed
-				// (perhaps due to insufficient gas). Just return an error that represents this.
-				ret = common.FromHex("0x")
-				err = ErrOvmExecutionFailed
-			} else if len(evm.Context.OriginalTargetResult) >= 96 {
+			if len(ret) >= 96 {
 				// We expect that EOA contracts return at least 96 bytes of data, where the first
 				// 32 bytes are the boolean success value and the next 64 bytes are unnecessary
 				// ABI encoding data. The actual return data starts at the 96th byte and can be
 				// empty.
-				success := evm.Context.OriginalTargetResult[:32]
-				ret = evm.Context.OriginalTargetResult[96:]
+				success := ret[:32]
+				ret = ret[96:]
 
 				if !bytes.Equal(success, AbiBytesTrue) && !bytes.Equal(success, AbiBytesFalse) {
 					// If the first 32 bytes not either are the ABI encoding of "true" or "false",
@@ -390,17 +348,6 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas
 					// If the first 32 bytes are the ABI encoding of "false", then we need to add an
 					// artificial error that represents the revert.
 					err = errExecutionReverted
-
-					// We also currently need to add an extra four empty bytes to the return data
-					// to appease ethers.js. Our return correctly inserts the four specific bytes
-					// that represent a "string error" to clients, but somehow the returndata size
-					// is a multiple of 32 (when we expect size % 32 == 4). ethers.js checks that
-					// [size % 32 == 4] before trying to decode a string error result. Adding these
-					// four empty bytes tricks ethers into correctly decoding the error string.
-					// ovmTODO: Figure out how to actually deal with this.
-					// ovmTODO: This may actually be completely broken if the first four bytes of
-					// the return data are **not** the specific "string error" bytes.
-					ret = append(ret, make([]byte, 4)...)
 				}
 			} else {
 				// User hasn't conformed the standard format, just return "null" for the success

packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol

@@ -162,11 +162,14 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
     )
         override
         public
+        returns (
+            bytes memory
+        )
     {
         // Make sure that run() is not re-enterable.  This condition should always be satisfied
         // Once run has been called once, due to the behavior of _isValidInput().
         if (transactionContext.ovmNUMBER != DEFAULT_UINT256) {
-            return;
+            return bytes("");
         }
 
         // Store our OVM_StateManager instance (significantly easier than attempting to pass the
@@ -194,15 +197,15 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
         // reverts for INVALID_STATE_ACCESS.
         if (_isValidInput(_transaction) == false) {
             _resetContext();
-            return;
+            return bytes("");
         }
 
         // TEMPORARY: Gas metering is disabled for minnet.
         // // Check gas right before the call to get total gas consumed by OVM transaction.
         // uint256 gasProvided = gasleft();
 
         // Run the transaction, make sure to meter the gas usage.
-        ovmCALL(
+        (, bytes memory returndata) = ovmCALL(
             _transaction.gasLimit - gasMeterConfig.minTransactionGasLimit,
             _transaction.entrypoint,
             _transaction.data
@@ -215,6 +218,8 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
 
         // Wipe the execution context.
         _resetContext();
+
+        return returndata;
     }
 
 

packages/contracts/contracts/optimistic-ethereum/OVM/predeploys/OVM_SequencerEntrypoint.sol

@@ -2,9 +2,6 @@
 // @unsupported: evm
 pragma solidity >0.5.0 <0.8.0;
 
-/* Interface Imports */
-import { iOVM_ECDSAContractAccount } from "../../iOVM/accounts/iOVM_ECDSAContractAccount.sol";
-
 /* Library Imports */
 import { Lib_EIP155Tx } from "../../libraries/codec/Lib_EIP155Tx.sol";
 import { Lib_ExecutionManagerWrapper } from "../../libraries/wrappers/Lib_ExecutionManagerWrapper.sol";
@@ -69,6 +66,21 @@ contract OVM_SequencerEntrypoint {
         }
 
         // Forward the transaction over to the EOA.
-        iOVM_ECDSAContractAccount(target).execute(encodedTx);
+        (bool success, bytes memory returndata) = target.call(
+            abi.encodeWithSignature(
+                "execute(bytes)",
+                encodedTx
+            )
+        );
+
+        if (success) {
+            assembly {
+                return(add(returndata, 0x20), mload(returndata))
+            }
+        } else {
+            assembly {
+                revert(add(returndata, 0x20), mload(returndata))
+            }
+        }
     }
 }

packages/contracts/contracts/optimistic-ethereum/iOVM/execution/iOVM_ExecutionManager.sol

@@ -75,7 +75,7 @@ interface iOVM_ExecutionManager {
     function run(
         Lib_OVMCodec.Transaction calldata _transaction,
         address _txStateManager
-    ) external;
+    ) external returns (bytes memory);
 
 
     /*******************

packages/contracts/test/contracts/OVM/accounts/OVM_ECDSAContractAccount.spec.ts

@@ -137,7 +137,6 @@ describe('OVM_ECDSAContractAccount', () => {
 
       // The ovmCALL is the 2nd call because the first call transfers the fee.
       const ovmCALL: any = Mock__OVM_ExecutionManager.smocked.ovmCALL.calls[1]
-      expect(ovmCALL._gasLimit).to.equal(DEFAULT_EIP155_TX.gasLimit)
       expect(ovmCALL._address).to.equal(DEFAULT_EIP155_TX.to)
       expect(ovmCALL._calldata).to.equal(DEFAULT_EIP155_TX.data)
     })

packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager.gas-spec.ts

@@ -110,7 +110,7 @@ describe('OVM_ExecutionManager gas consumption', () => {
       )
       console.log(`calculated gas cost of ${gasCost}`)
 
-      const benchmark: number = 105_000
+      const benchmark: number = 106_000
       expect(gasCost).to.be.lte(benchmark)
       expect(gasCost).to.be.gte(
         benchmark - 1_000,