ethereum-optimism · maurelian · Mar 15, 2021 · Mar 15, 2021 · Mar 8, 2021 · Apr 15, 2021
diff --git a/.changeset/ten-baboons-compete.md b/.changeset/ten-baboons-compete.md
@@ -0,0 +1,7 @@
+---
+"@eth-optimism/contracts": minor
+---
+
+- Reduce nonce size to uint64
+- Ban ovmCALLER opcode when it would return ZERO
+- Add value transfer in OVM_ECDSAContractAccount
@@ -101,13 +101,10 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
         // Transfer fee to relayer.
         address relayer = Lib_SafeExecutionManagerWrapper.safeCALLER();
         uint256 fee = Lib_SafeMathWrapper.mul(decodedTx.gasLimit, decodedTx.gasPrice);
-        (bool success, ) = Lib_SafeExecutionManagerWrapper.safeCALL(
-            gasleft(),
-            ETH_ERC20_ADDRESS,
-            abi.encodeWithSignature("transfer(address,uint256)", relayer, fee)
-        );
         Lib_SafeExecutionManagerWrapper.safeREQUIRE(
-            success == true,
+            _attemptETHTransfer(
+                relayer, fee
+            ),
             "Fee was not transferred to relayer."
         );
 
@@ -130,11 +127,50 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
             // cases, but since this is a contract we'd end up bumping the nonce twice.
             Lib_SafeExecutionManagerWrapper.safeINCREMENTNONCE();
 
-            return Lib_SafeExecutionManagerWrapper.safeCALL(
-                gasleft(),
-                decodedTx.to,
-                decodedTx.data
-            );
+            if (decodedTx.value > 0) {
+                Lib_SafeExecutionManagerWrapper.safeREQUIRE(
+                    decodedTx.data.length == 0,
+                    "Sending ETH with data is currently unsupported."
+                );
+
+                return (
+                    _attemptETHTransfer(decodedTx.to, decodedTx.value),
+                    bytes('')
+                );
+            } else {
+                return Lib_SafeExecutionManagerWrapper.safeCALL(
+                    gasleft(),
+                    decodedTx.to,
+                    decodedTx.data
+                );
+            }
         }
     }
+
+    /**
+     * Attempts to tansfer OVM_ETH.
+     * @param _to Address to send the L2 ETH to.
+     * @param _value Amount of L2 ETH to send.
+     * @return Whether the transfer was successful.
+     */
+    function _attemptETHTransfer(
+        address _to,
+        uint256 _value
+    )
+        internal
+        returns(
+            bool
+        )
+    {
+        (bool success, ) = Lib_SafeExecutionManagerWrapper.safeCALL(
+            gasleft(),
+            ETH_ERC20_ADDRESS,
+            abi.encodeWithSignature(
+                "transfer(address,uint256)",
+                _to,
+                _value
+            )
+        );
+        return success;
+    }
 }
