revert manual exec on inner revert (#333)

contracts/gas-snapshots/ccip.gas-snapshot

@@ -95,12 +95,12 @@ CommitStore_verify:testPausedReverts() (gas: 18438)
 CommitStore_verify:testTooManyLeavesReverts() (gas: 36830)
 DefensiveExampleTest:testHappyPathSuccess() (gas: 174862)
 DefensiveExampleTest:testRecovery() (gas: 399786)
-E2E:testE2E_3MessagesSuccess_gas() (gas: 887723)
+E2E:testE2E_3MessagesSuccess_gas() (gas: 887843)
 EVM2EVMOffRamp__releaseOrMintTokens:testRateLimitErrorsReverts() (gas: 454832)
 EVM2EVMOffRamp__releaseOrMintTokens:testTokenHandlingErrorReverts() (gas: 105010)
 EVM2EVMOffRamp__releaseOrMintTokens:testUnsupportedTokenReverts() (gas: 19784)
 EVM2EVMOffRamp__releaseOrMintTokens:test_releaseOrMintTokensSuccess() (gas: 140631)
-EVM2EVMOffRamp__report:testReportSuccess() (gas: 127236)
+EVM2EVMOffRamp__report:testReportSuccess() (gas: 127276)
 EVM2EVMOffRamp__trialExecute:testRateLimitErrorSuccess() (gas: 174934)
 EVM2EVMOffRamp__trialExecute:testTokenHandlingErrorIsCaughtSuccess() (gas: 183256)
 EVM2EVMOffRamp__trialExecute:test_trialExecuteSuccess() (gas: 236792)
@@ -111,31 +111,31 @@ EVM2EVMOffRamp_applyPoolUpdates:testPoolAlreadyExistsReverts() (gas: 4926374)
 EVM2EVMOffRamp_applyPoolUpdates:testPoolDoesNotExistReverts() (gas: 2399537)
 EVM2EVMOffRamp_applyPoolUpdates:testTokenPoolMismatchReverts() (gas: 4928727)
 EVM2EVMOffRamp_ccipReceive:testReverts() (gas: 17072)
-EVM2EVMOffRamp_constructor:testCommitStoreAlreadyInUseReverts() (gas: 168848)
-EVM2EVMOffRamp_constructor:testConstructorSuccess() (gas: 5730956)
-EVM2EVMOffRamp_constructor:testTokenConfigMismatchReverts() (gas: 145215)
-EVM2EVMOffRamp_constructor:testZeroOnRampAddressReverts() (gas: 2538111)
-EVM2EVMOffRamp_execute:testAlreadyExecutedReverts() (gas: 137288)
+EVM2EVMOffRamp_constructor:testCommitStoreAlreadyInUseReverts() (gas: 168876)
+EVM2EVMOffRamp_constructor:testConstructorSuccess() (gas: 5748402)
+EVM2EVMOffRamp_constructor:testTokenConfigMismatchReverts() (gas: 145243)
+EVM2EVMOffRamp_constructor:testZeroOnRampAddressReverts() (gas: 2538140)
+EVM2EVMOffRamp_execute:testAlreadyExecutedReverts() (gas: 137328)
 EVM2EVMOffRamp_execute:testEmptyReportReverts() (gas: 19016)
 EVM2EVMOffRamp_execute:testInvalidMessageIdReverts() (gas: 34063)
 EVM2EVMOffRamp_execute:testInvalidSourceChainReverts() (gas: 49347)
 EVM2EVMOffRamp_execute:testManualExecutionNotYetEnabledReverts() (gas: 44104)
 EVM2EVMOffRamp_execute:testMessageTooLargeReverts() (gas: 150111)
 EVM2EVMOffRamp_execute:testPausedReverts() (gas: 74942)
-EVM2EVMOffRamp_execute:testReceiverErrorSuccess() (gas: 163658)
+EVM2EVMOffRamp_execute:testReceiverErrorSuccess() (gas: 163698)
 EVM2EVMOffRamp_execute:testRootNotCommittedReverts() (gas: 38980)
 EVM2EVMOffRamp_execute:testRouterYULCallReverts() (gas: 413652)
-EVM2EVMOffRamp_execute:testSingleMessageNoTokensSuccess() (gas: 173465)
-EVM2EVMOffRamp_execute:testSingleMessageToNonCCIPReceiverSuccess() (gas: 246243)
-EVM2EVMOffRamp_execute:testSingleMessagesNoTokensSuccess_gas() (gas: 114632)
-EVM2EVMOffRamp_execute:testSkippedIncorrectNonceStillExecutesSuccess() (gas: 324071)
+EVM2EVMOffRamp_execute:testSingleMessageNoTokensSuccess() (gas: 173545)
+EVM2EVMOffRamp_execute:testSingleMessageToNonCCIPReceiverSuccess() (gas: 246283)
+EVM2EVMOffRamp_execute:testSingleMessagesNoTokensSuccess_gas() (gas: 114672)
+EVM2EVMOffRamp_execute:testSkippedIncorrectNonceStillExecutesSuccess() (gas: 324111)
 EVM2EVMOffRamp_execute:testSkippedIncorrectNonceSuccess() (gas: 51868)
-EVM2EVMOffRamp_execute:testStrictUntouchedToSuccessSuccess() (gas: 130854)
+EVM2EVMOffRamp_execute:testStrictUntouchedToSuccessSuccess() (gas: 130894)
 EVM2EVMOffRamp_execute:testTokenDataMismatchReverts() (gas: 49702)
-EVM2EVMOffRamp_execute:testTwoMessagesWithTokensAndGESuccess() (gas: 443316)
-EVM2EVMOffRamp_execute:testTwoMessagesWithTokensSuccess_gas() (gas: 402490)
+EVM2EVMOffRamp_execute:testTwoMessagesWithTokensAndGESuccess() (gas: 443470)
+EVM2EVMOffRamp_execute:testTwoMessagesWithTokensSuccess_gas() (gas: 402570)
 EVM2EVMOffRamp_execute:testUnexpectedTokenDataReverts() (gas: 33032)
-EVM2EVMOffRamp_execute:testUnhealthyReverts() (gas: 419917)
+EVM2EVMOffRamp_execute:testUnhealthyReverts() (gas: 419997)
 EVM2EVMOffRamp_execute:testUnsupportedNumberOfTokensReverts() (gas: 61416)
 EVM2EVMOffRamp_execute:testUnsupportedTokenReverts() (gas: 130374)
 EVM2EVMOffRamp_executeSingleMessage:testMessageSenderReverts() (gas: 20545)
@@ -145,23 +145,24 @@ EVM2EVMOffRamp_executeSingleMessage:testNonContractWithTokensSuccess() (gas: 205
 EVM2EVMOffRamp_executeSingleMessage:testTokenHandlingErrorReverts() (gas: 140032)
 EVM2EVMOffRamp_executeSingleMessage:testTokensSuccess() (gas: 234077)
 EVM2EVMOffRamp_executeSingleMessage:testZeroGasDONExecutionReverts() (gas: 48581)
-EVM2EVMOffRamp_execute_upgrade:testV2NonceNewSenderStartsAtZeroSuccess() (gas: 232027)
-EVM2EVMOffRamp_execute_upgrade:testV2NonceStartsAtV1NonceSuccess() (gas: 279912)
-EVM2EVMOffRamp_execute_upgrade:testV2OffRampNonceSkipsIfMsgInFlightSuccess() (gas: 261079)
-EVM2EVMOffRamp_execute_upgrade:testV2SenderNoncesReadsPreviousRampSuccess() (gas: 225202)
-EVM2EVMOffRamp_execute_upgrade:testV2Success() (gas: 131369)
+EVM2EVMOffRamp_execute_upgrade:testV2NonceNewSenderStartsAtZeroSuccess() (gas: 232107)
+EVM2EVMOffRamp_execute_upgrade:testV2NonceStartsAtV1NonceSuccess() (gas: 280032)
+EVM2EVMOffRamp_execute_upgrade:testV2OffRampNonceSkipsIfMsgInFlightSuccess() (gas: 261159)
+EVM2EVMOffRamp_execute_upgrade:testV2SenderNoncesReadsPreviousRampSuccess() (gas: 225322)
+EVM2EVMOffRamp_execute_upgrade:testV2Success() (gas: 131409)
 EVM2EVMOffRamp_getDestinationToken:testGetDestinationTokenSuccess() (gas: 32786)
 EVM2EVMOffRamp_getDestinationToken:testUnsupportedTokenReverts() (gas: 13763)
 EVM2EVMOffRamp_getDestinationTokens:testGetDestinationTokensSuccess() (gas: 26091)
 EVM2EVMOffRamp_getExecutionState:testFillExecutionStateSuccess() (gas: 3047481)
 EVM2EVMOffRamp_getExecutionState:test_GetExecutionStateSuccess() (gas: 76354)
-EVM2EVMOffRamp_manuallyExecute:testLowGasLimitManualExecSuccess() (gas: 496879)
-EVM2EVMOffRamp_manuallyExecute:testManualExecForkedChainReverts() (gas: 25977)
-EVM2EVMOffRamp_manuallyExecute:testManualExecGasLimitMismatchReverts() (gas: 43615)
-EVM2EVMOffRamp_manuallyExecute:testManualExecInvalidGasLimitReverts() (gas: 26018)
-EVM2EVMOffRamp_manuallyExecute:testManualExecSuccess() (gas: 189562)
-EVM2EVMOffRamp_manuallyExecute:testManualExecWithGasOverrideSuccess() (gas: 190189)
-EVM2EVMOffRamp_manuallyExecute:testReentrancyManualExecuteFAILS() (gas: 1856211)
+EVM2EVMOffRamp_manuallyExecute:testLowGasLimitManualExecSuccess() (gas: 497074)
+EVM2EVMOffRamp_manuallyExecute:testManualExecFailedTxReverts() (gas: 188016)
+EVM2EVMOffRamp_manuallyExecute:testManualExecForkedChainReverts() (gas: 26018)
+EVM2EVMOffRamp_manuallyExecute:testManualExecGasLimitMismatchReverts() (gas: 43673)
+EVM2EVMOffRamp_manuallyExecute:testManualExecInvalidGasLimitReverts() (gas: 26052)
+EVM2EVMOffRamp_manuallyExecute:testManualExecSuccess() (gas: 189664)
+EVM2EVMOffRamp_manuallyExecute:testManualExecWithGasOverrideSuccess() (gas: 190291)
+EVM2EVMOffRamp_manuallyExecute:testReentrancyManualExecuteFAILS() (gas: 1835231)
 EVM2EVMOffRamp_metadataHash:testMetadataHashSuccess() (gas: 6086)
 EVM2EVMOffRamp_setDynamicConfig:testNonOwnerReverts() (gas: 44539)
 EVM2EVMOffRamp_setDynamicConfig:testRouterZeroAddressReverts() (gas: 38587)

contracts/gas-snapshots/functions.gas-snapshot

@@ -102,11 +102,11 @@ FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_Reve
 FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_RevertIfStartIsAfterEnd() (gas: 13436)
 FunctionsSubscriptions_GetSubscriptionsInRange:test_GetSubscriptionsInRange_Success() (gas: 59568)
 FunctionsSubscriptions_GetTotalBalance:test_GetTotalBalance_Success() (gas: 15032)
-FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoCalldata(uint96) (runs: 256, μ: 28401, ~: 28401)
-FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoSubscription(uint96) (runs: 256, μ: 30913, ~: 30913)
-FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNotLink(uint96) (runs: 256, μ: 14248, ~: 14248)
-FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfPaused(uint96) (runs: 256, μ: 35870, ~: 35870)
-FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_Success(uint96) (runs: 256, μ: 59685, ~: 59685)
+FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoCalldata() (gas: 27586)
+FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNoSubscription() (gas: 30051)
+FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfCallerIsNotLink() (gas: 13388)
+FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_RevertIfPaused() (gas: 34989)
+FunctionsSubscriptions_OnTokenTransfer:test_OnTokenTransfer_Success() (gas: 56279)
 FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfAmountMoreThanBalance() (gas: 20745)
 FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfBalanceInvariant() (gas: 189)
 FunctionsSubscriptions_OracleWithdraw:test_OracleWithdraw_RevertIfNoAmount() (gas: 15638)

contracts/scripts/native_solc_compile_all_ccip

@@ -41,7 +41,7 @@ compileContractLowOpts () {
 
 # Solc produces and overwrites intermediary contracts.
 # Contracts should be ordered in reverse-import-complexity-order to minimize overwrite risks.
-compileContract ccip/offRamp/EVM2EVMOffRamp.sol
+compileContractLowOpts ccip/offRamp/EVM2EVMOffRamp.sol
 compileContract ccip/applications/PingPongDemo.sol
 compileContract ccip/applications/SelfFundedPingPong.sol
 compileContractLowOpts ccip/onRamp/EVM2EVMOnRamp.sol
@@ -59,7 +59,6 @@ compileContract ccip/ARMProxy.sol
 # Test helpers
 compileContract ccip/test/helpers/BurnMintERC677Helper.sol
 compileContract ccip/test/helpers/CommitStoreHelper.sol
-compileContract ccip/test/helpers/EVM2EVMOffRampHelper.sol
 compileContract ccip/test/helpers/CustomTokenPool.sol
 compileContract ccip/test/helpers/receivers/MaybeRevertMessageReceiver.sol
 compileContract ccip/test/mocks/MockARM.sol

contracts/src/v0.8/ccip/offRamp/EVM2EVMOffRamp.sol

@@ -99,7 +99,7 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
 
   // STATIC CONFIG
   // solhint-disable-next-line chainlink-solidity/all-caps-constant-storage-variables
-  string public constant override typeAndVersion = "EVM2EVMOffRamp 1.2.0";
+  string public constant override typeAndVersion = "EVM2EVMOffRamp 1.3.0-dev";
   /// @dev Commit store address on the destination chain
   address internal immutable i_commitStore;
   /// @dev ChainSelector of the source chain
@@ -343,6 +343,14 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
       (Internal.MessageExecutionState newState, bytes memory returnData) = _trialExecute(message, offchainTokenData);
       _setExecutionState(message.sequenceNumber, newState);
 
+      // Since it's hard to estimate whether manual execution will succeed, we
+      // revert the entire transaction if it fails. This will show the user if
+      // their manual exec will fail before they submit it.
+      if (manualExecution && newState == Internal.MessageExecutionState.FAILURE) {
+        // If manual execution fails, we revert the entire transaction.
+        revert ExecutionError(returnData);
+      }
+
       // The only valid prior states are UNTOUCHED and FAILURE (checked above)
       // The only valid post states are FAILURE and SUCCESS (checked below)
       if (newState != Internal.MessageExecutionState.FAILURE && newState != Internal.MessageExecutionState.SUCCESS)

contracts/src/v0.8/ccip/test/offRamp/EVM2EVMOffRamp.t.sol

@@ -74,7 +74,7 @@ contract EVM2EVMOffRamp_constructor is EVM2EVMOffRampSetup {
     assertEq(block.number, blockNumber);
 
     // OffRamp initial values
-    assertEq("EVM2EVMOffRamp 1.2.0", s_offRamp.typeAndVersion());
+    assertEq("EVM2EVMOffRamp 1.3.0-dev", s_offRamp.typeAndVersion());
     assertEq(OWNER, s_offRamp.owner());
   }
 
@@ -978,6 +978,28 @@ contract EVM2EVMOffRamp_manuallyExecute is EVM2EVMOffRampSetup {
     s_offRamp.manuallyExecute(_generateReportFromMessages(messages), gasLimits);
   }
 
+  function testManualExecFailedTxReverts() public {
+    Internal.EVM2EVMMessage[] memory messages = _generateBasicMessages();
+
+    messages[0].receiver = address(s_reverting_receiver);
+    messages[0].messageId = Internal._hash(messages[0], s_offRamp.metadataHash());
+
+    s_offRamp.execute(_generateReportFromMessages(messages), new uint256[](0));
+
+    s_reverting_receiver.setRevert(true);
+
+    vm.expectRevert(
+      abi.encodeWithSelector(
+        EVM2EVMOffRamp.ExecutionError.selector,
+        abi.encodeWithSelector(
+          EVM2EVMOffRamp.ReceiverError.selector,
+          abi.encodeWithSelector(MaybeRevertMessageReceiver.CustomError.selector, bytes(""))
+        )
+      )
+    );
+    s_offRamp.manuallyExecute(_generateReportFromMessages(messages), _getGasLimitsFromMessages(messages));
+  }
+
   function testReentrancyManualExecuteFAILS() public {
     uint256 tokenAmount = 1e9;
     IERC20 tokenToAbuse = IERC20(s_destFeeToken);
@@ -1002,16 +1024,21 @@ contract EVM2EVMOffRamp_manuallyExecute is EVM2EVMOffRampSetup {
     // sets the report to be repeated on the ReentrancyAbuser to be able to replay
     receiver.setPayload(report);
 
-    s_offRamp.manuallyExecute(report, _getGasLimitsFromMessages(messages));
-
     // The first entry should be fine and triggers the second entry. This one fails
-    // but since it's an inner tx of the first one it is caught in the cry-catch.
-    // This failure of the inner tx flags the outer tx as failed.
-    assertEq(
-      uint256(s_offRamp.getExecutionState(messages[0].sequenceNumber)),
-      uint256(Internal.MessageExecutionState.FAILURE)
+    // but since it's an inner tx of the first one it is caught in the try-catch.
+    // Since this is manual exec, the entire tx fails on any failure.
+    vm.expectRevert(
+      abi.encodeWithSelector(
+        EVM2EVMOffRamp.ExecutionError.selector,
+        abi.encodeWithSelector(
+          EVM2EVMOffRamp.ReceiverError.selector,
+          abi.encodeWithSelector(EVM2EVMOffRamp.AlreadyExecuted.selector, messages[0].sequenceNumber)
+        )
+      )
     );
 
+    s_offRamp.manuallyExecute(report, _getGasLimitsFromMessages(messages));
+
     // Since the tx failed we don't release the tokens
     assertEq(tokenToAbuse.balanceOf(address(receiver)), balancePre);
   }

contracts/src/v0.8/functions/tests/v1_X/FunctionsSubscriptions.t.sol

@@ -310,53 +310,31 @@ contract FunctionsSubscriptions_OwnerWithdraw is FunctionsFulfillmentSetup {
 
 /// @notice #onTokenTransfer
 contract FunctionsSubscriptions_OnTokenTransfer is FunctionsSubscriptionSetup {
-  function test_OnTokenTransfer_RevertIfPaused(uint96 fundingAmount) public {
-    // Funding amount must be less than LINK total supply
-    vm.assume(fundingAmount < 1_000_000_000 * 1e18);
-    vm.assume(fundingAmount > 0);
-
+  function test_OnTokenTransfer_RevertIfPaused() public {
     s_functionsRouter.pause();
     vm.expectRevert("Pausable: paused");
-    s_linkToken.transferAndCall(address(s_functionsRouter), fundingAmount, abi.encode(s_subscriptionId));
+    s_linkToken.transferAndCall(address(s_functionsRouter), 1e18, abi.encode(s_subscriptionId));
   }
 
-  function test_OnTokenTransfer_RevertIfCallerIsNotLink(uint96 fundingAmount) public {
-    // Funding amount must be less than LINK total supply
-    vm.assume(fundingAmount < 1_000_000_000 * 1e18);
-    vm.assume(fundingAmount > 0);
-
+  function test_OnTokenTransfer_RevertIfCallerIsNotLink() public {
     vm.expectRevert(FunctionsSubscriptions.OnlyCallableFromLink.selector);
-    s_functionsRouter.onTokenTransfer(address(s_functionsRouter), fundingAmount, abi.encode(s_subscriptionId));
+    s_functionsRouter.onTokenTransfer(address(s_functionsRouter), 1e18, abi.encode(s_subscriptionId));
   }
 
-  function test_OnTokenTransfer_RevertIfCallerIsNoCalldata(uint96 fundingAmount) public {
-    // Funding amount must be less than LINK total supply
-    vm.assume(fundingAmount < 1_000_000_000 * 1e18);
-    vm.assume(fundingAmount > 0);
-
+  function test_OnTokenTransfer_RevertIfCallerIsNoCalldata() public {
     vm.expectRevert(FunctionsSubscriptions.InvalidCalldata.selector);
-    s_linkToken.transferAndCall(address(s_functionsRouter), fundingAmount, new bytes(0));
+    s_linkToken.transferAndCall(address(s_functionsRouter), 1e18, new bytes(0));
   }
 
-  function test_OnTokenTransfer_RevertIfCallerIsNoSubscription(uint96 fundingAmount) public {
-    // Funding amount must be less than LINK total supply
-    vm.assume(fundingAmount < 1_000_000_000 * 1e18);
-    vm.assume(fundingAmount > 0);
-
+  function test_OnTokenTransfer_RevertIfCallerIsNoSubscription() public {
     vm.expectRevert(FunctionsSubscriptions.InvalidSubscription.selector);
     uint64 invalidSubscriptionId = 123456789;
-    s_linkToken.transferAndCall(address(s_functionsRouter), fundingAmount, abi.encode(invalidSubscriptionId));
+    s_linkToken.transferAndCall(address(s_functionsRouter), 1e18, abi.encode(invalidSubscriptionId));
   }
 
-  function test_OnTokenTransfer_Success(uint96 fundingAmount) public {
+  function test_OnTokenTransfer_Success() public {
+    uint96 fundingAmount = 1e19;
     uint96 subscriptionBalanceBefore = s_functionsRouter.getSubscription(s_subscriptionId).balance;
-
-    // Funding amount must be less than LINK total supply
-    uint96 TOTAL_LINK = 1_000_000_000 * 1e18;
-    // Some of the total supply is already in the subscription account
-    vm.assume(fundingAmount < TOTAL_LINK - subscriptionBalanceBefore);
-    vm.assume(fundingAmount > 0);
-
     s_linkToken.transferAndCall(address(s_functionsRouter), fundingAmount, abi.encode(s_subscriptionId));
     uint96 subscriptionBalanceAfter = s_functionsRouter.getSubscription(s_subscriptionId).balance;
     assertEq(subscriptionBalanceBefore + fundingAmount, subscriptionBalanceAfter);