♻️ Remove need to return results from ERC7821 execute (#1220)

Author: Vectorized

src/accounts/ERC7821.sol

@@ -28,8 +28,7 @@ contract ERC7821 is Receiver {
     /*                    EXECUTION OPERATIONS                    */
     /*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/
 
-    /// @dev Executes the `calls` in `executionData` and returns the results.
-    /// The `results` are the returned data from each call.
+    /// @dev Executes the calls in `executionData`.
     /// Reverts and bubbles up error if any call fails.
     ///
     /// `executionData` encoding:
@@ -49,12 +48,7 @@ contract ERC7821 is Receiver {
     ///
     /// `opData` may be used to store additional data for authentication,
     /// paymaster data, gas limits, etc.
-    function execute(bytes32 mode, bytes calldata executionData)
-        public
-        payable
-        virtual
-        returns (bytes[] memory)
-    {
+    function execute(bytes32 mode, bytes calldata executionData) public payable virtual {
         uint256 id = _executionModeId(mode);
         Call[] calldata calls;
         bytes calldata opData;
@@ -64,7 +58,7 @@ contract ERC7821 is Receiver {
                 mstore(0x00, 0x7f181275) // `UnsupportedExecutionMode()`.
                 revert(0x1c, 0x04)
             }
-            // Use inline assembly to extract the `calls` and optional `opData` efficiently.
+            // Use inline assembly to extract the calls and optional `opData` efficiently.
             opData.length := 0
             let o := add(executionData.offset, calldataload(executionData.offset))
             calls.offset := add(o, 0x20)
@@ -76,7 +70,7 @@ contract ERC7821 is Receiver {
                 opData.length := calldataload(q)
             }
         }
-        return _execute(mode, executionData, calls, opData);
+        _execute(mode, executionData, calls, opData);
     }
 
     /// @dev Provided for execution mode support detection.
@@ -105,15 +99,15 @@ contract ERC7821 is Receiver {
         }
     }
 
-    /// @dev Executes the `calls` and returns the results.
+    /// @dev Executes the calls.
     /// Reverts and bubbles up error if any call fails.
     /// The `mode` and `executionData` are passed along in case there's a need to use them.
     function _execute(
         bytes32 mode,
         bytes calldata executionData,
         Call[] calldata calls,
         bytes calldata opData
-    ) internal virtual returns (bytes[] memory) {
+    ) internal virtual {
         // Silence compiler warning on unused variables.
         mode = mode;
         executionData = executionData;
@@ -127,20 +121,10 @@ contract ERC7821 is Receiver {
         revert(); // In your override, replace this with logic to operate on `opData`.
     }
 
-    /// @dev Executes the `calls` and returns the results.
+    /// @dev Executes the calls.
     /// Reverts and bubbles up error if any call fails.
     /// `extraData` can be any supplementary data (e.g. a memory pointer, some hash).
-    function _execute(Call[] calldata calls, bytes32 extraData)
-        internal
-        virtual
-        returns (bytes[] memory results)
-    {
-        /// @solidity memory-safe-assembly
-        assembly {
-            results := mload(0x40) // Grab the free memory pointer.
-            mstore(results, calls.length) // Store the length of results.
-            mstore(0x40, add(add(results, 0x20), shl(5, calls.length))) // Allocate memory.
-        }
+    function _execute(Call[] calldata calls, bytes32 extraData) internal virtual {
         uint256 n = calls.length << 5;
         for (uint256 j; j != n;) {
             address target;
@@ -158,36 +142,27 @@ contract ERC7821 is Receiver {
                 data.length := calldataload(o)
                 j := add(j, 0x20)
             }
-            bytes memory r = _execute(target, value, data, extraData);
-            /// @solidity memory-safe-assembly
-            assembly {
-                mstore(add(results, j), r) // Set `results[i]` to `r`.
-            }
+            _execute(target, value, data, extraData);
         }
     }
 
-    /// @dev Executes the `calls` and returns the result.
+    /// @dev Executes the call.
     /// Reverts and bubbles up error if any call fails.
     /// `extraData` can be any supplementary data (e.g. a memory pointer, some hash).
     function _execute(address target, uint256 value, bytes calldata data, bytes32 extraData)
         internal
         virtual
-        returns (bytes memory result)
     {
         /// @solidity memory-safe-assembly
         assembly {
-            result := mload(0x40) // Grab the free memory pointer.
-            calldatacopy(result, data.offset, data.length)
-            if iszero(call(gas(), target, value, result, data.length, codesize(), 0x00)) {
+            extraData := extraData // Silence unused variable compiler warning.
+            let m := mload(0x40) // Grab the free memory pointer.
+            calldatacopy(m, data.offset, data.length)
+            if iszero(call(gas(), target, value, m, data.length, codesize(), 0x00)) {
                 // Bubble up the revert if the call reverts.
-                returndatacopy(result, 0x00, returndatasize())
-                revert(result, returndatasize())
+                returndatacopy(m, 0x00, returndatasize())
+                revert(m, returndatasize())
             }
-            mstore(result, returndatasize()) // Store the length.
-            let o := add(result, 0x20)
-            returndatacopy(o, 0x00, returndatasize()) // Copy the returndata.
-            mstore(0x40, add(o, returndatasize())) // Allocate the memory.
-            extraData := extraData // Silence unused variable compiler warning.
         }
     }
 }

src/accounts/Timelock.sol

@@ -305,7 +305,7 @@ contract Timelock is ERC7821, EnumerableRoles {
         bytes calldata executionData,
         Call[] calldata calls,
         bytes calldata opData
-    ) internal virtual override(ERC7821) returns (bytes[] memory results) {
+    ) internal virtual override(ERC7821) {
         if (!_hasRole(OPEN_ROLE_HOLDER, EXECUTOR_ROLE)) _checkRole(EXECUTOR_ROLE);
         bytes32 id;
         uint256 s;
@@ -332,7 +332,7 @@ contract Timelock is ERC7821, EnumerableRoles {
                 }
             }
         }
-        results = _execute(calls, id);
+        _execute(calls, id);
         /// @solidity memory-safe-assembly
         assembly {
             // Recheck the operation after the calls, in case of reentrancy.

test/ERC7821.t.sol

@@ -48,14 +48,7 @@ contract ERC7821Test is SoladyTest {
         bytes memory data = abi.encode(calls);
         vm.resumeGasMetering();
 
-        bytes[] memory results = mbe.execute{value: _totalValue(calls)}(_SUPPORTED_MODE, data);
-
-        vm.pauseGasMetering();
-
-        assertEq(results.length, 2);
-        assertEq(abi.decode(results[0], (bytes)), "hehe");
-        assertEq(abi.decode(results[1], (bytes32)), keccak256("lol"));
-        vm.resumeGasMetering();
+        mbe.execute{value: _totalValue(calls)}(_SUPPORTED_MODE, data);
     }
 
     function testERC7821(bytes memory opData) public {
@@ -71,14 +64,9 @@ contract ERC7821Test is SoladyTest {
         calls[1].value = 789;
         calls[1].data = abi.encodeWithSignature("returnsHash(bytes)", "lol");
 
-        bytes[] memory results =
-            mbe.execute{value: _totalValue(calls)}(_SUPPORTED_MODE, _encode(calls, opData));
+        mbe.execute{value: _totalValue(calls)}(_SUPPORTED_MODE, _encode(calls, opData));
 
         assertEq(mbe.lastOpData(), opData);
-
-        assertEq(results.length, 2);
-        assertEq(abi.decode(results[0], (bytes)), "hehe");
-        assertEq(abi.decode(results[1], (bytes32)), keccak256("lol"));
     }
 
     function testERC7821ForRevert() public {
@@ -124,14 +112,7 @@ contract ERC7821Test is SoladyTest {
             }
         }
 
-        bytes[] memory results = mbe.executeDirect{value: _totalValue(calls)}(calls);
-        for (uint256 i; i < calls.length; ++i) {
-            if (payloads[i].mode == 0) {
-                assertEq(abi.decode(results[i], (bytes)), payloads[i].data);
-            } else {
-                assertEq(abi.decode(results[i], (bytes32)), keccak256(payloads[i].data));
-            }
-        }
+        mbe.executeDirect{value: _totalValue(calls)}(calls);
 
         if (calls.length != 0 && _randomChance(32)) {
             calls[_randomUniform() % calls.length].data =

test/utils/mocks/MockERC7821.sol

@@ -13,21 +13,15 @@ contract MockERC7821 is ERC7821, Brutalizer {
         internal
         virtual
         override
-        returns (bytes[] memory)
     {
         lastOpData = opData;
-        return _execute(calls, bytes32(0));
+        _execute(calls, bytes32(0));
     }
 
-    function executeDirect(Call[] calldata calls)
-        public
-        payable
-        virtual
-        returns (bytes[] memory results)
-    {
+    function executeDirect(Call[] calldata calls) public payable virtual {
         _misalignFreeMemoryPointer();
         _brutalizeMemory();
-        results = _execute(calls, bytes32(0));
+        _execute(calls, bytes32(0));
         _checkMemory();
     }
 }