Fix: ToB-08 (indefinite stuck disputes) (#1449)

* Track last Notary attestation nonce * Enforce ascending attestation nonces * Add tests * Add success tests as well
synapsecns · Oct 23, 2023 · 10f3dff · 10f3dff
1 parent 5874ba9
commit 10f3dff
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 2 deletions.
diff --git a/packages/contracts-core/contracts/Destination.sol b/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} from "./libs/Errors.sol";
+import {IndexOutOfRange, NotaryInDispute, OutdatedNonce} from "./libs/Errors.sol";
 import {ChainGas, GasData} from "./libs/stack/GasData.sol";
 import {AgentStatus, DestinationStatus} from "./libs/Structures.sol";
 // ═════════════════════════════ INTERNAL IMPORTS ══════════════════════════════
@@ -52,6 +52,9 @@ contract Destination is ExecutionHub, DestinationEvents, InterfaceDestination {
     /// @inheritdoc InterfaceDestination
     DestinationStatus public destStatus;
 
+    /// @inheritdoc InterfaceDestination
+    mapping(uint32 => uint32) public lastAttestationNonce;
+
     /// @dev Stored lookup data for all accepted Notary Attestations
     StoredAttData[] internal _storedAttestations;
 
@@ -98,6 +101,10 @@ contract Destination is ExecutionHub, DestinationEvents, InterfaceDestination {
         if (rootPassed) return false;
         // This will revert if payload is not an attestation
         Attestation att = attPayload.castToAttestation();
+        // Check that this Notary hasn't used a more fresh nonce
+        uint32 attNonce = att.nonce();
+        if (attNonce <= lastAttestationNonce[notaryIndex]) revert OutdatedNonce();
+        lastAttestationNonce[notaryIndex] = attNonce;
         // This will revert if snapshot root has been previously submitted
         _saveAttestation(att, notaryIndex, sigIndex);
         _storedAttestations.push(StoredAttData({agentRoot: agentRoot, dataHash: att.dataHash()}));

diff --git a/packages/contracts-core/contracts/interfaces/InterfaceDestination.sol b/packages/contracts-core/contracts/interfaces/InterfaceDestination.sol
@@ -81,4 +81,10 @@ interface InterfaceDestination {
      * Returns Agent Merkle Root to be passed to LightManager once its optimistic period is over.
      */
     function nextAgentRoot() external view returns (bytes32);
+
+    /**
+     * @notice Returns the nonce of the last attestation submitted by a Notary with a given agent index.
+     * @dev Will return zero if the Notary hasn't submitted any attestations yet.
+     */
+    function lastAttestationNonce(uint32 notaryIndex) external view returns (uint32);
 }
diff --git a/packages/contracts-core/test/mocks/DestinationMock.t.sol b/packages/contracts-core/test/mocks/DestinationMock.t.sol
@@ -29,4 +29,6 @@ contract DestinationMock is ExecutionHubMock, AgentSecuredMock, InterfaceDestina
     function destStatus() external view returns (uint40 snapRootTime, uint40 agentRootTime, uint32 notaryIndex) {}
 
     function nextAgentRoot() external view returns (bytes32) {}
+
+    function lastAttestationNonce(uint32 notaryIndex) external view returns (uint32) {}
 }
diff --git a/packages/contracts-core/test/suite/Destination.t.sol b/packages/contracts-core/test/suite/Destination.t.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.17;
 
-import {CallerNotInbox, NotaryInDispute} from "../../contracts/libs/Errors.sol";
+import {CallerNotInbox, NotaryInDispute, OutdatedNonce} from "../../contracts/libs/Errors.sol";
 import {SNAPSHOT_MAX_STATES} from "../../contracts/libs/memory/Snapshot.sol";
 import {DisputeFlag} from "../../contracts/libs/Structures.sol";
 import {IAgentSecured} from "../../contracts/interfaces/IAgentSecured.sol";
@@ -91,6 +91,7 @@ contract DestinationTest is ExecutionHubTest {
     }
 
     function test_submitAttestation(RawAttestation memory ra, uint32 rootSubmittedAt) public {
+        vm.assume(ra.nonce != 0);
         RawSnapshot memory rs = Random(ra.snapRoot).nextSnapshot();
         ra._snapGasHash = rs.snapGasHash();
         ra.setDataHash();
@@ -103,9 +104,11 @@ contract DestinationTest is ExecutionHubTest {
         lightInbox.submitAttestation(attPayload, attSig, ra._agentRoot, snapGas);
         (uint48 snapRootTime,,) = InterfaceDestination(localDestination()).destStatus();
         assertEq(snapRootTime, rootSubmittedAt);
+        assertEq(InterfaceDestination(localDestination()).lastAttestationNonce(agentIndex[notary]), ra.nonce);
     }
 
     function test_submitAttestation_updatesAgentRoot(RawAttestation memory ra, uint32 rootSubmittedAt) public {
+        vm.assume(ra.nonce != 0);
         RawSnapshot memory rs = Random(ra.snapRoot).nextSnapshot();
         ra._snapGasHash = rs.snapGasHash();
         ra.setDataHash();
@@ -131,6 +134,7 @@ contract DestinationTest is ExecutionHubTest {
         uint32 firstRootSubmittedAt,
         uint32 timePassed
     ) public {
+        vm.assume(firstRA.nonce != 0 && secondRA.nonce != 0);
         bytes32 agentRootLM = lightManager.agentRoot();
         vm.assume(firstRA._agentRoot != agentRootLM);
         vm.assume(firstRA.snapRoot != secondRA.snapRoot);
@@ -170,6 +174,33 @@ contract DestinationTest is ExecutionHubTest {
         InterfaceDestination(localDestination()).acceptAttestation(agentIndex[notary], 0, "", 0, new ChainGas[](0));
     }
 
+    function test_acceptAttestation_revert_lowerNonce() public {
+        Random memory random = Random("salt");
+        RawAttestation memory firstRA = random.nextAttestation({nonce: 2});
+        test_submitAttestation({ra: firstRA, rootSubmittedAt: 1000});
+        skip(100);
+        RawSnapshot memory rawSnap = random.nextSnapshot();
+        uint256[] memory snapGas = rawSnap.snapGas();
+        RawAttestation memory secondRA = random.nextAttestation({rawSnap: rawSnap, nonce: 1});
+        address notary = domains[DOMAIN_LOCAL].agent;
+        (bytes memory attPayload, bytes memory attSig) = signAttestation(notary, secondRA);
+        vm.expectRevert(OutdatedNonce.selector);
+        lightInbox.submitAttestation(attPayload, attSig, secondRA._agentRoot, snapGas);
+    }
+
+    function test_acceptAttestation_revert_sameNonce() public {
+        Random memory random = Random("salt");
+        RawSnapshot memory rawSnap = random.nextSnapshot();
+        uint256[] memory snapGas = rawSnap.snapGas();
+        RawAttestation memory ra = random.nextAttestation({rawSnap: rawSnap, nonce: 1});
+        address notary = domains[DOMAIN_LOCAL].agent;
+        (bytes memory attPayload, bytes memory attSig) = signAttestation(notary, ra);
+        lightInbox.submitAttestation(attPayload, attSig, ra._agentRoot, snapGas);
+        skip(100);
+        vm.expectRevert(OutdatedNonce.selector);
+        lightInbox.submitAttestation(attPayload, attSig, ra._agentRoot, snapGas);
+    }
+
     function test_acceptAttestation_notAccepted_agentRootUpdated(
         RawAttestation memory firstRA,
         uint32 firstRootSubmittedAt
@@ -190,6 +221,34 @@ contract DestinationTest is ExecutionHubTest {
         assertEq(lightManager.agentRoot(), firstRA._agentRoot);
     }
 
+    function test_acceptAttestation_success_diffNotary_lowerNonce() public {
+        Random memory random = Random("salt");
+        RawAttestation memory firstRA = random.nextAttestation({nonce: 2});
+        test_submitAttestation({ra: firstRA, rootSubmittedAt: 1000});
+        skip(100);
+        RawSnapshot memory rawSnap = random.nextSnapshot();
+        uint256[] memory snapGas = rawSnap.snapGas();
+        RawAttestation memory secondRA = random.nextAttestation({rawSnap: rawSnap, nonce: 1});
+        address notary = domains[DOMAIN_LOCAL].agents[1];
+        (bytes memory attPayload, bytes memory attSig) = signAttestation(notary, secondRA);
+        bool success = lightInbox.submitAttestation(attPayload, attSig, secondRA._agentRoot, snapGas);
+        assertTrue(success);
+    }
+
+    function test_acceptAttestation_success_diffNotary_diffAtt_sameNonce() public {
+        Random memory random = Random("salt");
+        RawAttestation memory firstRA = random.nextAttestation({nonce: 2});
+        test_submitAttestation({ra: firstRA, rootSubmittedAt: 1000});
+        skip(100);
+        RawSnapshot memory rawSnap = random.nextSnapshot();
+        uint256[] memory snapGas = rawSnap.snapGas();
+        RawAttestation memory secondRA = random.nextAttestation({rawSnap: rawSnap, nonce: 2});
+        address notary = domains[DOMAIN_LOCAL].agents[1];
+        (bytes memory attPayload, bytes memory attSig) = signAttestation(notary, secondRA);
+        bool success = lightInbox.submitAttestation(attPayload, attSig, secondRA._agentRoot, snapGas);
+        assertTrue(success);
+    }
+
     function test_passAgentRoot_optimisticPeriodNotOver(
         RawAttestation memory ra,
         uint32 rootSubmittedAt,