don't revert on rate limit errors

smartcontractkit · Sep 27, 2023 · 062a191 · 062a191
1 parent 1f14684
commit 062a191
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 38 deletions.
diff --git a/contracts/gas-snapshots/ccip.gas-snapshot b/contracts/gas-snapshots/ccip.gas-snapshot
@@ -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)
@@ -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)
@@ -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)

diff --git a/contracts/src/v0.8/ccip/offRamp/EVM2EVMOffRamp.sol b/contracts/src/v0.8/ccip/offRamp/EVM2EVMOffRamp.sol
@@ -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 {
@@ -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());

diff --git a/contracts/src/v0.8/ccip/test/offRamp/EVM2EVMOffRamp.t.sol b/contracts/src/v0.8/ccip/test/offRamp/EVM2EVMOffRamp.t.sol
@@ -1137,9 +1137,7 @@ contract EVM2EVMOffRamp__trialExecute is EVM2EVMOffRampSetup {
     assertEq(startingBalance, dstToken0.balanceOf(OWNER));
   }
 
-  // Reverts
-
-  function testRateLimitErrorReverts() public {
+  function testRateLimitErrorSuccess() public {
     uint256[] memory amounts = new uint256[](2);
     amounts[0] = 1000;
     amounts[1] = 50;
@@ -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 error) = 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), error);
   }
 }
 
@@ -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,