Merge pull request #112 from BootNodeDev/refund-wrong-chain

Check msg origin and settler when refunding

Author: lmcorbalan

solidity/package.json

@@ -69,6 +69,8 @@
     "openOrder": "forge script script/OpenOrder.s.sol:OpenOrder -f $NETWORK --broadcast --verify --slow -vvv",
     "run:openOrder": "dotenv-run-script openOrder",
     "enrollRouter": "forge script script/EnrollRouter.s.sol:EnrollRouter -f $NETWORK --broadcast --slow -vvv",
-    "run:enrollRouter": "dotenv-run-script enrollRouter"
+    "run:enrollRouter": "dotenv-run-script enrollRouter",
+    "upgradeSimple": "forge script script/UpgradeSimple.s.sol:UpgradeSimple -f $NETWORK --broadcast --slow -vvv",
+    "run:upgradeSimple": "dotenv-run-script upgradeSimple"
   }
 }

solidity/script/UpgradeSimple.s.sol

@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity >=0.8.25 <0.9.0;
+
+import { Script } from "forge-std/Script.sol";
+import { console2 } from "forge-std/console2.sol";
+
+import { ITransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
+import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
+
+import { Hyperlane7683 } from "../src/Hyperlane7683.sol";
+
+/// @dev See the Solidity Scripting tutorial: https://book.getfoundry.sh/tutorials/solidity-scripting
+contract UpgradeSimple is Script {
+    function run() public {
+        uint256 proxyAdminOwner = vm.envUint("PROXY_ADMIN_OWNER_PK");
+
+        address routerProxy = vm.envAddress("ROUTER");
+        address proxyAdmin = vm.envAddress("PROXY_ADMIN");
+
+        vm.startBroadcast(proxyAdminOwner);
+
+        address newRouterImpl = deployImplementation();
+
+        ProxyAdmin(proxyAdmin).upgrade(ITransparentUpgradeableProxy(routerProxy), newRouterImpl);
+
+        vm.stopBroadcast();
+
+        // solhint-disable-next-line no-console
+        console2.log("New Implementation:", newRouterImpl);
+    }
+
+    function deployImplementation() internal returns (address routerImpl) {
+        address mailbox = vm.envAddress("MAILBOX");
+        address permit2 = vm.envAddress("PERMIT2");
+
+        return address(new Hyperlane7683(mailbox, permit2));
+    }
+}

solidity/src/Base7683.sol

@@ -226,7 +226,7 @@ abstract contract Base7683 is IOriginSettler, IDestinationSettler {
      * @param _orderId Unique order identifier for this order
      * @param _originData Data emitted on the origin to parameterize the fill
      * @param _fillerData Data provided by the filler to inform the fill or express their preferences. It should
-     * contain the bytes32 encoded address of the receiver which is the used at settlement time
+     * contain the bytes32 encoded address of the receiver which is used at settlement time
      */
     function fill(bytes32 _orderId, bytes calldata _originData, bytes calldata _fillerData) external payable virtual {
         if (orderStatus[_orderId] != UNKNOWN) revert InvalidOrderStatus();
@@ -244,7 +244,7 @@ abstract contract Base7683 is IOriginSettler, IDestinationSettler {
      * @dev Pays the filler the amount locked when the orders were opened.
      * The settled status should not be changed here but rather on the origin chain. To allow the filler to retry in
      * case some error occurs.
-     * Ensuring the order is not settled in the origin chain is the responsibility of the caller.
+     * Ensuring the order is eligible for settling in the origin chain is the responsibility of the caller.
      * @param _orderIds An array of IDs for the orders to settle.
      */
     function settle(bytes32[] calldata _orderIds) external payable {
@@ -267,7 +267,7 @@ abstract contract Base7683 is IOriginSettler, IDestinationSettler {
      * @notice Refunds a batch of expired GaslessCrossChainOrders on the chain where the orders were opened.
      * The refunded status should not be changed here but rather on the origin chain. To allow the user to retry in
      * case some error occurs.
-     * Ensuring the order is not refunded in the origin chain is the responsibility of the caller.
+     * Ensuring the order is eligible for refunding in the origin chain is the responsibility of the caller.
      * @param _orders An array of GaslessCrossChainOrders to refund.
      */
     function refund(GaslessCrossChainOrder[] memory _orders) external payable {
@@ -289,7 +289,7 @@ abstract contract Base7683 is IOriginSettler, IDestinationSettler {
      * @notice Refunds a batch of expired OnchainCrossChainOrder on the chain where the orders were opened.
      * The refunded status should not be changed here but rather on the origin chain. To allow the user to retry in
      * case some error occurs.
-     * Ensuring the order is not refunded in the origin chain is the responsibility of the caller.
+     * Ensuring the order is eligible for refunding the origin chain is the responsibility of the caller.
      * @param _orders An array of GaslessCrossChainOrders to refund.
      */
     function refund(OnchainCrossChainOrder[] memory _orders) external payable {

solidity/src/BasicSwap7683.sol

@@ -85,6 +85,9 @@ abstract contract BasicSwap7683 is Base7683 {
 
     /**
      * @dev Settles multiple orders by dispatching the settlement instructions.
+     * The proper status of all the orders (filled) is validated on the Base7683 before calling this function.
+     * It assumes that all orders were originated in the same originDomain so it uses the the one from the first one for
+     * dispatching the message, but if some order differs on the originDomain it can be re-settle later.
      * @param _orderIds The IDs of the orders to settle.
      * @param _ordersOriginData The original data of the orders.
      * @param _ordersFillerData The filler data for the orders.
@@ -98,85 +101,135 @@ abstract contract BasicSwap7683 is Base7683 {
         override
     {
         // at this point we are sure all orders are filled, use the first order to get the originDomain
-        // if some order differs on the originDomain ir can be re-settle later
+        // if some order differs on the originDomain it can be re-settle later
         _dispatchSettle(OrderEncoder.decode(_ordersOriginData[0]).originDomain, _orderIds, _ordersFillerData);
     }
 
     /**
      * @dev Refunds multiple OnchainCrossChain orders by dispatching refund instructions.
+     * The proper status of all the orders (NOT filled and expired) is validated on the Base7683 before calling this
+     * function.
+     * It assumes that all orders were originated in the same originDomain so it uses the the one from the first one for
+     * dispatching the message, but if some order differs on the originDomain it can be re-refunded later.
      * @param _orders The orders to refund.
      * @param _orderIds The IDs of the orders to refund.
      */
     function _refundOrders(OnchainCrossChainOrder[] memory _orders, bytes32[] memory _orderIds) internal override {
-        // at this point we are sure all orders are filled, use the first order to get the originDomain
-        // if some order differs on the originDomain ir can be re-refunded later
         _dispatchRefund(OrderEncoder.decode(_orders[0].orderData).originDomain, _orderIds);
     }
 
     /**
      * @dev Refunds multiple GaslessCrossChain orders by dispatching refund instructions.
+     * The proper status of all the orders (NOT filled and expired) is validated on the Base7683 before calling this
+     * function.
+     * It assumes that all orders were originated in the same originDomain so it uses the the one from the first one for
+     * dispatching the message, but if some order differs on the originDomain it can be re-refunded later.
      * @param _orders The orders to refund.
      * @param _orderIds The IDs of the orders to refund.
      */
     function _refundOrders(GaslessCrossChainOrder[] memory _orders, bytes32[] memory _orderIds) internal override {
-        // at this point we are sure all orders are filled, use the first order to get the originDomain
-        // if some order differs on the originDomain ir can be re-refunded later
         _dispatchRefund(OrderEncoder.decode(_orders[0].orderData).originDomain, _orderIds);
     }
 
     /**
      * @dev Handles settling an individual order, should be called by the inheriting contract when receiving a setting
      * instruction from a remote chain.
+     * @param _messageOrigin The domain from which the message originates.
+     * @param _messageSender The address of the sender on the origin domain.
      * @param _orderId The ID of the order to settle.
      * @param _receiver The receiver address (encoded as bytes32).
      */
-    function _handleSettleOrder(bytes32 _orderId, bytes32 _receiver) internal virtual {
-        // check if the order is opened to ensure it belongs to this domain, skip otherwise
-        if (orderStatus[_orderId] != OPENED) return;
-
-        (,bytes memory _orderData) = abi.decode(openOrders[_orderId], (bytes32, bytes));
-        OrderData memory orderData = OrderEncoder.decode(_orderData);
+    function _handleSettleOrder(
+        uint32 _messageOrigin,
+        bytes32 _messageSender,
+        bytes32 _orderId,
+        bytes32 _receiver
+    ) internal virtual {
+        (
+            bool isEligible,
+            OrderData memory orderData
+        ) = _checkOrderEligibility(_messageOrigin, _messageSender, _orderId);
+
+        if (!isEligible) return;
 
         orderStatus[_orderId] = SETTLED;
 
         address receiver = TypeCasts.bytes32ToAddress(_receiver);
         address inputToken = TypeCasts.bytes32ToAddress(orderData.inputToken);
 
-        if (inputToken == address(0)) {
-            Address.sendValue(payable(receiver), orderData.amountIn);
-        } else {
-            IERC20(inputToken).safeTransfer(receiver, orderData.amountIn);
-        }
+        _transferTokenOut(inputToken, receiver, orderData.amountIn);
 
         emit Settled(_orderId, receiver);
     }
 
     /**
      * @dev Handles refunding an individual order, should be called by the inheriting contract when receiving a
      * refunding instruction from a remote chain.
+     * @param _messageOrigin The domain from which the message originates.
+     * @param _messageSender The address of the sender on the origin domain.
      * @param _orderId The ID of the order to refund.
      */
-    function _handleRefundOrder(bytes32 _orderId) internal virtual {
-        // check if the order is opened to ensure it belongs to this domain, skip otherwise
-        if (orderStatus[_orderId] != OPENED) return;
+    function _handleRefundOrder(uint32 _messageOrigin, bytes32 _messageSender, bytes32 _orderId) internal virtual {
+        (
+            bool isEligible,
+            OrderData memory orderData
+        ) = _checkOrderEligibility(_messageOrigin, _messageSender, _orderId);
 
-        (,bytes memory _orderData) = abi.decode(openOrders[_orderId], (bytes32, bytes));
-        OrderData memory orderData = OrderEncoder.decode(_orderData);
+        if (!isEligible) return;
 
         orderStatus[_orderId] = REFUNDED;
 
         address orderSender = TypeCasts.bytes32ToAddress(orderData.sender);
         address inputToken = TypeCasts.bytes32ToAddress(orderData.inputToken);
 
-        if (inputToken == address(0)) {
-            Address.sendValue(payable(orderSender), orderData.amountIn);
-        } else {
-            IERC20(inputToken).safeTransfer(orderSender, orderData.amountIn);
-        }
+        _transferTokenOut(inputToken, orderSender, orderData.amountIn);
 
         emit Refunded(_orderId, orderSender);
     }
 
+    /**
+    * @notice Checks if order is eligible for settlement or refund .
+    * @dev Order must be OPENED and the message was sent from the appropriated chain and contract.
+    * @param _messageOrigin The origin domain of the message.
+    * @param _messageSender The sender identifier of the message.
+    * @param _orderId The unique identifier of the order.
+    * @return A boolean indicating if the order is valid, and the decoded OrderData structure.
+    */
+    function _checkOrderEligibility(
+        uint32 _messageOrigin,
+        bytes32 _messageSender,
+        bytes32 _orderId
+    ) internal virtual returns (bool, OrderData memory) {
+        OrderData memory orderData;
+
+        // check if the order is opened to ensure it belongs to this domain, skip otherwise
+        if (orderStatus[_orderId] != OPENED) return (false, orderData);
+
+        (,bytes memory _orderData) = abi.decode(openOrders[_orderId], (bytes32, bytes));
+        orderData = OrderEncoder.decode(_orderData);
+
+        if (orderData.destinationDomain != _messageOrigin || orderData.destinationSettler != _messageSender)
+            return (false, orderData);
+
+        return (true, orderData);
+    }
+
+    /**
+    * @notice Transfers tokens or ETH out of the contract.
+    * @dev If _token is the zero address, transfers ETH using a safe method; otherwise, performs an ERC20 token
+    * transfer.
+    * @param _token The address of the token to transfer (use address(0) for ETH).
+    * @param _to The recipient address.
+    * @param _amount The amount of tokens or ETH to transfer.
+    */
+    function _transferTokenOut(address _token, address _to, uint256 _amount) internal {
+        if (_token == address(0)) {
+            Address.sendValue(payable(_to), _amount);
+        } else {
+            IERC20(_token).safeTransfer(_to, _amount);
+        }
+    }
+
     /**
      * @dev Gets the ID of a GaslessCrossChainOrder.
      * @param _order The GaslessCrossChainOrder to compute the ID for.

solidity/src/Hyperlane7683.sol

@@ -84,19 +84,19 @@ contract Hyperlane7683 is GasRouter, BasicSwap7683 {
     /**
      * @notice Handles incoming messages.
      * @dev Decodes the message and processes settlement or refund operations accordingly.
-     * _originDomain The domain from which the message originates (unused in this implementation).
-     * _sender The address of the sender on the origin domain (unused in this implementation).
+     * @param _messageOrigin The domain from which the message originates (unused in this implementation).
+     * @param _messageSender The address of the sender on the origin domain (unused in this implementation).
      * @param _message The encoded message received via Hyperlane.
      */
-    function _handle(uint32, bytes32, bytes calldata _message) internal virtual override {
+    function _handle(uint32 _messageOrigin, bytes32 _messageSender, bytes calldata _message) internal virtual override {
         (bool _settle, bytes32[] memory _orderIds, bytes[] memory _ordersFillerData) =
             Hyperlane7683Message.decode(_message);
 
         for (uint256 i = 0; i < _orderIds.length; i++) {
             if (_settle) {
-                _handleSettleOrder(_orderIds[i], abi.decode(_ordersFillerData[i], (bytes32)));
+                _handleSettleOrder(_messageOrigin, _messageSender, _orderIds[i], abi.decode(_ordersFillerData[i], (bytes32)));
             } else {
-                _handleRefundOrder(_orderIds[i]);
+                _handleRefundOrder(_messageOrigin, _messageSender, _orderIds[i]);
             }
         }
     }