fix(task-auditor): close the test-data dispute reward-pool drain (#205)

Documentation/technical/contracts/DINShared.md

@@ -237,7 +237,7 @@ Used by: `DINTaskCoordinator`
 | `TA_CannotSetTestDataAssignedFlag` | State is not `AuditorsBatchesCreated` |
 | `TA_FlagMustBeTrue` | `setTestDataAssignedFlag` called with `flag = false` |
 | `TA_FlagAlreadySet` | Flag was already set for this GI |
-| `TA_NotAssignedAuditor` | Score commit/reveal from auditor not assigned to the batch |
+| `TA_NotAssignedAuditor` | Score commit/reveal, or `openTestDataDispute`, from an auditor not assigned to the batch |
 | `TA_InvalidModelIndex` | Model index not assigned to this batch |
 | `TA_CannotSetAuditScore` | Declared but unused — left over from the pre-commit-reveal `setAuditScorenEligibility`. |
 | `TA_ScoreOutOfRange` | Score > 100 (checked at reveal time) |

Documentation/technical/contracts/DINTaskAuditor.md

@@ -69,7 +69,7 @@ Constants: `MAX_REGISTERED_AUDITORS = 300`; `MAX_LM_SUBMISSIONS = 10000` (a plai
 | `s1SlashFractionBps` | 3000 (30%) | `setS1SlashFractionBps` (1 – 10 000) |
 | `s3DeviationThreshold` | 40 (on the 0–100 scale) | `setS3DeviationThreshold` (≤ 100) |
 | `s3SlashingEnabled` | `false` (shadow mode) | `setS3SlashingEnabled` |
-| `disputeBondAmount` | 0 | `setDisputeBondAmount` |
+| `disputeBondAmount` | 100 DIN (`100 * 1e18`) | `setDisputeBondAmount` |
 | `disputeWindowBlocks` | 7200 (~1 day on Optimism) | `setDisputeWindowBlocks` |
 | `disputePenaltyBps` | 2500 (25% of the GI pool) | `setDisputePenaltyBps` (≤ 10 000) |
 
@@ -93,8 +93,9 @@ The reward split and the S1 fraction are explicitly provisional (MECHANISM_DESIG
 | Paired coordinator (`onlyTaskCoordinator`) | `updatePassScore`, `createAuditorsBatches`, `setTestDataAssignedFlag`, `finalizeEvaluation`, `slashAuditors`, `settleRewards`, `decrementAuditorRegistrations` |
 | `owner()` (model owner) | `assignAuditTestDataset`, `reassignAuditTestDataset`, all setters |
 | Assigned auditor (`onlyAssignedAuditor`) | `commitAuditScore`, `revealAuditScore` |
+| Auditor of the disputed batch (`isBatchAuditor`) | `openTestDataDispute` |
 | Any active validator | `registerDINAuditor` |
-| Any address | `submitLocalModel`, `depositRewards`, `claimReward`, `claimRewards`, `openTestDataDispute`, `resolveTestDataDispute`, `closeExpiredDispute`, views |
+| Any address | `submitLocalModel`, `depositRewards`, `claimReward`, `claimRewards`, `closeExpiredDispute`, views |
 
 ---
 
@@ -161,9 +162,9 @@ Lets a batch auditor challenge the model owner's test data.
 | Step | Who | Effect |
 |------|-----|--------|
 | `isEncryptionKeyEmpty(gi, batchId, auditor)` | View | Free check: an auditor who received no key has grounds to dispute |
-| `openTestDataDispute(gi, batchId)` | Anyone | Needs a stored commitment; pulls `disputeBondAmount` DIN (0 by default); window = `disputeWindowBlocks` |
-| `resolveTestDataDispute(gi, batchId, K, plaintextHash)` | **Anyone**, within the window | Recomputes the commitment. **Match →** dispute false: bond forfeited. **Mismatch →** upheld: bond returned; `disputePenaltyBps` of `giRewardPool[gi]` removed as a penalty; batch marked `pendingReassignment` |
-| `closeExpiredDispute(gi, batchId)` | Anyone, after the window | Bond forfeited as above |
+| `openTestDataDispute(gi, batchId)` | An auditor of that batch (`TA_NotAssignedAuditor` otherwise) | Needs a stored commitment; pulls `disputeBondAmount` DIN (100 DIN by default); window = `disputeWindowBlocks` |
+| `resolveTestDataDispute(gi, batchId, K, plaintextHash)` | Owner, within the window | The owner reveals `K` and the plaintext hash, and the commitment is recomputed. **Match →** dispute false: bond forfeited. **Mismatch →** upheld: bond returned; `disputePenaltyBps` of `giRewardPool[gi]` removed as a penalty; batch marked `pendingReassignment` |
+| `closeExpiredDispute(gi, batchId)` | Anyone, after the window | The owner didn't answer, so the dispute is **upheld** with the same effects as a mismatch. Emits `DisputeExpired(gi, batchId)`, then `TestDataDisputeUpheld` |
 | `reassignAuditTestDataset(…)` | Owner | New CID, keys and commitment for a batch pending reassignment |
 
 Forfeited bonds and penalties are split 50% burned / 50% forwarded to `slashTreasury()`; both halves are burned if no treasury is set. `treasuryAccrued` is a running counter of everything routed out this way (including the burned part). No tokens are held against it.
@@ -195,7 +196,7 @@ Registration & data: `DINAuditorRegistered`, `LocalModelSubmitted`, `AuditorsBat
 
 Read alongside the [foundry/src security review](../audits/foundry-src-security-review.md).
 
-- **No. 1 — Test-data disputes can be won by the challenger alone.** `resolveTestDataDispute` is callable by anyone, and any commitment *mismatch* upholds the dispute. A challenger can call it with an arbitrary `K` and win: bond back, the model owner's GI pool cut by `disputePenaltyBps`, and the batch blocked until reassignment. Only the model owner revealing the real `K` should be able to reach the "match" branch, and a mismatch from a non-owner caller should not count as evidence. `disputeBondAmount` defaults to 0, so this costs the challenger nothing and can be repeated after every reassignment. Tracked in issue No. 205.
+- **No. 1 — Fixed: a test-data dispute can no longer be won by the challenger alone.** `resolveTestDataDispute` used to be callable by anyone, and any commitment mismatch upheld the dispute. So any address could pass a junk `K` and win: bond back, the model owner's GI pool cut by `disputePenaltyBps`, and the batch blocked until reassignment. With a 0 default bond this was free and repeatable. Now only an auditor of the batch can open a dispute, the bond defaults to 100 DIN, and only the owner can resolve. An owner who doesn't answer within the window loses through `closeExpiredDispute` (issue No. 205). Remaining trust assumption: the owner reveals evidence about their own data, and a bad plaintext behind a correct `K` can't be proven on-chain. Decentralised adjudication is tracked in issue No. 181.
 - **No. 2 — Fixed: commit hashes are now bound to the auditor.** The old hash, `keccak256(abi.encodePacked(score, vote, salt))`, carried no address, GI, batch or model. An assigned auditor could copy a peer's commit hash, wait for the peer's reveal, and replay it for a free vote. The hash now binds `msg.sender`, `gi`, `batchId` and `modelIndex` (§7), as the aggregation commits on the coordinator do (issue No. 192).
 - **No. 3 — Committed-but-unrevealed is slashed as a liveness miss.** An auditor who commits and then withholds the reveal pays the S1 fraction (`AUD_NO_VOTE`, 30% of `minStake` by default). That is less than the full-`minStake` S3 slash a revealed outlier would pay once `s3SlashingEnabled` is on, so an auditor who sees they will be in the minority can choose not to reveal. Whether this case gets its own reason code and fraction is open in issue No. 201 (Part B).
 - **No. 4 — Unclaimable remainders.** If no model is approved (`giTotalApprovedScore == 0`), or nobody reveals, or no aggregator weight exists, that role's pool share stays in the contract with no reclaim path.
@@ -219,3 +220,4 @@ Read alongside the [foundry/src security review](../audits/foundry-src-security-
 - Treasury shares and forfeitures forwarded to the platform treasury (`slashTreasury()`), replacing the per-contract treasury address (issue No. 152).
 - `createAuditorsBatches` takes the coordinator's locked audit seed (issue No. 156 H-2, PR No. 191).
 - The audit commit hash binds the auditor and the slot: `keccak256(abi.encode(score, vote, salt, msg.sender, gi, batchId, modelIndex))` replaces `keccak256(abi.encodePacked(score, vote, salt))` (issue No. 192). Function signatures and the ABI are unchanged; in-flight commits made under the old formula can't be revealed after the switch.
+- Test-data disputes (issue No. 205): only an auditor of the batch can open one; `resolveTestDataDispute` is owner-only; `closeExpiredDispute` now upholds an unanswered dispute instead of forfeiting the bond; `disputeBondAmount` defaults to 100 DIN. `DisputeExpired` drops its `bondForfeited` field.

dincli/abis/DINTaskAuditor.json

@@ -1940,12 +1940,6 @@
           "type": "uint256",
           "indexed": true,
           "internalType": "uint256"
-        },
-        {
-          "name": "bondForfeited",
-          "type": "uint256",
-          "indexed": false,
-          "internalType": "uint256"
         }
       ],
       "anonymous": false

foundry/src/DINTaskAuditor.sol

@@ -1,3 +1,3 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity ^0.8.28;
 
@@ -283,7 +283,9 @@
     }
     mapping(uint256 => mapping(uint256 => DisputeRecord)) public testDataDisputes;
 
-    uint256 public disputeBondAmount;           // DIN; 0 at deploy, DAO-settable
+    // Non-zero by default so each dispute attempt costs something (issue #205);
+    // matches DINTaskCoordinator.disputeBond. Revisit with issue #155's values.
+    uint256 public disputeBondAmount = 100 * 1e18; // DIN; owner-settable
     uint256 public disputeWindowBlocks = 7200;  // ~1 day on Optimism (~2s blocks)
     uint256 public disputePenaltyBps = 2500;    // 25% of giRewardPool[gi] forfeited on owner loss
 
@@ -355,7 +357,9 @@
     event TestDataDisputeResolvedFalse(uint256 indexed gi, uint256 indexed batchId, address indexed disputer, uint256 bondForfeited);
     event TestDataDisputeUpheld(uint256 indexed gi, uint256 indexed batchId, address indexed disputer, uint256 bondReturned, uint256 ownerPenalty);
     event BatchPendingReassignment(uint256 indexed gi, uint256 indexed batchId);
-    event DisputeExpired(uint256 indexed gi, uint256 indexed batchId, uint256 bondForfeited);
+    /// @dev Emitted when an unanswered dispute is closed after its window; it is
+    ///      then upheld (TestDataDisputeUpheld follows). Issue #205.
+    event DisputeExpired(uint256 indexed gi, uint256 indexed batchId);
 
     event EligibilityVoted(
         uint256 indexed gi,
@@ -1462,15 +1466,21 @@
         return encryptedTestDataKey[gi][batchId][auditor].length == 0;
     }
 
-    /// @notice Opens a test-data dispute for a batch. Requires a DIN bond.
-    ///         The disputer must call resolveTestDataDispute within the challenge
-    ///         window; failure to do so forfeits the bond (closeExpiredDispute).
+    /// @notice Opens a test-data dispute for a batch. Only an auditor of that
+    ///         batch can open one, and it requires the DIN bond.
+    ///         The model owner must then answer with resolveTestDataDispute
+    ///         within the challenge window; if the owner stays silent, anyone
+    ///         can call closeExpiredDispute and the dispute is upheld.
+    /// @dev Issue #205: opening was unrestricted and the bond defaulted to 0,
+    ///      so any address could drain giRewardPool through repeated disputes.
     /// @param gi GI index.
     /// @param batchId Batch to dispute.
     function openTestDataDispute(
         uint256 gi,
         uint256 batchId
     ) external nonReentrant {
+        if (batchId >= auditBatches[gi].length) revert TA_BatchDoesNotExist();
+        if (!isBatchAuditor[gi][batchId][msg.sender]) revert TA_NotAssignedAuditor();
         if (testDataCommitments[gi][batchId] == bytes32(0)) revert TA_NoCommitmentStored();
         DisputeRecord storage d = testDataDisputes[gi][batchId];
         if (d.active) revert TA_DisputeAlreadyActive();
@@ -1492,11 +1502,18 @@
         emit TestDataDisputeOpened(gi, batchId, msg.sender, disputeBondAmount, expires);
     }
 
-    /// @notice Resolves an active dispute by revealing K and the actual plaintext hash.
-    ///         Anyone may call this — the disputer is the beneficiary if upheld.
+    /// @notice The model owner answers an active dispute by revealing K and the
+    ///         actual plaintext hash.
     ///         Commitment check: keccak256(abi.encodePacked(gi, batchId, keccak256(K), plaintextHash))
     ///         Match  → dispute false → disputer's bond forfeited (50% burn / 50% treasury).
     ///         Mismatch → dispute upheld → bond returned, owner's giRewardPool[gi] penalised.
+    /// @dev Owner-only (issue #205). When anyone could call this, a caller-chosen
+    ///      junk K always produced a mismatch, so any address could uphold a
+    ///      dispute and burn 25% of the GI reward pool. Only the party holding the
+    ///      data can now reveal; a non-matching owner reveal still upholds.
+    ///      Trust assumption: the owner judges disputes about their own test data
+    ///      (tracked for mainnet in issue #181); the silence rule in
+    ///      closeExpiredDispute keeps that from being a free veto.
     /// @param gi GI index.
     /// @param batchId Batch under dispute.
     /// @param K The raw symmetric key the model owner used to encrypt the test data.
@@ -1506,7 +1523,7 @@
         uint256 batchId,
         bytes calldata K,
         bytes32 plaintextHash
-    ) external nonReentrant {
+    ) external onlyOwner nonReentrant {
         DisputeRecord storage d = testDataDisputes[gi][batchId];
         if (!d.active) revert TA_NoActiveDispute();
         if (block.number > d.expiresAtBlock) revert TA_DisputeWindowClosed();
@@ -1523,40 +1540,47 @@
             _burnAndForward(bond);
             emit TestDataDisputeResolvedFalse(gi, batchId, d.disputer, bond);
         } else {
-            // Dispute upheld — return bond, penalise owner's reward pool
-            uint256 bond = d.bond;
-            address disputer = d.disputer;
-            d.active = false;
-            d.pendingReassignment = true;
-
-            if (bond > 0) {
-                dinToken.safeTransfer(disputer, bond);
-            }
-
-            uint256 penalty = (giRewardPool[gi] * disputePenaltyBps) / 10000;
-            if (penalty > 0 && giRewardPool[gi] >= penalty) {
-                giRewardPool[gi] -= penalty;
-                _burnAndForward(penalty);
-            }
-
-            emit TestDataDisputeUpheld(gi, batchId, disputer, bond, penalty);
-            emit BatchPendingReassignment(gi, batchId);
+            _upholdTestDataDispute(gi, batchId, d);
         }
     }
 
-    /// @notice Closes an expired dispute and forfeits the disputer's bond.
-    ///         Callable by anyone once the challenge window has elapsed without resolution.
+    /// @notice Closes a dispute the model owner didn't answer within the
+    ///         challenge window. The dispute is upheld: bond returned to the
+    ///         disputer, owner's giRewardPool[gi] penalised, batch flagged for
+    ///         reassignment. Callable by anyone once the window has elapsed.
+    /// @dev Issue #205: silence counts against the party who holds the data.
+    ///      Before, an unanswered dispute forfeited the disputer's bond, which
+    ///      combined with the open resolve made the owner's silence costless.
     function closeExpiredDispute(uint256 gi, uint256 batchId) external nonReentrant {
         DisputeRecord storage d = testDataDisputes[gi][batchId];
         if (!d.active) revert TA_NoActiveDispute();
         if (block.number <= d.expiresAtBlock) revert TA_DisputeWindowClosed();
 
+        emit DisputeExpired(gi, batchId);
+        _upholdTestDataDispute(gi, batchId, d);
+    }
+
+    /// @dev Upheld test-data dispute: return the bond, penalise the owner's GI
+    ///      reward pool by disputePenaltyBps, and block the batch until
+    ///      reassignAuditTestDataset.
+    function _upholdTestDataDispute(uint256 gi, uint256 batchId, DisputeRecord storage d) internal {
         uint256 bond = d.bond;
+        address disputer = d.disputer;
         d.active = false;
+        d.pendingReassignment = true;
 
-        _burnAndForward(bond);
+        if (bond > 0) {
+            dinToken.safeTransfer(disputer, bond);
+        }
+
+        uint256 penalty = (giRewardPool[gi] * disputePenaltyBps) / 10000;
+        if (penalty > 0 && giRewardPool[gi] >= penalty) {
+            giRewardPool[gi] -= penalty;
+            _burnAndForward(penalty);
+        }
 
-        emit DisputeExpired(gi, batchId, bond);
+        emit TestDataDisputeUpheld(gi, batchId, disputer, bond, penalty);
+        emit BatchPendingReassignment(gi, batchId);
     }
 
     /// @notice Re-assigns test data for a batch after the model owner lost a dispute.