Skip to content

Commit

Permalink
Drop gas introspection in receiver contract
Browse files Browse the repository at this point in the history
  • Loading branch information
@ezynda3
ezynda3 committed Dec 6, 2024
1 parent 4b24a76 commit 56aebd5
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 87 deletions.
39 changes: 8 additions & 31 deletions src/Periphery/ReceiverAcrossV3.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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 ///
Expand Down Expand Up @@ -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)
Expand All @@ -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);

Expand Down
59 changes: 3 additions & 56 deletions test/solidity/Periphery/ReceiverAcrossV3.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit 56aebd5

Please sign in to comment.