Audit-202409

contracts/ILCPClientErrors.sol

@@ -34,6 +34,7 @@ interface ILCPClientErrors {
     error LCPClientUpdateStateEmittedStatesMustNotEmpty();
     error LCPClientUpdateStatePrevStateIdMustNotEmpty();
     error LCPClientUpdateStateUnexpectedPrevStateId();
+    error LCPClientUpdateStateInconsistentConsensusState();
 
     error LCPClientMisbehaviourPrevStatesMustNotEmpty();
 

contracts/LCPClientBase.sol

@@ -100,7 +100,8 @@ abstract contract LCPClientBase is ILightClient, ILCPClientErrors {
     /**
      * @dev initializeClient initializes a new client with the given state.
      *      If succeeded, it returns heights at which the consensus state are stored.
-     *      The function must be only called by IBCHandler.
+     *      This function is guaranteed by the IBC contract to be called only once for each `clientId`.
+     * @param clientId the client identifier which is unique within the IBC handler
      */
     function initializeClient(
         string calldata clientId,
@@ -433,16 +434,22 @@ abstract contract LCPClientBase is ILightClient, ILCPClientErrors {
 
         LCPCommitment.validationContextEval(pmsg.context, block.timestamp * 1e9);
 
-        uint128 latestHeight = clientState.latest_height.toUint128();
         uint128 postHeight = pmsg.postHeight.toUint128();
-        if (latestHeight < postHeight) {
-            clientState.latest_height = pmsg.postHeight;
-        }
-
         consensusState = clientStorage.consensusStates[postHeight];
+        if (consensusState.stateId != bytes32(0)) {
+            if (consensusState.stateId != pmsg.postStateId || consensusState.timestamp != uint64(pmsg.timestamp)) {
+                revert LCPClientUpdateStateInconsistentConsensusState();
+            }
+            // return empty heights if the consensus state is already stored
+            return heights;
+        }
         consensusState.stateId = pmsg.postStateId;
         consensusState.timestamp = uint64(pmsg.timestamp);
 
+        uint128 latestHeight = clientState.latest_height.toUint128();
+        if (latestHeight < postHeight) {
+            clientState.latest_height = pmsg.postHeight;
+        }
         heights = new Height.Data[](1);
         heights[0] = pmsg.postHeight;
         return heights;

test/LCPClientTest.t.sol

@@ -118,13 +118,26 @@ contract LCPClientTest is BasicTest {
         }
 
         TestData[] memory dataList = readTestDataList(commandStartNumber);
+        bool firstUpdate = true;
         for (uint256 i = 0; i < dataList.length; i++) {
             if (dataList[i].cmd == Command.UpdateClient) {
                 UpdateClientMessage.Data memory message = createUpdateClientMessage(dataList[i].path);
                 Height.Data[] memory heights = lc.updateClient(clientId, message);
                 require(heights.length == 1, "heights length must be 1");
                 console.log("revision_height:");
                 console.log(heights[0].revision_height);
+                if (firstUpdate) {
+                    firstUpdate = false;
+                } else {
+                    // repeat updateClient to check the state is not changed
+                    message = createUpdateClientMessage(dataList[i].path);
+                    // staticcall is expected to succeed because updateClient does not update the state if the message is already processed
+                    (bool success, bytes memory ret) = address(lc).staticcall(
+                        abi.encodeWithSelector(LCPClientBase.updateClient.selector, clientId, message)
+                    );
+                    heights = abi.decode(ret, (Height.Data[]));
+                    require(heights.length == 0, "heights length must be 0");
+                }
             } else if (dataList[i].cmd == Command.VerifyMembership) {
                 (
                     Height.Data memory height,