diff --git a/.changeset/ten-baboons-compete.md b/.changeset/ten-baboons-compete.md new file mode 100644 index 000000000000..66a3b3f6af3a --- /dev/null +++ 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 diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol index ee013f1e6934..1e13cc04b23b 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/accounts/OVM_ECDSAContractAccount.sol @@ -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; + } } diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol index dba6fa1db2ff..b05273245471 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol @@ -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); diff --git a/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_StateManager.sol b/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_StateManager.sol index 2d8a95bc13e6..0f08a30d976b 100644 --- a/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_StateManager.sol +++ b/packages/contracts/contracts/optimistic-ethereum/OVM/execution/OVM_StateManager.sol @@ -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; diff --git a/packages/contracts/contracts/optimistic-ethereum/iOVM/execution/iOVM_ExecutionManager.sol b/packages/contracts/contracts/optimistic-ethereum/iOVM/execution/iOVM_ExecutionManager.sol index 9be045cfc748..3f4cfe889004 100644 --- a/packages/contracts/contracts/optimistic-ethereum/iOVM/execution/iOVM_ExecutionManager.sol +++ b/packages/contracts/contracts/optimistic-ethereum/iOVM/execution/iOVM_ExecutionManager.sol @@ -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; diff --git a/packages/contracts/contracts/optimistic-ethereum/iOVM/execution/iOVM_StateManager.sol b/packages/contracts/contracts/optimistic-ethereum/iOVM/execution/iOVM_StateManager.sol index 1c4870a5841a..9497752ad9ba 100644 --- a/packages/contracts/contracts/optimistic-ethereum/iOVM/execution/iOVM_StateManager.sol +++ b/packages/contracts/contracts/optimistic-ethereum/iOVM/execution/iOVM_StateManager.sol @@ -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; diff --git a/packages/contracts/contracts/optimistic-ethereum/libraries/codec/Lib_OVMCodec.sol b/packages/contracts/contracts/optimistic-ethereum/libraries/codec/Lib_OVMCodec.sol index 0cb7245956a5..4c3c94a7424b 100644 --- a/packages/contracts/contracts/optimistic-ethereum/libraries/codec/Lib_OVMCodec.sol +++ b/packages/contracts/contracts/optimistic-ethereum/libraries/codec/Lib_OVMCodec.sol @@ -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]) diff --git a/packages/contracts/contracts/optimistic-ethereum/libraries/rlp/Lib_RLPReader.sol b/packages/contracts/contracts/optimistic-ethereum/libraries/rlp/Lib_RLPReader.sol index 5e81c5de2ae6..b0b198996a8a 100644 --- a/packages/contracts/contracts/optimistic-ethereum/libraries/rlp/Lib_RLPReader.sol +++ b/packages/contracts/contracts/optimistic-ethereum/libraries/rlp/Lib_RLPReader.sol @@ -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. diff --git a/packages/contracts/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol b/packages/contracts/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol index 14dc89bdf8ed..b0885d6a879f 100644 --- a/packages/contracts/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol +++ b/packages/contracts/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol @@ -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)); } /** diff --git a/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager/nuisance-gas.spec.ts b/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager/nuisance-gas.spec.ts index 821ee3fd9b82..0b60db0926ca 100644 --- a/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager/nuisance-gas.spec.ts +++ b/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager/nuisance-gas.spec.ts @@ -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, }, }, }, diff --git a/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager/ovmCREATE.spec.ts b/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager/ovmCREATE.spec.ts index 147fac7be9f0..c4caf60103cd 100644 --- a/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager/ovmCREATE.spec.ts +++ b/packages/contracts/test/contracts/OVM/execution/OVM_ExecutionManager/ovmCREATE.spec.ts @@ -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: [ {