fix(contracts-communication): default settings for Guard service

packages/contracts-communication/contracts/InterchainClientV1.sol

@@ -429,8 +429,11 @@ contract InterchainClientV1 is Ownable, InterchainClientV1Events, IInterchainCli
             revert InterchainClientV1__ReceiverZeroRequiredResponses(receiver);
         }
         // Verify against the Guard if the app opts in to use it
-        _assertNoGuardConflict(_getGuard(appConfig), entry);
-        uint256 finalizedResponses = _getFinalizedResponsesCount(approvedModules, entry, appConfig.optimisticPeriod);
+        address guard = _getGuard(appConfig);
+        _assertNoGuardConflict(guard, entry);
+        // Optimistic period is not used if there's no Guard configured
+        uint256 optimisticPeriod = guard == address(0) ? 0 : appConfig.optimisticPeriod;
+        uint256 finalizedResponses = _getFinalizedResponsesCount(approvedModules, entry, optimisticPeriod);
         if (finalizedResponses < appConfig.requiredResponses) {
             revert InterchainClientV1__ResponsesAmountBelowMin(finalizedResponses, appConfig.requiredResponses);
         }

packages/contracts-communication/contracts/apps/ICAppV1.sol

@@ -5,7 +5,7 @@ import {AbstractICApp, InterchainTxDescriptor} from "./AbstractICApp.sol";
 
 import {InterchainAppV1Events} from "../events/InterchainAppV1Events.sol";
 import {IInterchainAppV1} from "../interfaces/IInterchainAppV1.sol";
-import {AppConfigV1, APP_CONFIG_GUARD_DEFAULT} from "../libs/AppConfig.sol";
+import {AppConfigV1, APP_CONFIG_GUARD_DISABLED} from "../libs/AppConfig.sol";
 import {OptionsV1} from "../libs/Options.sol";
 import {TypeCasts} from "../libs/TypeCasts.sol";
 
@@ -107,7 +107,7 @@ abstract contract ICAppV1 is AbstractICApp, AccessControlEnumerable, InterchainA
     /// - requiredResponses: the number of module responses required for accepting the message
     /// - optimisticPeriod: the minimum time after which the module responses are considered final
     function setAppConfigV1(uint256 requiredResponses, uint256 optimisticPeriod) external onlyRole(IC_GOVERNOR_ROLE) {
-        if (requiredResponses == 0 || optimisticPeriod == 0) {
+        if (requiredResponses == 0) {
             revert InterchainApp__AppConfigInvalid(requiredResponses, optimisticPeriod);
         }
         _requiredResponses = SafeCast.toUint16(requiredResponses);
@@ -253,9 +253,9 @@ abstract contract ICAppV1 is AbstractICApp, AccessControlEnumerable, InterchainA
     }
 
     /// @dev Returns the guard flag and address in the app config.
-    /// By default, the ICApp is using the Client-provided guard, but it can be overridden in the derived contract.
+    /// By default, the ICApp does not opt in for any guard, but it can be overridden in the derived contracts.
     function _getGuardConfig() internal view virtual returns (uint8 guardFlag, address guard) {
-        return (APP_CONFIG_GUARD_DEFAULT, address(0));
+        return (APP_CONFIG_GUARD_DISABLED, address(0));
     }
 
     /// @dev Returns the address of the Execution Service to use for sending messages.

packages/contracts-communication/contracts/apps/examples/PingPongApp.sol

@@ -3,6 +3,7 @@ pragma solidity 0.8.20;
 
 import {ICAppV1} from "../ICAppV1.sol";
 
+import {APP_CONFIG_GUARD_DEFAULT} from "../../libs/AppConfig.sol";
 import {InterchainTxDescriptor} from "../../libs/InterchainTransaction.sol";
 import {OptionsV1} from "../../libs/Options.sol";
 
@@ -88,4 +89,11 @@ contract PingPongApp is ICAppV1 {
         gasLimit = gasLimit_;
         emit GasLimitSet(gasLimit_);
     }
+
+    /// @dev Returns the guard flag and address in the app config.
+    /// By default, the ICApp does not opt in for any guard, but it can be overridden in the derived contracts.
+    /// PingPong app opts in for the default guard.
+    function _getGuardConfig() internal pure override returns (uint8 guardFlag, address guard) {
+        return (APP_CONFIG_GUARD_DEFAULT, address(0));
+    }
 }