@@ -460,7 +460,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
         override
         public
         returns (
-            uint256 _nonce
+            uint64 _nonce
         )
     {
         return _getAccountNonce(ovmADDRESS());
@@ -475,7 +475,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
         notStatic
     {
         address account = ovmADDRESS();
-        uint256 nonce = _getAccountNonce(account);
+        uint64 nonce = _getAccountNonce(account);
 
         // Prevent overflow.
         if (nonce + 1 > nonce) {
@@ -1050,15 +1050,15 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
      * This function sanitizes the return types for creation messages to match calls (bool, bytes),
      * by being an external function which the EM can call, that mimics the success/fail case of the CREATE.
      * This allows for consistent handling of both types of messages in _handleExternalMessage().
-     * Having this step occur as a separate call frame also allows us to easily revert the 
+     * Having this step occur as a separate call frame also allows us to easily revert the
      * contract deployment in the event that the code is unsafe.
-     * 
-     * @param _gasLimit Amount of gas to be passed into this creation.
+     *
+     * param _gasLimit Amount of gas to be passed into this creation.
      * @param _creationCode Code to pass into CREATE for deployment.
      * @param _address OVM address being deployed to.
      */
     function safeCREATE(
-        uint _gasLimit,
+        uint, // _gasLimit
         bytes memory _creationCode,
         address _address
     )
@@ -1097,7 +1097,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
         if (ethAddress == address(0)) {
             // If the creation fails, the EVM lets us grab its revert data. This may contain a revert flag
             // to be used above in _handleExternalMessage, so we pass the revert data back up unmodified.
-            assembly { 
+            assembly {
                 returndatacopy(0,0,returndatasize())
                 revert(0, returndatasize())
             }
@@ -1167,7 +1167,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
      */
     function _setAccountNonce(
         address _address,
-        uint256 _nonce
+        uint64 _nonce
     )
         internal
     {
@@ -1185,7 +1185,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
     )
         internal
         returns (
-            uint256 _nonce
+            uint64 _nonce
         )
     {
         _checkAccountLoad(_address);

@@ -207,7 +207,7 @@ contract OVM_StateManager is iOVM_StateManager {
      */
     function setAccountNonce(
         address _address,
-        uint256 _nonce
+        uint64 _nonce
     )
         override
         public
@@ -228,7 +228,7 @@ contract OVM_StateManager is iOVM_StateManager {
         public
         view
         returns (
-            uint256
+            uint64
         )
     {
         return accounts[_address].nonce;

@@ -117,7 +117,7 @@ interface iOVM_ExecutionManager {
      * Account Abstraction Opcodes *
      ******************************/
 
-    function ovmGETNONCE() external returns (uint256 _nonce);
+    function ovmGETNONCE() external returns (uint64 _nonce);
     function ovmINCREMENTNONCE() external;
     function ovmCREATEEOA(bytes32 _messageHash, uint8 _v, bytes32 _r, bytes32 _s) external;
 

@@ -45,8 +45,8 @@ interface iOVM_StateManager {
     function getAccount(address _address) external view returns (Lib_OVMCodec.Account memory _account);
     function hasAccount(address _address) external view returns (bool _exists);
     function hasEmptyAccount(address _address) external view returns (bool _exists);
-    function setAccountNonce(address _address, uint256 _nonce) external;
-    function getAccountNonce(address _address) external view returns (uint256 _nonce);
+    function setAccountNonce(address _address, uint64 _nonce) external;
+    function getAccountNonce(address _address) external view returns (uint64 _nonce);
     function getAccountEthAddress(address _address) external view returns (address _ethAddress);
     function getAccountStorageRoot(address _address) external view returns (bytes32 _storageRoot);
     function initPendingAccount(address _address) external;

@@ -34,7 +34,7 @@ library Lib_OVMCodec {
      ***********/
 
     struct Account {
-        uint256 nonce;
+        uint64 nonce;
         uint256 balance;
         bytes32 storageRoot;
         bytes32 codeHash;
@@ -43,7 +43,7 @@ library Lib_OVMCodec {
     }
 
     struct EVMAccount {
-        uint256 nonce;
+        uint64 nonce;
         uint256 balance;
         bytes32 storageRoot;
         bytes32 codeHash;
@@ -307,7 +307,7 @@ library Lib_OVMCodec {
         // index-by-index circumvents this issue.
         raw[0] = Lib_RLPWriter.writeBytes(
             Lib_Bytes32Utils.removeLeadingZeros(
-                bytes32(_account.nonce)
+                bytes32(uint256(_account.nonce))
             )
         );
         raw[1] = Lib_RLPWriter.writeBytes(
@@ -338,7 +338,7 @@ library Lib_OVMCodec {
         Lib_RLPReader.RLPItem[] memory accountState = Lib_RLPReader.readList(_encoded);
 
         return EVMAccount({
-            nonce: Lib_RLPReader.readUint256(accountState[0]),
+            nonce: uint64(Lib_RLPReader.readUint256(accountState[0])),
             balance: Lib_RLPReader.readUint256(accountState[1]),
             storageRoot: Lib_RLPReader.readBytes32(accountState[2]),
             codeHash: Lib_RLPReader.readBytes32(accountState[3])

@@ -425,6 +425,28 @@ library Lib_RLPReader {
         );
     }
 
+    /**
+     * Reads an RLP Uint64 value into a uint64.
+     * @param _in RLP uint64 value.
+     * @return Decoded uint64.
+     */
+    function readUint64(
+        RLPItem memory _in
+    )
+        internal
+        pure
+        returns (
+            uint64
+        )
+    {
+        require(
+            _in.length <= 9,
+            "Invalid RLP uint64 value."
+        );
+
+        return uint64(readUint256(_in));
+    }
+
     /**
      * Reads the raw bytes of an RLP item.
      * @param _in RLP item to read.

@@ -6,10 +6,10 @@ import { Lib_ErrorUtils } from "../utils/Lib_ErrorUtils.sol";
 
 /**
  * @title Lib_SafeExecutionManagerWrapper
- * @dev The Safe Execution Manager Wrapper provides functions which facilitate writing OVM safe 
- * code using the standard solidity compiler, by routing all its operations through the Execution 
+ * @dev The Safe Execution Manager Wrapper provides functions which facilitate writing OVM safe
+ * code using the standard solidity compiler, by routing all its operations through the Execution
  * Manager.
- * 
+ *
  * Compiler used: solc
  * Runtime target: OVM
  */
@@ -195,7 +195,7 @@ library Lib_SafeExecutionManagerWrapper {
     function safeGETNONCE()
         internal
         returns (
-            uint256 _nonce
+            uint64 _nonce
         )
     {
         bytes memory returndata = _safeExecutionManagerInteraction(
@@ -204,7 +204,7 @@ library Lib_SafeExecutionManagerWrapper {
             )
         );
 
-        return abi.decode(returndata, (uint256));
+        return abi.decode(returndata, (uint64));
     }
 
     /**

@@ -188,7 +188,7 @@ const test_nuisanceGas: TestDefinition = {
             // This is because there is natural gas consumption between the ovmCALL(GAS/2) and ovmCREATE, which allots nuisance gas via _getNuisanceGasLimit.
             // This means that the ovmCREATE exception, DOES consumes all nuisance gas allotted, but that allotment
             // is less than the full OVM_TX_GAS_LIMIT / 2 which is alloted to the parent ovmCALL.
-            nuisanceGasLeft: 4184829,
+            nuisanceGasLeft: 4185958,
           },
         },
       },

@@ -744,6 +744,24 @@ const test_ovmCREATE: TestDefinition = {
         },
       ],
     },
+    {
+      name: 'ovmCREATE(UNSAFE_CODE)',
+      steps: [
+        {
+          functionName: 'ovmCREATE',
+          functionParams: {
+            bytecode: UNSAFE_BYTECODE,
+          },
+          expectedReturnStatus: true,
+          expectedReturnValue: {
+            address: constants.AddressZero,
+            revertData: encodeSolidityError(
+              'Constructor attempted to deploy unsafe bytecode.'
+            ),
+          },
+        },
+      ],
+    },
   ],
   subTests: [
     {