From fdcf4955c003c3a78aa01bc757c98f23f0c9a993 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 23 Oct 2023 14:27:47 +0100 Subject: [PATCH 01/13] Add `DisputeStatus` struct and a getter for it --- .../contracts/interfaces/IAgentSecured.sol | 11 ++++++++++- packages/contracts-core/contracts/libs/Structures.sol | 10 ++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/contracts-core/contracts/interfaces/IAgentSecured.sol b/packages/contracts-core/contracts/interfaces/IAgentSecured.sol index 467af05106..49f7137c3c 100644 --- a/packages/contracts-core/contracts/interfaces/IAgentSecured.sol +++ b/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); } diff --git a/packages/contracts-core/contracts/libs/Structures.sol b/packages/contracts-core/contracts/libs/Structures.sol index 5a0320ffbe..6fd778a495 100644 --- a/packages/contracts-core/contracts/libs/Structures.sol +++ b/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. From 6f75e0ddbd959cb6fe5c021e587c03897bff7346 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 23 Oct 2023 14:28:02 +0100 Subject: [PATCH 02/13] Add empty implementation for the new getter --- packages/contracts-core/contracts/base/AgentSecured.sol | 5 ++++- .../contracts-core/test/mocks/base/AgentSecuredMock.t.sol | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/contracts-core/contracts/base/AgentSecured.sol b/packages/contracts-core/contracts/base/AgentSecured.sol index a77f5a00be..f38b5d3a20 100644 --- a/packages/contracts-core/contracts/base/AgentSecured.sol +++ b/packages/contracts-core/contracts/base/AgentSecured.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.17; // ══════════════════════════════ LIBRARY IMPORTS ══════════════════════════════ 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"; @@ -78,6 +78,9 @@ abstract contract AgentSecured is MessagingBase, IAgentSecured { return _getAgent(index); } + /// @inheritdoc IAgentSecured + function latestDisputeStatus(uint32 agentIndex) external view returns (DisputeStatus memory) {} + // ══════════════════════════════════════════════ INTERNAL VIEWS ═══════════════════════════════════════════════════ /// @dev Returns status of the given agent: (flag, domain, index). diff --git a/packages/contracts-core/test/mocks/base/AgentSecuredMock.t.sol b/packages/contracts-core/test/mocks/base/AgentSecuredMock.t.sol index 13f08d868b..64144d3374 100644 --- a/packages/contracts-core/test/mocks/base/AgentSecuredMock.t.sol +++ b/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) {} } From 87c00735fa572a2d18506363ee1fb2679b7d33e6 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 23 Oct 2023 18:14:28 +0100 Subject: [PATCH 03/13] First draft for dispute tests in `AgentSecured` --- .../test/suite/base/AgentSecured.t.sol | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/contracts-core/test/suite/base/AgentSecured.t.sol b/packages/contracts-core/test/suite/base/AgentSecured.t.sol index 4785d7a2a1..4c30be0ed2 100644 --- a/packages/contracts-core/test/suite/base/AgentSecured.t.sol +++ b/packages/contracts-core/test/suite/base/AgentSecured.t.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.17; +import {CallerNotAgentManager} from "../../../contracts/libs/Errors.sol"; +import {IAgentSecured} from "../../../contracts/interfaces/IAgentSecured.sol"; import {Origin} from "../../utils/SynapseTest.t.sol"; import {MessagingBaseTest} from "./MessagingBase.t.sol"; @@ -14,4 +16,46 @@ abstract contract AgentSecuredTest is MessagingBaseTest { // Etch the implementation code to effectively update the values of immutables vm.etch(localOrigin(), address(impl).code); } + + // ════════════════════════════════════════════ TESTS: OPEN DISPUTE ════════════════════════════════════════════════ + + function openTestDispute() public { + vm.prank(localAgentManager()); + localAgentSecured().openDispute(1, 2); + } + + function test_openDispute() public { + // TODO: add expectations + openTestDispute(); + } + + function test_openDispute_revert_onlyAgentManager(address caller) public { + vm.assume(caller != localAgentManager()); + vm.expectRevert(CallerNotAgentManager.selector); + vm.prank(caller); + localAgentSecured().openDispute(1, 2); + } + + // ══════════════════════════════════════════ TESTS: RESOLVE DISPUTE ═══════════════════════════════════════════════ + + function test_resolveDispute() public { + openTestDispute(); + // TODO: add expectations + vm.prank(localAgentManager()); + localAgentSecured().resolveDispute(1, 2); + } + + function test_resolveDispute_revert_onlyAgentManager(address caller) public { + vm.assume(caller != localAgentManager()); + vm.expectRevert(CallerNotAgentManager.selector); + vm.prank(caller); + localAgentSecured().resolveDispute(1, 2); + } + + // ══════════════════════════════════════════════════ HELPERS ══════════════════════════════════════════════════════ + + /// @dev Returns tested contract as `IAgentSecured` + function localAgentSecured() internal view returns (IAgentSecured) { + return IAgentSecured(localContract()); + } } From 00617b4e3798788edea83928bb85c86e30504c37 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 23 Oct 2023 18:49:44 +0100 Subject: [PATCH 04/13] Add tests for the expected AgentSecured dispute workflows --- .../test/suite/base/AgentSecured.t.sol | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/packages/contracts-core/test/suite/base/AgentSecured.t.sol b/packages/contracts-core/test/suite/base/AgentSecured.t.sol index 4c30be0ed2..9fca903708 100644 --- a/packages/contracts-core/test/suite/base/AgentSecured.t.sol +++ b/packages/contracts-core/test/suite/base/AgentSecured.t.sol @@ -2,12 +2,12 @@ pragma solidity 0.8.17; import {CallerNotAgentManager} from "../../../contracts/libs/Errors.sol"; +import {DisputeFlag, DisputeStatus} from "../../../contracts/libs/Structures.sol"; import {IAgentSecured} from "../../../contracts/interfaces/IAgentSecured.sol"; import {Origin} from "../../utils/SynapseTest.t.sol"; import {MessagingBaseTest} from "./MessagingBase.t.sol"; abstract contract AgentSecuredTest is MessagingBaseTest { - // TODO: unit tests for AgentSecured // ═══════════════════════════════════════════ UPDATE IMPLEMENTATION ═══════════════════════════════════════════════ function updateOrigin(uint32 domain, address agentManager, address inbox_, address gasOracle_) public { @@ -19,14 +19,30 @@ abstract contract AgentSecuredTest is MessagingBaseTest { // ════════════════════════════════════════════ TESTS: OPEN DISPUTE ════════════════════════════════════════════════ - function openTestDispute() public { + function openTestDispute(uint32 guardIndex, uint32 notaryIndex) public { + vm.warp(1234); vm.prank(localAgentManager()); - localAgentSecured().openDispute(1, 2); + localAgentSecured().openDispute(guardIndex, notaryIndex); + } + + function resolveTestDispute(uint32 slashedIndex, uint32 rivalIndex) public { + vm.warp(5678); + vm.prank(localAgentManager()); + localAgentSecured().resolveDispute(slashedIndex, rivalIndex); + } + + function checkLatestDisputeStatus(uint32 index, DisputeStatus memory expected) public { + DisputeStatus memory actual = localAgentSecured().latestDisputeStatus(index); + assertEq(uint8(actual.flag), uint8(expected.flag), "!flag"); + assertEq(actual.openedAt, expected.openedAt, "!openedAt"); + assertEq(actual.resolvedAt, expected.resolvedAt, "!resolvedAt"); } function test_openDispute() public { - // TODO: add expectations - openTestDispute(); + DisputeStatus memory expected = DisputeStatus({flag: DisputeFlag.Pending, openedAt: 1234, resolvedAt: 0}); + openTestDispute({guardIndex: 1, notaryIndex: 2}); + checkLatestDisputeStatus(1, expected); + checkLatestDisputeStatus(2, expected); } function test_openDispute_revert_onlyAgentManager(address caller) public { @@ -39,13 +55,14 @@ abstract contract AgentSecuredTest is MessagingBaseTest { // ══════════════════════════════════════════ TESTS: RESOLVE DISPUTE ═══════════════════════════════════════════════ function test_resolveDispute() public { - openTestDispute(); - // TODO: add expectations - vm.prank(localAgentManager()); - localAgentSecured().resolveDispute(1, 2); + openTestDispute({guardIndex: 1, notaryIndex: 2}); + resolveTestDispute({slashedIndex: 1, rivalIndex: 2}); + checkLatestDisputeStatus(1, DisputeStatus({flag: DisputeFlag.Slashed, openedAt: 1234, resolvedAt: 5678})); + checkLatestDisputeStatus(2, DisputeStatus({flag: DisputeFlag.None, openedAt: 1234, resolvedAt: 5678})); } function test_resolveDispute_revert_onlyAgentManager(address caller) public { + openTestDispute({guardIndex: 1, notaryIndex: 2}); vm.assume(caller != localAgentManager()); vm.expectRevert(CallerNotAgentManager.selector); vm.prank(caller); From c17b56429adbb4ccde2ef222368331cbed59af4a Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 23 Oct 2023 19:07:49 +0100 Subject: [PATCH 05/13] Track agent dispute statuses in agent-secured contracts --- packages/contracts-core/contracts/Summit.sol | 3 +- .../contracts/base/AgentSecured.sol | 31 +++++++++++++------ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/packages/contracts-core/contracts/Summit.sol b/packages/contracts-core/contracts/Summit.sol index 57f6a2de8a..efcc5a1e03 100644 --- a/packages/contracts-core/contracts/Summit.sol +++ b/packages/contracts-core/contracts/Summit.sol @@ -211,7 +211,8 @@ 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. diff --git a/packages/contracts-core/contracts/base/AgentSecured.sol b/packages/contracts-core/contracts/base/AgentSecured.sol index f38b5d3a20..db0ad36af3 100644 --- a/packages/contracts-core/contracts/base/AgentSecured.sol +++ b/packages/contracts-core/contracts/base/AgentSecured.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.17; // ══════════════════════════════ LIBRARY IMPORTS ══════════════════════════════ +import {ChainContext} from "../libs/ChainContext.sol"; import {CallerNotAgentManager, CallerNotInbox} from "../libs/Errors.sol"; import {AgentStatus, DisputeFlag, DisputeStatus} from "../libs/Structures.sol"; // ═════════════════════════════ INTERNAL IMPORTS ══════════════════════════════ @@ -29,8 +30,8 @@ abstract contract AgentSecured is MessagingBase, IAgentSecured { // ══════════════════════════════════════════════════ 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 +57,23 @@ abstract contract AgentSecured is MessagingBase, IAgentSecured { /// @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 ═══════════════════════════════════════════════════════ @@ -79,7 +89,9 @@ abstract contract AgentSecured is MessagingBase, IAgentSecured { } /// @inheritdoc IAgentSecured - function latestDisputeStatus(uint32 agentIndex) external view returns (DisputeStatus memory) {} + function latestDisputeStatus(uint32 agentIndex) external view returns (DisputeStatus memory) { + return _disputes[agentIndex]; + } // ══════════════════════════════════════════════ INTERNAL VIEWS ═══════════════════════════════════════════════════ @@ -95,6 +107,7 @@ abstract contract AgentSecured is MessagingBase, IAgentSecured { /// @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; + // TODO: add timeout for Notaries that just won the dispute. + return _disputes[agentIndex].flag != DisputeFlag.None; } } From 4fd03e3b3f52b458f5114269411613407be9e472 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Tue, 24 Oct 2023 17:57:09 +0100 Subject: [PATCH 06/13] Use current block.timestamp in test disputes --- packages/contracts-core/test/suite/base/AgentSecured.t.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/contracts-core/test/suite/base/AgentSecured.t.sol b/packages/contracts-core/test/suite/base/AgentSecured.t.sol index 9fca903708..9b999defd4 100644 --- a/packages/contracts-core/test/suite/base/AgentSecured.t.sol +++ b/packages/contracts-core/test/suite/base/AgentSecured.t.sol @@ -20,13 +20,11 @@ abstract contract AgentSecuredTest is MessagingBaseTest { // ════════════════════════════════════════════ TESTS: OPEN DISPUTE ════════════════════════════════════════════════ function openTestDispute(uint32 guardIndex, uint32 notaryIndex) public { - vm.warp(1234); vm.prank(localAgentManager()); localAgentSecured().openDispute(guardIndex, notaryIndex); } function resolveTestDispute(uint32 slashedIndex, uint32 rivalIndex) public { - vm.warp(5678); vm.prank(localAgentManager()); localAgentSecured().resolveDispute(slashedIndex, rivalIndex); } @@ -40,6 +38,7 @@ abstract contract AgentSecuredTest is MessagingBaseTest { function test_openDispute() public { DisputeStatus memory expected = DisputeStatus({flag: DisputeFlag.Pending, openedAt: 1234, resolvedAt: 0}); + vm.warp(1234); openTestDispute({guardIndex: 1, notaryIndex: 2}); checkLatestDisputeStatus(1, expected); checkLatestDisputeStatus(2, expected); @@ -55,7 +54,9 @@ abstract contract AgentSecuredTest is MessagingBaseTest { // ══════════════════════════════════════════ TESTS: RESOLVE DISPUTE ═══════════════════════════════════════════════ function test_resolveDispute() public { + vm.warp(1234); openTestDispute({guardIndex: 1, notaryIndex: 2}); + vm.warp(5678); resolveTestDispute({slashedIndex: 1, rivalIndex: 2}); checkLatestDisputeStatus(1, DisputeStatus({flag: DisputeFlag.Slashed, openedAt: 1234, resolvedAt: 5678})); checkLatestDisputeStatus(2, DisputeStatus({flag: DisputeFlag.None, openedAt: 1234, resolvedAt: 5678})); From 7b54ce227b7c13c37336ace2008385448c286a58 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 26 Oct 2023 18:43:35 +0100 Subject: [PATCH 07/13] Define "expected behavior" tests for `Destination` --- .../contracts/libs/Constants.sol | 2 + .../contracts-core/contracts/libs/Errors.sol | 1 + .../test/suite/Destination.t.sol | 233 ++++++++++++++++-- .../test/suite/DestinationSynapse.t.sol | 84 +++++-- 4 files changed, 290 insertions(+), 30 deletions(-) diff --git a/packages/contracts-core/contracts/libs/Constants.sol b/packages/contracts-core/contracts/libs/Constants.sol index 8eba0ea593..a90e002902 100644 --- a/packages/contracts-core/contracts/libs/Constants.sol +++ b/packages/contracts-core/contracts/libs/Constants.sol @@ -44,6 +44,8 @@ bytes32 constant STATE_INVALID_SALT = keccak256("STATE_INVALID_SALT"); /// @dev Optimistic period for new agent roots in LightManager uint32 constant AGENT_ROOT_OPTIMISTIC_PERIOD = 1 days; 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) diff --git a/packages/contracts-core/contracts/libs/Errors.sol b/packages/contracts-core/contracts/libs/Errors.sol index 02f3ab8ef7..55ac9fea30 100644 --- a/packages/contracts-core/contracts/libs/Errors.sol +++ b/packages/contracts-core/contracts/libs/Errors.sol @@ -77,6 +77,7 @@ error AgentUnknown(); error DisputeAlreadyResolved(); error DisputeNotOpened(); error DisputeNotStuck(); +error DisputeTimeoutNotOver(); error GuardInDispute(); error NotaryInDispute(); diff --git a/packages/contracts-core/test/suite/Destination.t.sol b/packages/contracts-core/test/suite/Destination.t.sol index cebaabf2f0..44844e72d4 100644 --- a/packages/contracts-core/test/suite/Destination.t.sol +++ b/packages/contracts-core/test/suite/Destination.t.sol @@ -1,7 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.17; -import {CallerNotInbox, NotaryInDispute, OutdatedNonce} from "../../contracts/libs/Errors.sol"; +import {DISPUTE_TIMEOUT_NOTARY} from "../../contracts/libs/Constants.sol"; +import {CallerNotInbox, DisputeTimeoutNotOver, 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"; @@ -194,6 +195,46 @@ contract DestinationTest is ExecutionHubTest { InterfaceDestination(localDestination()).acceptAttestation(agentIndex[notary], 0, "", 0, new ChainGas[](0)); } + function test_acceptAttestation_revert_notaryWonDisputeTimeout() public { + address notary = domains[DOMAIN_LOCAL].agent; + address guard = domains[0].agent; + + Random memory random = Random("salt"); + RawSnapshot memory rawSnap = random.nextSnapshot(); + RawAttestation memory ra = random.nextAttestation({rawSnap: rawSnap, nonce: 1}); + uint256[] memory snapGas = rawSnap.snapGas(); + (bytes memory attPayload, bytes memory attSig) = signAttestation(notary, ra); + + openTestDispute({guardIndex: agentIndex[guard], notaryIndex: agentIndex[notary]}); + skip(7 days); + resolveTestDispute({slashedIndex: agentIndex[guard], rivalIndex: agentIndex[notary]}); + skip(DISPUTE_TIMEOUT_NOTARY - 1); + vm.expectRevert(DisputeTimeoutNotOver.selector); + lightInbox.submitAttestation(attPayload, attSig, ra._agentRoot, snapGas); + } + + function test_acceptAttestation_afterNotaryDisputeTimeout() public { + address notary = domains[DOMAIN_LOCAL].agent; + address guard = domains[0].agent; + + Random memory random = Random("salt"); + RawSnapshot memory rawSnap = random.nextSnapshot(); + bytes32 snapRoot = rawSnap.castToSnapshot().calculateRoot(); + // Sanity check + assert(testedEH().getAttestationNonce(snapRoot) == 0); + RawAttestation memory ra = random.nextAttestation({rawSnap: rawSnap, nonce: 1}); + uint256[] memory snapGas = rawSnap.snapGas(); + (bytes memory attPayload, bytes memory attSig) = signAttestation(notary, ra); + + openTestDispute({guardIndex: agentIndex[guard], notaryIndex: agentIndex[notary]}); + skip(7 days); + resolveTestDispute({slashedIndex: agentIndex[guard], rivalIndex: agentIndex[notary]}); + skip(DISPUTE_TIMEOUT_NOTARY); + lightInbox.submitAttestation(attPayload, attSig, ra._agentRoot, snapGas); + // Snapshot root should be known to ExecutionHub (Destination) + assertGt(testedEH().getAttestationNonce(snapRoot), 0); + } + function test_acceptAttestation_revert_lowerNonce() public { Random memory random = Random("salt"); RawAttestation memory firstRA = random.nextAttestation({nonce: 2}); @@ -296,6 +337,124 @@ contract DestinationTest is ExecutionHubTest { assertEq(lightManager.agentRoot(), ra._agentRoot); } + /// @dev Submits a mock attestation and opens a dispute for the Notary that signed it. + function prepareAgentRootDisputeTest() internal returns (bytes32 newAgentRoot) { + address notary = domains[DOMAIN_LOCAL].agent; + address guard = domains[0].agent; + + Random memory random = Random("salt"); + RawSnapshot memory rawSnap = random.nextSnapshot(); + RawAttestation memory ra = random.nextAttestation({rawSnap: rawSnap, nonce: 1}); + newAgentRoot = ra._agentRoot; + uint256[] memory snapGas = rawSnap.snapGas(); + + (bytes memory attPayload, bytes memory attSig) = signAttestation(notary, ra); + lightInbox.submitAttestation(attPayload, attSig, ra._agentRoot, snapGas); + // Sanity check + assert(InterfaceDestination(localDestination()).nextAgentRoot() == newAgentRoot); + openTestDispute({guardIndex: agentIndex[guard], notaryIndex: agentIndex[notary]}); + } + + /// @dev Resolves test dispute above in favor of the Notary. + function prepareNotaryWonDisputeTest() internal { + address notary = domains[DOMAIN_LOCAL].agent; + address guard = domains[0].agent; + resolveTestDispute({slashedIndex: agentIndex[guard], rivalIndex: agentIndex[notary]}); + } + + function test_passAgentRoot_notaryInDispute_optimisticPeriodNotOver() public { + bytes32 oldRoot = lightManager.agentRoot(); + prepareAgentRootDisputeTest(); + skip(AGENT_ROOT_OPTIMISTIC_PERIOD - 1); + (bool rootPassed, bool rootPending) = InterfaceDestination(localDestination()).passAgentRoot(); + // Should not pass the root + assertFalse(rootPassed); + assertEq(lightManager.agentRoot(), oldRoot); + // Should clear pending + assertFalse(rootPending); + assertEq(InterfaceDestination(localDestination()).nextAgentRoot(), oldRoot); + } + + function test_passAgentRoot_notaryInDispute_optimisticPeriodOver() public { + bytes32 oldRoot = lightManager.agentRoot(); + prepareAgentRootDisputeTest(); + skip(AGENT_ROOT_OPTIMISTIC_PERIOD); + (bool rootPassed, bool rootPending) = InterfaceDestination(localDestination()).passAgentRoot(); + // Should not pass the root + assertFalse(rootPassed); + assertEq(lightManager.agentRoot(), oldRoot); + // Should clear pending + assertFalse(rootPending); + assertEq(InterfaceDestination(localDestination()).nextAgentRoot(), oldRoot); + } + + function test_passAgentRoot_notaryWonDisputeTimeout_optimisticPeriodNotOver() public { + bytes32 oldRoot = lightManager.agentRoot(); + bytes32 newRoot = prepareAgentRootDisputeTest(); + skip(AGENT_ROOT_OPTIMISTIC_PERIOD - DISPUTE_TIMEOUT_NOTARY); + prepareNotaryWonDisputeTest(); + 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(); + // Should not pass the root + assertFalse(rootPassed); + assertEq(lightManager.agentRoot(), oldRoot); + // Should not clear pending + assertTrue(rootPending); + assertEq(InterfaceDestination(localDestination()).nextAgentRoot(), newRoot); + } + + function test_passAgentRoot_notaryWonDisputeTimeout_optimisticPeriodOver() public { + bytes32 oldRoot = lightManager.agentRoot(); + bytes32 newRoot = prepareAgentRootDisputeTest(); + skip(AGENT_ROOT_OPTIMISTIC_PERIOD - (DISPUTE_TIMEOUT_NOTARY - 1)); + prepareNotaryWonDisputeTest(); + 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(); + // Should not pass the root + assertFalse(rootPassed); + assertEq(lightManager.agentRoot(), oldRoot); + // Should not clear pending + assertTrue(rootPending); + assertEq(InterfaceDestination(localDestination()).nextAgentRoot(), newRoot); + } + + function test_passAgentRoot_afterNotaryDisputeTimeout_optimisticPeriodNotOver() public { + bytes32 oldRoot = lightManager.agentRoot(); + bytes32 newRoot = prepareAgentRootDisputeTest(); + skip(AGENT_ROOT_OPTIMISTIC_PERIOD - 1 - DISPUTE_TIMEOUT_NOTARY); + prepareNotaryWonDisputeTest(); + 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(); + // Should not pass the root + assertFalse(rootPassed); + assertEq(lightManager.agentRoot(), oldRoot); + // Should not clear pending + assertTrue(rootPending); + assertEq(InterfaceDestination(localDestination()).nextAgentRoot(), newRoot); + } + + function test_passAgentRoot_afterNotaryDisputeTimeout_optimisticPeriodOver() public { + bytes32 newRoot = prepareAgentRootDisputeTest(); + skip(AGENT_ROOT_OPTIMISTIC_PERIOD - DISPUTE_TIMEOUT_NOTARY); + prepareNotaryWonDisputeTest(); + 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(); + // Should pass the root + assertTrue(rootPassed); + assertEq(lightManager.agentRoot(), newRoot); + // Should clear pending + assertFalse(rootPending); + assertEq(InterfaceDestination(localDestination()).nextAgentRoot(), newRoot); + } + // ═════════════════════════════════════════════════ GAS DATA ══════════════════════════════════════════════════════ function test_getGasData(Random memory random) public { @@ -387,20 +546,50 @@ contract DestinationTest is ExecutionHubTest { assertEq(dataMaturity, 0); } - function test_getGasData_notaryInDispute(Random memory random) public { - RawSnapshot memory firstSnap; - firstSnap.states = new RawState[](2); - firstSnap.states[0] = random.nextState({origin: DOMAIN_REMOTE, nonce: random.nextUint32()}); - firstSnap.states[1] = random.nextState({origin: DOMAIN_SYNAPSE, nonce: random.nextUint32()}); - RawAttestation memory ra = random.nextAttestation(firstSnap, random.nextUint32()); - address firstNotary = domains[DOMAIN_LOCAL].agents[0]; - (bytes memory attPayload, bytes memory attSig) = signAttestation(firstNotary, ra); - uint256[] memory firstSnapGas = firstSnap.snapGas(); - // Submit first attestation - lightInbox.submitAttestation(attPayload, attSig, ra._agentRoot, firstSnapGas); - skip(random.nextUint32()); - // Open dispute - openDispute({guard: domains[0].agent, notary: firstNotary}); + function prepareGasDataDisputeTest() internal returns (GasData remoteGasData, GasData synapseGasData) { + address notary = domains[DOMAIN_LOCAL].agent; + address guard = domains[0].agent; + + Random memory random = Random("salt"); + RawSnapshot memory rawSnap = RawSnapshot(new RawState[](2)); + rawSnap.states[0] = random.nextState({origin: DOMAIN_REMOTE, nonce: 1}); + rawSnap.states[1] = random.nextState({origin: DOMAIN_SYNAPSE, nonce: 2}); + remoteGasData = rawSnap.states[0].castToState().gasData(); + synapseGasData = rawSnap.states[1].castToState().gasData(); + + RawAttestation memory ra = random.nextAttestation({rawSnap: rawSnap, nonce: 1}); + uint256[] memory snapGas = rawSnap.snapGas(); + (bytes memory attPayload, bytes memory attSig) = signAttestation(notary, ra); + lightInbox.submitAttestation(attPayload, attSig, ra._agentRoot, snapGas); + // Sanity checks + { + (GasData gasData,) = InterfaceDestination(localDestination()).getGasData(DOMAIN_REMOTE); + assert(GasData.unwrap(gasData) == GasData.unwrap(remoteGasData)); + } + { + (GasData gasData,) = InterfaceDestination(localDestination()).getGasData(DOMAIN_SYNAPSE); + assert(GasData.unwrap(gasData) == GasData.unwrap(synapseGasData)); + } + openTestDispute({guardIndex: agentIndex[guard], notaryIndex: agentIndex[notary]}); + } + + function test_getGasData_notaryInDispute() public { + prepareGasDataDisputeTest(); + skip(7 days); + // Check getGasData + (GasData gasData, uint256 dataMaturity) = InterfaceDestination(localDestination()).getGasData(DOMAIN_REMOTE); + assertEq(GasData.unwrap(gasData), 0); + assertEq(dataMaturity, 0); + (gasData, dataMaturity) = InterfaceDestination(localDestination()).getGasData(DOMAIN_SYNAPSE); + assertEq(GasData.unwrap(gasData), 0); + assertEq(dataMaturity, 0); + } + + function test_getGasData_notaryWonDisputeTimeout() public { + prepareGasDataDisputeTest(); + skip(7 days); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY - 1); // Check getGasData (GasData gasData, uint256 dataMaturity) = InterfaceDestination(localDestination()).getGasData(DOMAIN_REMOTE); assertEq(GasData.unwrap(gasData), 0); @@ -410,6 +599,20 @@ contract DestinationTest is ExecutionHubTest { assertEq(dataMaturity, 0); } + function test_getGasData_afterNotaryDisputeTimeout() public { + (GasData remoteGasData, GasData synapseGasData) = prepareGasDataDisputeTest(); + skip(7 days); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY); + // Check getGasData + (GasData gasData, uint256 dataMaturity) = InterfaceDestination(localDestination()).getGasData(DOMAIN_REMOTE); + assertEq(GasData.unwrap(gasData), GasData.unwrap(remoteGasData)); + assertEq(dataMaturity, 7 days + DISPUTE_TIMEOUT_NOTARY); + (gasData, dataMaturity) = InterfaceDestination(localDestination()).getGasData(DOMAIN_SYNAPSE); + assertEq(GasData.unwrap(gasData), GasData.unwrap(synapseGasData)); + assertEq(dataMaturity, 7 days + DISPUTE_TIMEOUT_NOTARY); + } + // ══════════════════════════════════════════════════ HELPERS ══════════════════════════════════════════════════════ /// @notice Prepares execution of the created messages diff --git a/packages/contracts-core/test/suite/DestinationSynapse.t.sol b/packages/contracts-core/test/suite/DestinationSynapse.t.sol index 1fea16d751..2550fb19d1 100644 --- a/packages/contracts-core/test/suite/DestinationSynapse.t.sol +++ b/packages/contracts-core/test/suite/DestinationSynapse.t.sol @@ -1,7 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.17; -import {CallerNotInbox, NotaryInDispute} from "../../contracts/libs/Errors.sol"; +import {DISPUTE_TIMEOUT_NOTARY} from "../../contracts/libs/Constants.sol"; +import {CallerNotInbox, DisputeTimeoutNotOver, NotaryInDispute} from "../../contracts/libs/Errors.sol"; import {ChainGas, GasData, InterfaceDestination} from "../../contracts/interfaces/InterfaceDestination.sol"; import {Random} from "../utils/libs/Random.t.sol"; @@ -193,20 +194,45 @@ contract DestinationSynapseTest is ExecutionHubTest { assertEq(dataMaturity, 0); } - function test_getGasData_notaryInDispute(Random memory random) public { - RawSnapshot memory firstSnap; - firstSnap.states = new RawState[](2); - firstSnap.states[0] = random.nextState({origin: DOMAIN_REMOTE, nonce: 1}); - firstSnap.states[1] = random.nextState({origin: DOMAIN_LOCAL, nonce: 1}); - address firstNotary = domains[DOMAIN_LOCAL].agents[0]; - (bytes memory firstSnapPayload, bytes memory firstNotarySignature) = signSnapshot(firstNotary, firstSnap); - (, bytes memory firstGuardSignature) = signSnapshot(domains[0].agent, firstSnap); - inbox.submitSnapshot(firstSnapPayload, firstGuardSignature); - inbox.submitSnapshot(firstSnapPayload, firstNotarySignature); - uint256 firstSkipTime = random.nextUint32(); - skip(firstSkipTime); - // Open dispute - openDispute({guard: domains[0].agent, notary: firstNotary}); + function prepareGasDataDisputeTest() internal returns (GasData remoteGasData, GasData localGasData) { + address notary = domains[DOMAIN_LOCAL].agent; + address reportGuard = domains[0].agent; + address snapshotGuard = domains[0].agents[1]; + + Random memory random = Random("salt"); + RawSnapshot memory rawSnap = RawSnapshot(new RawState[](2)); + rawSnap.states[0] = random.nextState({origin: DOMAIN_REMOTE, nonce: 1}); + rawSnap.states[1] = random.nextState({origin: DOMAIN_LOCAL, nonce: 2}); + remoteGasData = rawSnap.states[0].castToState().gasData(); + localGasData = rawSnap.states[1].castToState().gasData(); + + // Another Guard signs the snapshot + (bytes memory snapPayload, bytes memory guardSignature) = signSnapshot(snapshotGuard, rawSnap); + inbox.submitSnapshot(snapPayload, guardSignature); + // Notary signs the snapshot + (, bytes memory notarySignature) = signSnapshot(notary, rawSnap); + inbox.submitSnapshot(snapPayload, notarySignature); + // Sanity checks + { + (GasData gasData,) = InterfaceDestination(localDestination()).getGasData(DOMAIN_REMOTE); + assert(GasData.unwrap(gasData) == GasData.unwrap(remoteGasData)); + } + { + (GasData gasData,) = InterfaceDestination(localDestination()).getGasData(DOMAIN_LOCAL); + assert(GasData.unwrap(gasData) == GasData.unwrap(localGasData)); + } + openTestDispute({guardIndex: agentIndex[reportGuard], notaryIndex: agentIndex[notary]}); + } + + function prepareNotaryWonDisputeTest() internal { + address notary = domains[DOMAIN_LOCAL].agent; + address guard = domains[0].agent; + resolveTestDispute({slashedIndex: agentIndex[guard], rivalIndex: agentIndex[notary]}); + } + + function test_getGasData_notaryInDispute() public { + prepareGasDataDisputeTest(); + skip(7 days); // Check getGasData (GasData gasData, uint256 dataMaturity) = InterfaceDestination(localDestination()).getGasData(DOMAIN_REMOTE); assertEq(GasData.unwrap(gasData), 0); @@ -216,6 +242,34 @@ contract DestinationSynapseTest is ExecutionHubTest { assertEq(dataMaturity, 0); } + function test_getGasData_notaryWonDisputeTimeout() public { + prepareGasDataDisputeTest(); + skip(7 days); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY - 1); + // Check getGasData + (GasData gasData, uint256 dataMaturity) = InterfaceDestination(localDestination()).getGasData(DOMAIN_REMOTE); + assertEq(GasData.unwrap(gasData), 0); + assertEq(dataMaturity, 0); + (gasData, dataMaturity) = InterfaceDestination(localDestination()).getGasData(DOMAIN_LOCAL); + assertEq(GasData.unwrap(gasData), 0); + assertEq(dataMaturity, 0); + } + + function test_getGasData_afterNotaryDisputeTimeout() public { + (GasData remoteGasData, GasData localGasData) = prepareGasDataDisputeTest(); + skip(7 days); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY); + // Check getGasData + (GasData gasData, uint256 dataMaturity) = InterfaceDestination(localDestination()).getGasData(DOMAIN_REMOTE); + assertEq(GasData.unwrap(gasData), GasData.unwrap(remoteGasData)); + assertEq(dataMaturity, 7 days + DISPUTE_TIMEOUT_NOTARY); + (gasData, dataMaturity) = InterfaceDestination(localDestination()).getGasData(DOMAIN_LOCAL); + assertEq(GasData.unwrap(gasData), GasData.unwrap(localGasData)); + assertEq(dataMaturity, 7 days + DISPUTE_TIMEOUT_NOTARY); + } + // ══════════════════════════════════════════════════ HELPERS ══════════════════════════════════════════════════════ /// @notice Prepares execution of the created messages From 35e9211ead722ed4b2f21897337380c5f01f4207 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 26 Oct 2023 21:09:32 +0100 Subject: [PATCH 08/13] Define "expected behavior" tests for `Summit` --- .../contracts-core/test/suite/Summit.t.sol | 48 +++++++++- .../test/suite/SummitTips.t.sol | 96 +++++++++++++++++++ 2 files changed, 142 insertions(+), 2 deletions(-) diff --git a/packages/contracts-core/test/suite/Summit.t.sol b/packages/contracts-core/test/suite/Summit.t.sol index fe5270a23d..89b7ad2c35 100644 --- a/packages/contracts-core/test/suite/Summit.t.sol +++ b/packages/contracts-core/test/suite/Summit.t.sol @@ -1,11 +1,11 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.17; -import {CallerNotInbox, NotaryInDispute} from "../../contracts/libs/Errors.sol"; +import {CallerNotInbox, DisputeTimeoutNotOver, NotaryInDispute} from "../../contracts/libs/Errors.sol"; import {IAgentSecured} from "../../contracts/interfaces/IAgentSecured.sol"; import {InterfaceDestination} from "../../contracts/interfaces/InterfaceDestination.sol"; import {ISnapshotHub} from "../../contracts/interfaces/ISnapshotHub.sol"; -import {SNAPSHOT_TREE_HEIGHT} from "../../contracts/libs/Constants.sol"; +import {DISPUTE_TIMEOUT_NOTARY, SNAPSHOT_TREE_HEIGHT} from "../../contracts/libs/Constants.sol"; import {MerkleMath} from "../../contracts/libs/merkle/MerkleMath.sol"; import {InterfaceSummit} from "../../contracts/Summit.sol"; @@ -403,6 +403,50 @@ contract SummitTest is AgentSecuredTest { inbox.submitSnapshot(snapPayload, guardSig); } + // ═════════════════════════════════════════ TESTS: NOTARY WON DISPUTE ═════════════════════════════════════════════ + + function prepareSubmitSnapshotDisputeTest() internal returns (bytes memory snapPayload, bytes memory notarySig) { + address notary = domains[DOMAIN_LOCAL].agent; + address reportGuard = domains[0].agent; + address snapshotGuard = domains[0].agents[1]; + + Random memory random = Random("salt"); + RawSnapshot memory rawSnap = random.nextSnapshot(); + // Another Guard signs the snapshot + bytes memory guardSignature; + (snapPayload, guardSignature) = signSnapshot(snapshotGuard, rawSnap); + notarySig = signSnapshot(notary, snapPayload); + inbox.submitSnapshot(snapPayload, guardSignature); + openTestDispute({guardIndex: agentIndex[reportGuard], notaryIndex: agentIndex[notary]}); + } + + /// @dev Resolves test dispute above in favor of the Notary. + function prepareNotaryWonDisputeTest() internal { + address notary = domains[DOMAIN_LOCAL].agent; + address guard = domains[0].agent; + resolveTestDispute({slashedIndex: agentIndex[guard], rivalIndex: agentIndex[notary]}); + } + + function test_submitSnapshot_revert_notaryWonDisputeTimeout() public { + (bytes memory snapPayload, bytes memory notarySig) = prepareSubmitSnapshotDisputeTest(); + skip(7 days); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY - 1); + vm.expectRevert(DisputeTimeoutNotOver.selector); + inbox.submitSnapshot(snapPayload, notarySig); + } + + function test_submitSnapshot_afterNotaryDisputeTimeout() public { + address notary = domains[DOMAIN_LOCAL].agent; + (bytes memory snapPayload, bytes memory notarySig) = prepareSubmitSnapshotDisputeTest(); + skip(7 days); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY); + inbox.submitSnapshot(snapPayload, notarySig); + (bytes memory snapPayload_,) = ISnapshotHub(summit).getNotarySnapshot(0); + assertEq(snapPayload_, snapPayload); + } + // ═════════════════════════════════════════════════ OVERRIDES ═════════════════════════════════════════════════════ function localContract() public view override returns (address) { diff --git a/packages/contracts-core/test/suite/SummitTips.t.sol b/packages/contracts-core/test/suite/SummitTips.t.sol index ec66fd8619..aa43410982 100644 --- a/packages/contracts-core/test/suite/SummitTips.t.sol +++ b/packages/contracts-core/test/suite/SummitTips.t.sol @@ -1,9 +1,11 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.17; +import {DISPUTE_TIMEOUT_NOTARY} from "../../contracts/libs/Constants.sol"; import { AgentNotNotary, CallerNotInbox, + DisputeTimeoutNotOver, IncorrectSnapshotRoot, IncorrectTipsProof, NotaryInDispute, @@ -125,6 +127,19 @@ contract SummitTipsTest is AgentSecuredTest { Summit(localContract()).initialize(); } + function prepareNotaryInDisputeTest() internal { + address notary = domains[DOMAIN_REMOTE].agent; + address guard = domains[0].agent; + openTestDispute({guardIndex: agentIndex[guard], notaryIndex: agentIndex[notary]}); + } + + /// @dev Resolves test dispute above in favor of the Notary. + function prepareNotaryWonDisputeTest() internal { + address notary = domains[DOMAIN_REMOTE].agent; + address guard = domains[0].agent; + resolveTestDispute({slashedIndex: agentIndex[guard], rivalIndex: agentIndex[notary]}); + } + // ══════════════════════════════════════════ TESTS: SUBMIT RECEIPTS ═══════════════════════════════════════════════ function test_submitReceipt( @@ -183,6 +198,35 @@ contract SummitTipsTest is AgentSecuredTest { inbox.submitReceipt(rcptPayload, rcptSignature, tips.encodeTips(), rtp.headerHash, rtp.bodyHash); } + function test_submitReceipt_revert_notaryWonDisputeTimeout() public { + address notary = domains[DOMAIN_REMOTE].agent; + (RawExecReceipt memory re, RawTips memory tips, RawTipsProof memory rtp) = mockReceipt("First"); + prepareReceipt(re, tips, rtp, false, 0, false); + (bytes memory rcptPayload, bytes memory rcptSignature) = signReceipt(notary, re); + // Put DOMAIN_REMOTE notary in Dispute + prepareNotaryInDisputeTest(); + skip(7 days); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY - 1); + vm.expectRevert(DisputeTimeoutNotOver.selector); + inbox.submitReceipt(rcptPayload, rcptSignature, tips.encodeTips(), rtp.headerHash, rtp.bodyHash); + } + + function test_submitReceipt_afterNotaryDisputeTimeout() public checkQueueLength(1) { + address notary = domains[DOMAIN_REMOTE].agent; + (RawExecReceipt memory re, RawTips memory tips, RawTipsProof memory rtp) = mockReceipt("First"); + prepareReceipt(re, tips, rtp, false, 0, false); + (bytes memory rcptPayload, bytes memory rcptSignature) = signReceipt(notary, re); + // Put DOMAIN_REMOTE notary in Dispute + prepareNotaryInDisputeTest(); + skip(7 days); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY); + bool wasAccepted = + inbox.submitReceipt(rcptPayload, rcptSignature, tips.encodeTips(), rtp.headerHash, rtp.bodyHash); + assertTrue(wasAccepted); + } + function test_submitReceipt_revert_unknownSnapRoot() public { (RawExecReceipt memory re, RawTips memory tips, RawTipsProof memory rtp) = mockReceipt("First"); prepareReceipt(re, tips, rtp, false, 0, false); @@ -297,6 +341,32 @@ contract SummitTipsTest is AgentSecuredTest { assertFalse(InterfaceSummit(summit).distributeTips()); } + /// @dev When Notary in "Dispute Won Timeout" mode, the receipt is moved to the end of the queue, + /// therefore two receipts in the queue are expected in the modifier. + function test_distributeTips_attestationNotary_wonDisputeTimeout() public checkQueueLength(2) { + // rcptNotary: agents[1], attNotary: agents[0] + prepareTwoReceiptTest(1, 0); + prepareNotaryInDisputeTest(); + skip(BONDING_OPTIMISTIC_PERIOD); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY - 1); + assertTrue(InterfaceSummit(summit).distributeTips()); + } + + /// @dev Should distribute tips if the "dispute won" timeout is over. + function test_distributeTips_attestationNotary_afterNotaryDisputeTimeout() public checkQueueLength(1) { + // rcptNotary: agents[1], attNotary: agents[0] + prepareTwoReceiptTest(1, 0); + prepareNotaryInDisputeTest(); + skip(BONDING_OPTIMISTIC_PERIOD); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY); + vm.recordLogs(); + assertTrue(InterfaceSummit(summit).distributeTips()); + // Should end up emitting TipsAwarded logs (TODO: better way to check this) + assertGt(vm.getRecordedLogs().length, 0); + } + function test_distributeTips_attestationNotaryDispute() public checkQueueLength(2) { // rcptNotary: agents[1], attNotary: agents[0] prepareTwoReceiptTest(1, 0); @@ -336,6 +406,32 @@ contract SummitTipsTest is AgentSecuredTest { assertTrue(InterfaceSummit(summit).distributeTips()); } + /// @dev When Notary in "Dispute Won Timeout" mode, the receipt is moved to the end of the queue, + /// therefore two receipts in the queue are expected in the modifier. + function test_distributeTips_receiptNotary_wonDisputeTimeout() public checkQueueLength(2) { + // rcptNotary: agents[0], attNotary: agents[1] + prepareTwoReceiptTest(0, 1); + prepareNotaryInDisputeTest(); + skip(BONDING_OPTIMISTIC_PERIOD); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY - 1); + assertTrue(InterfaceSummit(summit).distributeTips()); + } + + /// @dev Should distribute tips if the "dispute won" timeout is over. + function test_distributeTips_receiptNotary_afterNotaryDisputeTimeout() public checkQueueLength(1) { + // rcptNotary: agents[0], attNotary: agents[1] + prepareTwoReceiptTest(0, 1); + prepareNotaryInDisputeTest(); + skip(BONDING_OPTIMISTIC_PERIOD); + prepareNotaryWonDisputeTest(); + skip(DISPUTE_TIMEOUT_NOTARY); + vm.recordLogs(); + assertTrue(InterfaceSummit(summit).distributeTips()); + // Should end up emitting TipsAwarded logs (TODO: better way to check this) + assertGt(vm.getRecordedLogs().length, 0); + } + function test_distributeTips_receiptNotaryFraudulent() public checkQueueLength(1) { // rcptNotary: agents[0], attNotary: agents[1] prepareTwoReceiptTest(0, 1); From 23a28039b41c024abfa7b492f9b0ad39c6d38ef8 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:06:47 +0100 Subject: [PATCH 09/13] Define "expected behavior" tests for `ExecutionHub` --- .../test/suite/hubs/ExecutionHub.t.sol | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/packages/contracts-core/test/suite/hubs/ExecutionHub.t.sol b/packages/contracts-core/test/suite/hubs/ExecutionHub.t.sol index c2152040aa..8e609ec193 100644 --- a/packages/contracts-core/test/suite/hubs/ExecutionHub.t.sol +++ b/packages/contracts-core/test/suite/hubs/ExecutionHub.t.sol @@ -1,9 +1,11 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.17; +import {DISPUTE_TIMEOUT_NOTARY} from "../../../contracts/libs/Constants.sol"; import { AlreadyExecuted, AlreadyFailed, + DisputeTimeoutNotOver, GasLimitTooLow, GasSuppliedTooLow, IncorrectDestinationDomain, @@ -241,6 +243,56 @@ abstract contract ExecutionHubTest is AgentSecuredTest { verify_messageStatusNone(msgLeaf); } + function test_execute_base_revert_notaryWonDisputeTimeout() public { + address notary = domains[DOMAIN_LOCAL].agent; + address guard = domains[0].agent; + Random memory random = Random("Random Salt 1"); + // Create some simple data + (RawBaseMessage memory rbm, RawHeader memory rh, SnapshotMock memory sm) = createDataRevertTest(random); + // Create messages and get origin proof + RawMessage memory rm = createBaseMessages(rbm, rh, localDomain()); + msgPayload = rm.formatMessage(); + msgLeaf = rm.castToMessage().leaf(); + bytes32[] memory originProof = getLatestProof(rh.nonce - 1); + // Create snapshot proof + adjustSnapshot(sm); + (, bytes32[] memory snapProof) = prepareExecution(sm); + // initiate dispute + openTestDispute({guardIndex: agentIndex[guard], notaryIndex: agentIndex[notary]}); + // Make sure that optimistic period is over + skip(rh.optimisticPeriod); + resolveTestDispute({slashedIndex: agentIndex[guard], rivalIndex: agentIndex[notary]}); + skip(DISPUTE_TIMEOUT_NOTARY - 1); + vm.expectRevert(DisputeTimeoutNotOver.selector); + vm.prank(executor); + testedEH().execute(msgPayload, originProof, snapProof, sm.rsi.stateIndex, rbm.request.gasLimit); + } + + function test_execute_base_afterNotaryDisputeTimeout() public { + address notary = domains[DOMAIN_LOCAL].agent; + address guard = domains[0].agent; + Random memory random = Random("Random Salt 2"); + // Create some simple data + (RawBaseMessage memory rbm, RawHeader memory rh, SnapshotMock memory sm) = createDataRevertTest(random); + // Create messages and get origin proof + RawMessage memory rm = createBaseMessages(rbm, rh, localDomain()); + msgPayload = rm.formatMessage(); + msgLeaf = rm.castToMessage().leaf(); + bytes32[] memory originProof = getLatestProof(rh.nonce - 1); + // Create snapshot proof + adjustSnapshot(sm); + (bytes32 snapRoot, bytes32[] memory snapProof) = prepareExecution(sm); + // initiate dispute + openTestDispute({guardIndex: agentIndex[guard], notaryIndex: agentIndex[notary]}); + // Make sure that optimistic period is over + skip(rh.optimisticPeriod); + resolveTestDispute({slashedIndex: agentIndex[guard], rivalIndex: agentIndex[notary]}); + skip(DISPUTE_TIMEOUT_NOTARY); + vm.prank(executor); + testedEH().execute(msgPayload, originProof, snapProof, sm.rsi.stateIndex, rbm.request.gasLimit); + verify_messageStatus(msgLeaf, snapRoot, sm.rsi.stateIndex, MessageStatus.Success, executor, executor); + } + function test_execute_base_revert_snapRootUnknown(Random memory random) public { // Create some simple data (RawBaseMessage memory rbm, RawHeader memory rh, SnapshotMock memory sm) = createDataRevertTest(random); From 23ed1fe57c2d93f6c8911f0bd98b2da9de7f532c Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:09:18 +0100 Subject: [PATCH 10/13] Separate getters for two phases where Notary data is not yet trusted: - Notary in ongoing Dispute / slashed - Notary recently won a Dispute --- .../contracts-core/contracts/Destination.sol | 6 ++--- packages/contracts-core/contracts/Summit.sol | 4 +-- .../contracts/base/AgentSecured.sol | 27 ++++++++++++++++--- .../contracts/hubs/ExecutionHub.sol | 2 +- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/packages/contracts-core/contracts/Destination.sol b/packages/contracts-core/contracts/Destination.sol index 95b90d9e58..f71708abd6 100644 --- a/packages/contracts-core/contracts/Destination.sol +++ b/packages/contracts-core/contracts/Destination.sol @@ -94,7 +94,7 @@ contract Destination is ExecutionHub, DestinationEvents, InterfaceDestination { bytes32 agentRoot, ChainGas[] memory snapGas ) external onlyInbox returns (bool wasAccepted) { - if (_isInDispute(notaryIndex)) revert NotaryInDispute(); + if (_notaryDisputeExists(notaryIndex)) revert NotaryInDispute(); // 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,7 +128,7 @@ 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); @@ -175,7 +175,7 @@ contract Destination is ExecutionHub, DestinationEvents, 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)) { + if (storedGasData.submittedAt != 0 && !_notaryDisputeExists(storedGasData.notaryIndex)) { gasData = storedGasData.gasData; dataMaturity = block.timestamp - storedGasData.submittedAt; } diff --git a/packages/contracts-core/contracts/Summit.sol b/packages/contracts-core/contracts/Summit.sol index efcc5a1e03..0dcf409b0c 100644 --- a/packages/contracts-core/contracts/Summit.sol +++ b/packages/contracts-core/contracts/Summit.sol @@ -112,7 +112,7 @@ contract Summit is SnapshotHub, SummitEvents, InterfaceSummit { uint256 paddedTips, bytes memory rcptPayload ) external onlyInbox returns (bool wasAccepted) { - if (_isInDispute(rcptNotaryIndex)) revert NotaryInDispute(); + if (_notaryDisputeExists(rcptNotaryIndex)) revert NotaryInDispute(); // This will revert if payload is not a receipt body return _saveReceipt({ rcpt: rcptPayload.castToReceipt(), @@ -138,7 +138,7 @@ contract Summit is SnapshotHub, SummitEvents, InterfaceSummit { onlyInbox returns (bytes memory attPayload) { - if (_isInDispute(notaryIndex)) revert NotaryInDispute(); + if (_notaryDisputeExists(notaryIndex)) revert NotaryInDispute(); // This will revert if payload is not a snapshot return _acceptNotarySnapshot(snapPayload.castToSnapshot(), agentRoot, notaryIndex, sigIndex); } diff --git a/packages/contracts-core/contracts/base/AgentSecured.sol b/packages/contracts-core/contracts/base/AgentSecured.sol index db0ad36af3..08dd9f2c1b 100644 --- a/packages/contracts-core/contracts/base/AgentSecured.sol +++ b/packages/contracts-core/contracts/base/AgentSecured.sol @@ -2,6 +2,7 @@ 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, DisputeStatus} from "../libs/Structures.sol"; @@ -105,9 +106,27 @@ abstract contract AgentSecured is MessagingBase, IAgentSecured { 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) { - // TODO: add timeout for Notaries that just won the dispute. - return _disputes[agentIndex].flag != 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; } } diff --git a/packages/contracts-core/contracts/hubs/ExecutionHub.sol b/packages/contracts-core/contracts/hubs/ExecutionHub.sol index fec576fcdb..aef021cbc2 100644 --- a/packages/contracts-core/contracts/hubs/ExecutionHub.sol +++ b/packages/contracts-core/contracts/hubs/ExecutionHub.sol @@ -367,7 +367,7 @@ 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(); } /// @dev Formats the message execution receipt payload for the given hash and receipt data. From 96457d4601f1b68227a67c75d888042b1b2be0a2 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:11:30 +0100 Subject: [PATCH 11/13] Check both scenarios in ExecutionHub --- packages/contracts-core/contracts/hubs/ExecutionHub.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/contracts-core/contracts/hubs/ExecutionHub.sol b/packages/contracts-core/contracts/hubs/ExecutionHub.sol index aef021cbc2..d42367704b 100644 --- a/packages/contracts-core/contracts/hubs/ExecutionHub.sol +++ b/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, @@ -368,6 +369,8 @@ abstract contract ExecutionHub is AgentSecured, ReentrancyGuardUpgradeable, Exec if (rootData.submittedAt == 0) revert IncorrectSnapshotRoot(); // Check that Notary who submitted the attestation is not in dispute 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. From 101a9a59ea466ea90cf3b29bab0a64eb203bd4e5 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:37:14 +0100 Subject: [PATCH 12/13] Check both scenarios in Destination --- .../contracts-core/contracts/Destination.sol | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/contracts-core/contracts/Destination.sol b/packages/contracts-core/contracts/Destination.sol index f71708abd6..b71648f815 100644 --- 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, 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) { + // 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, @@ -133,6 +135,12 @@ contract Destination is ExecutionHub, DestinationEvents, InterfaceDestination { _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 && !_notaryDisputeExists(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 From 34d666ab000c75b8fb92343a64a4d039dd5b0600 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:21:00 +0100 Subject: [PATCH 13/13] Check both scenarios in Summit --- packages/contracts-core/contracts/Summit.sol | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/contracts-core/contracts/Summit.sol b/packages/contracts-core/contracts/Summit.sol index 0dcf409b0c..59b260d6c3 100644 --- a/packages/contracts-core/contracts/Summit.sol +++ b/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) { + // 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) { + // 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); } @@ -218,9 +228,10 @@ contract Summit is SnapshotHub, SummitEvents, InterfaceSummit { // 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; }