[cantina finding #11] do not revert inside _ccipReceive

cryptexfinance · Feb 23, 2025 · 529b594 · 529b594
1 parent 02d486a
commit 529b594
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 14 deletions.
diff --git 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
@@ -11,22 +11,31 @@ 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();
 
   /// @dev Error thrown when an unauthorized caller tries to execute a restricted function.
   /// @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
@@ -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
@@ -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