Auto relayer refund address handling improvement (#3428)

ethereum/contracts/relayer/libraries/Utils.sol

@@ -16,6 +16,16 @@ function pay(address payable receiver, LocalNative amount) returns (bool success
     success = true;
 }
 
+function pay(address payable receiver, LocalNative amount, uint256 gasBound) returns (bool success) {
+  uint256 amount_ = LocalNative.unwrap(amount);
+  if (amount_ != 0)
+    // TODO: we currently ignore the return data. Some users of this function might want to bubble up the return value though.
+    // Specifying a higher limit than 63/64 of the remaining gas caps it at that amount without throwing an exception.
+    (success,) = returnLengthBoundedCall(receiver, new bytes(0), gasBound, amount_, 0);
+  else
+    success = true;
+}
+
 function min(uint256 a, uint256 b) pure returns (uint256) {
   return a < b ? a : b;
 }
@@ -47,6 +57,18 @@ uint256 constant freeMemoryPtr = 0x40;
 uint256 constant memoryWord = 32;
 uint256 constant maskModulo32 = 0x1f;
 
+/**
+ * Overload with no 'value' and non-payable address
+ */
+function returnLengthBoundedCall(
+  address callee,
+  bytes memory callData,
+  uint256 gasLimit,
+  uint256 dataLengthBound
+) returns (bool success, bytes memory returnedData) {
+  return returnLengthBoundedCall(payable(callee), callData, gasLimit, 0, dataLengthBound);
+}
+
 /**
  * Implements call that truncates return data to a specific size to avoid excessive gas consumption for relayers
  * when a revert or unexpectedly large return value is produced by the call.

ethereum/contracts/relayer/wormholeRelayer/WormholeRelayerDelivery.sol

@@ -25,7 +25,7 @@ import {
 import {IWormholeReceiver} from "../../interfaces/relayer/IWormholeReceiver.sol";
 import {IDeliveryProvider} from "../../interfaces/relayer/IDeliveryProviderTyped.sol";
 
-import {pay, min, toWormholeFormat, fromWormholeFormat, returnLengthBoundedCall} from "../../relayer/libraries/Utils.sol";
+import {pay, pay, min, toWormholeFormat, fromWormholeFormat, returnLengthBoundedCall, returnLengthBoundedCall} from "../../relayer/libraries/Utils.sol";
 import {
     DeliveryInstruction,
     DeliveryOverride,
@@ -43,6 +43,10 @@ import {WormholeRelayerBase} from "./WormholeRelayerBase.sol";
 import "../../interfaces/relayer/TypedUnits.sol";
 import "../../relayer/libraries/ExecutionParameters.sol";
 
+uint256 constant QUOTE_LENGTH_BYTES = 32;
+
+uint256 constant GAS_LIMIT_EXTERNAL_CALL = 100_000;
+
 abstract contract WormholeRelayerDelivery is WormholeRelayerBase, IWormholeRelayerDelivery {
     using WormholeRelayerSerde for *; 
     using BytesParsing for bytes;
@@ -446,49 +450,63 @@ abstract contract WormholeRelayerDelivery is WormholeRelayerBase, IWormholeRelay
         uint16 refundChain,
         bytes32 refundAddress,
         LocalNative refundAmount,
-        bytes32 relayerAddress
+        bytes32 deliveryProvider
     ) private returns (RefundStatus) {
         // User requested refund on this chain
         if (refundChain == getChainId()) {
-            return pay(payable(fromWormholeFormat(refundAddress)), refundAmount)
+            return pay(payable(fromWormholeFormat(refundAddress)), refundAmount, GAS_LIMIT_EXTERNAL_CALL)
                 ? RefundStatus.REFUND_SENT
                 : RefundStatus.REFUND_FAIL;
         }
 
         // User requested refund on a different chain
         
-        IDeliveryProvider deliveryProvider = IDeliveryProvider(fromWormholeFormat(relayerAddress));
-        
         // Determine price of an 'empty' delivery
         // (Note: assumes refund chain is an EVM chain)
-        LocalNative baseDeliveryPrice;
+        (bool success, LocalNative baseDeliveryPrice) = untrustedBaseDeliveryPrice(fromWormholeFormat(deliveryProvider), refundChain);
-      
+        
-        try deliveryProvider.quoteDeliveryPrice(
+        // If the unstrusted call failed, or the refundAmount is not greater than the 'empty delivery price', then the refund does not go through
-            refundChain,
+        // Note: We first check 'refundAmount <= baseDeliveryPrice', in case an untrusted delivery provider returns a value that overflows once
-            TargetNative.wrap(0),
+        // the wormhole message fee is added to it
-            encodeEvmExecutionParamsV1(getEmptyEvmExecutionParamsV1())
+        unchecked {
-        ) returns (LocalNative quote, bytes memory) {
+            if (!success || (refundAmount <= baseDeliveryPrice) || (refundAmount <= getWormholeMessageFee() + baseDeliveryPrice)) {
-            baseDeliveryPrice = quote;
+                return RefundStatus.CROSS_CHAIN_REFUND_FAIL_NOT_ENOUGH;
-        } catch (bytes memory) {
+            }
-            return RefundStatus.CROSS_CHAIN_REFUND_FAIL_PROVIDER_NOT_SUPPORTED;
-        }
-
-        // If the refundAmount is not greater than the 'empty delivery price', the refund does not go through
-        if (refundAmount <= getWormholeMessageFee() + baseDeliveryPrice) {
-            return RefundStatus.CROSS_CHAIN_REFUND_FAIL_NOT_ENOUGH;
         }
         
+        return sendCrossChainRefund(refundChain, refundAddress, refundAmount, refundAmount - getWormholeMessageFee() - baseDeliveryPrice, deliveryProvider);
+    }
+
+    function untrustedBaseDeliveryPrice(address deliveryProvider, uint16 refundChain) internal returns (bool success, LocalNative baseDeliveryPrice) {
+        (bool externalCallSuccess, bytes memory returnData) = returnLengthBoundedCall(
+            deliveryProvider,
+            abi.encodeCall(IDeliveryProvider.quoteDeliveryPrice, (refundChain, TargetNative.wrap(0), encodeEvmExecutionParamsV1(getEmptyEvmExecutionParamsV1()))),
+            GAS_LIMIT_EXTERNAL_CALL,
+            QUOTE_LENGTH_BYTES
+        );
+        
+        if(externalCallSuccess && returnData.length == QUOTE_LENGTH_BYTES) {
+            baseDeliveryPrice = abi.decode(returnData, (LocalNative));
+            success = true;
+        } else {
+            success = false;
+        }
+    }
+
+    function sendCrossChainRefund(uint16 refundChain, bytes32 refundAddress, LocalNative sendAmount, LocalNative receiveAmount, bytes32 deliveryProvider) internal returns (RefundStatus status) {
         // Request a 'send' with 'paymentForExtraReceiverValue' equal to the refund minus the 'empty delivery price'
-        try IWormholeRelayerSend(address(this)).send{value: refundAmount.unwrap()}(
+        // We limit the gas because we are within a delivery, so thus the trust assumptions on the delivery provider are different
+        // Normally, in 'send', a revert is no problem; but here, we want to prevent such reverts in this try-catch
+        try IWormholeRelayerSend(address(this)).send{value: sendAmount.unwrap(), gas: GAS_LIMIT_EXTERNAL_CALL}(
             refundChain,
             bytes32(0),
             bytes(""),
             TargetNative.wrap(0),
-            refundAmount - getWormholeMessageFee() - baseDeliveryPrice,
+            receiveAmount,
             encodeEvmExecutionParamsV1(getEmptyEvmExecutionParamsV1()),
             refundChain,
             refundAddress,
-            fromWormholeFormat(relayerAddress),
+            fromWormholeFormat(deliveryProvider),
             new VaaKey[](0),
             CONSISTENCY_LEVEL_INSTANT
         ) returns (uint64) {

ethereum/contracts/relayer/wormholeRelayer/WormholeRelayerSend.sol

@@ -541,7 +541,7 @@ abstract contract WormholeRelayerSend is WormholeRelayerBase, IWormholeRelayerSe
         bytes32 targetAddress,
         bytes memory payload,
         TargetNative receiverValue,
-        LocalNative paymentForExtraReceiverValue,
+        LocalNative,
         bytes memory encodedExecutionParameters,
         uint16 refundChain,
         bytes32 refundAddress,

ethereum/forge-test/relayer/WormholeRelayer.t.sol

@@ -2213,4 +2213,71 @@ contract WormholeRelayerTests is Test {
             keccak256(setup.target.integration.getMessage()) == keccak256(message), "payload wrong"
         );
     }
+
+    function testProviderRefundAddressZeros(
+        GasParameters memory gasParams,
+        FeeParameters memory feeParams,
+        bytes memory message
+    ) public {
+        StandardSetupTwoChains memory setup = standardAssumeAndSetupTwoChains(
+            gasParams,
+            feeParams,
+            1000000
+        );
+        vm.recordLogs();
+        setup.source.deliveryProvider.updateTargetChainAddress(
+            setup.targetChain,
+            bytes32(0x0)
+        );
+        (LocalNative deliveryCost, ) = setup
+            .source
+            .coreRelayer
+            .quoteEVMDeliveryPrice(
+                setup.targetChain,
+                TargetNative.wrap(0),
+                Gas.wrap(gasParams.targetGasLimit)
+            );
+        setup.source.integration.sendMessageWithRefund{
+            value: LocalNative.unwrap(deliveryCost)
+        }(
+            message,
+            setup.targetChain,
+            gasParams.targetGasLimit,
+            0,
+            setup.sourceChain,
+            address(this)
+        );
+        genericRelayer.relay(setup.sourceChain);
+    }
+
+    function testSendTargetAddressZeros(
+        GasParameters memory gasParams,
+        FeeParameters memory feeParams,
+        bytes memory message
+    ) public {
+        StandardSetupTwoChains memory setup = standardAssumeAndSetupTwoChains(
+            gasParams,
+            feeParams,
+            1000000
+        );
+        vm.recordLogs();
+        (LocalNative deliveryCost, ) = setup
+            .source
+            .coreRelayer
+            .quoteEVMDeliveryPrice(
+                setup.targetChain,
+                TargetNative.wrap(25),
+                Gas.wrap(gasParams.targetGasLimit)
+            );
+        setup.source.coreRelayer.sendPayloadToEvm{
+            value: LocalNative.unwrap(deliveryCost)
+        }(
+            setup.targetChain,
+            address(0x1234123412341234123412341234123412341234),
+            message,
+            TargetNative.wrap(25),
+            Gas.wrap(gasParams.targetGasLimit)
+        );
+        genericRelayer.relay(setup.sourceChain);
+    }
 }