From 529b594f498ba0944868769665fd9fdcb2a809ee Mon Sep 17 00:00:00 2001
From: voith <voithjm1@gmail.com>
Date: Mon, 24 Feb 2025 03:03:37 +0530
Subject: [PATCH] [cantina finding #11] do not revert inside _ccipReceive

---
 contracts/ccip/GovernanceCCIPReceiver.sol       | 10 ++++++----
 .../ccip/interfaces/IGovernanceCCIPReceiver.sol | 17 +++++++++++++----
 test/ccip/GovernanceCCIPIntegrationTest.t.sol   | 12 ++++++++++--
 test/ccip/GovernanceCCIPReceiverTest.t.sol      | 14 ++++++++++----
 4 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/contracts/ccip/GovernanceCCIPReceiver.sol b/contracts/ccip/GovernanceCCIPReceiver.sol
index 65587fd..6e19fa4 100644
--- a/contracts/ccip/GovernanceCCIPReceiver.sol
+++ b/contracts/ccip/GovernanceCCIPReceiver.sol
@@ -53,9 +53,11 @@ contract GovernanceCCIPReceiver is IGovernanceCCIPReceiver, CCIPReceiver {
     require(target != address(0), TargetAddressCannotBeZero());
 
     // Execute payload
-    (bool success, ) = target.call(payload);
-    require(success, MessageCallFailed());
-
-    emit MessageExecuted(messageId, target, payload);
+    (bool success, bytes memory _error) = target.call(payload);
+    if (success) {
+      emit MessageExecutedSuccessfully(messageId, target, payload);
+    } else {
+      emit MessageExecutionFailed(messageId, target, payload, _error);
+    }
   }
 }
diff --git a/contracts/ccip/interfaces/IGovernanceCCIPReceiver.sol b/contracts/ccip/interfaces/IGovernanceCCIPReceiver.sol
index 24eab6a..571199d 100644
--- a/contracts/ccip/interfaces/IGovernanceCCIPReceiver.sol
+++ b/contracts/ccip/interfaces/IGovernanceCCIPReceiver.sol
@@ -11,12 +11,24 @@ interface IGovernanceCCIPReceiver {
   /// @notice Emitted when a message is successfully executed.
   /// @param target The target address on the destination chain.
   /// @param payload The payload of the message.
-  event MessageExecuted(
+  event MessageExecutedSuccessfully(
     bytes32 indexed messageId,
     address indexed target,
     bytes payload
   );
 
+  /// @notice Emitted when a message is successfully executed.
+  /// @param messageId The ccip message id.
+  /// @param target The target address on the destination chain.
+  /// @param payload The payload of the message.
+  /// @param _error The error returned by the message call.
+  event MessageExecutionFailed(
+    bytes32 indexed messageId,
+    address indexed target,
+    bytes payload,
+    bytes _error
+  );
+
   /// @dev Error thrown when a provided address is the zero address.
   error AddressCannotBeZero();
 
@@ -24,9 +36,6 @@ interface IGovernanceCCIPReceiver {
   /// @param caller The address of the unauthorized caller.
   error Unauthorized(address caller);
 
-  /// @dev Error thrown when the execution of a message fails.
-  error MessageCallFailed();
-
   /// @dev Error thrown when the target address is zero.
   error TargetAddressCannotBeZero();
 
diff --git a/test/ccip/GovernanceCCIPIntegrationTest.t.sol b/test/ccip/GovernanceCCIPIntegrationTest.t.sol
index 4bb3c70..100d38d 100644
--- a/test/ccip/GovernanceCCIPIntegrationTest.t.sol
+++ b/test/ccip/GovernanceCCIPIntegrationTest.t.sol
@@ -237,7 +237,11 @@ contract GovernanceCCIPIntegrationTest is Test {
 
     // Route the message to Polygon
     vm.expectEmit(true, true, true, true);
-    emit IGovernanceCCIPReceiver.MessageExecuted(messageId, target, payload);
+    emit IGovernanceCCIPReceiver.MessageExecutedSuccessfully(
+      messageId,
+      target,
+      payload
+    );
     ccipLocalSimulatorFork.switchChainAndRouteMessage(polygonMainnetForkId);
 
     // Verify the message was received and executed on Polygon
@@ -283,7 +287,11 @@ contract GovernanceCCIPIntegrationTest is Test {
     );
 
     vm.expectEmit(true, true, true, true);
-    emit IGovernanceCCIPReceiver.MessageExecuted(messageId, target, payload);
+    emit IGovernanceCCIPReceiver.MessageExecutedSuccessfully(
+      messageId,
+      target,
+      payload
+    );
     ccipLocalSimulatorFork.switchChainAndRouteMessage(polygonMainnetForkId);
 
     vm.selectFork(polygonMainnetForkId);
diff --git a/test/ccip/GovernanceCCIPReceiverTest.t.sol b/test/ccip/GovernanceCCIPReceiverTest.t.sol
index 518967b..c511e70 100644
--- a/test/ccip/GovernanceCCIPReceiverTest.t.sol
+++ b/test/ccip/GovernanceCCIPReceiverTest.t.sol
@@ -134,16 +134,22 @@ contract GovernanceCCIPReceiverTest is Test {
   /// @notice Test ccipReceive raises MessageCallFailed when execution fails
   function testCcipReceiveMessageCallFailed() public {
     address nonContract = address(0x9); // Not a contract, will fail execution
-
+    bytes memory payload = abi.encodeWithSignature("nonexistentFunction()");
     Client.Any2EVMMessage memory message = constructMessage(
       nonContract,
-      abi.encodeWithSignature("nonexistentFunction()"),
+      payload,
       mainnetChainSelector,
       mainnetSender
     );
 
     vm.prank(ccipRouter);
-    vm.expectRevert(IGovernanceCCIPReceiver.MessageCallFailed.selector);
+    vm.expectEmit(true, true, true, true);
+    emit IGovernanceCCIPReceiver.MessageExecutionFailed(
+      bytes32(""),
+      nonContract,
+      payload,
+      bytes("")
+    );
     receiver.ccipReceive(message);
   }
 
@@ -160,7 +166,7 @@ contract GovernanceCCIPReceiverTest is Test {
 
     vm.prank(ccipRouter);
     vm.expectEmit(true, true, true, true);
-    emit IGovernanceCCIPReceiver.MessageExecuted(
+    emit IGovernanceCCIPReceiver.MessageExecutedSuccessfully(
       bytes32(""),
       address(numberUpdater),
       payload