diff --git a/packages/contracts-core/contracts/Destination.sol b/packages/contracts-core/contracts/Destination.sol index b71648f815..0b879cefb8 100644 --- a/packages/contracts-core/contracts/Destination.sol +++ b/packages/contracts-core/contracts/Destination.sol @@ -98,10 +98,6 @@ contract Destination is ExecutionHub, DestinationEvents, InterfaceDestination { 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 @@ contract Destination is ExecutionHub, DestinationEvents, InterfaceDestination { _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,36 +117,36 @@ contract Destination is ExecutionHub, DestinationEvents, InterfaceDestination { // ═══════════════════════════════════════════ 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 ═══════════════════════════════════════════════════════ diff --git a/packages/contracts-core/contracts/interfaces/InterfaceDestination.sol b/packages/contracts-core/contracts/interfaces/InterfaceDestination.sol index 1b25bb6c0c..5476821de6 100644 --- a/packages/contracts-core/contracts/interfaces/InterfaceDestination.sol +++ b/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 diff --git a/packages/contracts-core/test/mocks/DestinationMock.t.sol b/packages/contracts-core/test/mocks/DestinationMock.t.sol index 5e8313ceeb..7495aef4f3 100644 --- a/packages/contracts-core/test/mocks/DestinationMock.t.sol +++ b/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, diff --git a/packages/contracts-core/test/suite/Destination.t.sol b/packages/contracts-core/test/suite/Destination.t.sol index 44844e72d4..ea0c69985e 100644 --- a/packages/contracts-core/test/suite/Destination.t.sol +++ b/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); diff --git a/packages/contracts-core/test/suite/DestinationSynapse.t.sol b/packages/contracts-core/test/suite/DestinationSynapse.t.sol index 2550fb19d1..26cc23c0e8 100644 --- a/packages/contracts-core/test/suite/DestinationSynapse.t.sol +++ b/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); }