From ec2ac96adbe52f946a74a8776a14ddd499ac6bd0 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Fri, 19 Mar 2021 14:24:32 -0700 Subject: [PATCH 1/3] Start working on ExecutionManager return data --- .../OVM/accounts/OVM_ProxyEOA.sol | 4 ++-- .../OVM/execution/OVM_ExecutionManager.sol | 10 ++++++++-- .../OVM/precompiles/OVM_SequencerEntrypoint.sol | 14 ++++++++++++-- .../iOVM/execution/iOVM_ExecutionManager.sol | 7 ++++++- .../Lib_SafeExecutionManagerWrapper.sol | 17 +++++++++++++---- 5 files changed, 41 insertions(+), 11 deletions(-) diff --git a/contracts/optimistic-ethereum/OVM/accounts/OVM_ProxyEOA.sol b/contracts/optimistic-ethereum/OVM/accounts/OVM_ProxyEOA.sol index 0f376f821..904b23d2a 100644 --- a/contracts/optimistic-ethereum/OVM/accounts/OVM_ProxyEOA.sol +++ b/contracts/optimistic-ethereum/OVM/accounts/OVM_ProxyEOA.sol @@ -58,8 +58,8 @@ contract OVM_ProxyEOA { return(add(returndata, 0x20), mload(returndata)) } } else { - Lib_SafeExecutionManagerWrapper.safeREVERT( - string(returndata) + Lib_SafeExecutionManagerWrapper.safeREVERTbytes( + returndata ); } } diff --git a/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol b/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol index af4c03b8c..51818cdb6 100644 --- a/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol +++ b/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol @@ -158,6 +158,10 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ) override public + returns ( + bool, + bytes memory + ) { require(transactionContext.ovmNUMBER == 0, "Only be callable at the start of a transaction"); // Store our OVM_StateManager instance (significantly easier than attempting to pass the @@ -185,7 +189,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { // reverts for INVALID_STATE_ACCESS. if (_isValidGasLimit(_transaction.gasLimit, _transaction.l1QueueOrigin) == false) { _resetContext(); - return; + return (false, bytes("")); } // TEMPORARY: Gas metering is disabled for minnet. @@ -193,7 +197,7 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { // uint256 gasProvided = gasleft(); // Run the transaction, make sure to meter the gas usage. - ovmCALL( + (bool success, bytes memory returndata) = ovmCALL( _transaction.gasLimit - gasMeterConfig.minTransactionGasLimit, _transaction.entrypoint, _transaction.data @@ -209,6 +213,8 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { // Reset the ovmStateManager. ovmStateManager = iOVM_StateManager(address(0)); + + return (success, returndata); } diff --git a/contracts/optimistic-ethereum/OVM/precompiles/OVM_SequencerEntrypoint.sol b/contracts/optimistic-ethereum/OVM/precompiles/OVM_SequencerEntrypoint.sol index bba7dd59c..4371719d4 100644 --- a/contracts/optimistic-ethereum/OVM/precompiles/OVM_SequencerEntrypoint.sol +++ b/contracts/optimistic-ethereum/OVM/precompiles/OVM_SequencerEntrypoint.sol @@ -89,13 +89,23 @@ contract OVM_SequencerEntrypoint { s ); - Lib_SafeExecutionManagerWrapper.safeCALL( + (bool success, bytes memory returndata) = Lib_SafeExecutionManagerWrapper.safeCALL( gasleft(), target, callbytes ); + + if (success) { + assembly { + return(add(returndata, 0x20), mload(returndata)) + } + } else { + Lib_SafeExecutionManagerWrapper.safeREVERTbytes( + returndata + ); + } } - + /********************** * Internal Functions * diff --git a/contracts/optimistic-ethereum/iOVM/execution/iOVM_ExecutionManager.sol b/contracts/optimistic-ethereum/iOVM/execution/iOVM_ExecutionManager.sol index 404822fc9..5e39c0776 100644 --- a/contracts/optimistic-ethereum/iOVM/execution/iOVM_ExecutionManager.sol +++ b/contracts/optimistic-ethereum/iOVM/execution/iOVM_ExecutionManager.sol @@ -75,7 +75,12 @@ interface iOVM_ExecutionManager { function run( Lib_OVMCodec.Transaction calldata _transaction, address _txStateManager - ) external; + ) + external + returns ( + bool, + bytes memory + ); /******************* diff --git a/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol b/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol index 5ebe12fb4..dbe205799 100644 --- a/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol +++ b/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol @@ -250,6 +250,19 @@ library Lib_SafeExecutionManagerWrapper { ); } + function safeREVERTbytes( + bytes memory _revertdata + ) + internal + { + _safeExecutionManagerInteraction( + abi.encodeWithSignature( + "ovmREVERT(bytes)", + _revertdata + ) + ); + } + /** * Performs a safe REVERT. * @param _reason String revert reason to pass along with the REVERT. @@ -355,10 +368,6 @@ library Lib_SafeExecutionManagerWrapper { assembly { revert(add(returndata, 0x20), mload(returndata)) } - } else if (returndata.length == 1) { - assembly { - return(0, 1) - } } else { return returndata; } From 7874d16ff674741d277473912208780a11395bd2 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Mon, 22 Mar 2021 17:54:09 -0700 Subject: [PATCH 2/3] Add a quick comment --- .../optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol b/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol index 51818cdb6..dabffef81 100644 --- a/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol +++ b/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol @@ -151,6 +151,8 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { * Starts the execution of a transaction via the OVM_ExecutionManager. * @param _transaction Transaction data to be executed. * @param _ovmStateManager iOVM_StateManager implementation providing account state. + * @return `true` if the top-level CALL succeeded, `false` if it reverted. + * @return Data returned by the top-level CALL. */ function run( Lib_OVMCodec.Transaction memory _transaction, From 7cd413bed882c1b9d55bf7d8406531ee749d5ac7 Mon Sep 17 00:00:00 2001 From: Kelvin Fichter Date: Mon, 22 Mar 2021 17:55:14 -0700 Subject: [PATCH 3/3] Add another comment --- .../Lib_SafeExecutionManagerWrapper.sol | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol b/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol index dbe205799..c59c644b5 100644 --- a/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol +++ b/contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol @@ -250,34 +250,38 @@ library Lib_SafeExecutionManagerWrapper { ); } - function safeREVERTbytes( - bytes memory _revertdata + /** + * Performs a safe REVERT. + * @param _reason String revert reason to pass along with the REVERT. + */ + function safeREVERT( + string memory _reason ) internal { _safeExecutionManagerInteraction( abi.encodeWithSignature( "ovmREVERT(bytes)", - _revertdata + Lib_ErrorUtils.encodeRevertString( + _reason + ) ) ); } /** - * Performs a safe REVERT. - * @param _reason String revert reason to pass along with the REVERT. + * Same as safeREVERT, but does *not* attach the sighash of Error(string). + * @param _revertdata Data to revert with. */ - function safeREVERT( - string memory _reason + function safeREVERTbytes( + bytes memory _revertdata ) internal { _safeExecutionManagerInteraction( abi.encodeWithSignature( "ovmREVERT(bytes)", - Lib_ErrorUtils.encodeRevertString( - _reason - ) + _revertdata ) ); }