fix(disputes): disputeFinalizesAt view, and an answer is one non-zero hash before the challenge deadline (D4, D5)

evm/src/AdManager.sol

@@ -62,8 +62,8 @@ contract AdManager is EscrowBase, IAdManager {
     /// @notice The maker's settlement halt (#422). While set, the co-signed `unlock` of every order
     ///         against every ad this maker owns is refused. Custody-authorized, instant both ways,
     ///         never pause-gated. Read by `unlock`, by the cancel doors (`finalizeCancel`,
-    ///         `finalizeDispute`) for their grace, and by `cancelFinalizesAt`: every evidence path
-    ///         ignores it, so a halt can delay a payout but never keep both sides.
+    ///         `finalizeDispute`) for their grace, and by `cancelFinalizesAt` / `disputeFinalizesAt`:
+    ///         every evidence path ignores it, so a halt can delay a payout but never keep both sides.
     mapping(address maker => bool) public halted;
 
     /// @notice When `maker` last resumed. A halt in force at any point at or after an order's
@@ -250,7 +250,7 @@ contract AdManager is EscrowBase, IAdManager {
         _requireSettleable(orderHash, nullifierHash);
         // #422: the maker's halt refuses the co-signed payout; evidence (`presentSettled`) still pays.
         // One cold read of the maker's flag on the metered path (~2.4k; UnlockGas re-baselined).
-        address maker = ads[params.adId].maker;
+        address maker = _adOf(params).maker;
         if (halted[maker]) revert AdManager__Halted(maker);
         // Gate 2 — root authenticity (the co-signed root); mandatory, reverts NoRootVerifier when unwired.
         _requireRootValid(
@@ -294,11 +294,9 @@ contract AdManager is EscrowBase, IAdManager {
         // #422 rule 3: when the co-signed payout was denied, the cancel waits for the order chain's
         // SETTLED evidence to be anchorable and presented, so a maker paid on the other chain cannot
         // also take the lock back.
-        if (_coSignDenied(params, orderHash, _presentationCutoff(orderHash, params))) {
-            _requireReached(_claimedWindowEnd(orderHash) + _evidenceGrace(params.orderChainId));
-        }
+        _requireReached(_finalizesAt(params, orderHash, Status.Claimed));
 
-        Ad storage ad = ads[params.adId];
+        Ad storage ad = _adOf(params);
         uint256 adAmount = _adAmount(params);
         ad.locked -= adAmount;
         _cancel(orderHash, params.adSettlementSigner, false);
@@ -316,7 +314,7 @@ contract AdManager is EscrowBase, IAdManager {
         bytes32 orderHash = _orderHash(params);
         // Only the order's two parties may file. Without this any address could dispute any live
         // order and, one short challenge period later, cancel it out from under both of them.
-        _requireParty(ads[params.adId].maker, params.orderRecipient.toAddressChecked());
+        _requireParty(_adOf(params).maker, params.orderRecipient.toAddressChecked());
         // The amount is validated by the hash the caller had to reproduce, which is why filing
         // starts here and not on the module: only this contract can vouch for it.
         _openDispute(
@@ -334,7 +332,7 @@ contract AdManager is EscrowBase, IAdManager {
         bytes32 orderHash = _orderHash(params);
         _requireStatus(orderHash, Status.Disputed);
         // Either party may call; the module refuses the filer, so only the other one gets through.
-        _requireParty(ads[params.adId].maker, params.orderRecipient.toAddressChecked());
+        _requireParty(_adOf(params).maker, params.orderRecipient.toAddressChecked());
         disputeModuleOf[orderHash].recordResponse(orderHash, msg.sender, evidence);
     }
 
@@ -343,22 +341,16 @@ contract AdManager is EscrowBase, IAdManager {
         bytes32 orderHash = _orderHash(params);
         _requireStatus(orderHash, Status.Disputed);
         // D5: the module the order was filed under, whatever is wired now.
-        IDisputeManager m = disputeModuleOf[orderHash];
-
-        (Dispute.Outcome outcome, bool windowOver, address initiator) = m.outcomeOf(orderHash, pausedSeconds);
+        (IDisputeManager m, Dispute.Outcome outcome, bool windowOver, address initiator) = _disputeOf(orderHash);
         if (!windowOver) revert Escrow__DisputeNotResolved(orderHash);
         // No ruling means the fallback: a mutual refund, the unified primitive's terminal (D4).
         if (outcome == Dispute.Outcome.None) outcome = Dispute.Outcome.MutualRefund;
         // #422 rule 3, this door too: every outcome but MakerForfeit hands the lock back to the maker,
-        // so a denied payout waits the evidence grace past the challenge deadline (review F1).
-        if (outcome != Dispute.Outcome.MakerForfeit) {
-            uint256 until = m.effectiveChallengeDeadline(orderHash);
-            if (_coSignDenied(params, orderHash, until)) {
-                _requireReached(until + _evidenceGrace(params.orderChainId));
-            }
-        }
+        // so a denied payout waits the evidence grace past the challenge deadline (review F1). The
+        // clock from the module and outcome already in hand: one read, not three.
+        _requireReached(_disputeEnd(params, orderHash, m, outcome));
 
-        Ad storage ad = ads[params.adId];
+        Ad storage ad = _adOf(params);
         uint256 adAmount = _adAmount(params);
         ad.locked -= adAmount;
         if (outcome == Dispute.Outcome.MakerForfeit) {
@@ -521,7 +513,7 @@ contract AdManager is EscrowBase, IAdManager {
 
         // Evidence beats arbitration: if this order was disputed, that dispute is now over and the
         // bond settles on what the proof shows, not on whatever the arbiter had ruled.
-        _closeDisputeByEvidence(orderHash, ads[params.adId].maker);
+        _closeDisputeByEvidence(orderHash, _adOf(params).maker);
         _fill(orderHash, params.adSettlementSigner, true);
         _payFromAd(params);
     }
@@ -574,14 +566,19 @@ contract AdManager is EscrowBase, IAdManager {
         return _hashOrder(p, block.chainid, address(this));
     }
 
+    /// @dev The order's ad: one keccak-of-calldata-string site instead of eight (EIP-170 margin).
+    function _adOf(OrderParams calldata p) private view returns (Ad storage) {
+        return ads[p.adId];
+    }
+
     /// @dev The signed amount in ad-chain units — what the lock reserved and the payout releases.
     function _adAmount(OrderParams calldata p) private pure returns (uint256) {
         return ProofBridgeUtils.scale(p.amount, p.orderDecimals, p.adDecimals);
     }
 
     /// @dev Pay the bridger's recipient from the ad, in the units the lock reserved.
     function _payFromAd(OrderParams calldata p) private {
-        Ad storage ad = ads[p.adId];
+        Ad storage ad = _adOf(p);
         uint256 adAmount = _adAmount(p);
         ad.balance -= adAmount;
         ad.locked -= adAmount;
@@ -614,7 +611,7 @@ contract AdManager is EscrowBase, IAdManager {
     ///      choose later. No registry wired means no lever 2 to read. #461: no "no usable slot" term —
     ///      the lock required one, so losing it by the cutoff is an expiry in the interval already.
     function _coSignDenied(OrderParams calldata p, bytes32 orderHash, uint256 until) private view returns (bool) {
-        address maker = ads[p.adId].maker;
+        address maker = _adOf(p).maker;
         if (halted[maker] || lastResumedAt[maker] >= p.deadline) return true;
         // #465: the registry the order was locked under, not the live one: a migration mid-order
         // would otherwise read a registry that never saw the kill.
@@ -646,12 +643,55 @@ contract AdManager is EscrowBase, IAdManager {
 
     /// @inheritdoc IAdManager
     function cancelFinalizesAt(OrderParams calldata params) external view returns (uint256) {
-        bytes32 orderHash = _orderHash(params);
-        if (_statusOf(orderHash) != Status.Claimed) return 0;
-        uint256 end = _claimedWindowEnd(orderHash);
-        return _coSignDenied(params, orderHash, _presentationCutoff(orderHash, params))
-            ? end + _evidenceGrace(params.orderChainId)
-            : end;
+        return _finalizesAt(params, _orderHash(params), Status.Claimed);
+    }
+
+    /// @inheritdoc IAdManager
+    function disputeFinalizesAt(OrderParams calldata params) external view returns (uint256) {
+        return _finalizesAt(params, _orderHash(params), Status.Disputed);
+    }
+
+    /// @dev The one clock both finalize doors wait on and both views report: the cancel window's end
+    ///      or the effective challenge deadline, plus the evidence grace when the co-signed payout was
+    ///      denied (#422) and the outcome hands the lock back (all but MakerForfeit). 0 off `status`.
+    function _finalizesAt(OrderParams calldata params, bytes32 orderHash, Status status)
+        private
+        view
+        returns (uint256 end)
+    {
+        if (_statusOf(orderHash) != status) return 0;
+        if (status == Status.Claimed) {
+            end = _claimedWindowEnd(orderHash);
+            if (_coSignDenied(params, orderHash, _presentationCutoff(orderHash, params))) {
+                end += _evidenceGrace(params.orderChainId);
+            }
+        } else {
+            (IDisputeManager m, Dispute.Outcome outcome,,) = _disputeOf(orderHash);
+            end = _disputeEnd(params, orderHash, m, outcome);
+        }
+    }
+
+    /// @dev The dispute half of `_finalizesAt`, for a caller that has already read the module and
+    ///      the outcome (`finalizeDispute`): the effective challenge deadline, plus the grace unless
+    ///      the ruling is MakerForfeit.
+    function _disputeEnd(OrderParams calldata params, bytes32 orderHash, IDisputeManager m, Dispute.Outcome outcome)
+        private
+        view
+        returns (uint256 end)
+    {
+        end = m.effectiveChallengeDeadline(orderHash);
+        if (outcome == Dispute.Outcome.MakerForfeit) return end;
+        if (_coSignDenied(params, orderHash, end)) end += _evidenceGrace(params.orderChainId);
+    }
+
+    /// @dev The module the order was filed under (D5) and its verdict, read at this escrow's pause clock.
+    function _disputeOf(bytes32 orderHash)
+        private
+        view
+        returns (IDisputeManager m, Dispute.Outcome outcome, bool windowOver, address initiator)
+    {
+        m = disputeModuleOf[orderHash];
+        (outcome, windowOver, initiator) = m.outcomeOf(orderHash, pausedSeconds);
     }
 
     /// @dev How long a denied order's cancel waits past its window: the order chain's anchor delay

evm/src/DisputeManager.sol

@@ -101,6 +101,12 @@ contract DisputeManager is IDisputeManager, TwoStepAdmin {
     error DisputeManager__ChallengeClosed(uint256 since);
     error DisputeManager__AlreadyRuled(bytes32 orderHash);
     error DisputeManager__NotResponder();
+    /// @notice D5: answers close with the challenge window, the instant the arbiter's ruling does.
+    error DisputeManager__ResponseWindowClosed(uint256 since);
+    /// @notice D5: the responder slot takes one answer; the first one stands.
+    error DisputeManager__AlreadyResponded(bytes32 orderHash);
+    /// @notice D5: an empty answer is refused, so "answered" is exactly "the slot is non-zero".
+    error DisputeManager__ZeroResponse();
     error DisputeManager__NothingToClaim();
 
     modifier onlyEscrow() {
@@ -222,7 +228,8 @@ contract DisputeManager is IDisputeManager, TwoStepAdmin {
                              THE DISPUTE ITSELF
     //////////////////////////////////////////////////////////////*/
 
-    /// @notice Record the counterparty's evidence hash. Moves no funds, posts no bond (D11).
+    /// @notice Record the counterparty's evidence hash. Moves no funds, posts no bond (D11). One
+    ///         non-zero answer, before the effective challenge deadline and before any ruling (D5).
     /// @dev Escrow-only. Authenticating "an address that is not the filer" here would let any
     ///      passer-by overwrite the genuine counterparty's hash — the slot is single, not an append
     ///      — so the responder's identity has to come from the side that knows the order's parties,
@@ -232,6 +239,14 @@ contract DisputeManager is IDisputeManager, TwoStepAdmin {
         Dispute.Record storage d = disputes[orderHash];
         if (d.initiator == address(0)) revert DisputeManager__NotDisputed(orderHash);
         if (responder == d.initiator) revert DisputeManager__NotResponder();
+        if (evidence == bytes32(0)) revert DisputeManager__ZeroResponse();
+        if (d.responderEvidence != bytes32(0)) revert DisputeManager__AlreadyResponded(orderHash);
+        // The ruling closes the answer window. The record's deadline becomes the ruling's finalize
+        // time, so without this an answer would land until then — of use only to a re-ruling, which
+        // is #454's question (T3), not this door's.
+        if (d.ruling != Dispute.Outcome.None) revert DisputeManager__AlreadyRuled(orderHash);
+        uint256 until_ = effectiveChallengeDeadline(orderHash);
+        if (block.timestamp >= until_) revert DisputeManager__ResponseWindowClosed(until_);
         d.responderEvidence = evidence;
         emit DisputeResponded(orderHash, responder, evidence);
     }

evm/src/interfaces/IAdManager.sol

@@ -159,8 +159,14 @@ interface IAdManager is IEscrow {
      * @notice When a claimed cancel really finalizes: the claim window's end plus the evidence grace
      *         when the co-signed payout was denied (#422). 0 when the order is not `Claimed`. The
      *         relayer's janitor asks this instead of computing the clock itself.
+     * @dev During a pause this under-reports: paused seconds accrue at `unpause`, so the time jumps
+     *      forward then. The doors are `whenNotPaused`, so acceptance never drifts from the view, but
+     *      a caller that cached it before a pause must re-read after the unpause.
      */
     function cancelFinalizesAt(OrderParams calldata params) external view returns (uint256);
+    /// @notice `cancelFinalizesAt`'s dispute twin: when `finalizeDispute` can run, grace included; 0 off `Disputed`.
+    /// @dev Same pause caveat as `cancelFinalizesAt`: re-read after an unpause.
+    function disputeFinalizesAt(OrderParams calldata params) external view returns (uint256);
     function fundAd(string calldata adId, uint256 amount) external payable;
     function withdrawFromAd(string calldata adId, uint256 amount, address to) external;
     function closeAd(string calldata adId, address to) external;
@@ -206,6 +212,7 @@ interface IAdManager is IEscrow {
     /// @notice Record the counterparty's evidence hash on an open dispute. Only the order's other
     ///         party may call it (D11): the responder slot is single, not an append, so anyone able
     ///         to write it could overwrite the genuine response a block before the arbiter reads it.
+    ///         One non-zero answer, before the effective challenge deadline (D5, the module refuses).
     function respondToDispute(OrderParams calldata params, bytes32 evidence) external;
     /// @notice Apply the module's outcome once its window is over, and settle the bond.
     function finalizeDispute(OrderParams calldata params) external;

evm/src/interfaces/IDisputeManager.sol

@@ -63,7 +63,8 @@ interface IDisputeManager {
     ) external payable returns (uint128 bond);
 
     /**
-     * @notice Record the counterparty's response to an open dispute.
+     * @notice Record the counterparty's response to an open dispute: one non-zero hash, strictly
+     *         before the effective challenge deadline (D5).
      * @dev Escrow-only: only the escrow knows who the order's two parties are, so only it can say
      *      that this responder is the one that did not file (D11). The module authenticating "some
      *      address that is not the filer" would let anyone overwrite the genuine counterparty's

evm/test/Dispute.t.sol

@@ -403,6 +403,78 @@ contract DisputeTest is AdManagerTest, CancellationHarness {
         assertEq(responderEvidence, bytes32("response"), "the counterparty's hash is recorded");
     }
 
+    /*//////////////// D5: one answer, inside the window, never empty ////////////////*/
+
+    /// D5: the slot holds one answer; a second one is refused and the first stands.
+    function test_d5_aSecondAnswerIsRefused() public {
+        (IAdManager.OrderParams memory p, bytes32 h) = _lockedOrder(26);
+        _file(p, filer);
+        vm.prank(maker);
+        adManager.respondToDispute(p, bytes32("first"));
+        vm.prank(maker);
+        vm.expectRevert(abi.encodeWithSelector(DisputeManager.DisputeManager__AlreadyResponded.selector, h));
+        adManager.respondToDispute(p, bytes32("second"));
+        (,,,,, bytes32 responderEvidence,,,) = dm.disputes(h);
+        assertEq(responderEvidence, bytes32("first"), "the first answer stands");
+    }
+
+    /// D5: answers close with the challenge window, the same instant the arbiter's ruling does.
+    function test_d5_anAnswerAtTheChallengeDeadlineIsRefused() public {
+        (IAdManager.OrderParams memory p, bytes32 h) = _lockedOrder(27);
+        _file(p, filer);
+        uint256 until = dm.effectiveChallengeDeadline(h);
+        vm.warp(until);
+        vm.prank(maker);
+        vm.expectRevert(abi.encodeWithSelector(DisputeManager.DisputeManager__ResponseWindowClosed.selector, until));
+        adManager.respondToDispute(p, bytes32("late"));
+        // One second earlier it lands.
+        vm.warp(until - 1);
+        vm.prank(maker);
+        adManager.respondToDispute(p, bytes32("on time"));
+    }
+
+    /// D5: the answer deadline is the effective one, so a pause after the filing moves it.
+    function test_d5_aPauseMovesTheAnswerDeadline() public {
+        (IAdManager.OrderParams memory p, bytes32 h) = _lockedOrder(29);
+        _file(p, filer);
+        uint256 until = dm.effectiveChallengeDeadline(h);
+        vm.prank(admin);
+        adManager.pause();
+        vm.warp(block.timestamp + 1 hours);
+        vm.prank(admin);
+        adManager.unpause();
+        assertEq(dm.effectiveChallengeDeadline(h), until + 1 hours);
+        vm.warp(until);
+        vm.prank(maker);
+        adManager.respondToDispute(p, bytes32("still open"));
+    }
+
+    /// D5 (R1, 2026-10-05): the ruling closes the answer window. Without the guard the record's
+    /// deadline is the ruling's finalize time, so a late answer lands with nothing left to act on it.
+    function test_d5_anAnswerAfterTheRulingIsRefused() public {
+        (IAdManager.OrderParams memory p, bytes32 h) = _lockedOrder(33);
+        _file(p, filer);
+        vm.prank(arbiter);
+        dm.resolveDispute(h, Dispute.Outcome.MutualRefund);
+        assertLt(block.timestamp, dm.effectiveChallengeDeadline(h), "the ruling's window is still open");
+        vm.prank(maker);
+        vm.expectRevert(abi.encodeWithSelector(DisputeManager.DisputeManager__AlreadyRuled.selector, h));
+        adManager.respondToDispute(p, bytes32("too late"));
+        (,,,,, bytes32 responderEvidence,,,) = dm.disputes(h);
+        assertEq(responderEvidence, bytes32(0), "nothing recorded");
+    }
+
+    /// D5: an empty answer is refused, so "answered" is exactly "the slot is non-zero".
+    function test_d5_aZeroAnswerIsRefused() public {
+        (IAdManager.OrderParams memory p, bytes32 h) = _lockedOrder(28);
+        _file(p, filer);
+        vm.prank(maker);
+        vm.expectRevert(DisputeManager.DisputeManager__ZeroResponse.selector);
+        adManager.respondToDispute(p, bytes32(0));
+        (,,,,, bytes32 responderEvidence,,,) = dm.disputes(h);
+        assertEq(responderEvidence, bytes32(0));
+    }
+
     /// S4, tightened by C-17: the bond is paid exactly. An overpaid or underpaid bond is refused,
     /// so no surplus is ever wrapped and nothing needs refunding.
     /// 49E-2: the bond is exact, so the amount is quoted by the contract, not re-derived by clients.