Fix: chi-01 (agent root passing)

packages/contracts-core/contracts/Destination.sol

@@ -98,10 +98,6 @@
         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,
-        // as the following agent check will fail.
-        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
@@ -112,6 +108,7 @@
         _saveAttestation(att, notaryIndex, sigIndex);
         _storedAttestations.push(StoredAttData({agentRoot: agentRoot, dataHash: att.dataHash()}));
         // Save Agent Root if required, and update the Destination's Status
+        bool rootPending = passAgentRoot();
         destStatus = _saveAgentRoot(rootPending, agentRoot, notaryIndex);
         _saveGasData(snapGas, notaryIndex);
         return true;
@@ -120,37 +117,37 @@
     // ═══════════════════════════════════════════ AGENT ROOT QUARANTINE ═══════════════════════════════════════════════
 
     /// @inheritdoc InterfaceDestination
-    function passAgentRoot() public returns (bool rootPassed, bool rootPending) {
+    function passAgentRoot() public returns (bool rootPending) {
         // Agent root is not passed on Synapse Chain, as it could be accessed via BondingManager
-        if (localDomain == synapseDomain) return (false, false);
+        if (localDomain == synapseDomain) return false;
         bytes32 oldRoot = IAgentManager(agentManager).agentRoot();
         bytes32 newRoot = _nextAgentRoot;
         // Check if agent root differs from the current one in LightManager
-        if (oldRoot == newRoot) return (false, false);
+        if (oldRoot == newRoot) return false;
         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 (_notaryDisputeExists(status.notaryIndex)) {
             // Remove the pending agent merkle root, as its signer is in dispute
             _nextAgentRoot = oldRoot;
-            return (false, false);
+            return 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);
+            return 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
-            return (false, true);
+            return true;
         }
         // `newRoot` signer was not disputed, and the root optimistic period is over.
         // Finally, pass the Agent Merkle Root to LightManager
         InterfaceLightManager(address(agentManager)).setAgentRoot(newRoot);
-        return (true, false);
+        return false;
     }
 
     // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════
 

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