packages/contracts-communication/contracts/libs/InterchainEntry.sol

@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.0;
 
-import {TypeCasts} from "./TypeCasts.sol";
-
 import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
 
 /// @notice Struct representing an entry in the Interchain DataBase.
@@ -39,7 +37,8 @@ library InterchainEntryLib {
         view
         returns (InterchainEntry memory entry)
     {
-        return InterchainEntry({srcChainId: SafeCast.toUint64(block.chainid), dbNonce: dbNonce, entryValue: entryValue});
+        uint64 srcChainId = SafeCast.toUint64(block.chainid);
+        return InterchainEntry({srcChainId: srcChainId, dbNonce: dbNonce, entryValue: entryValue});
     }
 
     /// @notice Returns the value of the entry: writer + digest hashed together

packages/contracts-communication/test/InterchainClientV1.Dst.t.sol

@@ -368,10 +368,13 @@ abstract contract InterchainClientV1DstTest is InterchainClientV1BaseTest {
             return;
         }
         uint256 actual = 0;
-        if (timeA == justVerTS() || timeA == OVER_VERIFIED) {
+        // If the guard is disabled, the last accepted timestamp is the initial timestamp - 1.
+        // Otherwise, it's the "just verified" timestamp: initial timestamp - optimistic period - 1.
+        uint256 lastAcceptedTS = (guardFlag == GUARD_DISABLED) ? INITIAL_TS - 1 : justVerTS();
+        if (timeA <= lastAcceptedTS && timeA != NOT_VERIFIED) {
             actual++;
         }
-        if (timeB == justVerTS() || timeB == OVER_VERIFIED) {
+        if (timeB <= lastAcceptedTS && timeB != NOT_VERIFIED) {
             actual++;
         }
         if (actual >= required) {

packages/contracts-communication/test/InterchainDB.Src.t.sol

@@ -1,13 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.20;
 
-import {
-    InterchainDB,
-    InterchainEntry,
-    InterchainEntryLib,
-    IInterchainDB,
-    InterchainDBEvents
-} from "../contracts/InterchainDB.sol";
+import {InterchainDB, InterchainEntry, IInterchainDB, InterchainDBEvents} from "../contracts/InterchainDB.sol";
 
 import {InterchainEntryLibHarness} from "./harnesses/InterchainEntryLibHarness.sol";
 import {VersionedPayloadLibHarness} from "./harnesses/VersionedPayloadLibHarness.sol";

packages/contracts-communication/test/apps/ICAppV1.Messaging.t.sol

@@ -1,17 +1,35 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.20;
 
+import {AppConfigV1, APP_CONFIG_GUARD_DISABLED} from "../../contracts/libs/AppConfig.sol";
+
 import {InterchainAppV1MessagingTest} from "./InterchainAppV1.Messaging.t.sol";
 import {IInterchainAppV1Harness} from "../interfaces/IInterchainAppV1Harness.sol";
 import {ICAppV1Harness} from "../harnesses/ICAppV1Harness.sol";
 
 import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol";
 
+// solhint-disable func-name-mixedcase
+// solhint-disable ordering
 contract ICAppV1MessagingTest is InterchainAppV1MessagingTest {
     /// @dev This should deploy the Interchain App V1 contract and give `governor`
     /// privileges to setup its interchain configuration.
     function deployICAppV1() internal override returns (IInterchainAppV1Harness app) {
         app = new ICAppV1Harness(address(this));
         IAccessControl(address(app)).grantRole(IC_GOVERNOR_ROLE, governor);
     }
+
+    function test_freshAppConfig() public {
+        ICAppV1Harness app = new ICAppV1Harness(address(this));
+        AppConfigV1 memory config = app.getAppConfigV1();
+        assertEq(config.requiredResponses, 0);
+        assertEq(config.optimisticPeriod, 0);
+        assertEq(config.guardFlag, APP_CONFIG_GUARD_DISABLED);
+        assertEq(config.guard, address(0));
+    }
+
+    function test_freshAppModules() public {
+        ICAppV1Harness app = new ICAppV1Harness(address(this));
+        assertEq(app.getModules(), new address[](0));
+    }
 }

packages/contracts-communication/test/apps/InterchainAppV1.Management.t.sol

@@ -398,6 +398,16 @@ abstract contract InterchainAppV1ManagementTest is InterchainAppV1Test {
         assertEq(appHarness.getAppConfigV1().optimisticPeriod, APP_OPTIMISTIC_PERIOD);
     }
 
+    function test_setAppConfigV1_whenNotSet_zeroPeriod() public {
+        expectEventAppConfigV1Set(APP_REQUIRED_RESPONSES, 0);
+        vm.recordLogs();
+        vm.prank(governor);
+        appHarness.setAppConfigV1(APP_REQUIRED_RESPONSES, 0);
+        assertEq(vm.getRecordedLogs().length, 1);
+        assertEq(appHarness.getAppConfigV1().requiredResponses, APP_REQUIRED_RESPONSES);
+        assertEq(appHarness.getAppConfigV1().optimisticPeriod, 0);
+    }
+
     function test_setAppConfigV1_whenSet() public {
         vm.prank(governor);
         appHarness.setAppConfigV1(APP_REQUIRED_RESPONSES, APP_OPTIMISTIC_PERIOD);
@@ -410,6 +420,18 @@ abstract contract InterchainAppV1ManagementTest is InterchainAppV1Test {
         assertEq(appHarness.getAppConfigV1().optimisticPeriod, APP_OPTIMISTIC_PERIOD + 1);
     }
 
+    function test_setAppConfigV1_whenSet_zeroPeriod() public {
+        vm.prank(governor);
+        appHarness.setAppConfigV1(APP_REQUIRED_RESPONSES, APP_OPTIMISTIC_PERIOD);
+        expectEventAppConfigV1Set(APP_REQUIRED_RESPONSES + 1, 0);
+        vm.recordLogs();
+        vm.prank(governor);
+        appHarness.setAppConfigV1(APP_REQUIRED_RESPONSES + 1, 0);
+        assertEq(vm.getRecordedLogs().length, 1);
+        assertEq(appHarness.getAppConfigV1().requiredResponses, APP_REQUIRED_RESPONSES + 1);
+        assertEq(appHarness.getAppConfigV1().optimisticPeriod, 0);
+    }
+
     function test_setAppConfigV1_revert_notGovernor(address caller) public {
         vm.assume(caller != governor);
         expectRevertUnauthorizedGovernor(caller);
@@ -423,12 +445,6 @@ abstract contract InterchainAppV1ManagementTest is InterchainAppV1Test {
         appHarness.setAppConfigV1(0, APP_OPTIMISTIC_PERIOD);
     }
 
-    function test_setAppConfigV1_revert_zeroOptimisticPeriod() public {
-        expectRevertAppConfigInvalid(APP_REQUIRED_RESPONSES, 0);
-        vm.prank(governor);
-        appHarness.setAppConfigV1(APP_REQUIRED_RESPONSES, 0);
-    }
-
     function test_setAppConfigV1_revert_zeroedAppConfig() public {
         expectRevertAppConfigInvalid(0, 0);
         vm.prank(governor);

packages/contracts-communication/test/apps/InterchainAppV1.Messaging.t.sol

@@ -128,7 +128,7 @@ abstract contract InterchainAppV1MessagingTest is InterchainAppV1Test {
             AppConfigV1({
                 requiredResponses: APP_REQUIRED_RESPONSES,
                 optimisticPeriod: APP_OPTIMISTIC_PERIOD,
-                guardFlag: 1,
+                guardFlag: 0,
                 guard: address(0)
             }).encodeAppConfigV1()
         );

packages/contracts-communication/test/integration/ICSetup.t.sol

@@ -29,8 +29,6 @@ abstract contract ICSetup is ProxyTest {
     uint64 public constant SRC_INITIAL_DB_NONCE = 10;
     uint64 public constant DST_INITIAL_DB_NONCE = 20;
 
-    uint256 public constant APP_OPTIMISTIC_PERIOD = 10 minutes;
-
     uint256 public constant INITIAL_TS = 1_704_067_200; // 2024-01-01 00:00:00 UTC
 
     InterchainEntryLibHarness public entryLibHarness;
@@ -128,10 +126,9 @@ abstract contract ICSetup is ProxyTest {
 
     function configureLocalApp() internal virtual {
         address app = localApp();
-        // For simplicity, we assume that the apps are deployed to the same address on both chains.
         ICAppV1(app).linkRemoteAppEVM(remoteChainId(), remoteApp());
         ICAppV1(app).addTrustedModule(address(module));
-        ICAppV1(app).setAppConfigV1({requiredResponses: 1, optimisticPeriod: APP_OPTIMISTIC_PERIOD});
+        ICAppV1(app).setAppConfigV1({requiredResponses: 1, optimisticPeriod: getAppOptimisticPeriod()});
         ICAppV1(app).setExecutionService(address(executionService));
         ICAppV1(app).addInterchainClient({client: address(icClient), updateLatest: true});
     }
@@ -167,7 +164,7 @@ abstract contract ICSetup is ProxyTest {
         }
     }
 
-    function isSourceChainTest() internal view returns (bool) {
+    function isSourceChainTest() internal pure returns (bool) {
         return isSourceChain(localChainId());
     }
 
@@ -189,8 +186,10 @@ abstract contract ICSetup is ProxyTest {
         return isSourceChainTest() ? dstApp : srcApp;
     }
 
+    /// @notice Should return the optimistic period for the app.
+    function getAppOptimisticPeriod() internal pure virtual returns (uint256);
     /// @notice Should return either `SRC_CHAIN_ID` or `DST_CHAIN_ID`.
-    function localChainId() internal view virtual returns (uint64);
+    function localChainId() internal pure virtual returns (uint64);
     /// @notice Should return either `SRC_CHAIN_ID` or `DST_CHAIN_ID`.
-    function remoteChainId() internal view virtual returns (uint64);
+    function remoteChainId() internal pure virtual returns (uint64);
 }

packages/contracts-communication/test/integration/PingPong.t.sol

@@ -3,7 +3,7 @@ pragma solidity 0.8.20;
 
 import {PingPongApp} from "../../contracts/apps/examples/PingPongApp.sol";
 
-import {ICIntegrationTest, InterchainEntry, InterchainTransaction} from "./ICIntegration.t.sol";
+import {ICIntegrationTest, InterchainTransaction} from "./ICIntegration.t.sol";
 
 import {OptionsV1} from "../../contracts/libs/Options.sol";
 
@@ -13,6 +13,8 @@ abstract contract PingPongIntegrationTest is ICIntegrationTest {
     uint256 public constant PING_PONG_BALANCE = 1000 ether;
     uint256 public constant COUNTER = 42;
 
+    uint256 public constant APP_OPTIMISTIC_PERIOD = 10 minutes;
+
     OptionsV1 public ppOptions = OptionsV1({gasLimit: 500_000, gasAirdrop: 0});
 
     event PingReceived(uint256 counter, uint64 dbNonce);
@@ -76,4 +78,8 @@ abstract contract PingPongIntegrationTest is ICIntegrationTest {
     function getDstMessage() internal pure override returns (bytes memory) {
         return abi.encode(COUNTER - 1);
     }
+
+    function getAppOptimisticPeriod() internal pure override returns (uint256) {
+        return APP_OPTIMISTIC_PERIOD;
+    }
 }

packages/contracts-communication/test/integration/legacy/LegacyPingPong.Dst.t.sol

@@ -163,38 +163,53 @@ contract LegacyPingPongDstIntegrationTest is LegacyPingPongIntegrationTest {
         executeTx(icOptions);
     }
 
-    function test_interchainExecute_revert_notConfirmed_guardMarked() public {
-        markInvalidByGuard(srcFullEntry);
-        expectClientRevertEntryConflict(guard);
-        executeTx(icOptions);
-    }
-
     function test_interchainExecute_revert_confirmed_sameBlock() public {
         module.verifyRemoteEntry(moduleEntry, moduleSignatures);
         expectClientRevertResponsesAmountBelowMin({actual: 0, required: 1});
         executeTx(icOptions);
     }
 
-    function test_interchainExecute_revert_confirmed_sameBlock_guardMarked() public {
-        module.verifyRemoteEntry(moduleEntry, moduleSignatures);
-        markInvalidByGuard(srcFullEntry);
-        expectClientRevertEntryConflict(guard);
-        executeTx(icOptions);
-    }
-
     function test_interchainExecute_revert_confirmed_periodMinusOneSecond() public {
         module.verifyRemoteEntry(moduleEntry, moduleSignatures);
         skip(APP_OPTIMISTIC_PERIOD);
         expectClientRevertResponsesAmountBelowMin({actual: 0, required: 1});
         executeTx(icOptions);
     }
 
+    /// @notice MessageBus doesn't opt in for the guard, so the guard conflict is not checked
+    function test_interchainExecute_state_db_guardMarked() public {
+        markInvalidByGuard(srcFullEntry);
+        test_interchainExecute_state_db();
+    }
+
+    function test_interchainExecute_state_execService_guardMarked() public {
+        markInvalidByGuard(srcFullEntry);
+        test_interchainExecute_state_execService();
+    }
+
+    function test_interchainExecute_state_legacyPingPong_guardMarked() public {
+        markInvalidByGuard(srcFullEntry);
+        test_interchainExecute_state_legacyPingPong();
+    }
+
+    function test_interchainExecute_state_synapseModule_guardMarked() public {
+        markInvalidByGuard(srcFullEntry);
+        test_interchainExecute_state_synapseModule();
+    }
+
+    function test_interchainExecute_revert_notConfirmed_guardMarked() public {
+        markInvalidByGuard(srcFullEntry);
+        test_interchainExecute_revert_notConfirmed();
+    }
+
+    function test_interchainExecute_revert_confirmed_sameBlock_guardMarked() public {
+        markInvalidByGuard(srcFullEntry);
+        test_interchainExecute_revert_confirmed_sameBlock();
+    }
+
     function test_interchainExecute_revert_confirmed_periodMinusOneSecond_guardMarked() public {
-        module.verifyRemoteEntry(moduleEntry, moduleSignatures);
-        skip(APP_OPTIMISTIC_PERIOD);
         markInvalidByGuard(srcFullEntry);
-        expectClientRevertEntryConflict(guard);
-        executeTx(icOptions);
+        test_interchainExecute_revert_confirmed_periodMinusOneSecond();
     }
 
     function test_interchainExecute_revert_alreadyExecuted() public {
@@ -218,8 +233,7 @@ contract LegacyPingPongDstIntegrationTest is LegacyPingPongIntegrationTest {
 
     function test_isExecutable_revert_notConfirmed_guardMarked() public {
         markInvalidByGuard(srcFullEntry);
-        expectClientRevertEntryConflict(guard);
-        icClient.isExecutable(encodedSrcTx);
+        test_isExecutable_revert_notConfirmed();
     }
 
     function test_isExecutable_revert_confirmed_sameBlock() public {
@@ -229,10 +243,8 @@ contract LegacyPingPongDstIntegrationTest is LegacyPingPongIntegrationTest {
     }
 
     function test_isExecutable_revert_confirmed_sameBlock_guardMarked() public {
-        module.verifyRemoteEntry(moduleEntry, moduleSignatures);
         markInvalidByGuard(srcFullEntry);
-        expectClientRevertEntryConflict(guard);
-        icClient.isExecutable(encodedSrcTx);
+        test_isExecutable_revert_confirmed_sameBlock();
     }
 
     function test_isExecutable_revert_confirmed_periodMinusOneSecond() public {
@@ -243,11 +255,8 @@ contract LegacyPingPongDstIntegrationTest is LegacyPingPongIntegrationTest {
     }
 
     function test_isExecutable_revert_confirmed_periodMinusOneSecond_guardMarked() public {
-        module.verifyRemoteEntry(moduleEntry, moduleSignatures);
-        skip(APP_OPTIMISTIC_PERIOD);
         markInvalidByGuard(srcFullEntry);
-        expectClientRevertEntryConflict(guard);
-        icClient.isExecutable(encodedSrcTx);
+        test_isExecutable_revert_confirmed_periodMinusOneSecond();
     }
 
     function test_isExecutable_revert_alreadyExecuted() public {