Skip to content

Commit

Permalink
fix bug arising from tokenAmounts containing the fee token
Browse files Browse the repository at this point in the history
  • Loading branch information
jhweintraub committed Jul 15, 2024
1 parent 4312d74 commit e21f356
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 22 deletions.
16 changes: 8 additions & 8 deletions contracts/gas-snapshots/ccip.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ BurnWithFromMintTokenPool_lockOrBurn:test_PoolBurnRevertNotHealthy_Revert() (gas
BurnWithFromMintTokenPool_lockOrBurn:test_PoolBurn_Success() (gas: 243568)
BurnWithFromMintTokenPool_lockOrBurn:test_Setup_Success() (gas: 24260)
CCIPClientTest:test_HappyPath_Success() (gas: 192504)
CCIPClientTest:test_ccipSend_withNonNativeFeetoken_andDestTokens_Success() (gas: 326914)
CCIPClientTest:test_ccipSend_withNonNativeFeetoken_andNoDestTokens_Success() (gas: 219955)
CCIPClientTest:test_ccipSend_with_NativeFeeToken_andDestTokens_Success() (gas: 376823)
CCIPClientTest:test_ccipSend_withNonNativeFeetoken_andDestTokens_Success() (gas: 326927)
CCIPClientTest:test_ccipSend_withNonNativeFeetoken_andNoDestTokens_Success() (gas: 219968)
CCIPClientTest:test_ccipSend_with_NativeFeeToken_andDestTokens_Success() (gas: 376836)
CCIPClientTest:test_modifyFeeToken_Success() (gas: 74452)
CCIPClientWithACKTest:test_ccipReceiveAndSendAck_Success() (gas: 331795)
CCIPClientWithACKTest:test_ccipReceiver_ack_with_invalidAckMessageHeaderBytes_Revert() (gas: 438714)
CCIPClientWithACKTest:test_ccipSendAndReceiveAck_in_return_Success() (gas: 348231)
CCIPClientWithACKTest:test_ccipSend_withNonNativeFeetoken_andNoDestTokens_Success() (gas: 241532)
CCIPClientWithACKTest:test_send_tokens_that_are_not_feeToken_Success() (gas: 552182)
CCIPClientWithACKTest:test_ccipSendAndReceiveAck_in_return_Success() (gas: 349366)
CCIPClientWithACKTest:test_ccipSend_withNonNativeFeetoken_andNoDestTokens_Success() (gas: 242666)
CCIPClientWithACKTest:test_send_tokens_that_are_not_feeToken_Success() (gas: 553089)
CCIPConfigSetup:test_getCapabilityConfiguration_Success() (gas: 9495)
CCIPConfig_ConfigStateMachine:test__computeConfigDigest_Success() (gas: 70755)
CCIPConfig_ConfigStateMachine:test__computeNewConfigWithMeta_InitToRunning_Success() (gas: 363647)
Expand Down Expand Up @@ -703,10 +703,10 @@ OCR2Base_transmit:test_UnAuthorizedTransmitter_Revert() (gas: 23484)
OCR2Base_transmit:test_UnauthorizedSigner_Revert() (gas: 39665)
OCR2Base_transmit:test_WrongNumberOfSignatures_Revert() (gas: 20557)
OnRampTokenPoolReentrancy:test_OnRampTokenPoolReentrancy_Success() (gas: 380711)
PingPong_example_ccipReceive:test_CcipReceive_Success() (gas: 179553)
PingPong_example_ccipReceive:test_CcipReceive_Success() (gas: 179566)
PingPong_example_plumbing:test_Pausing_Success() (gas: 17917)
PingPong_example_plumbing:test_typeAndVersion() (gas: 9786)
PingPong_example_startPingPong:test_StartPingPong_Success() (gas: 203446)
PingPong_example_startPingPong:test_StartPingPong_Success() (gas: 203459)
PriceRegistry_applyFeeTokensUpdates:test_ApplyFeeTokensUpdates_Success() (gas: 79823)
PriceRegistry_applyFeeTokensUpdates:test_OnlyCallableByOwner_Revert() (gas: 12580)
PriceRegistry_constructor:test_InvalidStalenessThreshold_Revert() (gas: 67418)
Expand Down
16 changes: 9 additions & 7 deletions contracts/src/v0.8/ccip/applications/external/CCIPClient.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ contract CCIPClient is CCIPReceiver {
feeToken: address(s_feeToken)
});

uint256 fee = IRouterClient(s_ccipRouter).getFee(destChainSelector, message);

// Additional tokens for fees do not need to be approved to the router since it is already handled by setting s_feeToken
// Fee transfers need first, that way balanceOf(address(this)) does not conflict with any tokens sent in tokenAmounts
// to support fee-token pre-funding
if ((address(s_feeToken) != address(0)) && (s_feeToken.balanceOf(address(this)) < fee)) {
IERC20(s_feeToken).safeTransferFrom(msg.sender, address(this), fee);
}

for (uint256 i = 0; i < tokenAmounts.length; ++i) {
// Transfer the tokens to pay for tokens in tokenAmounts
IERC20(tokenAmounts[i].token).safeTransferFrom(msg.sender, address(this), tokenAmounts[i].amount);
Expand All @@ -55,13 +64,6 @@ contract CCIPClient is CCIPReceiver {
}
}

uint256 fee = IRouterClient(s_ccipRouter).getFee(destChainSelector, message);

// Additional tokens for fees do not need to be approved to the router since it is already handled by setting s_feeToken
if ((address(s_feeToken) != address(0)) && (s_feeToken.balanceOf(address(this)) < fee)) {
IERC20(s_feeToken).safeTransferFrom(msg.sender, address(this), fee);
}

// messageId is only generated in the on-ramp, and therefore cannot be calcualated head of time.
// This necessitates breaking CEI but since the router is a trusted contract, any risks are negligible.
messageId = IRouterClient(s_ccipRouter).ccipSend{value: address(s_feeToken) == address(0) ? fee : 0}(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ contract CCIPClientWithACK is CCIPReceiverWithACK {
feeToken: address(s_feeToken)
});

uint256 fee = IRouterClient(s_ccipRouter).getFee(destChainSelector, message);

// Additional tokens for fees do not need to be approved to the router since it is already handled by setting s_feeToken
// Fee transfers need first, that way balanceOf(address(this)) does not conflict with any tokens sent in tokenAmounts
// to support fee-token pre-funding
if ((address(s_feeToken) != address(0)) && (s_feeToken.balanceOf(address(this)) < fee)) {
IERC20(s_feeToken).safeTransferFrom(msg.sender, address(this), fee);
}

for (uint256 i = 0; i < tokenAmounts.length; ++i) {
// Transfer the tokens to pay for tokens in tokenAmounts
IERC20(tokenAmounts[i].token).safeTransferFrom(msg.sender, address(this), tokenAmounts[i].amount);
Expand All @@ -47,13 +56,6 @@ contract CCIPClientWithACK is CCIPReceiverWithACK {
}
}

uint256 fee = IRouterClient(s_ccipRouter).getFee(destChainSelector, message);

// Additional tokens for fees do not need to be approved to the router since it is already handled by setting s_feeToken
if (address(s_feeToken) != address(0)) {
IERC20(s_feeToken).safeTransferFrom(msg.sender, address(this), fee);
}

messageId = IRouterClient(s_ccipRouter).ccipSend{value: address(s_feeToken) == address(0) ? fee : 0}(
destChainSelector, message
);
Expand Down

0 comments on commit e21f356

Please sign in to comment.