always catch all errors in offRamp (#1229)

## Motivation
In line with the multiOffRamp

---------

Co-authored-by: Adam Hamrick <adam.hamrick@smartcontract.com>

contracts/gas-snapshots/ccip.gas-snapshot

@@ -301,40 +301,40 @@ EVM2EVMOffRamp__releaseOrMintTokens:test_releaseOrMintTokens_InvalidEVMAddress_R
 EVM2EVMOffRamp__releaseOrMintTokens:test_releaseOrMintTokens_Success() (gas: 214151)
 EVM2EVMOffRamp__releaseOrMintTokens:test_releaseOrMintTokens_destDenominatedDecimals_Success() (gas: 306912)
 EVM2EVMOffRamp__report:test_Report_Success() (gas: 127459)
-EVM2EVMOffRamp__trialExecute:test_RateLimitError_Success() (gas: 255047)
-EVM2EVMOffRamp__trialExecute:test_TokenHandlingErrorIsCaught_Success() (gas: 263638)
-EVM2EVMOffRamp__trialExecute:test_TokenPoolIsNotAContract_Success() (gas: 335707)
+EVM2EVMOffRamp__trialExecute:test_RateLimitError_Success() (gas: 254710)
+EVM2EVMOffRamp__trialExecute:test_TokenHandlingErrorIsCaught_Success() (gas: 263301)
+EVM2EVMOffRamp__trialExecute:test_TokenPoolIsNotAContract_Success() (gas: 334438)
 EVM2EVMOffRamp__trialExecute:test_trialExecute_Success() (gas: 314443)
 EVM2EVMOffRamp_ccipReceive:test_Reverts() (gas: 17009)
-EVM2EVMOffRamp_constructor:test_CommitStoreAlreadyInUse_Revert() (gas: 153427)
-EVM2EVMOffRamp_constructor:test_Constructor_Success() (gas: 5464875)
-EVM2EVMOffRamp_constructor:test_ZeroOnRampAddress_Revert() (gas: 144183)
+EVM2EVMOffRamp_constructor:test_CommitStoreAlreadyInUse_Revert() (gas: 153241)
+EVM2EVMOffRamp_constructor:test_Constructor_Success() (gas: 5337708)
+EVM2EVMOffRamp_constructor:test_ZeroOnRampAddress_Revert() (gas: 143997)
 EVM2EVMOffRamp_execute:test_EmptyReport_Revert() (gas: 21345)
-EVM2EVMOffRamp_execute:test_InvalidMessageId_Revert() (gas: 36442)
+EVM2EVMOffRamp_execute:test_InvalidMessageId_Revert() (gas: 36486)
 EVM2EVMOffRamp_execute:test_InvalidSourceChain_Revert() (gas: 51701)
-EVM2EVMOffRamp_execute:test_InvalidSourcePoolAddress_Success() (gas: 473575)
+EVM2EVMOffRamp_execute:test_InvalidSourcePoolAddress_Success() (gas: 473216)
 EVM2EVMOffRamp_execute:test_ManualExecutionNotYetEnabled_Revert() (gas: 46423)
 EVM2EVMOffRamp_execute:test_MessageTooLarge_Revert() (gas: 152453)
-EVM2EVMOffRamp_execute:test_Paused_Revert() (gas: 101458)
-EVM2EVMOffRamp_execute:test_ReceiverError_Success() (gas: 165036)
-EVM2EVMOffRamp_execute:test_RetryFailedMessageWithoutManualExecution_Revert() (gas: 177824)
+EVM2EVMOffRamp_execute:test_Paused_Revert() (gas: 101503)
+EVM2EVMOffRamp_execute:test_ReceiverError_Success() (gas: 164818)
+EVM2EVMOffRamp_execute:test_RetryFailedMessageWithoutManualExecution_Revert() (gas: 177584)
 EVM2EVMOffRamp_execute:test_RootNotCommitted_Revert() (gas: 41317)
-EVM2EVMOffRamp_execute:test_RouterYULCall_Revert() (gas: 402506)
 EVM2EVMOffRamp_execute:test_SingleMessageNoTokensUnordered_Success() (gas: 159387)
-EVM2EVMOffRamp_execute:test_SingleMessageNoTokens_Success() (gas: 174622)
+EVM2EVMOffRamp_execute:test_SingleMessageNoTokens_Success() (gas: 174600)
 EVM2EVMOffRamp_execute:test_SingleMessageToNonCCIPReceiver_Success() (gas: 248634)
-EVM2EVMOffRamp_execute:test_SingleMessagesNoTokensSuccess_gas() (gas: 115017)
-EVM2EVMOffRamp_execute:test_SkippedIncorrectNonceStillExecutes_Success() (gas: 409338)
+EVM2EVMOffRamp_execute:test_SingleMessagesNoTokensSuccess_gas() (gas: 115040)
+EVM2EVMOffRamp_execute:test_SkippedIncorrectNonceStillExecutes_Success() (gas: 409316)
 EVM2EVMOffRamp_execute:test_SkippedIncorrectNonce_Success() (gas: 54173)
 EVM2EVMOffRamp_execute:test_StrictUntouchedToSuccess_Success() (gas: 132056)
-EVM2EVMOffRamp_execute:test_TokenDataMismatch_Revert() (gas: 52200)
+EVM2EVMOffRamp_execute:test_TokenDataMismatch_Revert() (gas: 52178)
 EVM2EVMOffRamp_execute:test_TwoMessagesWithTokensAndGE_Success() (gas: 560178)
-EVM2EVMOffRamp_execute:test_TwoMessagesWithTokensSuccess_gas() (gas: 499424)
-EVM2EVMOffRamp_execute:test_UnexpectedTokenData_Revert() (gas: 35442)
+EVM2EVMOffRamp_execute:test_TwoMessagesWithTokensSuccess_gas() (gas: 499466)
+EVM2EVMOffRamp_execute:test_UnexpectedTokenData_Revert() (gas: 35486)
 EVM2EVMOffRamp_execute:test_Unhealthy_Revert() (gas: 546987)
-EVM2EVMOffRamp_execute:test_UnsupportedNumberOfTokens_Revert() (gas: 64045)
+EVM2EVMOffRamp_execute:test_UnsupportedNumberOfTokens_Revert() (gas: 64023)
 EVM2EVMOffRamp_execute:test__execute_SkippedAlreadyExecutedMessageUnordered_Success() (gas: 123223)
-EVM2EVMOffRamp_execute:test__execute_SkippedAlreadyExecutedMessage_Success() (gas: 143388)
+EVM2EVMOffRamp_execute:test__execute_SkippedAlreadyExecutedMessage_Success() (gas: 143411)
+EVM2EVMOffRamp_execute:test_execute_RouterYULCall_Success() (gas: 428233)
 EVM2EVMOffRamp_executeSingleMessage:test_MessageSender_Revert() (gas: 20582)
 EVM2EVMOffRamp_executeSingleMessage:test_NonContractWithTokens_Success() (gas: 281891)
 EVM2EVMOffRamp_executeSingleMessage:test_NonContract_Success() (gas: 20231)
@@ -351,15 +351,15 @@ EVM2EVMOffRamp_execute_upgrade:test_V2_Success() (gas: 131682)
 EVM2EVMOffRamp_getAllRateLimitTokens:test_GetAllRateLimitTokens_Success() (gas: 38408)
 EVM2EVMOffRamp_getExecutionState:test_FillExecutionState_Success() (gas: 3213556)
 EVM2EVMOffRamp_getExecutionState:test_GetExecutionState_Success() (gas: 83091)
-EVM2EVMOffRamp_manuallyExecute:test_LowGasLimitManualExec_Success() (gas: 483328)
-EVM2EVMOffRamp_manuallyExecute:test_ManualExecFailedTx_Revert() (gas: 186413)
+EVM2EVMOffRamp_manuallyExecute:test_LowGasLimitManualExec_Success() (gas: 483110)
+EVM2EVMOffRamp_manuallyExecute:test_ManualExecFailedTx_Revert() (gas: 185977)
 EVM2EVMOffRamp_manuallyExecute:test_ManualExecForkedChain_Revert() (gas: 25824)
 EVM2EVMOffRamp_manuallyExecute:test_ManualExecGasLimitMismatch_Revert() (gas: 43449)
 EVM2EVMOffRamp_manuallyExecute:test_ManualExecInvalidGasLimit_Revert() (gas: 25927)
-EVM2EVMOffRamp_manuallyExecute:test_ManualExecWithGasOverride_Success() (gas: 188518)
-EVM2EVMOffRamp_manuallyExecute:test_ManualExec_Success() (gas: 187965)
-EVM2EVMOffRamp_manuallyExecute:test_ReentrancyManualExecuteFails() (gas: 2027441)
-EVM2EVMOffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched_Success() (gas: 143803)
+EVM2EVMOffRamp_manuallyExecute:test_ManualExecWithGasOverride_Success() (gas: 188300)
+EVM2EVMOffRamp_manuallyExecute:test_ManualExec_Success() (gas: 187747)
+EVM2EVMOffRamp_manuallyExecute:test_ReentrancyManualExecuteFails() (gas: 2027223)
+EVM2EVMOffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched_Success() (gas: 143585)
 EVM2EVMOffRamp_metadataHash:test_MetadataHash_Success() (gas: 8871)
 EVM2EVMOffRamp_setDynamicConfig:test_NonOwner_Revert() (gas: 40429)
 EVM2EVMOffRamp_setDynamicConfig:test_RouterZeroAddress_Revert() (gas: 38804)

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

@@ -435,18 +435,9 @@ contract EVM2EVMOffRamp is IAny2EVMOffRamp, AggregateRateLimiter, ITypeAndVersio
   ) internal returns (Internal.MessageExecutionState, bytes memory) {
     try this.executeSingleMessage(message, offchainTokenData) {}
     catch (bytes memory err) {
-      if (
-        ReceiverError.selector == bytes4(err) || TokenHandlingError.selector == bytes4(err)
-          || Internal.InvalidEVMAddress.selector == bytes4(err) || InvalidDataLength.selector == bytes4(err)
-          || CallWithExactGas.NoContract.selector == bytes4(err) || NotACompatiblePool.selector == bytes4(err)
-      ) {
-        // If CCIP receiver execution is not successful, bubble up receiver revert data,
-        // prepended by the 4 bytes of ReceiverError.selector, TokenHandlingError.selector or InvalidPoolAddress.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);
-      }
-      // If revert is not caused by CCIP receiver, it is unexpected, bubble up the revert.
-      revert ExecutionError(err);
+      // return the message execution state as FAILURE and the revert data
+      // 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);
     }
     // If message execution succeeded, no CCIP receiver return data is expected, return with empty bytes.
     return (Internal.MessageExecutionState.SUCCESS, "");

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

@@ -611,6 +611,27 @@ contract EVM2EVMOffRamp_execute is EVM2EVMOffRampSetup {
     s_offRamp.execute(_generateReportFromMessages(messages), new uint256[](0));
   }
 
+  function test_execute_RouterYULCall_Success() public {
+    Internal.EVM2EVMMessage[] memory messages = _generateSingleBasicMessage();
+
+    // gas limit too high, Router's external call should revert
+    messages[0].gasLimit = 1e36;
+    messages[0].receiver = address(new ConformingReceiver(address(s_destRouter), s_destFeeToken));
+    messages[0].messageId = Internal._hash(messages[0], s_offRamp.metadataHash());
+
+    Internal.ExecutionReport memory executionReport = _generateReportFromMessages(messages);
+
+    vm.expectEmit();
+    emit EVM2EVMOffRamp.ExecutionStateChanged(
+      messages[0].sequenceNumber,
+      messages[0].messageId,
+      Internal.MessageExecutionState.FAILURE,
+      abi.encodeWithSelector(CallWithExactGas.NotEnoughGasForCall.selector)
+    );
+
+    s_offRamp.execute(executionReport, new uint256[](0));
+  }
+
   // Reverts
 
   function test_InvalidMessageId_Revert() public {
@@ -723,24 +744,6 @@ contract EVM2EVMOffRamp_execute is EVM2EVMOffRampSetup {
     s_offRamp.execute(executionReport, new uint256[](0));
   }
 
-  function test_RouterYULCall_Revert() public {
-    Internal.EVM2EVMMessage[] memory messages = _generateSingleBasicMessage();
-
-    // gas limit too high, Router's external call should revert
-    messages[0].gasLimit = 1e36;
-    messages[0].receiver = address(new ConformingReceiver(address(s_destRouter), s_destFeeToken));
-    messages[0].messageId = Internal._hash(messages[0], s_offRamp.metadataHash());
-
-    Internal.ExecutionReport memory executionReport = _generateReportFromMessages(messages);
-
-    vm.expectRevert(
-      abi.encodeWithSelector(
-        EVM2EVMOffRamp.ExecutionError.selector, abi.encodeWithSelector(CallWithExactGas.NotEnoughGasForCall.selector)
-      )
-    );
-    s_offRamp.execute(executionReport, new uint256[](0));
-  }
-
   function test_RetryFailedMessageWithoutManualExecution_Revert() public {
     Internal.EVM2EVMMessage[] memory messages = _generateSingleBasicMessage();
 

core/gethwrappers/ccip/generation/generated-wrapper-dependency-versions-do-not-edit.txt

@@ -12,7 +12,7 @@ commit_store_helper: ../../../contracts/solc/v0.8.24/CommitStoreHelper/CommitSto
 ether_sender_receiver: ../../../contracts/solc/v0.8.24/EtherSenderReceiver/EtherSenderReceiver.abi ../../../contracts/solc/v0.8.24/EtherSenderReceiver/EtherSenderReceiver.bin 09510a3f773f108a3c231e8d202835c845ded862d071ec54c4f89c12d868b8de
 evm_2_evm_multi_offramp: ../../../contracts/solc/v0.8.24/EVM2EVMMultiOffRamp/EVM2EVMMultiOffRamp.abi ../../../contracts/solc/v0.8.24/EVM2EVMMultiOffRamp/EVM2EVMMultiOffRamp.bin 25a7bf3aa46252844c7afabc15db1051e7b6a717e296fc4c6e2f2f93d16033c5
 evm_2_evm_multi_onramp: ../../../contracts/solc/v0.8.24/EVM2EVMMultiOnRamp/EVM2EVMMultiOnRamp.abi ../../../contracts/solc/v0.8.24/EVM2EVMMultiOnRamp/EVM2EVMMultiOnRamp.bin 9478aedc9f0072fbdafb54a6f82248de1efbcd7bdff18a90d8556b9aaff67455
-evm_2_evm_offramp: ../../../contracts/solc/v0.8.24/EVM2EVMOffRamp/EVM2EVMOffRamp.abi ../../../contracts/solc/v0.8.24/EVM2EVMOffRamp/EVM2EVMOffRamp.bin a8c23c9280a713544eae0a0b8841a9caf97e616338d31ebc62501d8b4ab0eed6
+evm_2_evm_offramp: ../../../contracts/solc/v0.8.24/EVM2EVMOffRamp/EVM2EVMOffRamp.abi ../../../contracts/solc/v0.8.24/EVM2EVMOffRamp/EVM2EVMOffRamp.bin 6a3ae8f29cfc5aa52b5e4438b3196be8a542a952e828065b964fd8156dd3d96e
 evm_2_evm_onramp: ../../../contracts/solc/v0.8.24/EVM2EVMOnRamp/EVM2EVMOnRamp.abi ../../../contracts/solc/v0.8.24/EVM2EVMOnRamp/EVM2EVMOnRamp.bin 116d5cb8447a1af61664a8d1db2d76086c042a3228337bc5cd49b9abd3e815f7
 lock_release_token_pool: ../../../contracts/solc/v0.8.24/LockReleaseTokenPool/LockReleaseTokenPool.abi ../../../contracts/solc/v0.8.24/LockReleaseTokenPool/LockReleaseTokenPool.bin 95a93517b01f51c35d82711a0015995f4804820ed67f6b46b785c4c94815df93
 lock_release_token_pool_and_proxy: ../../../contracts/solc/v0.8.24/LockReleaseTokenPoolAndProxy/LockReleaseTokenPoolAndProxy.abi ../../../contracts/solc/v0.8.24/LockReleaseTokenPoolAndProxy/LockReleaseTokenPoolAndProxy.bin 05e308151b5adc9ba8d33385b8f82d55aad638652fe50e3ea8b09b1d0bbfd367

integration-tests/ccip-tests/actions/ccip_helpers.go

@@ -3034,13 +3034,12 @@ func (lane *CCIPLane) ExecuteManually(options ...ManualExecutionOption) error {
 // validationOptions are used in the ValidateRequests function to specify which phase is expected to fail and how
 type validationOptions struct {
 	phaseExpectedToFail  testreporters.Phase // the phase expected to fail
-	phaseShouldExist     bool                // for some phases, their lack of existence is a failure, for others their existence can also have a failure state
 	expectedErrorMessage string              // if provided, we're looking for a specific error message
 	timeout              time.Duration       // timeout for the validation
 }
 
 // ValidationOptionFunc is a function that can be passed to ValidateRequests to specify which phase is expected to fail
-type ValidationOptionFunc func(logger *zerolog.Logger, opts *validationOptions)
+type ValidationOptionFunc func(opts *validationOptions)
 
 // PhaseSpecificValidationOptionFunc can specify how exactly you want a phase to fail
 type PhaseSpecificValidationOptionFunc func(*validationOptions)
@@ -3059,39 +3058,19 @@ func WithTimeout(timeout time.Duration) PhaseSpecificValidationOptionFunc {
 	}
 }
 
-// ShouldExist specifies that a specific phase should exist, but be in a failed state. This is only applicable to the `ExecStateChanged` phase.
-func ShouldExist() PhaseSpecificValidationOptionFunc {
-	return func(opts *validationOptions) {
-		opts.phaseShouldExist = true
-	}
-}
-
 // ExpectPhaseToFail specifies that a specific phase is expected to fail.
 // You can optionally provide an expected error message, if you don't have one in mind, just pass an empty string.
 // shouldExist is used to specify whether the phase should exist or not, which is only applicable to the `ExecStateChanged` phase.
 // If you expect the `ExecStateChanged` events to be there, but in a "failed" state, set this to true.
 // It will otherwise be ignored.
 func ExpectPhaseToFail(phase testreporters.Phase, phaseSpecificOptions ...PhaseSpecificValidationOptionFunc) ValidationOptionFunc {
-	return func(logger *zerolog.Logger, opts *validationOptions) {
+	return func(opts *validationOptions) {
 		opts.phaseExpectedToFail = phase
 		for _, f := range phaseSpecificOptions {
 			if f != nil {
 				f(opts)
 			}
 		}
-		if phase == testreporters.ExecStateChanged {
-			if opts.expectedErrorMessage != "" {
-				logger.Warn().Msg("You are overriding the expected error message for the ExecStateChanged phase. This can cause unexpected behavior and is generally not recommended.")
-			} else if !opts.phaseShouldExist {
-				opts.expectedErrorMessage = "ExecutionStateChanged event not found for seq num"
-			} else {
-				opts.expectedErrorMessage = "ExecutionStateChanged event state - expected"
-			}
-		}
-		if phase != testreporters.ExecStateChanged && opts.phaseShouldExist {
-			logger.Warn().Msg("phaseShouldExist is only applicable to the ExecStateChanged phase. Ignoring for other phases.")
-			opts.phaseShouldExist = false
-		}
 	}
 }
 
@@ -3102,7 +3081,7 @@ func (lane *CCIPLane) ValidateRequests(validationOptionFuncs ...ValidationOption
 	var opts validationOptions
 	for _, f := range validationOptionFuncs {
 		if f != nil {
-			f(lane.Logger, &opts)
+			f(&opts)
 		}
 	}
 	for txHash, ccipReqs := range lane.SentReqs {

integration-tests/ccip-tests/smoke/ccip_test.go

@@ -840,7 +840,7 @@ func TestSmokeCCIPManuallyExecuteAfterExecutionFailingDueToInsufficientGas(t *te
 			// send with insufficient gas for ccip-receive to fail
 			err := tc.lane.SendRequests(1, big.NewInt(0))
 			require.NoError(t, err)
-			tc.lane.ValidateRequests(actions.ExpectPhaseToFail(testreporters.ExecStateChanged, actions.ShouldExist()))
+			tc.lane.ValidateRequests(actions.ExpectPhaseToFail(testreporters.ExecStateChanged))
 			// wait for events
 			err = tc.lane.Dest.Common.ChainClient.WaitForEvents()
 			require.NoError(t, err)
@@ -882,7 +882,6 @@ func testOffRampRateLimits(t *testing.T, rateLimiterConfig contracts.RateLimiter
 		contracts.OffRampContract: contracts.V1_5_0_dev,
 	})
 	require.NoError(t, err, "Required contract versions not met")
-	require.True(t, TestCfg.SelectedNetworks[0].Simulated, "This test relies on timing assumptions and should only be run on simulated networks")
 	require.False(t, pointer.GetBool(TestCfg.TestGroupInput.ExistingDeployment), "This test modifies contract state and cannot be run on existing deployments")
 
 	// Set the default permissionless exec threshold lower so that we can manually execute the transactions faster
@@ -975,13 +974,9 @@ func testOffRampRateLimits(t *testing.T, rateLimiterConfig contracts.RateLimiter
 			tc.lane.RecordStateBeforeTransfer()
 			err = tc.lane.SendRequests(1, big.NewInt(actions.DefaultDestinationGasLimit))
 			require.NoError(t, err, "Failed to send rate limited token transfer")
-			// Expect the ExecutionStateChanged event to never show up
-			// Since we're looking to confirm that an event has NOT occurred, this can lead to some imperfect assumptions and results
-			// We set the timeout to stop waiting for the event after a minute
-			// 99% of transactions occur in under a minute in ideal simulated conditions, so this is an okay assumption there
-			// but on real chains this risks false negatives
-			// If we don't set this timeout, this test can take a long time and hold up CI
-			tc.lane.ValidateRequests(actions.ExpectPhaseToFail(testreporters.ExecStateChanged, actions.WithTimeout(time.Minute)))
+
+			// We should see the ExecStateChanged phase fail on the OffRamp
+			tc.lane.ValidateRequests(actions.ExpectPhaseToFail(testreporters.ExecStateChanged))
 			tc.lane.Logger.Info().
 				Str("Token", limitedSrcToken.ContractAddress.Hex()).
 				Msg("Limited token transfer failed on destination chain (a good thing in this context)")