Fix: ToB-11 (dispute collusion)

packages/contracts-core/contracts/Destination.sol

@@ -5,7 +5,7 @@ pragma solidity 0.8.17;
 import {Attestation, AttestationLib} from "./libs/memory/Attestation.sol";
 import {ByteString} from "./libs/memory/ByteString.sol";
 import {AGENT_ROOT_OPTIMISTIC_PERIOD} from "./libs/Constants.sol";
-import {IndexOutOfRange, NotaryInDispute, OutdatedNonce} from "./libs/Errors.sol";
+import {IndexOutOfRange, DisputeTimeoutNotOver, NotaryInDispute, OutdatedNonce} from "./libs/Errors.sol";
 import {ChainGas, GasData} from "./libs/stack/GasData.sol";
 import {AgentStatus, DestinationStatus} from "./libs/Structures.sol";
 import {ChainContext} from "./libs/ChainContext.sol";
@@ -94,7 +94,9 @@ contract Destination is ExecutionHub, DestinationEvents, InterfaceDestination {
         bytes32 agentRoot,
         ChainGas[] memory snapGas
     ) external onlyInbox returns (bool wasAccepted) {
-        if (_isInDispute(notaryIndex)) revert NotaryInDispute();
+        // Check that we can trust the Notary data: they are not in dispute, and the dispute timeout is over (if any)
+        if (_notaryDisputeExists(notaryIndex)) revert NotaryInDispute();
+        if (_notaryDisputeTimeout(notaryIndex)) revert DisputeTimeoutNotOver();
         // First, try passing current agent merkle root
         (bool rootPassed, bool rootPending) = passAgentRoot();
         // Don't accept attestation, if the agent root was updated in LightManager,
@@ -128,11 +130,17 @@ contract Destination is ExecutionHub, DestinationEvents, InterfaceDestination {
         DestinationStatus memory status = destStatus;
         // Invariant: Notary who supplied `newRoot` was registered as active against `oldRoot`
         // So we just need to check the Dispute status of the Notary
-        if (_isInDispute(status.notaryIndex)) {
+        if (_notaryDisputeExists(status.notaryIndex)) {
             // Remove the pending agent merkle root, as its signer is in dispute
             _nextAgentRoot = oldRoot;
             return (false, false);
         }
+        // If Notary recently won a Dispute, we can optimistically assume that their passed root is valid.
+        // However, we need to wait until the Dispute timeout is over, before passing the new root to LightManager.
+        if (_notaryDisputeTimeout(status.notaryIndex)) {
+            // We didn't pass anything, but there is a pending root
+            return (false, true);
+        }
         // Check if agent root optimistic period is over
         if (status.agentRootTime + AGENT_ROOT_OPTIMISTIC_PERIOD > block.timestamp) {
             // We didn't pass anything, but there is a pending root
@@ -174,12 +182,20 @@ contract Destination is ExecutionHub, DestinationEvents, InterfaceDestination {
     /// @inheritdoc InterfaceDestination
     function getGasData(uint32 domain) external view returns (GasData gasData, uint256 dataMaturity) {
         StoredGasData memory storedGasData = _storedGasData[domain];
-        // Check if there is a stored gas data for the domain, and if the notary who provided the data is not in dispute
-        if (storedGasData.submittedAt != 0 && !_isInDispute(storedGasData.notaryIndex)) {
+        // Form the data to return only if it exists and we can trust it:
+        // - There is stored gas data for the domain
+        // - Notary who provided the data is not in dispute
+        // - Notary who provided the data is not in post-dispute timeout period
+        // forgefmt: disable-next-item
+        if (
+            storedGasData.submittedAt != 0 &&
+            !_notaryDisputeExists(storedGasData.notaryIndex) &&
+            !_notaryDisputeTimeout(storedGasData.notaryIndex)
+        ) {
             gasData = storedGasData.gasData;
             dataMaturity = block.timestamp - storedGasData.submittedAt;
         }
-        // Return empty values if there is no data for the domain, or if the notary who provided the data is in dispute
+        // Return empty values if there is no data for the domain, or the notary who provided the data can't be trusted.
     }
 
     /// @inheritdoc InterfaceDestination

packages/contracts-core/contracts/Summit.sol

@@ -5,7 +5,13 @@ pragma solidity 0.8.17;
 import {AttestationLib} from "./libs/memory/Attestation.sol";
 import {ByteString} from "./libs/memory/ByteString.sol";
 import {BONDING_OPTIMISTIC_PERIOD, TIPS_GRANULARITY} from "./libs/Constants.sol";
-import {MustBeSynapseDomain, NotaryInDispute, TipsClaimMoreThanEarned, TipsClaimZero} from "./libs/Errors.sol";
+import {
+    MustBeSynapseDomain,
+    DisputeTimeoutNotOver,
+    NotaryInDispute,
+    TipsClaimMoreThanEarned,
+    TipsClaimZero
+} from "./libs/Errors.sol";
 import {Receipt, ReceiptLib} from "./libs/memory/Receipt.sol";
 import {Snapshot, SnapshotLib} from "./libs/memory/Snapshot.sol";
 import {AgentFlag, AgentStatus, DisputeFlag, MessageStatus} from "./libs/Structures.sol";
@@ -112,7 +118,9 @@ contract Summit is SnapshotHub, SummitEvents, InterfaceSummit {
         uint256 paddedTips,
         bytes memory rcptPayload
     ) external onlyInbox returns (bool wasAccepted) {
-        if (_isInDispute(rcptNotaryIndex)) revert NotaryInDispute();
+        // Check that we can trust the receipt Notary data: they are not in dispute, and the dispute timeout is over.
+        if (_notaryDisputeExists(rcptNotaryIndex)) revert NotaryInDispute();
+        if (_notaryDisputeTimeout(rcptNotaryIndex)) revert DisputeTimeoutNotOver();
         // This will revert if payload is not a receipt body
         return _saveReceipt({
             rcpt: rcptPayload.castToReceipt(),
@@ -138,7 +146,9 @@ contract Summit is SnapshotHub, SummitEvents, InterfaceSummit {
         onlyInbox
         returns (bytes memory attPayload)
     {
-        if (_isInDispute(notaryIndex)) revert NotaryInDispute();
+        // Check that we can trust the snapshot Notary data: they are not in dispute, and the dispute timeout is over.
+        if (_notaryDisputeExists(notaryIndex)) revert NotaryInDispute();
+        if (_notaryDisputeTimeout(notaryIndex)) revert DisputeTimeoutNotOver();
         // This will revert if payload is not a snapshot
         return _acceptNotarySnapshot(snapPayload.castToSnapshot(), agentRoot, notaryIndex, sigIndex);
     }
@@ -211,15 +221,17 @@ contract Summit is SnapshotHub, SummitEvents, InterfaceSummit {
     /// - Notary was slashed => receipt is invalided and deleted
     /// - Notary is in Dispute => receipt handling is postponed
     function _checkNotaryDisputed(bytes32 messageHash, uint32 notaryIndex) internal returns (bool queuePopped) {
-        DisputeFlag flag = _disputes[notaryIndex];
+        // TODO: add timeout for Notaries that just won the dispute.
+        DisputeFlag flag = _disputes[notaryIndex].flag;
         if (flag == DisputeFlag.Slashed) {
             // Notary has been slashed, so we can't trust their statement.
             // Honest Notaries are incentivized to resubmit the Receipt or Attestation if it was in fact valid.
             _deleteFromQueue(messageHash);
             queuePopped = true;
-        } else if (flag == DisputeFlag.Pending) {
-            // Notary is not slashed, but is in Dispute. To keep the tips flow going we add the receipt to the back of
-            // the queue, hoping that by the next interaction the dispute will have been resolved.
+        } else if (flag == DisputeFlag.Pending || _notaryDisputeTimeout(notaryIndex)) {
+            // Notary is in the ongoing Dispute, or has recently won one. We postpone the receipt handling.
+            // To keep the tips flow going we add the receipt to the back of the queue,
+            // hoping that by the next interaction the dispute will have been resolved.
             _moveToBack();
             queuePopped = true;
         }

packages/contracts-core/contracts/base/AgentSecured.sol

@@ -2,8 +2,10 @@
 pragma solidity 0.8.17;
 
 // ══════════════════════════════ LIBRARY IMPORTS ══════════════════════════════
+import {DISPUTE_TIMEOUT_NOTARY} from "../libs/Constants.sol";
+import {ChainContext} from "../libs/ChainContext.sol";
 import {CallerNotAgentManager, CallerNotInbox} from "../libs/Errors.sol";
-import {AgentStatus, DisputeFlag} from "../libs/Structures.sol";
+import {AgentStatus, DisputeFlag, DisputeStatus} from "../libs/Structures.sol";
 // ═════════════════════════════ INTERNAL IMPORTS ══════════════════════════════
 import {IAgentManager} from "../interfaces/IAgentManager.sol";
 import {IAgentSecured} from "../interfaces/IAgentSecured.sol";
@@ -29,8 +31,8 @@
 
     // ══════════════════════════════════════════════════ STORAGE ══════════════════════════════════════════════════════
 
-    // (agent index => their dispute flag: None/Pending/Slashed)
-    mapping(uint32 => DisputeFlag) internal _disputes;
+    // (agent index => their dispute status: flag, openedAt, resolvedAt)
+    mapping(uint32 => DisputeStatus) internal _disputes;
 
     /// @dev gap for upgrade safety
     uint256[49] private __GAP; // solhint-disable-line var-name-mixedcase
@@ -56,14 +58,23 @@
 
     /// @inheritdoc IAgentSecured
     function openDispute(uint32 guardIndex, uint32 notaryIndex) external onlyAgentManager {
-        _disputes[guardIndex] = DisputeFlag.Pending;
-        _disputes[notaryIndex] = DisputeFlag.Pending;
+        uint40 openedAt = ChainContext.blockTimestamp();
+        DisputeStatus memory status = DisputeStatus({flag: DisputeFlag.Pending, openedAt: openedAt, resolvedAt: 0});
+        _disputes[guardIndex] = status;
+        _disputes[notaryIndex] = status;
     }
 
     /// @inheritdoc IAgentSecured
-    function resolveDispute(uint32 slashedIndex, uint32 honestIndex) external onlyAgentManager {
-        _disputes[slashedIndex] = DisputeFlag.Slashed;
-        if (honestIndex != 0) delete _disputes[honestIndex];
+    function resolveDispute(uint32 slashedIndex, uint32 rivalIndex) external onlyAgentManager {
+        // Update the dispute status of the slashed agent first.
+        uint40 resolvedAt = ChainContext.blockTimestamp();
+        _disputes[slashedIndex].flag = DisputeFlag.Slashed;
+        _disputes[slashedIndex].resolvedAt = resolvedAt;
+        // Mark the rival agent as not disputed, if there was an ongoing dispute.
+        if (rivalIndex != 0) {
+            _disputes[rivalIndex].flag = DisputeFlag.None;
+            _disputes[rivalIndex].resolvedAt = resolvedAt;
+        }
     }
 
     // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════
@@ -78,6 +89,11 @@
         return _getAgent(index);
     }
 
+    /// @inheritdoc IAgentSecured
+    function latestDisputeStatus(uint32 agentIndex) external view returns (DisputeStatus memory) {
+        return _disputes[agentIndex];
+    }
+
     // ══════════════════════════════════════════════ INTERNAL VIEWS ═══════════════════════════════════════════════════
 
     /// @dev Returns status of the given agent: (flag, domain, index).
@@ -90,8 +106,27 @@
         return IAgentManager(agentManager).getAgent(index);
     }
 
-    /// @dev Checks if the agent with the given index is in a dispute.
-    function _isInDispute(uint32 agentIndex) internal view returns (bool) {
-        return _disputes[agentIndex] != DisputeFlag.None;
+    /// @dev Checks if a Dispute exists for the given Notary. This function returns true, if
+    /// Notary is in ongoing Dispute, or if Dispute was resolved not in Notary's favor.
+    /// In both cases we can't trust Notary's data.
+    /// Note: Agent-Secured contracts can trust Notary data only if both `_notaryDisputeExists` and
+    /// `_notaryDisputeTimeout` return false.
+    function _notaryDisputeExists(uint32 notaryIndex) internal view returns (bool) {
+        return _disputes[notaryIndex].flag != DisputeFlag.None;
+    }
+
+    /// @dev Checks if a Notary recently won a Dispute and is still in the "post-dispute" timeout period.
+    /// In this period we still can't trust Notary's data, though we can optimistically assume that
+    /// that the data will be correct after the timeout (assuming no new Disputes are opened).
+    /// Note: Agent-Secured contracts can trust Notary data only if both `_notaryDisputeExists` and
+    /// `_notaryDisputeTimeout` return false.
+    function _notaryDisputeTimeout(uint32 notaryIndex) internal view returns (bool) {
+        DisputeStatus memory status = _disputes[notaryIndex];
+        // Exit early if Notary is in ongoing Dispute / slashed.
+        if (status.flag != DisputeFlag.None) return false;
+        // Check if Notary has been in any Dispute at all.
+        if (status.openedAt == 0) return false;
+        // Otherwise check if the Dispute timeout is still active.
+        return block.timestamp < status.resolvedAt + DISPUTE_TIMEOUT_NOTARY;
     }
 }

packages/contracts-core/contracts/hubs/ExecutionHub.sol

@@ -9,6 +9,7 @@ import {ORIGIN_TREE_HEIGHT, SNAPSHOT_TREE_HEIGHT} from "../libs/Constants.sol";
 import {
     AlreadyExecuted,
     AlreadyFailed,
+    DisputeTimeoutNotOver,
     DuplicatedSnapshotRoot,
     IncorrectDestinationDomain,
     IncorrectMagicValue,
@@ -367,7 +368,9 @@ abstract contract ExecutionHub is AgentSecured, ReentrancyGuardUpgradeable, Exec
         // Check if snapshot root has been submitted
         if (rootData.submittedAt == 0) revert IncorrectSnapshotRoot();
         // Check that Notary who submitted the attestation is not in dispute
-        if (_isInDispute(rootData.notaryIndex)) revert NotaryInDispute();
+        if (_notaryDisputeExists(rootData.notaryIndex)) revert NotaryInDispute();
+        // Check that Notary who submitted the attestation isn't in post-dispute timeout
+        if (_notaryDisputeTimeout(rootData.notaryIndex)) revert DisputeTimeoutNotOver();
     }
 
     /// @dev Formats the message execution receipt payload for the given hash and receipt data.

packages/contracts-core/contracts/interfaces/IAgentSecured.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.17;
 
-import {AgentStatus} from "../libs/Structures.sol";
+import {AgentStatus, DisputeStatus} from "../libs/Structures.sol";
 
 interface IAgentSecured {
     /**
@@ -53,4 +53,13 @@ interface IAgentSecured {
      * @return status   Status for the given agent: (flag, domain, index)
      */
     function getAgent(uint256 index) external view returns (address agent, AgentStatus memory status);
+
+    /**
+     * @notice Returns (flag, openedAt, resolvedAt) that describes the latest status of
+     * the latest dispute for an agent with a given index.
+     * @dev Will return empty values if agent with given index doesn't exist.
+     * @param agentIndex    Agent index in the Agent Merkle Tree
+     * @return              Latest dispute status for the given agent: (flag, openedAt, resolvedAt)
+     */
+    function latestDisputeStatus(uint32 agentIndex) external view returns (DisputeStatus memory);
 }

packages/contracts-core/contracts/libs/Constants.sol

@@ -46,6 +46,8 @@ uint32 constant AGENT_ROOT_OPTIMISTIC_PERIOD = 1 days;
 /// @dev Timeout between the agent root could be proposed and resolved in LightManager
 uint32 constant AGENT_ROOT_PROPOSAL_TIMEOUT = 12 hours;
 uint32 constant BONDING_OPTIMISTIC_PERIOD = 1 days;
+/// @dev Amount of time that the Notary will not be considered active after they won a dispute
+uint32 constant DISPUTE_TIMEOUT_NOTARY = 12 hours;
 /// @dev Amount of time without fresh data from Notaries before contract owner can resolve stuck disputes manually
 uint256 constant FRESH_DATA_TIMEOUT = 4 hours;
 /// @dev Maximum bytes per message = 2 KiB (somewhat arbitrarily set to begin)

packages/contracts-core/contracts/libs/Errors.sol

@@ -82,6 +82,7 @@ error NotStuck();
 
 error DisputeAlreadyResolved();
 error DisputeNotOpened();
+error DisputeTimeoutNotOver();
 error GuardInDispute();
 error NotaryInDispute();
 

packages/contracts-core/contracts/libs/Structures.sol

@@ -54,6 +54,16 @@ enum DisputeFlag {
     Slashed
 }
 
+/// @notice Struct for storing dispute status of an agent.
+/// @param flag         Dispute flag: None/Pending/Slashed.
+/// @param openedAt     Timestamp when the latest agent dispute was opened (zero if not opened).
+/// @param resolvedAt   Timestamp when the latest agent dispute was resolved (zero if not resolved).
+struct DisputeStatus {
+    DisputeFlag flag;
+    uint40 openedAt;
+    uint40 resolvedAt;
+}
+
 // ════════════════════════════════ DESTINATION ════════════════════════════════
 
 /// @notice Struct representing the status of Destination contract.

packages/contracts-core/test/mocks/base/AgentSecuredMock.t.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.17;
 
-import {AgentStatus, IAgentSecured} from "../../../contracts/interfaces/IAgentSecured.sol";
+import {AgentStatus, DisputeStatus, IAgentSecured} from "../../../contracts/interfaces/IAgentSecured.sol";
 
 contract AgentSecuredMock is IAgentSecured {
     /// @notice Prevents this contract from being included in the coverage report
@@ -18,4 +18,6 @@ contract AgentSecuredMock is IAgentSecured {
     function agentStatus(address agent) external view returns (AgentStatus memory) {}
 
     function getAgent(uint256 index) external view returns (address agent, AgentStatus memory status) {}
+
+    function latestDisputeStatus(uint32 agentIndex) external view returns (DisputeStatus memory) {}
 }