Skip to content

Commit

Permalink
don't revert on rate limit errors (#170)
Browse files Browse the repository at this point in the history
* don't revert on rate limit errors

* make wrappers

* don't shadow error name
  • Loading branch information
RensR authored Sep 29, 2023
1 parent f961982 commit 9639d1a
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 46 deletions.
18 changes: 9 additions & 9 deletions contracts/gas-snapshots/ccip.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,13 @@ CommitStore_verify:testTooManyLeavesReverts() (gas: 36830)
DefensiveExampleTest:testHappyPathSuccess() (gas: 174828)
DefensiveExampleTest:testRecovery() (gas: 399728)
E2E:testE2E_3MessagesSuccess_gas() (gas: 863423)
EVM2EVMOffRamp__releaseOrMintTokens:testRateLimitErrorsReverts() (gas: 428056)
EVM2EVMOffRamp__releaseOrMintTokens:testTokenHandlingErrorReverts() (gas: 100322)
EVM2EVMOffRamp__releaseOrMintTokens:testRateLimitErrorsReverts() (gas: 426826)
EVM2EVMOffRamp__releaseOrMintTokens:testTokenHandlingErrorReverts() (gas: 100041)
EVM2EVMOffRamp__releaseOrMintTokens:testUnsupportedTokenReverts() (gas: 18159)
EVM2EVMOffRamp__releaseOrMintTokens:test_releaseOrMintTokensSuccess() (gas: 139008)
EVM2EVMOffRamp__report:testReportSuccess() (gas: 126666)
EVM2EVMOffRamp__trialExecute:testRateLimitErrorReverts() (gas: 150522)
EVM2EVMOffRamp__trialExecute:testTokenHandlingErrorIsCaughtSuccess() (gas: 179946)
EVM2EVMOffRamp__trialExecute:testRateLimitErrorSuccess() (gas: 171346)
EVM2EVMOffRamp__trialExecute:testTokenHandlingErrorIsCaughtSuccess() (gas: 179665)
EVM2EVMOffRamp__trialExecute:test_trialExecuteSuccess() (gas: 228022)
EVM2EVMOffRamp_applyPoolUpdates:testApplyPoolUpdatesSuccess() (gas: 2548304)
EVM2EVMOffRamp_applyPoolUpdates:testInvalidTokenPoolConfigReverts() (gas: 17359)
Expand All @@ -102,10 +102,10 @@ EVM2EVMOffRamp_applyPoolUpdates:testPoolAlreadyExistsReverts() (gas: 5153053)
EVM2EVMOffRamp_applyPoolUpdates:testPoolDoesNotExistReverts() (gas: 2512963)
EVM2EVMOffRamp_applyPoolUpdates:testTokenPoolMismatchReverts() (gas: 5155382)
EVM2EVMOffRamp_ccipReceive:testReverts() (gas: 17094)
EVM2EVMOffRamp_constructor:testCommitStoreAlreadyInUseReverts() (gas: 169257)
EVM2EVMOffRamp_constructor:testConstructorSuccess() (gas: 6004286)
EVM2EVMOffRamp_constructor:testTokenConfigMismatchReverts() (gas: 145623)
EVM2EVMOffRamp_constructor:testZeroOnRampAddressReverts() (gas: 2651907)
EVM2EVMOffRamp_constructor:testCommitStoreAlreadyInUseReverts() (gas: 169122)
EVM2EVMOffRamp_constructor:testConstructorSuccess() (gas: 5915423)
EVM2EVMOffRamp_constructor:testTokenConfigMismatchReverts() (gas: 145488)
EVM2EVMOffRamp_constructor:testZeroOnRampAddressReverts() (gas: 2651772)
EVM2EVMOffRamp_execute:testAlreadyExecutedReverts() (gas: 136534)
EVM2EVMOffRamp_execute:testEmptyReportReverts() (gas: 18990)
EVM2EVMOffRamp_execute:testInvalidMessageIdReverts() (gas: 33939)
Expand Down Expand Up @@ -133,7 +133,7 @@ EVM2EVMOffRamp_executeSingleMessage:testMessageSenderReverts() (gas: 20494)
EVM2EVMOffRamp_executeSingleMessage:testNoTokensSuccess() (gas: 47668)
EVM2EVMOffRamp_executeSingleMessage:testNonContractSuccess() (gas: 20042)
EVM2EVMOffRamp_executeSingleMessage:testNonContractWithTokensSuccess() (gas: 196939)
EVM2EVMOffRamp_executeSingleMessage:testTokenHandlingErrorReverts() (gas: 136903)
EVM2EVMOffRamp_executeSingleMessage:testTokenHandlingErrorReverts() (gas: 136622)
EVM2EVMOffRamp_executeSingleMessage:testTokensSuccess() (gas: 225488)
EVM2EVMOffRamp_executeSingleMessage:testZeroGasDONExecutionReverts() (gas: 48191)
EVM2EVMOffRamp_execute_upgrade:testV2NonceNewSenderStartsAtZeroSuccess() (gas: 230624)
Expand Down
22 changes: 4 additions & 18 deletions contracts/src/v0.8/ccip/offRamp/EVM2EVMOffRamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, TypeAndVersion
try this.executeSingleMessage(message, offchainTokenData) {} catch (bytes memory err) {
if (ReceiverError.selector == bytes4(err) || TokenHandlingError.selector == bytes4(err)) {
// If CCIP receiver execution is not successful, bubble up receiver revert data,
// prepended by the 4 bytes of ReceiverError.selector
// prepended by the 4 bytes of ReceiverError.selector or TokenHandlingError.selector
// Max length of revert data is Router.MAX_RET_BYTES, max length of err is 4 + Router.MAX_RET_BYTES
return (Internal.MessageExecutionState.FAILURE, err);
} else {
Expand Down Expand Up @@ -589,23 +589,9 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, TypeAndVersion
i_sourceChainSelector,
abi.encode(sourceTokenData[i], offchainTokenData[i])
)
{} catch (
/// @dev we only want to revert on rate limiting errors, any other errors are
/// wrapped in a TokenHandlingError, which is caught above and handled gracefully.
bytes memory err
) {
bytes4 errSig = bytes4(err);
if (
RateLimiter.BucketOverfilled.selector == errSig ||
RateLimiter.AggregateValueMaxCapacityExceeded.selector == errSig ||
RateLimiter.AggregateValueRateLimitReached.selector == errSig ||
RateLimiter.TokenMaxCapacityExceeded.selector == errSig ||
RateLimiter.TokenRateLimitReached.selector == errSig
) {
revert TokenRateLimitError(err);
} else {
revert TokenHandlingError(err);
}
{} catch (bytes memory err) {
/// @dev wrap and rethrow the error so we can catch it lower in the stack
revert TokenHandlingError(err);
}

destTokenAmounts[i].token = address(pool.getToken());
Expand Down
26 changes: 11 additions & 15 deletions contracts/src/v0.8/ccip/test/offRamp/EVM2EVMOffRamp.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -1102,12 +1102,12 @@ contract EVM2EVMOffRamp__trialExecute is EVM2EVMOffRampSetup {
IERC20 dstToken0 = IERC20(s_destTokens[0]);
uint256 startingBalance = dstToken0.balanceOf(message.receiver);

(Internal.MessageExecutionState newState, bytes memory error) = s_offRamp.trialExecute(
(Internal.MessageExecutionState newState, bytes memory err) = s_offRamp.trialExecute(
message,
new bytes[](message.tokenAmounts.length)
);
assertEq(uint256(Internal.MessageExecutionState.SUCCESS), uint256(newState));
assertEq("", error);
assertEq("", err);

// Check that the tokens were transferred
assertEq(startingBalance + amounts[0], dstToken0.balanceOf(message.receiver));
Expand All @@ -1126,20 +1126,18 @@ contract EVM2EVMOffRamp__trialExecute is EVM2EVMOffRampSetup {
Internal.EVM2EVMMessage memory message = _generateAny2EVMMessageWithTokens(1, amounts);
MaybeRevertingBurnMintTokenPool(s_destPools[1]).setShouldRevert(errorMessage);

(Internal.MessageExecutionState newState, bytes memory error) = s_offRamp.trialExecute(
(Internal.MessageExecutionState newState, bytes memory err) = s_offRamp.trialExecute(
message,
new bytes[](message.tokenAmounts.length)
);
assertEq(uint256(Internal.MessageExecutionState.FAILURE), uint256(newState));
assertEq(abi.encodeWithSelector(EVM2EVMOffRamp.TokenHandlingError.selector, errorMessage), error);
assertEq(abi.encodeWithSelector(EVM2EVMOffRamp.TokenHandlingError.selector, errorMessage), err);

// Expect the balance to remain the same
assertEq(startingBalance, dstToken0.balanceOf(OWNER));
}

// Reverts

function testRateLimitErrorReverts() public {
function testRateLimitErrorSuccess() public {
uint256[] memory amounts = new uint256[](2);
amounts[0] = 1000;
amounts[1] = 50;
Expand All @@ -1149,14 +1147,12 @@ contract EVM2EVMOffRamp__trialExecute is EVM2EVMOffRampSetup {
Internal.EVM2EVMMessage memory message = _generateAny2EVMMessageWithTokens(1, amounts);
MaybeRevertingBurnMintTokenPool(s_destPools[1]).setShouldRevert(errorMessage);

vm.expectRevert(
abi.encodeWithSelector(
EVM2EVMOffRamp.ExecutionError.selector,
abi.encodeWithSelector(EVM2EVMOffRamp.TokenRateLimitError.selector, errorMessage)
)
(Internal.MessageExecutionState newState, bytes memory err) = s_offRamp.trialExecute(
message,
new bytes[](message.tokenAmounts.length)
);

s_offRamp.trialExecute(message, new bytes[](message.tokenAmounts.length));
assertEq(uint256(Internal.MessageExecutionState.FAILURE), uint256(newState));
assertEq(abi.encodeWithSelector(EVM2EVMOffRamp.TokenHandlingError.selector, errorMessage), err);
}
}

Expand Down Expand Up @@ -1244,7 +1240,7 @@ contract EVM2EVMOffRamp__releaseOrMintTokens is EVM2EVMOffRampSetup {
for (uint256 i = 0; i < rateLimitErrors.length; ++i) {
MaybeRevertingBurnMintTokenPool(s_destPools[1]).setShouldRevert(rateLimitErrors[i]);

vm.expectRevert(abi.encodeWithSelector(EVM2EVMOffRamp.TokenRateLimitError.selector, rateLimitErrors[i]));
vm.expectRevert(abi.encodeWithSelector(EVM2EVMOffRamp.TokenHandlingError.selector, rateLimitErrors[i]));

s_offRamp.releaseOrMintTokens(
srcTokenAmounts,
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ burn_mint_token_pool: ../../../contracts/solc/v0.8.19/BurnMintTokenPool.abi ../.
commit_store: ../../../contracts/solc/v0.8.19/CommitStore.abi ../../../contracts/solc/v0.8.19/CommitStore.bin 7d80f085f8249ef15e0850bf03698806c972125156f3476a1dafca54598f7f4d
commit_store_helper: ../../../contracts/solc/v0.8.19/CommitStoreHelper.abi ../../../contracts/solc/v0.8.19/CommitStoreHelper.bin 0891a1662bda54db424d12e80a56533cfd9859f7d94a3be147ad3caa46cad072
custom_token_pool: ../../../contracts/solc/v0.8.19/CustomTokenPool.abi ../../../contracts/solc/v0.8.19/CustomTokenPool.bin 79ab937aa4493bf31fb0e57affd00555aad75205c90268e89674c28ea9e5e48f
evm_2_evm_offramp: ../../../contracts/solc/v0.8.19/EVM2EVMOffRamp.abi ../../../contracts/solc/v0.8.19/EVM2EVMOffRamp.bin 8a0bd8a428ae8d071d326a2d4e1f7a2d35da08fef7307b6e1f15e108eb23770e
evm_2_evm_offramp_helper: ../../../contracts/solc/v0.8.19/EVM2EVMOffRampHelper.abi ../../../contracts/solc/v0.8.19/EVM2EVMOffRampHelper.bin 9e06fb409bcc9b1728c1a327d404640796c2834e7798728224b8ad4e5745c4a7
evm_2_evm_offramp: ../../../contracts/solc/v0.8.19/EVM2EVMOffRamp.abi ../../../contracts/solc/v0.8.19/EVM2EVMOffRamp.bin bfea579e0b5100b6e90c2ea4d1533789ff037104fd03ba590e135772a0cf0997
evm_2_evm_offramp_helper: ../../../contracts/solc/v0.8.19/EVM2EVMOffRampHelper.abi ../../../contracts/solc/v0.8.19/EVM2EVMOffRampHelper.bin fe22966f5d97a8f386a4f01f901a308592336ea52c8fd06311f4bf1a961033ad
evm_2_evm_onramp: ../../../contracts/solc/v0.8.19/EVM2EVMOnRamp.abi ../../../contracts/solc/v0.8.19/EVM2EVMOnRamp.bin 1c21fbc9b49521a9a783c40b0674297bfcb18a64100dae10173682e38ea199fc
lock_release_token_pool: ../../../contracts/solc/v0.8.19/LockReleaseTokenPool.abi ../../../contracts/solc/v0.8.19/LockReleaseTokenPool.bin 7f7a28f55f9fb63669cd8038a7f99e31431acd6d15ddeeb6a77188eb0bf85d58
maybe_revert_message_receiver: ../../../contracts/solc/v0.8.19/MaybeRevertMessageReceiver.abi ../../../contracts/solc/v0.8.19/MaybeRevertMessageReceiver.bin aaa90eac8cc555ee4b0fbe57d1fb8d72d6689b29510b238177c97ab9b7979ac5
Expand Down

0 comments on commit 9639d1a

Please sign in to comment.