Merge pull request #75 from base-org/jack/gas-improvements

Jack/gas improvements

Author: jackchuma

Diff for: contracts/src/ERC7786Base.sol

@@ -59,6 +59,9 @@ contract ERC7786Base {
     /// @param selector The selector of the attribute that was not found
     error AttributeNotFound(bytes4 selector);
 
+    /// @notice This error is thrown when the call attributes array contains more than one element
+    error MaxOneAttributeExpected();
+
     /// @notice Locates an attribute in the attributes array
     ///
     /// @custom:reverts If the attribute is not found
@@ -100,15 +103,19 @@ contract ERC7786Base {
     /// @notice Locates an attribute value in the attributes array
     ///
     /// @param attributes The attributes array to search
-    /// @param selector   The selector of the attribute to find
     ///
     /// @return value The value of the attribute found
-    function _locateAttributeValue(bytes[] calldata attributes, bytes4 selector) internal pure returns (uint256) {
-        for (uint256 i; i < attributes.length; i++) {
-            if (bytes4(attributes[i]) == selector) {
-                return abi.decode(attributes[i][4:], (uint256));
-            }
+    function _locateAttributeValue(bytes[] calldata attributes) internal pure returns (uint256) {
+        if (attributes.length > 1) {
+            revert MaxOneAttributeExpected();
         }
-        return 0;
+
+        uint256 value;
+
+        if (attributes.length == 1 && bytes4(attributes[0]) == _VALUE_ATTRIBUTE_SELECTOR) {
+            value = abi.decode(attributes[0][4:], (uint256));
+        }
+
+        return value;
     }
 }

Diff for: contracts/src/RIP7755Inbox.sol

@@ -2,7 +2,6 @@
 pragma solidity 0.8.24;
 
 import {IEntryPoint} from "account-abstraction/interfaces/IEntryPoint.sol";
-import {Address} from "openzeppelin-contracts/contracts/utils/Address.sol";
 import {CAIP10} from "openzeppelin-contracts/contracts/utils/CAIP10.sol";
 import {CAIP2} from "openzeppelin-contracts/contracts/utils/CAIP2.sol";
 import {Strings} from "openzeppelin-contracts/contracts/utils/Strings.sol";
@@ -18,7 +17,6 @@ import {Paymaster} from "./Paymaster.sol";
 /// @notice An inbox contract within RIP-7755. This contract's sole purpose is to route requested transactions on
 ///         destination chains and store record of their fulfillment.
 contract RIP7755Inbox is ERC7786Base, Paymaster {
-    using Address for address payable;
     using Strings for string;
 
     struct MainStorage {
@@ -35,6 +33,16 @@ contract RIP7755Inbox is ERC7786Base, Paymaster {
         address fulfiller;
     }
 
+    /// @notice A message structure used for internal processing
+    struct InternalMessage {
+        /// @dev The fulfiller address allowed to claim on source chain
+        address fulfiller;
+        /// @dev Boolean value specifying if the request represents an ERC-4337 UserOp
+        bool isUserOp;
+        /// @dev Address of the specified precheck contract. This is optional.
+        address precheckContract;
+    }
+
     /// @notice Main storage location used as the base for the fulfillmentInfo mapping following EIP-7201. Derived from
     ///         the equation keccak256(abi.encode(uint256(keccak256(bytes("RIP-7755"))) - 1)) & ~bytes32(uint256(0xff))
     bytes32 private constant _MAIN_STORAGE_LOCATION = 0xfd1017d80ffe8da8a74488ee7408c9efa1877e094afa95857de95797c1228500;
@@ -81,21 +89,29 @@ contract RIP7755Inbox is ERC7786Base, Paymaster {
         Message[] calldata messages,
         bytes[] calldata globalAttributes
     ) external payable returns (bytes4) {
-        address fulfiller = _getFulfiller(globalAttributes);
-        _revertIfUserOp(globalAttributes);
+        InternalMessage memory m = _processAttributes(globalAttributes);
+
+        if (m.fulfiller == address(0)) {
+            revert AttributeNotFound(_FULFILLER_ATTRIBUTE_SELECTOR);
+        }
+
+        if (m.isUserOp) {
+            revert UserOp();
+        }
+
         bytes32 messageId = getRequestId(sourceChain, sender, messages, globalAttributes);
 
-        _runPrecheck(sourceChain, sender, messages, globalAttributes);
+        _runPrecheck(sourceChain, sender, messages, globalAttributes, m.precheckContract);
 
         if (_getFulfillmentInfo(messageId).timestamp != 0) {
             revert CallAlreadyFulfilled();
         }
 
-        _setFulfillmentInfo(messageId, fulfiller);
+        _setFulfillmentInfo(messageId, m.fulfiller);
 
         _sendCallsAndValidateMsgValue(messages);
 
-        emit CallFulfilled({requestHash: messageId, fulfilledBy: fulfiller});
+        emit CallFulfilled({requestHash: messageId, fulfilledBy: m.fulfiller});
 
         return 0x675b049b; // this function's sig
     }
@@ -134,8 +150,8 @@ contract RIP7755Inbox is ERC7786Base, Paymaster {
         uint256 valueSent;
 
         for (uint256 i; i < messages.length; i++) {
-            address payable to = payable(messages[i].receiver.parseAddress());
-            uint256 value = _locateAttributeValue(messages[i].attributes, _VALUE_ATTRIBUTE_SELECTOR);
+            address to = messages[i].receiver.parseAddress();
+            uint256 value = _locateAttributeValue(messages[i].attributes);
             _call(to, messages[i].payload, value);
 
             unchecked {
@@ -148,11 +164,16 @@ contract RIP7755Inbox is ERC7786Base, Paymaster {
         }
     }
 
-    function _call(address payable to, bytes memory data, uint256 value) private {
-        if (data.length == 0) {
-            to.sendValue(value);
-        } else {
-            to.functionCallWithValue(data, value);
+    function _call(address to, bytes memory data, uint256 value) private {
+        bytes memory result;
+        /// @solidity memory-safe-assembly
+        assembly {
+            result := mload(0x40)
+            if iszero(call(gas(), to, value, add(data, 0x20), mload(data), codesize(), 0x00)) {
+                // Bubble up the revert if the call reverts.
+                returndatacopy(result, 0x00, returndatasize())
+                revert(result, returndatasize())
+            }
         }
     }
 
@@ -167,39 +188,35 @@ contract RIP7755Inbox is ERC7786Base, Paymaster {
         string calldata sourceChain, // [CAIP-2] chain identifier
         string calldata sender, // [CAIP-10] account address
         Message[] calldata messages,
-        bytes[] calldata attributes
+        bytes[] calldata attributes,
+        address precheck
     ) private view {
-        (bool found, bytes calldata precheckAttribute) =
-            _locateAttributeUnchecked(attributes, _PRECHECK_ATTRIBUTE_SELECTOR);
-
-        if (!found) {
+        if (precheck == address(0)) {
             return;
         }
 
-        address precheckContract = abi.decode(precheckAttribute[4:], (address));
-        IPrecheckContract(precheckContract).precheckCall(sourceChain, sender, messages, attributes, msg.sender);
+        IPrecheckContract(precheck).precheckCall(sourceChain, sender, messages, attributes, msg.sender);
     }
 
     function _getFulfillmentInfo(bytes32 requestHash) private view returns (FulfillmentInfo memory) {
         MainStorage storage $ = _getMainStorage();
         return $.fulfillmentInfo[requestHash];
     }
 
-    function _getFulfiller(bytes[] calldata attributes) private pure returns (address) {
-        bytes calldata fulfillerAttribute = _locateAttribute(attributes, _FULFILLER_ATTRIBUTE_SELECTOR);
-        return abi.decode(fulfillerAttribute[4:], (address));
-    }
-
-    function _revertIfUserOp(bytes[] calldata attributes) private pure {
-        (bool found, bytes calldata userOpAttribute) =
-            _locateAttributeUnchecked(attributes, _USER_OP_ATTRIBUTE_SELECTOR);
-        if (found) {
-            bool isUserOp = abi.decode(userOpAttribute[4:], (bool));
+    function _processAttributes(bytes[] calldata attributes) private pure returns (InternalMessage memory) {
+        InternalMessage memory message;
 
-            if (isUserOp) {
-                revert UserOp();
+        for (uint256 i; i < attributes.length; i++) {
+            if (bytes4(attributes[i]) == _FULFILLER_ATTRIBUTE_SELECTOR) {
+                message.fulfiller = abi.decode(attributes[i][4:], (address));
+            } else if (bytes4(attributes[i]) == _USER_OP_ATTRIBUTE_SELECTOR) {
+                message.isUserOp = abi.decode(attributes[i][4:], (bool));
+            } else if (bytes4(attributes[i]) == _PRECHECK_ATTRIBUTE_SELECTOR) {
+                message.precheckContract = abi.decode(attributes[i][4:], (address));
             }
         }
+
+        return message;
     }
 
     function _filterOutFulfiller(bytes[] calldata attributes) private pure returns (bytes[] memory) {

Diff for: contracts/src/libraries/StateValidator.sol

@@ -2,7 +2,6 @@
 pragma solidity ^0.8.0;
 
 import {RLPReader} from "optimism/packages/contracts-bedrock/src/libraries/rlp/RLPReader.sol";
-import {MerkleTrie} from "optimism/packages/contracts-bedrock/src/libraries/trie/MerkleTrie.sol";
 import {SecureMerkleTrie} from "optimism/packages/contracts-bedrock/src/libraries/trie/SecureMerkleTrie.sol";
 
 import {SSZ} from "./SSZ.sol";
@@ -113,10 +112,12 @@ library StateValidator {
         bytes32 stateRoot,
         AccountProofParameters memory accountProofParams
     ) internal pure returns (bool) {
-        // Derive the account key that shows up in the execution client's merkle trie
-        bytes memory accountKey = abi.encodePacked(keccak256(abi.encodePacked(account)));
         // Use the account proof to derive the RLP-encoded account metadata
-        bytes memory encodedAccount = MerkleTrie.get(accountKey, accountProofParams.accountProof, stateRoot);
+        bytes memory encodedAccount = SecureMerkleTrie.get({
+            _key: abi.encodePacked(account),
+            _proof: accountProofParams.accountProof,
+            _root: stateRoot
+        });
 
         // Extract storage root from account data
         bytes32 storageRoot = _extractStorageRoot(encodedAccount);

Diff for: contracts/test/RIP7755Inbox.t.sol

@@ -41,6 +41,13 @@ contract RIP7755InboxTest is BaseTest {
         _setUp();
     }
 
+    function test_executeMessages_reverts_ifNoFulfiller() external {
+        TestMessage memory m = _initMessage(false, true);
+
+        vm.expectRevert(abi.encodeWithSelector(AttributeNotFound.selector, _FULFILLER_ATTRIBUTE_SELECTOR));
+        inbox.executeMessages(m.sourceChain, m.sender, m.messages, _filterOutFulfiller(m.attributes));
+    }
+
     function test_executeMessages_reverts_userOp() external {
         TestMessage memory m = _initMessage(false, true);
 
@@ -126,6 +133,21 @@ contract RIP7755InboxTest is BaseTest {
         assertEq(ALICE.balance, 0);
     }
 
+    function test_executeMessages_reverts_tooManyAttributes(uint256 amount) external {
+        TestMessage memory m = _initMessage(false, false);
+
+        bytes[] memory attributes = new bytes[](2);
+
+        _appendMessage(m, Message({receiver: ALICE.toChecksumHexString(), payload: "", attributes: attributes}));
+
+        vm.deal(FILLER, amount);
+        vm.prank(FILLER);
+        vm.expectRevert(MaxOneAttributeExpected.selector);
+        inbox.executeMessages(m.sourceChain, m.sender, m.messages, m.attributes);
+
+        assertEq(ALICE.balance, 0);
+    }
+
     function test_executeMessages_reverts_ifTargetContractReverts() external {
         TestMessage memory m = _initMessage(false, false);