imp: no-op instead of error on already processed packets (#106)

.github/workflows/foundry.yml

@@ -79,9 +79,17 @@ jobs:
       - name: "Set up environment"
         uses: ./.github/setup
 
+      - name: Install lcov
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y lcov
+
       - name: "Run coverage"
         run: forge coverage --report lcov
 
+      - name: "Remove the test directory from the coverage report"
+        run: lcov --remove lcov.info 'test/**' -o lcov.info
+
       - uses: codecov/codecov-action@v4
         with:
           exclude: test

src/ICS26Router.sol

@@ -6,6 +6,7 @@ import { IICS26Router } from "./interfaces/IICS26Router.sol";
 import { IICS02Client } from "./interfaces/IICS02Client.sol";
 import { ICS02Client } from "./ICS02Client.sol";
 import { IIBCStore } from "./interfaces/IIBCStore.sol";
+import { IICS24HostErrors } from "./errors/IICS24HostErrors.sol";
 import { IBCStore } from "./utils/IBCStore.sol";
 import { IICS26RouterErrors } from "./errors/IICS26RouterErrors.sol";
 import { Ownable } from "@openzeppelin/access/Ownable.sol";
@@ -138,6 +139,13 @@ contract ICS26Router is IICS26Router, IICS26RouterErrors, Ownable, ReentrancyGua
 
         ICS02_CLIENT.getClient(msg_.packet.destChannel).membership(membershipMsg);
 
+        // recvPacket will no-op if the packet receipt already exists
+        // solhint-disable-next-line no-empty-blocks
+        try IBC_STORE.setPacketReceipt(msg_.packet) { }
+        catch (bytes memory reason) {
+            return noopOnCorrectReason(reason, IICS24HostErrors.IBCPacketReceiptAlreadyExists.selector);
+        }
+
         bytes[] memory acks = new bytes[](1);
         acks[0] = getIBCApp(payload.destPort).onRecvPacket(
             IIBCAppCallbacks.OnRecvPacketCallback({
@@ -152,8 +160,6 @@ contract ICS26Router is IICS26Router, IICS26RouterErrors, Ownable, ReentrancyGua
 
         writeAcknowledgement(msg_.packet, acks);
 
-        IBC_STORE.setPacketReceipt(msg_.packet);
-
         emit RecvPacket(msg_.packet);
     }
 
@@ -171,13 +177,6 @@ contract ICS26Router is IICS26Router, IICS26RouterErrors, Ownable, ReentrancyGua
             IBCInvalidCounterparty(cInfo.clientId, msg_.packet.destChannel)
         );
 
-        // this will revert if the packet commitment does not exist
-        bytes32 storedCommitment = IBC_STORE.deletePacketCommitment(msg_.packet);
-        require(
-            storedCommitment == ICS24Host.packetCommitmentBytes32(msg_.packet),
-            IBCPacketCommitmentMismatch(storedCommitment, ICS24Host.packetCommitmentBytes32(msg_.packet))
-        );
-
         bytes memory commitmentPath =
             ICS24Host.packetAcknowledgementCommitmentPathCalldata(msg_.packet.destChannel, msg_.packet.sequence);
         bytes[] memory acks = new bytes[](1);
@@ -194,6 +193,16 @@ contract ICS26Router is IICS26Router, IICS26RouterErrors, Ownable, ReentrancyGua
 
         ICS02_CLIENT.getClient(msg_.packet.sourceChannel).membership(membershipMsg);
 
+        // ackPacket will no-op if the packet commitment does not exist
+        try IBC_STORE.deletePacketCommitment(msg_.packet) returns (bytes32 storedCommitment) {
+            require(
+                storedCommitment == ICS24Host.packetCommitmentBytes32(msg_.packet),
+                IBCPacketCommitmentMismatch(storedCommitment, ICS24Host.packetCommitmentBytes32(msg_.packet))
+            );
+        } catch (bytes memory reason) {
+            return noopOnCorrectReason(reason, IICS24HostErrors.IBCPacketCommitmentNotFound.selector);
+        }
+
         getIBCApp(payload.sourcePort).onAcknowledgementPacket(
             IIBCAppCallbacks.OnAcknowledgementPacketCallback({
                 sourceChannel: msg_.packet.sourceChannel,
@@ -222,13 +231,6 @@ contract ICS26Router is IICS26Router, IICS26RouterErrors, Ownable, ReentrancyGua
             IBCInvalidCounterparty(cInfo.clientId, msg_.packet.destChannel)
         );
 
-        // this will revert if the packet commitment does not exist
-        bytes32 storedCommitment = IBC_STORE.deletePacketCommitment(msg_.packet);
-        require(
-            storedCommitment == ICS24Host.packetCommitmentBytes32(msg_.packet),
-            IBCPacketCommitmentMismatch(storedCommitment, ICS24Host.packetCommitmentBytes32(msg_.packet))
-        );
-
         bytes memory receiptPath =
             ICS24Host.packetReceiptCommitmentPathCalldata(msg_.packet.destChannel, msg_.packet.sequence);
         ILightClientMsgs.MsgMembership memory nonMembershipMsg = ILightClientMsgs.MsgMembership({
@@ -244,6 +246,16 @@ contract ICS26Router is IICS26Router, IICS26RouterErrors, Ownable, ReentrancyGua
             IBCInvalidTimeoutTimestamp(msg_.packet.timeoutTimestamp, counterpartyTimestamp)
         );
 
+        // timeoutPacket will no-op if the packet commitment does not exist
+        try IBC_STORE.deletePacketCommitment(msg_.packet) returns (bytes32 storedCommitment) {
+            require(
+                storedCommitment == ICS24Host.packetCommitmentBytes32(msg_.packet),
+                IBCPacketCommitmentMismatch(storedCommitment, ICS24Host.packetCommitmentBytes32(msg_.packet))
+            );
+        } catch (bytes memory reason) {
+            return noopOnCorrectReason(reason, IICS24HostErrors.IBCPacketCommitmentNotFound.selector);
+        }
+
         getIBCApp(payload.sourcePort).onTimeoutPacket(
             IIBCAppCallbacks.OnTimeoutPacketCallback({
                 sourceChannel: msg_.packet.sourceChannel,
@@ -264,4 +276,20 @@ contract ICS26Router is IICS26Router, IICS26RouterErrors, Ownable, ReentrancyGua
         IBC_STORE.commitPacketAcknowledgement(packet, acks);
         emit WriteAcknowledgement(packet, acks);
     }
+
+    /// @notice No-op if the reason is correct, otherwise reverts with the same reason
+    /// @dev Only to be used in catch blocks
+    /// @param reason The reason to check
+    /// @param correctReason The correct reason
+    function noopOnCorrectReason(bytes memory reason, bytes4 correctReason) private {
+        if (bytes4(reason) == correctReason) {
+            emit Noop();
+        } else {
+            // reverts with the same reason
+            // solhint-disable-next-line no-inline-assembly
+            assembly ("memory-safe") {
+                revert(add(reason, 32), mload(reason))
+            }
+        }
+    }
 }

src/interfaces/IICS26Router.sol

@@ -9,11 +9,6 @@ import { IICS02Client } from "./IICS02Client.sol";
 /// @title ICS26 Router Interface
 /// @notice IICS26Router is an interface for the IBC Eureka router
 interface IICS26Router is IICS26RouterMsgs {
-    /// @notice Emitted when an IBC application is added to the router
-    /// @param portId The port identifier
-    /// @param app The address of the IBC application contract
-    event IBCAppAdded(string portId, address app);
-
     /// @notice Returns the IBC storage contract
     /// @return The address of the IBC stotage contract
     function IBC_STORE() external view returns (IIBCStore);
@@ -53,6 +48,10 @@ interface IICS26Router is IICS26RouterMsgs {
 
     // --------------------- Events --------------------- //
 
+    /// @notice Emitted when an IBC application is added to the router
+    /// @param portId The port identifier
+    /// @param app The address of the IBC application contract
+    event IBCAppAdded(string portId, address app);
     /// @notice Emitted when a packet is sent
     /// @param packet The sent packet
     event SendPacket(Packet packet);
@@ -70,4 +69,6 @@ interface IICS26Router is IICS26RouterMsgs {
     /// @param packet The packet that was acknowledged
     /// @param acknowledgement The acknowledgement data
     event AckPacket(Packet packet, bytes acknowledgement);
+    /// @notice Emitted when a redundant relay occurs
+    event Noop();
 }