From 56aebd5ff63c09a7bd4911eab3f3075a0de9761b Mon Sep 17 00:00:00 2001 From: Ed Zynda Date: Fri, 6 Dec 2024 12:14:17 +0300 Subject: [PATCH] Drop gas introspection in receiver contract --- src/Periphery/ReceiverAcrossV3.sol | 39 +++--------- .../solidity/Periphery/ReceiverAcrossV3.t.sol | 59 +------------------ 2 files changed, 11 insertions(+), 87 deletions(-) diff --git a/src/Periphery/ReceiverAcrossV3.sol b/src/Periphery/ReceiverAcrossV3.sol index b99565c38..edb3ececc 100644 --- a/src/Periphery/ReceiverAcrossV3.sol +++ b/src/Periphery/ReceiverAcrossV3.sol @@ -16,13 +16,9 @@ import { SafeTransferLib } from "solady/utils/SafeTransferLib.sol"; contract ReceiverAcrossV3 is ILiFi, TransferrableOwnership { using SafeTransferLib for address; - /// Error /// - error InsufficientGasLimit(); - /// Storage /// IExecutor public immutable executor; address public immutable spokepool; - uint256 public immutable recoverGas; /// Modifiers /// modifier onlySpokepool() { @@ -36,13 +32,11 @@ contract ReceiverAcrossV3 is ILiFi, TransferrableOwnership { constructor( address _owner, address _executor, - address _spokepool, - uint256 _recoverGas + address _spokepool ) TransferrableOwnership(_owner) { owner = _owner; executor = IExecutor(_executor); spokepool = _spokepool; - recoverGas = _recoverGas; } /// External Methods /// @@ -98,6 +92,7 @@ contract ReceiverAcrossV3 is ILiFi, TransferrableOwnership { /// Private Methods /// /// @notice Performs a swap before completing a cross-chain transaction + // @notice Since Across will always send wrappedNative to contract, we do not need a native handling here /// @param _transactionId the transaction id associated with the operation /// @param _swapData array of data needed for swaps /// @param assetId address of the token received from the source chain (not to be confused with StargateV2's assetIds which are uint16 values) @@ -110,34 +105,16 @@ contract ReceiverAcrossV3 is ILiFi, TransferrableOwnership { address payable receiver, uint256 amount ) private { - // since Across will always send wrappedNative to contract, we do not need a native handling here - uint256 cacheGasLeft = gasleft(); - - // We introduced this handling to prevent relayers from under-estimating our destination transactions and then - // running into out-of-gas errors which would cause the bridged tokens to be refunded to the receiver. This is - // an emergency behaviour but testing showed that this would happen very frequently. - // Reverting transactions that dont have enough gas helps to make sure that transactions will get correctly estimated - // by the relayers on source chain and thus improves the success rate of destination calls. - if (cacheGasLeft < recoverGas) { - // case A: not enough gas left to execute calls - // @dev: we removed the handling to send bridged funds to receiver in case of insufficient gas - // as it's better for AcrossV3 to revert these cases instead - revert InsufficientGasLimit(); - } - - // case 2b: enough gas left to execute calls assetId.safeApprove(address(executor), 0); assetId.safeApprove(address(executor), amount); try - executor.swapAndCompleteBridgeTokens{ - gas: cacheGasLeft - recoverGas - }(_transactionId, _swapData, assetId, receiver) + executor.swapAndCompleteBridgeTokens( + _transactionId, + _swapData, + assetId, + receiver + ) {} catch { - cacheGasLeft = gasleft(); - // if the only gas left here is the recoverGas then the swap must have failed due to out-of-gas error and in this - // case we want to revert (again, to force relayers to estimate our destination calls with sufficient gas limit) - if (cacheGasLeft <= recoverGas) revert InsufficientGasLimit(); - // send the bridged (and unswapped) funds to receiver address assetId.safeTransfer(receiver, amount); diff --git a/test/solidity/Periphery/ReceiverAcrossV3.t.sol b/test/solidity/Periphery/ReceiverAcrossV3.t.sol index 36f35b21f..d726c3259 100644 --- a/test/solidity/Periphery/ReceiverAcrossV3.t.sol +++ b/test/solidity/Periphery/ReceiverAcrossV3.t.sol @@ -19,15 +19,11 @@ contract ReceiverAcrossV3Test is TestBase { bytes32 guid = bytes32("12345"); address receiverAddress = USER_RECEIVER; - uint256 public constant RECOVER_GAS_VALUE = 100000; address stargateRouter; Executor executor; ERC20Proxy erc20Proxy; event ExecutorSet(address indexed executor); - event RecoverGasSet(uint256 indexed recoverGas); - - error InsufficientGasLimit(); function setUp() public { customBlockNumberForForking = 20024274; @@ -38,8 +34,7 @@ contract ReceiverAcrossV3Test is TestBase { receiver = new ReceiverAcrossV3( address(this), address(executor), - SPOKEPOOL_MAINNET, - RECOVER_GAS_VALUE + SPOKEPOOL_MAINNET ); vm.label(address(receiver), "ReceiverAcrossV3"); vm.label(address(executor), "Executor"); @@ -50,13 +45,11 @@ contract ReceiverAcrossV3Test is TestBase { receiver = new ReceiverAcrossV3( address(this), address(executor), - SPOKEPOOL_MAINNET, - RECOVER_GAS_VALUE + SPOKEPOOL_MAINNET ); assertEq(address(receiver.executor()) == address(executor), true); assertEq(receiver.spokepool() == SPOKEPOOL_MAINNET, true); - assertEq(receiver.recoverGas() == RECOVER_GAS_VALUE, true); } function test_OwnerCanPullERC20Token() public { @@ -168,52 +161,6 @@ contract ReceiverAcrossV3Test is TestBase { assertTrue(dai.balanceOf(receiverAddress) == amountOutMin); } - function test_willRevertIfGasIsLessThanRecoverGas() public { - // mock-send bridged funds to receiver contract - deal(ADDRESS_USDC, address(receiver), defaultUSDCAmount); - - // encode payload with mock data like Stargate would according to: - (bytes memory payload, ) = _getValidAcrossV3Payload( - ADDRESS_USDC, - ADDRESS_DAI - ); - - // fake a sendCompose from USDC pool on ETH mainnet - vm.startPrank(SPOKEPOOL_MAINNET); - - vm.expectRevert(abi.encodeWithSelector(InsufficientGasLimit.selector)); - - receiver.handleV3AcrossMessage{ gas: RECOVER_GAS_VALUE }( - ADDRESS_USDC, - defaultUSDCAmount, - address(0), - payload - ); - } - - function test_willRevertIfDestCallRunsOutOfGas() public { - // mock-send bridged funds to receiver contract - deal(ADDRESS_USDC, address(receiver), defaultUSDCAmount); - - // encode payload with mock data like Stargate would according to: - (bytes memory payload, ) = _getValidAcrossV3Payload( - ADDRESS_USDC, - ADDRESS_DAI - ); - - // fake a sendCompose from USDC pool on ETH mainnet - vm.startPrank(SPOKEPOOL_MAINNET); - - vm.expectRevert(abi.encodeWithSelector(InsufficientGasLimit.selector)); - - receiver.handleV3AcrossMessage{ gas: RECOVER_GAS_VALUE + 150000 }( - ADDRESS_USDC, - defaultUSDCAmount, - address(0), - payload - ); - } - function test_willReturnFundsToUserIfDstCallFails() public { // mock-send bridged funds to receiver contract deal(ADDRESS_USDC, address(receiver), defaultUSDCAmount); @@ -248,7 +195,7 @@ contract ReceiverAcrossV3Test is TestBase { defaultUSDCAmount, block.timestamp ); - receiver.handleV3AcrossMessage{ gas: RECOVER_GAS_VALUE + 200000 }( + receiver.handleV3AcrossMessage( ADDRESS_USDC, defaultUSDCAmount, address(0),