@@ -7,11 +7,9 @@ interface InterfaceDestination {
     /**
      * @notice Attempts to pass a quarantined Agent Merkle Root to a local Light Manager.
      * @dev Will do nothing, if root optimistic period is not over.
-     * Note: both returned values can not be true.
-     * @return rootPassed   Whether the agent merkle root was passed to LightManager
      * @return rootPending  Whether there is a pending agent merkle root left
      */
-    function passAgentRoot() external returns (bool rootPassed, bool rootPending);
+    function passAgentRoot() external returns (bool rootPending);
 
     /**
      * @notice Accepts an attestation, which local `AgentManager` verified to have been signed

packages/contracts-core/test/mocks/DestinationMock.t.sol

@@ -10,7 +10,7 @@ contract DestinationMock is ExecutionHubMock, AgentSecuredMock, InterfaceDestina
     /// @notice Prevents this contract from being included in the coverage report
     function testDestinationMock() external {}
 
-    function passAgentRoot() external returns (bool rootPassed, bool rootPending) {}
+    function passAgentRoot() external returns (bool rootPending) {}
 
     function acceptAttestation(
         uint32 notaryIndex,

packages/contracts-core/test/suite/Destination.t.sol

@@ -41,8 +41,7 @@ contract DestinationTest is ExecutionHubTest {
         // Check version
         assertEq(dst.version(), LATEST_VERSION, "!version");
         // Check pending Agent Merkle Root
-        (bool rootPassed, bool rootPending) = dst.passAgentRoot();
-        assertFalse(rootPassed);
+        (bool rootPending) = dst.passAgentRoot();
         assertFalse(rootPending);
     }
 
@@ -262,22 +261,34 @@ contract DestinationTest is ExecutionHubTest {
         lightInbox.submitAttestation(attPayload, attSig, ra._agentRoot, snapGas);
     }
 
-    function test_acceptAttestation_notAccepted_agentRootUpdated(
+    function test_acceptAttestation_passesAgentRoot(
         RawAttestation memory firstRA,
+        RawAttestation memory secondRA,
         uint32 firstRootSubmittedAt
     ) public {
         bytes32 agentRootLM = lightManager.agentRoot();
         vm.assume(firstRA._agentRoot != agentRootLM);
+        vm.assume(firstRA.snapRoot != secondRA.snapRoot);
+        vm.assume(secondRA.nonce > firstRA.nonce);
         test_submitAttestation(firstRA, firstRootSubmittedAt);
         skip(AGENT_ROOT_OPTIMISTIC_PERIOD);
-        // Mock a call from lightInbox, could as well use the empty values as they won't be checked for validity
-        vm.prank(address(lightInbox));
-        assertFalse(InterfaceDestination(localDestination()).acceptAttestation(0, 0, "", 0, new ChainGas[](0)));
+        // Form a second attestation: Notary 1
+        RawSnapshot memory rs = Random(secondRA.snapRoot).nextSnapshot();
+        secondRA._snapGasHash = rs.snapGasHash();
+        secondRA.setDataHash();
+        uint256[] memory snapGas = rs.snapGas();
+        address notaryS = domains[DOMAIN_LOCAL].agents[1];
+        (bytes memory attPayload, bytes memory attSig) = signAttestation(notaryS, secondRA);
+        uint256 newRootTimestamp = block.timestamp;
+        assertTrue(lightInbox.submitAttestation(attPayload, attSig, secondRA._agentRoot, snapGas));
         (uint40 snapRootTime, uint40 agentRootTime, uint32 index) =
             InterfaceDestination(localDestination()).destStatus();
-        assertEq(snapRootTime, firstRootSubmittedAt);
-        assertEq(agentRootTime, firstRootSubmittedAt);
-        assertEq(index, agentIndex[domains[DOMAIN_LOCAL].agent]);
+        // Dest status should point to the new root
+        assertEq(snapRootTime, newRootTimestamp);
+        assertEq(agentRootTime, newRootTimestamp);
+        assertEq(index, agentIndex[notaryS]);
+        // New agent root should be pending in Destination
+        assertEq(InterfaceDestination(localDestination()).nextAgentRoot(), secondRA._agentRoot);
         // Should update the Agent Merkle Root
         assertEq(lightManager.agentRoot(), firstRA._agentRoot);
     }
@@ -321,8 +332,7 @@ contract DestinationTest is ExecutionHubTest {
         test_submitAttestation_updatesAgentRoot(ra, rootSubmittedAt);
         timePassed = timePassed % AGENT_ROOT_OPTIMISTIC_PERIOD;
         skip(timePassed);
-        (bool rootPassed, bool rootPending) = InterfaceDestination(localDestination()).passAgentRoot();
-        assertFalse(rootPassed);
+        bool rootPending = InterfaceDestination(localDestination()).passAgentRoot();
         assertTrue(rootPending);
         assertEq(lightManager.agentRoot(), agentRootLM);
     }
@@ -331,8 +341,7 @@ contract DestinationTest is ExecutionHubTest {
         // Submit attestation that updates `nextAgentRoot`
         test_submitAttestation_updatesAgentRoot(ra, rootSubmittedAt);
         skip(AGENT_ROOT_OPTIMISTIC_PERIOD);
-        (bool rootPassed, bool rootPending) = InterfaceDestination(localDestination()).passAgentRoot();
-        assertTrue(rootPassed);
+        bool rootPending = InterfaceDestination(localDestination()).passAgentRoot();
         assertFalse(rootPending);
         assertEq(lightManager.agentRoot(), ra._agentRoot);
     }
@@ -366,9 +375,8 @@ contract DestinationTest is ExecutionHubTest {
         bytes32 oldRoot = lightManager.agentRoot();
         prepareAgentRootDisputeTest();
         skip(AGENT_ROOT_OPTIMISTIC_PERIOD - 1);
-        (bool rootPassed, bool rootPending) = InterfaceDestination(localDestination()).passAgentRoot();
+        bool rootPending = InterfaceDestination(localDestination()).passAgentRoot();
         // Should not pass the root
-        assertFalse(rootPassed);
         assertEq(lightManager.agentRoot(), oldRoot);
         // Should clear pending
         assertFalse(rootPending);
@@ -379,9 +387,8 @@ contract DestinationTest is ExecutionHubTest {
         bytes32 oldRoot = lightManager.agentRoot();
         prepareAgentRootDisputeTest();
         skip(AGENT_ROOT_OPTIMISTIC_PERIOD);
-        (bool rootPassed, bool rootPending) = InterfaceDestination(localDestination()).passAgentRoot();
+        bool rootPending = InterfaceDestination(localDestination()).passAgentRoot();
         // Should not pass the root
-        assertFalse(rootPassed);
         assertEq(lightManager.agentRoot(), oldRoot);
         // Should clear pending
         assertFalse(rootPending);
@@ -396,9 +403,8 @@ contract DestinationTest is ExecutionHubTest {
         skip(DISPUTE_TIMEOUT_NOTARY - 1);
         // Time since attestation was submitted: AGENT_ROOT_OPTIMISTIC_PERIOD - 1
         // Time sinceNotary won the dispute: DISPUTE_TIMEOUT_NOTARY - 1
-        (bool rootPassed, bool rootPending) = InterfaceDestination(localDestination()).passAgentRoot();
+        bool rootPending = InterfaceDestination(localDestination()).passAgentRoot();
         // Should not pass the root
-        assertFalse(rootPassed);
         assertEq(lightManager.agentRoot(), oldRoot);
         // Should not clear pending
         assertTrue(rootPending);
@@ -413,9 +419,8 @@ contract DestinationTest is ExecutionHubTest {
         skip(DISPUTE_TIMEOUT_NOTARY - 1);
         // Time since attestation was submitted: AGENT_ROOT_OPTIMISTIC_PERIOD
         // Time sinceNotary won the dispute: DISPUTE_TIMEOUT_NOTARY - 1
-        (bool rootPassed, bool rootPending) = InterfaceDestination(localDestination()).passAgentRoot();
+        bool rootPending = InterfaceDestination(localDestination()).passAgentRoot();
         // Should not pass the root
-        assertFalse(rootPassed);
         assertEq(lightManager.agentRoot(), oldRoot);
         // Should not clear pending
         assertTrue(rootPending);
@@ -430,9 +435,8 @@ contract DestinationTest is ExecutionHubTest {
         skip(DISPUTE_TIMEOUT_NOTARY);
         // Time since attestation was submitted: AGENT_ROOT_OPTIMISTIC_PERIOD - 1
         // Time sinceNotary won the dispute: DISPUTE_TIMEOUT_NOTARY
-        (bool rootPassed, bool rootPending) = InterfaceDestination(localDestination()).passAgentRoot();
+        bool rootPending = InterfaceDestination(localDestination()).passAgentRoot();
         // Should not pass the root
-        assertFalse(rootPassed);
         assertEq(lightManager.agentRoot(), oldRoot);
         // Should not clear pending
         assertTrue(rootPending);
@@ -446,9 +450,8 @@ contract DestinationTest is ExecutionHubTest {
         skip(DISPUTE_TIMEOUT_NOTARY);
         // Time since attestation was submitted: AGENT_ROOT_OPTIMISTIC_PERIOD
         // Time sinceNotary won the dispute: DISPUTE_TIMEOUT_NOTARY
-        (bool rootPassed, bool rootPending) = InterfaceDestination(localDestination()).passAgentRoot();
+        bool rootPending = InterfaceDestination(localDestination()).passAgentRoot();
         // Should pass the root
-        assertTrue(rootPassed);
         assertEq(lightManager.agentRoot(), newRoot);
         // Should clear pending
         assertFalse(rootPending);

packages/contracts-core/test/suite/DestinationSynapse.t.sol

@@ -33,8 +33,7 @@ contract DestinationSynapseTest is ExecutionHubTest {
         // Check version
         assertEq(dst.version(), LATEST_VERSION, "!version");
         // Check pending Agent Merkle Root
-        (bool rootPassed, bool rootPending) = dst.passAgentRoot();
-        assertFalse(rootPassed);
+        bool rootPending = dst.passAgentRoot();
         assertFalse(rootPending);
     }