diff --git a/Documentation/technical/contracts/DINShared.md b/Documentation/technical/contracts/DINShared.md index 9102870..5083868 100644 --- a/Documentation/technical/contracts/DINShared.md +++ b/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) | diff --git a/Documentation/technical/contracts/DINTaskAuditor.md b/Documentation/technical/contracts/DINTaskAuditor.md index f53398c..85e63b6 100644 --- a/Documentation/technical/contracts/DINTaskAuditor.md +++ b/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. diff --git a/dincli/abis/DINTaskAuditor.json b/dincli/abis/DINTaskAuditor.json index 38149e2..14b5166 100644 --- a/dincli/abis/DINTaskAuditor.json +++ b/dincli/abis/DINTaskAuditor.json @@ -1940,12 +1940,6 @@ "type": "uint256", "indexed": true, "internalType": "uint256" - }, - { - "name": "bondForfeited", - "type": "uint256", - "indexed": false, - "internalType": "uint256" } ], "anonymous": false diff --git a/foundry/src/DINTaskAuditor.sol b/foundry/src/DINTaskAuditor.sol index b3f6455..bc6d9e9 100644 --- a/foundry/src/DINTaskAuditor.sol +++ b/foundry/src/DINTaskAuditor.sol @@ -283,7 +283,9 @@ contract DINTaskAuditor is Ownable, ReentrancyGuardTransient { } 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 @@ contract DINTaskAuditor is Ownable, ReentrancyGuardTransient { 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 @@ contract DINTaskAuditor is Ownable, ReentrancyGuardTransient { 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 @@ contract DINTaskAuditor is Ownable, ReentrancyGuardTransient { 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 @@ contract DINTaskAuditor is Ownable, ReentrancyGuardTransient { 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 @@ contract DINTaskAuditor is Ownable, ReentrancyGuardTransient { _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. diff --git a/foundry/test/EncryptedTestData.t.sol b/foundry/test/EncryptedTestData.t.sol index 1c3a48b..23adfb3 100644 --- a/foundry/test/EncryptedTestData.t.sol +++ b/foundry/test/EncryptedTestData.t.sol @@ -29,7 +29,8 @@ import {DinValidatorStake} from "../src/DinValidatorStake.sol"; import {DINModelRegistry} from "../src/DINModelRegistry.sol"; import {DINTaskCoordinator} from "../src/DINTaskCoordinator.sol"; import {DINTaskAuditor} from "../src/DINTaskAuditor.sol"; -import {GIstates, TA_NoCommitmentStored, TA_DisputeAlreadyActive, TA_NoActiveDispute, TA_DisputeWindowClosed} from "../src/DINShared.sol"; +import {GIstates, TA_NoCommitmentStored, TA_DisputeAlreadyActive, TA_NoActiveDispute, TA_DisputeWindowClosed, TA_NotAssignedAuditor} from "../src/DINShared.sol"; +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; contract EncryptedTestDataTest is Test { DinToken tokenImpl; @@ -52,7 +53,11 @@ contract EncryptedTestDataTest is Test { address auditor3 = makeAddr("auditor3"); address client1 = makeAddr("client1"); address client2 = makeAddr("client2"); - address disputer = makeAddr("disputer"); + // Issue #205: only an auditor of the disputed batch can open a dispute, so + // `disputer` is set to batch 0's first auditor by _assignBatch0. `outsider` + // holds no role in the batch. + address disputer; + address outsider = makeAddr("outsider"); // Fixed test vectors — not real crypto, just values we control for // commitment reconstruction in tests. @@ -208,6 +213,7 @@ contract EncryptedTestDataTest is Test { commitment = _buildCommitment(1, 0, TEST_K, TEST_PLAINTEXT_HASH); vm.prank(modelOwner); ta.assignAuditTestDataset(1, 0, TEST_ENC_CID, keys, commitment); + disputer = auditors[0]; } /// @dev Opens a dispute on batch 0 of GI 1 with `from` as disputer. @@ -253,7 +259,8 @@ contract EncryptedTestDataTest is Test { function test_openDispute_noCommitment_reverts() public { _runToAuditorBatchesCreated(); - vm.prank(disputer); + (, address[] memory auditors,,) = ta.getAuditorsBatch(1, 0); + vm.prank(auditors[0]); vm.expectRevert(TA_NoCommitmentStored.selector); ta.openTestDataDispute(1, 0); } @@ -326,9 +333,9 @@ contract EncryptedTestDataTest is Test { uint256 poolBefore = ta.giRewardPool(1); uint256 disputerBefore = token.balanceOf(disputer); - // Wrong K — commitment won't match → dispute upheld + // The owner reveals a K that doesn't match its own commitment -> upheld. bytes memory wrongK = abi.encodePacked(bytes32(uint256(0xBAD))); - vm.prank(disputer); + vm.prank(modelOwner); ta.resolveTestDataDispute(1, 0, wrongK, TEST_PLAINTEXT_HASH); assertEq(token.balanceOf(disputer), disputerBefore + BOND, "bond should be returned to disputer"); @@ -346,7 +353,7 @@ contract EncryptedTestDataTest is Test { _openDispute(disputer); bytes memory wrongK = abi.encodePacked(bytes32(uint256(0xBAD))); - vm.prank(disputer); + vm.prank(modelOwner); ta.resolveTestDataDispute(1, 0, wrongK, TEST_PLAINTEXT_HASH); // pendingReassignment is set — opening another dispute should revert @@ -385,7 +392,7 @@ contract EncryptedTestDataTest is Test { // keccak256(abi.encodePacked(1, 0, keccak256(TEST_K), wrongPlaintext)) // which won't match → upheld. bytes memory wrongK = abi.encodePacked(bytes32(uint256(0xBADBAD))); - vm.prank(disputer); + vm.prank(modelOwner); ta.resolveTestDataDispute(1, 0, wrongK, TEST_PLAINTEXT_HASH); assertTrue(_disputePending(1, 0)); } @@ -409,24 +416,96 @@ contract EncryptedTestDataTest is Test { ta.resolveTestDataDispute(1, 0, TEST_K, TEST_PLAINTEXT_HASH); } - function test_closeExpiredDispute_forfeitsBond() public { + /// @dev Issue #205: an unanswered dispute is upheld, not forfeited -- + /// silence counts against the party who holds the data. + function test_closeExpiredDispute_ownerSilent_upholds() public { _runToAuditorBatchesCreated(); _assignBatch0(); vm.prank(modelOwner); ta.setDisputeBondAmount(BOND); _openDispute(disputer); - uint256 expires = _disputeExpires(1, 0); - uint256 supplyBefore = token.totalSupply(); - uint256 treasuryBefore = ta.treasuryAccrued(); + uint256 poolBefore = ta.giRewardPool(1); + uint256 disputerBefore = token.balanceOf(disputer); + vm.roll(_disputeExpires(1, 0) + 1); - vm.roll(expires + 1); + vm.expectEmit(true, true, false, true, address(ta)); + emit DINTaskAuditor.DisputeExpired(1, 0); + vm.prank(outsider); // permissionless ta.closeExpiredDispute(1, 0); - // No slash treasury set — both halves burned; counter tracks full forfeited amount. - assertEq(token.totalSupply(), supplyBefore - BOND, "full bond burned when no slash treasury set"); - assertEq(ta.treasuryAccrued(), treasuryBefore + BOND, "treasuryAccrued tracks full forfeited bond"); + assertEq(token.balanceOf(disputer), disputerBefore + BOND, "bond returned to disputer"); + uint256 expectedPenalty = (poolBefore * ta.disputePenaltyBps()) / 10000; + assertEq(ta.giRewardPool(1), poolBefore - expectedPenalty, "owner's reward pool penalised"); assertFalse(_disputeActive(1, 0)); + assertTrue(_disputePending(1, 0), "batch flagged for reassignment"); + } + + function test_closeExpiredDispute_beforeWindowEnds_reverts() public { + _runToAuditorBatchesCreated(); + _assignBatch0(); + _openDispute(disputer); + + vm.expectRevert(TA_DisputeWindowClosed.selector); + ta.closeExpiredDispute(1, 0); + } + + // ───────────────────────────────────────────────────────────────────────── + // Issue #205: nobody but the owner can resolve, nobody but a batch + // auditor can open, and the bond is non-zero by default. + // ───────────────────────────────────────────────────────────────────────── + + function test_defaultDisputeBond_isNonZero() public { + _runToAuditorBatchesCreated(); + assertEq(ta.disputeBondAmount(), 100 * 1e18); + } + + function test_openDispute_nonBatchAuditor_reverts() public { + _runToAuditorBatchesCreated(); + _assignBatch0(); + _fundDin(outsider, BOND + 10 ether); + + vm.prank(outsider); + vm.expectRevert(TA_NotAssignedAuditor.selector); + ta.openTestDataDispute(1, 0); + } + + function test_resolveDispute_nonOwner_reverts() public { + _runToAuditorBatchesCreated(); + _assignBatch0(); + _openDispute(disputer); + + bytes memory junkK = abi.encodePacked(bytes32(uint256(0xBAD))); + vm.prank(disputer); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, disputer)); + ta.resolveTestDataDispute(1, 0, junkK, bytes32(0)); + assertTrue(_disputeActive(1, 0), "dispute still open for the owner to answer"); + } + + /// @dev The #205 failure scenario: an outsider with no DIN and no role, at + /// the old zero default bond, upheld disputes with a junk K and burned + /// 25% of the GI pool per round. Neither step is reachable now: the + /// outsider can't open, and a batch auditor who opens can't resolve. + function test_issue205_junkKeyDrain_isClosed() public { + _runToAuditorBatchesCreated(); + _assignBatch0(); + vm.prank(modelOwner); + ta.setDisputeBondAmount(0); // the old default: free attempts + uint256 poolBefore = ta.giRewardPool(1); + bytes memory junkK = abi.encodePacked(bytes32(uint256(0xBAD))); + + vm.prank(outsider); + vm.expectRevert(TA_NotAssignedAuditor.selector); + ta.openTestDataDispute(1, 0); + + vm.prank(disputer); + ta.openTestDataDispute(1, 0); + vm.prank(disputer); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, disputer)); + ta.resolveTestDataDispute(1, 0, junkK, bytes32(0)); + + assertEq(ta.giRewardPool(1), poolBefore, "pool untouched"); + assertFalse(_disputePending(1, 0), "batch not blocked"); } // ───────────────────────────────────────────────────────────────────────── @@ -442,7 +521,7 @@ contract EncryptedTestDataTest is Test { // Uphold dispute to set pendingReassignment bytes memory wrongK = abi.encodePacked(bytes32(uint256(0xBAD))); - vm.prank(disputer); + vm.prank(modelOwner); ta.resolveTestDataDispute(1, 0, wrongK, TEST_PLAINTEXT_HASH); assertTrue(_disputePending(1, 0)); diff --git a/foundry/test/TreasuryForwarding.t.sol b/foundry/test/TreasuryForwarding.t.sol index f3256b1..1674341 100644 --- a/foundry/test/TreasuryForwarding.t.sol +++ b/foundry/test/TreasuryForwarding.t.sol @@ -568,14 +568,15 @@ contract TreasuryForwardingTest is Test { ta.assignAuditTestDataset(2, 0, TEST_ENC_CID, keys, commitment); } - /// @dev Funds and stakes the disputer, sets bond, opens dispute on gi=2 batch=0. + /// @dev Sets the bond and opens a dispute on gi=2 batch=0 from that batch's + /// first auditor (issue #205: only a batch auditor can open one). function _openTestDataDispute() internal returns (uint256 bond) { vm.prank(modelOwner); ta.setDisputeBondAmount(DISPUTE_BOND); bond = ta.disputeBondAmount(); - address disputer = makeAddr("disputer"); - _fundAndStake(disputer); + (, address[] memory batchAuditors,,) = ta.getAuditorsBatch(2, 0); + address disputer = batchAuditors[0]; _fundDin(disputer, bond); vm.startPrank(disputer); token.approve(address(ta), type(uint256).max); @@ -638,8 +639,9 @@ contract TreasuryForwardingTest is Test { assertEq(taBefore - token.balanceOf(address(ta)), bond + penalty, "task contract retains none of bond + penalty"); } - /// closeExpiredDispute burns 50% and sends 50% to treasury. - function test_closeExpiredDispute_burns50pct_sends50pctToTreasury() public { + /// closeExpiredDispute (owner silent) upholds the dispute (issue #205): the + /// bond goes back to the disputer and the penalty is burned 50% / sent 50%. + function test_closeExpiredDispute_upheld_penaltyBurns50pct_sends50pctToTreasury() public { _runToTestDataAssigned(); uint256 bond = _openTestDataDispute(); @@ -650,6 +652,7 @@ contract TreasuryForwardingTest is Test { (, , uint256 expiresAtBlock,,) = ta.testDataDisputes(2, 0); vm.roll(expiresAtBlock + 1); + uint256 penalty = (ta.giRewardPool(2) * ta.disputePenaltyBps()) / 10000; uint256 supplyBefore = token.totalSupply(); uint256 treasuryBefore = token.balanceOf(treasury); uint256 accruedBefore = ta.treasuryAccrued(); @@ -659,10 +662,10 @@ contract TreasuryForwardingTest is Test { uint256 burned = supplyBefore - token.totalSupply(); uint256 treasuryReceived = token.balanceOf(treasury) - treasuryBefore; - assertEq(burned, bond / 2, "50% burned on expired dispute"); - assertEq(treasuryReceived, bond - bond / 2, "50% to treasury on expired dispute"); - assertEq(ta.treasuryAccrued() - accruedBefore, bond, "treasuryAccrued delta == bond"); - assertEq(taBefore - token.balanceOf(address(ta)), bond, "task contract retains none of the bond"); + assertEq(burned, penalty / 2, "50% of penalty burned on expired dispute"); + assertEq(treasuryReceived, penalty - penalty / 2, "50% of penalty to treasury on expired dispute"); + assertEq(ta.treasuryAccrued() - accruedBefore, penalty, "treasuryAccrued delta == penalty"); + assertEq(taBefore - token.balanceOf(address(ta)), bond + penalty, "bond refunded + penalty routed out"); } }