fix(task-auditor): DINTaskAuditor size review (#201 A4) + dual-role registration guard (#180)

Developer/design/adversarial-threat-model.md

@@ -80,9 +80,9 @@ entry in `foundry/test/P3Adversarial.t.sol`.
 |---|---|
 | **Attacker** | One address registered as both auditor and aggregator in the same GI; model owner tries to steer the batch-assignment draw (formerly by choosing which block to submit batch-creation calls in) |
 | **Capability** | One address (auditor + aggregator), staking MIN_STAKE. Before issue #156 H-2 (PR #191) the owner chose which block `autoCreateTier1AndTier2` and `createAuditorsBatches` were submitted in, picking a `blockhash(block.number - 1)` that routed the poisoned model into the colluding T1 batch. Both now read a locked, future-block-anchored `aggSeed`/`auditSeed` (see Row 2), so that lever is closed; what remains is the BL-26 re-roll (decline to lock, wait out the ~256-block window) and TA-4 |
-| **Protocol response today** | Nothing prevents dual-role registration: `registerDINaggregator` (`DINTaskCoordinator.sol:386`) and `registerDINAuditor` (`DINTaskAuditor.sol:660`) have no cross-role check. Both increment `activeRegistrationCount`, so the concurrent-registration cap only raises the minimum stake required (≥ 2 concurrent registrations' worth) — it does not block. The cap is also off by default (`maxConcurrentRegistrationsPerStakeUnit == 0`, `DINTaskCoordinator.sol:406`). Note: a per-address cross-role guard is Sybil-bypassable (second address with its own stake), so the stake cost may be the only practical defence in any case. |
-| **Expected test outcome** | **KNOWN GAP** ([#180](https://github.com/InfiniteZeroFoundation/DevNet/issues/180); the [#156](https://github.com/InfiniteZeroFoundation/DevNet/issues/156) block-selection half is fixed, residuals in BL-26) — the test registers one address in both roles for the same GI and asserts it succeeds. The "colluding aggregators skip outlier clipping" step is off-chain (model-dependent, not testable in Forge). |
-| **Trust assumption** | None that defends this — gap pending #180 (role-separation or stake-cost documentation) and #156 (stronger batch-shuffle seed). |
+| **Protocol response today** | One address can no longer hold both roles in a GI: `registerDINAuditor` (`DINTaskAuditor.sol:674`) reverts with `TA_DualRoleNotAllowed` when the caller is already an aggregator for that GI, read from the coordinator's `isDINAggregator` (#180). The check sits on the auditor side because aggregator registration (`registerDINaggregator`, `DINTaskCoordinator.sol:401`; states 6–7) always comes before auditor registration (8–9). It is per address: a second address with its own stake still gets both roles (Sybil), so the remaining defence is that the attack costs a second stake and is visible on-chain. The concurrent-registration cap (off by default, `maxConcurrentRegistrationsPerStakeUnit == 0`) can raise that cost further when it is non-zero. |
+| **Expected test outcome** | **DEFENDED** for the single-address case ([#180](https://github.com/InfiniteZeroFoundation/DevNet/issues/180), `StakingEnforcement.t.sol` `test_registerDINAuditor_aggregatorSameGI_reverts`); the Sybil variant is documented by `test_registerDINAuditor_secondAddressWithOwnStake_registers`. The [#156](https://github.com/InfiniteZeroFoundation/DevNet/issues/156) block-selection half is fixed, residuals in BL-26. The "colluding aggregators skip outlier clipping" step is off-chain (model-dependent, not testable in Forge). PR #218's `test_knownGap_dualRoleRegistration` flips to `test_defended_…` when it rebases on this change. |
+| **Trust assumption** | Operators don't split one stake-holder across two funded addresses to take both roles (Sybil); the cost is a second stake. Batch-shuffle residuals as in BL-26. |
 
 ---
 
@@ -164,7 +164,7 @@ entry in `foundry/test/P3Adversarial.t.sol`.
 |---|---|---|---|
 | S3 graduating from shadow mode | [#38](https://github.com/InfiniteZeroFoundation/DevNet/issues/38) | Row 4 (auditor bloc poisoned model) | `s3SlashingEnabled` set to `true` after threshold calibration |
 | T1 wrong CID — finalCID not replaced, dissenter not refunded | [#194](https://github.com/InfiniteZeroFoundation/DevNet/issues/194) (#181 covers who adjudicates, not repairing the outcome) | Row 5 (T1 wrong CID) | `settleRecomputation(confirmed=true)` replaces `finalCID` and refunds the honest dissenter |
-| Dual-role registration guard | [#180](https://github.com/InfiniteZeroFoundation/DevNet/issues/180) | Row 6 (cross-role + owner block-selection) | Per-address role-separation guard added, or the stake cost documented as the intended defence (with a non-zero concurrent-registration cap) |
+| Dual-role registration guard | [#180](https://github.com/InfiniteZeroFoundation/DevNet/issues/180) | Row 6 (cross-role + owner block-selection) | **Flipped by task_061026_20 Part B:** per-address guard in `registerDINAuditor` (`TA_DualRoleNotAllowed`). The Sybil variant (second funded address) remains, bounded by stake cost |
 | Stronger batch-shuffle seed | [#156](https://github.com/InfiniteZeroFoundation/DevNet/issues/156) | Rows 2, 6 (owner block-selection) | **Flipped by PR #191:** shuffles seeded by the locked, future-block `aggSeed`/`auditSeed` rather than at owner-controlled call time. Residuals (decline-to-lock re-roll, post-lock pool reshaping) tracked in BL-26 |
 | Decentralized dispute adjudication | [#181](https://github.com/InfiniteZeroFoundation/DevNet/issues/181) | Row 7 (owner suppresses dispute) and bond-locked-forever variant | `resolveDispute` gated by multi-sig, DAO committee, or fraud-proof; timeout added for unresolved disputes |
 | Dispute timeout for unresolved disputes | [#181](https://github.com/InfiniteZeroFoundation/DevNet/issues/181) | Row 7 (owner never resolves → bond locked) | `expireDispute` (or a new function) reachable when `d.resolved == false` after a deadline |
@@ -179,5 +179,5 @@ entry in `foundry/test/P3Adversarial.t.sol`.
 1. **Row 5 — T1 wrong CID: KNOWN GAP.**
    `settleRecomputation(confirmed=true)` does not replace `finalCID` and does not refund the honest dissenter's `AGG_T1_BAD_CONSENSUS` slash, so the attack partially succeeds regardless of TA-1. COST-BOUNDED was rejected because it would pass a test while the honest dissenter is never made whole. Test: `test_knownGap_t1WrongCID`.
 
-2. **Row 6 — Dual-role registration: KNOWN GAP.**
-   Not accepted as intended; [#180](https://github.com/InfiniteZeroFoundation/DevNet/issues/180) stays open. A per-address cross-role guard is Sybil-bypassable (a second address with its own stake), so #180 decides between adding the guard and documenting the stake cost as the defence. The concurrent-registration cap only bounds that cost when it is non-zero.
+2. **Row 6 — Dual-role registration: DEFENDED (single address).**
+   Not accepted as intended. [#180](https://github.com/InfiniteZeroFoundation/DevNet/issues/180) chose the per-address guard (PR #219 Decision 2): `registerDINAuditor` refuses an address already registered as an aggregator for the GI. It is Sybil-bypassable (a second address with its own stake), which the tests document; the concurrent-registration cap bounds that cost further only when it is non-zero.

Documentation/technical/contracts/DINShared.md

@@ -158,6 +158,7 @@ Used by: `DINTaskCoordinator`, `DINTaskAuditor`
 interface IDINTaskCoordinator {
     function GI() external view returns (uint256);
     function GIstate() external view returns (GIstates);
+    function isDINAggregator(uint256 gi, address aggregator) external view returns (bool);
     function aggregatorWeight(uint256 gi, address aggregator) external view returns (uint256);
 }
 ```
@@ -168,6 +169,7 @@ Used by: `DINTaskAuditor`
 |--------|---------|
 | `GI()` | Current GI counter, for `onlyCurrentGI` and `depositRewards` validation |
 | `GIstate()` | Lifecycle gate for every auditor-side phase |
+| `isDINAggregator` | Read by `registerDINAuditor` to refuse an address already registered as an aggregator for the GI (issue No. 180) |
 | `aggregatorWeight` | Read at `claimReward` time to compute each aggregator's weighted share |
 
 ### 3.3 `IDINTaskAuditor`
@@ -227,6 +229,7 @@ Used by: `DINTaskCoordinator`
 | `TA_AuditorRegistrationNotOpen` | Registration attempted outside registration window |
 | `TA_WrongGI` | Global Iteration mismatch |
 | `TA_AuditorAlreadyRegistered` | Duplicate auditor registration for same GI |
+| `TA_DualRoleNotAllowed` | `registerDINAuditor` by an address already registered as an aggregator for the same GI (issue No. 180) |
 | `TA_LMSubmissionsNotOpen` | Local model submission attempted outside submission window |
 | `TA_AlreadySubmitted` | Client has already submitted a model this GI |
 | `TA_MaxLMSubmissionsReached` | Submission count reached `MAX_LM_SUBMISSIONS` (10,000) |

Documentation/technical/contracts/DINTaskAuditor.md

@@ -77,12 +77,14 @@ The reward split and the S1 fraction are explicitly provisional (MECHANISM_DESIG
 
 ### 3.4 Per-GI data
 
-- **Registration:** `dinAuditors[gi]`, `isRegisteredAuditor[gi][addr]`.
+Mappings marked *(internal)* have no auto-getter; read them through the view named next to them (issue No. 201 Part A item 4).
+
+- **Registration:** `dinAuditors[gi]` *(internal; read with `getDINtaskAuditors(gi)`)*, `isRegisteredAuditor[gi][addr]`.
 - **Submissions:** `lmSubmissions[gi]` (`LMSubmission`: client, modelCID, submittedAt, eligible, evaluated, approved, finalMedianScore), `clientHasSubmitted`, `clientSubmissionIndex`.
-- **Batches:** `auditBatches[gi]` (`AuditBatch`: batchId, auditors, modelIndexes, testDataCID), `isBatchAuditor`, `isBatchModelIndex`.
+- **Batches:** `auditBatches[gi]` *(internal; read with `getAuditorsBatch(gi, batchId)`)* (`AuditBatch`: batchId, auditors, modelIndexes, testDataCID), `isBatchAuditor`, `isBatchModelIndex`.
 - **Scoring:** `auditScoreCommits`, `hasCommittedLM`, `auditScores`, `LMeligibleVote`, `hasAuditedLM` (all keyed `[gi][batchId][auditor][modelIndex]`).
-- **Test data:** `encryptedTestDataKey[gi][batchId][auditor]`, `testDataCommitments[gi][batchId]`, `Is_testdataCIDs_Assigned[gi]`, `testDataDisputes[gi][batchId]`.
-- **Rewards:** `giRewardPool[gi]`, `giRewardSnapshot[gi]`, `giTotalApprovedScore[gi]`, `giTotalAuditWeight[gi]`, `auditorGIWeight[gi][addr]`, `rewardClaimed[gi][addr]`, `claimable[addr]`, `treasuryAccrued`.
+- **Test data:** `encryptedTestDataKey[gi][batchId][auditor]`, `testDataCommitments[gi][batchId]`, `Is_testdataCIDs_Assigned[gi]` *(internal; only guards `setTestDataAssignedFlag` against a second call)*, `testDataDisputes[gi][batchId]`.
+- **Rewards:** `giRewardPool[gi]`, `giRewardSnapshot[gi]`, `giTotalApprovedScore[gi]`, `giTotalAuditWeight[gi]`, `auditorGIWeight[gi][addr]` *(internal; feeds `claimReward`)*, `rewardClaimed[gi][addr]`, `claimable[addr]`, `treasuryAccrued`.
 
 ---
 
@@ -101,7 +103,7 @@ The reward split and the S1 fraction are explicitly provisional (MECHANISM_DESIG
 
 ## 5. Registration & Submissions
 
-- **`registerDINAuditor(gi)`** — requires the coordinator state `DINauditorsRegistrationStarted`. Checks: not already registered, fewer than 300 registered (`TA_RegistrationCapReached`), `isValidatorActive` (`TA_AuditorNotActive`), stake ≥ the model floor (`TA_StakeBelowModelFloor`), and the concurrent-registration cap (`TA_ConcurrentRegistrationCapReached`). Then calls `incrementActiveRegistration` and emits `DINAuditorRegistered`.
+- **`registerDINAuditor(gi)`** — requires the coordinator state `DINauditorsRegistrationStarted`. Checks: not already registered, not already an aggregator for the same GI (`TA_DualRoleNotAllowed`, read from the coordinator's `isDINAggregator`; issue No. 180), fewer than 300 registered (`TA_RegistrationCapReached`), `isValidatorActive` (`TA_AuditorNotActive`), stake ≥ the model floor (`TA_StakeBelowModelFloor`), and the concurrent-registration cap (`TA_ConcurrentRegistrationCapReached`). Then calls `incrementActiveRegistration` and emits `DINAuditorRegistered`.
 - **`submitLocalModel(cid, gi)`** — state `LMSstarted`; one submission per address per GI; up to 10 000 per GI. No stake or registration needed.
 
 ---
@@ -221,3 +223,5 @@ Read alongside the [foundry/src security review](../audits/foundry-src-security-
 - `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. Disputes can't be opened once the GI is settled (`TA_RewardsAlreadySettled`), and one upheld after settlement takes no pool penalty, so settled claims stay funded (PR No. 215 review).
+- Size review (issue No. 201 Part A item 4): `auditBatches`, `dinAuditors`, `Is_testdataCIDs_Assigned` and `auditorGIWeight` lose their auto-getters (nothing outside the contract read them; `getAuditorsBatch` / `getDINtaskAuditors` cover the first two). `assignAuditTestDataset` and `reassignAuditTestDataset` share `_storeTestData` and `_batchAt`, and `_burnAndForward` forwards its half through `_forwardToTreasury`. Behaviour and events are unchanged; runtime size drops from 22,718 B to 21,188 B.
+- Dual-role guard (issue No. 180): `registerDINAuditor` reverts with `TA_DualRoleNotAllowed` when the caller is already an aggregator for the same GI. The check is on the auditor side because aggregators always register first; it is per address, so a second address with its own stake can still take the auditor role. `IDINTaskCoordinator` gains `isDINAggregator`.

Documentation/technical/contracts/DinValidatorStake.md

@@ -309,7 +309,7 @@ Until step 8, slasher management through the coordinator reverts `ValidatorStake
 - **No. 1 — S6 is not wired:** `recordNoParticipation` exists and is tested, but no task contract calls it, so the S6 counter never moves in practice.
 - **No. 2 — S5 escalation on a blacklisted validator reverts the whole slash:** escalation calls `_jailInternal`, which reverts `ValidatorIsBlacklisted`. A task contract's slashing loop hitting a blacklisted repeat offender would revert, not just skip that validator.
 - **No. 3 — Jail exit does not require `reactivate()`:** after `jailedUntil` passes, any status-syncing call (e.g. `stake`) recomputes the status, bypassing `reactivate()`'s `StakeBelowFloor` check (the later sync still requires `≥ MIN_STAKE` for `Active`).
-- **No. 4 — Stale NatSpec:** `setModelStakeBounds` / `setMaxConcurrentRegistrationsPerStakeUnit` say "not yet enforced", but the task contracts enforce both (§8). `getModelStakeMin` says "set by the model owner", but the setter is `onlyOwner` (DIN-Representative). `modelMinStakeBounds[].max` is never read.
+- **No. 4 — Stale NatSpec (partly fixed, issue No. 180):** the `setModelStakeBounds` / `setMaxConcurrentRegistrationsPerStakeUnit` NatSpec now says where the task contracts enforce both (§8), `activeRegistrationCount` names `releaseGIRegistrationSlots` as the decrement, and `getModelStakeMin` names the `onlyOwner` (DIN-Representative) setter instead of "the model owner". Still open: `modelMinStakeBounds[].max` is never read.
 - **No. 5 — Recidivism is per model:** the S5 ring is keyed by calling contract, so a validator faulting across many models never escalates unless it hits the threshold within one model.
 - **No. 6 — Blacklisted funds are frozen:** blacklisted validators cannot unstake or claim; there is no recovery path other than unblacklisting.
 - **No. 7 — Custody meets upgradeability:** see §13.

dincli/abis/DINTaskAuditor.json

@@ -40,25 +40,6 @@
       ],
       "stateMutability": "view"
     },
-    {
-      "type": "function",
-      "name": "Is_testdataCIDs_Assigned",
-      "inputs": [
-        {
-          "name": "",
-          "type": "uint256",
-          "internalType": "uint256"
-        }
-      ],
-      "outputs": [
-        {
-          "name": "",
-          "type": "bool",
-          "internalType": "bool"
-        }
-      ],
-      "stateMutability": "view"
-    },
     {
       "type": "function",
       "name": "LMeligibleVote",
@@ -158,35 +139,6 @@
       "outputs": [],
       "stateMutability": "nonpayable"
     },
-    {
-      "type": "function",
-      "name": "auditBatches",
-      "inputs": [
-        {
-          "name": "",
-          "type": "uint256",
-          "internalType": "uint256"
-        },
-        {
-          "name": "",
-          "type": "uint256",
-          "internalType": "uint256"
-        }
-      ],
-      "outputs": [
-        {
-          "name": "batchId",
-          "type": "uint256",
-          "internalType": "uint256"
-        },
-        {
-          "name": "testDataCID",
-          "type": "bytes",
-          "internalType": "bytes"
-        }
-      ],
-      "stateMutability": "view"
-    },
     {
       "type": "function",
       "name": "auditScoreCommits",
@@ -255,30 +207,6 @@
       ],
       "stateMutability": "view"
     },
-    {
-      "type": "function",
-      "name": "auditorGIWeight",
-      "inputs": [
-        {
-          "name": "",
-          "type": "uint256",
-          "internalType": "uint256"
-        },
-        {
-          "name": "",
-          "type": "address",
-          "internalType": "address"
-        }
-      ],
-      "outputs": [
-        {
-          "name": "",
-          "type": "uint256",
-          "internalType": "uint256"
-        }
-      ],
-      "stateMutability": "view"
-    },
     {
       "type": "function",
       "name": "claimReward",
@@ -467,30 +395,6 @@
       "outputs": [],
       "stateMutability": "nonpayable"
     },
-    {
-      "type": "function",
-      "name": "dinAuditors",
-      "inputs": [
-        {
-          "name": "",
-          "type": "uint256",
-          "internalType": "uint256"
-        },
-        {
-          "name": "",
-          "type": "uint256",
-          "internalType": "uint256"
-        }
-      ],
-      "outputs": [
-        {
-          "name": "",
-          "type": "address",
-          "internalType": "address"
-        }
-      ],
-      "stateMutability": "view"
-    },
     {
       "type": "function",
       "name": "dinToken",
@@ -2559,6 +2463,11 @@
       "name": "TA_DisputeWindowClosed",
       "inputs": []
     },
+    {
+      "type": "error",
+      "name": "TA_DualRoleNotAllowed",
+      "inputs": []
+    },
     {
       "type": "error",
       "name": "TA_EmptyCommitHash",

foundry/src/DINShared.sol

@@ -112,6 +112,12 @@ interface IDINTaskCoordinator {
 
     function GIstate() external view returns (GIstates);
 
+    /// @dev Whether `aggregator` registered as an aggregator for GI `gi`
+    ///      (the coordinator's public mapping). Read by
+    ///      DINTaskAuditor.registerDINAuditor to refuse dual-role registration
+    ///      (issue No. 180).
+    function isDINAggregator(uint256 gi, address aggregator) external view returns (bool);
+
     /// @dev Per-(aggregator, finalized-batch) settlement weight for a GI,
     ///      read by DINTaskAuditor.claimReward at claim time (BL-10, #125).
     function aggregatorWeight(
@@ -364,6 +370,10 @@ error TA_RewardsNotSettled();
 ///      settled; claims pay from the snapshot, so a pool penalty could no
 ///      longer be taken without underfunding them.
 error TA_RewardsAlreadySettled();
+/// @dev registerDINAuditor was called by an address already registered as an
+///      aggregator for the same GI. One address can't hold both roles in a GI
+///      (issue No. 180); a second address with its own stake still can.
+error TA_DualRoleNotAllowed();
 /// @dev Caller already claimed their reward for this GI.
 error TA_RewardAlreadyClaimed();
 /// @dev Caller earned no reward for this GI (not a participant, or scored zero).