docs(tasks-plan): umermjd11 task plan 051026-1 — contract follow-ups: DINTaskAuditor size (#201 A4), #180 dual-role guard, #193 cross-model S5

[umermjd11]

Task plan for review: Developer/tasks-plan/umermjd11/task-plan-051026-1.md. It's written against develop @ 740a613. Same flow as before: review here, I apply amendments, then it's forwarded as one tasks/ spec.

Narrowed to the contract follow-ups. The dincli, integration-suite and docs items moved to the companion plan #229 (task-plan-051026-2: contracts check + richer GI states → dincli → full-GI suite on foundry → docs). The plan has a section on how the two coordinate.

#	Item	Summary	Estimate
TP-1	#201 Part A item 4	DINTaskAuditor size review, changing getter visibility only. Prototype: −320 B for the recommended set of four, which moves the margin from 1,858 to 2,178 B (out of the warn band). Up to −666 B if all seven go. The getters plan 2's commands read stay public	1 d
TP-2	#180	Per-address cross-role guard in registerDINAuditor. Aggregators always register first, so the guard goes on the auditor side. +103 B, measured. Also the amendment-4 NatSpec fix	0.5 d
TP-3	#193	Two-level S5: the per-slasher ring stays, plus a global per-validator ring keyed on block.timestamp. Timestamps are monotonic, so the trim stays safe. Also closes the S1/S2 split within one model	1.5 d

Decisions requested:

1. The TP-1 getter set.
2. For security: dual-role registration allows one operator to control both the auditor filter and T1 aggregator in the same GI #180: the guard, or relying on stake cost.
3. For security: S5 recidivism is per-model, so a validator can spread missed votes across models and never escalate #193: option A, B or C, and the defaults.

PR #218 (P3Adversarial.t.sol) encodes Row 6 and Row 11 as known gaps. Whichever lands second flips them; see Sequencing.

Size figures come from scratch builds on 740a613 (via_ir, 200 runs). Docs-only PR. check_doc_links.py Developer is green, and it merges cleanly with #229.

🤖 Generated with Claude Code

[umermjd11]

Split into two plans:

• This PR (task-plan-051026-1) is now contract follow-ups only: TP-1 DINTaskAuditor size (Contract size headroom for all contracts (DINTaskCoordinator −9 B over EIP-170 with PR No. 197) + slash reason for committed-but-unrevealed across commit-reveal flows #201 A4), TP-2 security: dual-role registration allows one operator to control both the auditor filter and T1 aggregator in the same GI #180 guard, TP-3 security: S5 recidivism is per-model, so a validator can spread missed votes across models and never escalate #193 S5. About 3 days.
• docs(tasks-plan): umermjd11 task plan 051026-2 — full-GI alignment: contracts check + richer GI states, dincli commands, integration suite on foundry, GI docs #229 (task-plan-051026-2) takes the former TP-4, TP-5 and TP-6 and widens them into full-GI alignment, in this order: contracts check plus richer GI states (AuditSeedLocked, AuditTestDataAssigned, AggSeedLocked) → dincli commands → tests/dincli running a complete GI on the foundry contracts → GI docs.
• Integration-suite CI is a separate follow-up: ci: run the tests/dincli full-GI integration suite in a separate CI job #228.

Decisions 4–5 moved to #229 with the items they belonged to. Each plan has a coordination section (getter set, DINShared.sol, the DINTaskAuditor ABI, #180 vs the suite's accounts). The two branches merge cleanly together.

🤖 Generated with Claude Code

[umeradl]

Review: deep verification

Reviewed against develop in an isolated worktree at PR head a9b5f27 (merge-base 740a613). develop has moved to a1fcce2 since (PR No. 217, which only touches Developer/tasks/task_021026_19.md). This PR adds a new file, so git merge-tree --write-tree is clean locally, and GitHub reports mergeable: MERGEABLE, mergeStateStatus: CLEAN. File:line references were re-resolved at 740a613. The size claims were re-measured by prototyping TP-1 and TP-2 on a real via_ir build of develop's foundry/.

1. Status check and references

Verified:

• No. 180: registerDINAuditor (DINTaskAuditor.sol:667) and registerDINaggregator (DINTaskCoordinator.sol:401) have no cross-role check. isDINAggregator is a public mapping on the coordinator (:34). The states really are ordered aggregator registration (6–7) before auditor registration (8–9) (DINShared.sol:17-20), so the auditor side is the only place the guard can fire. ✅
• Stale NatSpec: "not yet enforced" at DinValidatorStake.sol:451 and :462, and "decremented at endGI time" at :121. ✅
• No. 193: _partialSlashGIs is mapping(address => mapping(address => uint256[])) keyed by [validator][msg.sender] (:146, comment :136-146); slashPartial at :294; setS5RecidivismParams at :583; uint256[50] private __gap at :174. ✅
• The S5 discrepancy: the threshold branch does _applySlash(validator, MIN_STAKE, "S5_RECIDIVISM") + _jailInternal(validator, s5JailDuration, …), with s5JailDuration = 7 days at init. Developer/design/MECHANISM_DESIGN.md:81 defines S5 per validator ("Both" roles), and :87 says "entire slashable stake (active + unbonding) + blacklist". ✅ The path is Developer/design/MECHANISM_DESIGN.md; the plan writes MECHANISM_DESIGN.md.
• Threat model: Row 6 and Row 11 are both KNOWN GAP, and Judgment call 2 rules dual-role not intended. Row 6 still cites DINTaskCoordinator.sol:386 / DINTaskAuditor.sol:660, which TP-2 says it will refresh. ✅

2. TP-1 getter readers

DINTaskAuditor has 16 public mappings, as stated. Repo-wide readers outside the contract (foundry tests/scripts and Python, excluding dincli/abis/):

Getter	Plan says	Found
auditBatches	1 foundry test	0. The only hit is a comment (RewardEngine.t.sol:895). No test calls .auditBatches(
dinAuditors	none	none ✅
Is_testdataCIDs_Assigned	none	none ✅. It's written at :1028 and only read by its own double-set guard at :1026
auditorGIWeight	none	none ✅
rewardClaimed	none	none ✅ (keep for plan 2's claim command)
testDataDisputes	2 foundry tests	2 ✅
giRewardSnapshot	2 foundry tests	2 ✅

So the "switch the one foundry test from auditBatches to getAuditorsBatch" step has nothing to switch.

3. Sizes, prototyped

Verified — matches the plan to the byte, or within 1 B. Scratch edits on 740a613's foundry/, via_ir, 200 runs, runtime from out/DINTaskAuditor.sol/DINTaskAuditor.json:

Variant	Plan	Measured	Margin
develop	22,718 B	22,718 B	1,858 B (warn)
TP-1 four getters → internal	−320 B → 22,398 B	22,398 B	2,178 B ✅ out of the warn band
TP-2 guard alone (isDINAggregator in IDINTaskCoordinator, new TA_DualRoleNotAllowed, check in registerDINAuditor)	+103 B → 22,821 B	22,820 B (+102)	1,756 B
TP-1 + TP-2	about 22,501 B	22,500 B	2,076 B ✅ still out of the warn band

So the recommended four plus the guard leave DINTaskAuditor 28 B above the 2,048 B warn line. That's thin: plan 2's commitment check (§4) would likely push it back into the warn band (still far from the 1,024 B fail line). The coordinator is unchanged by both TPs. All scratch edits were reverted, and the worktree is clean.

4. Interplay

• PR No. 218 (P3Adversarial.t.sol, reviewed separately) has test_knownGap_dualRoleRegistration (Row 6) and test_knownGap_perSlasherS5Evasion (Row 11). The flip rule in Sequencing covers both. A note from that review: Row 11's test calls slashPartial directly as each task contract, which is exactly the path TP-3 changes, so it's a good regression check for option A.
• task-plan-051026-2 (PR No. 229): its review asks TP-2 there to make setTestDataAssignedFlag check every batch's stored commitment, so that AuditTestDataAssigned means something. That adds bytes to DINTaskAuditor as well, so the two plans' auditor budgets should be added up in one place.

5. Discussion No. 216 follow-up

Discussion No. 216 closed task_021026_19, but Developer/tasks/task_021026_19.md still says **Status:** Open (assigned) on develop. Neither PR No. 217 nor the closing touched it. This plan links that task as its "Previous plan", so its forwarding PR is the natural place to set it to done.

---

Amendments

1. task_021026_19 status: in the forwarding PR (the one that adds this plan's task_DDMMYY_n.md), set Developer/tasks/task_021026_19.md's **Status:** to closed/complete, with a link to Discussion No. 216.
2. TP-1 readers: auditBatches has no test reader (only a comment at RewardEngine.t.sol:895). Drop the "switch the one foundry test" step and fix the table cell.
3. TP-1 sizes: the measured numbers confirm the table (22,398 B; guard +102 B; together 22,500 B, 2,076 B margin). Note in TP-1 that this leaves only 28 B above the warn line, so the extra fold review (dispute/reassignment paths) is worth measuring, not optional.
4. TP-1 budget: add a line for plan 2's commitment check (see §4), so the auditor's margin after both plans is known before either merges.
5. Paths: MECHANISM_DESIGN.md → Developer/design/MECHANISM_DESIGN.md (TP-3 and the status table).

Everything else checks out at 740a613. check_doc_links.py Developer resolves, and git diff --check is clean. The reviewer decisions are in a separate comment.

[umeradl]

Files changed (1) — as of a9b5f27 (PR head)

Diffed against merge-base 740a613 (develop). develop has moved to a1fcce2 since (PR No. 217, Developer/tasks/task_021026_19.md only), and that doesn't overlap this PR's new file. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: CLEAN. A local git merge-tree --write-tree dry run is clean (exit 0).

Developer/tasks-plan/umermjd11/task-plan-051026-1.md

Field	Value
Change	New
Lines	+173/-0
Diff (what exactly is in this PR)	A three-item contract plan: TP-1 DINTaskAuditor size review (issue No. 201 Part A item 4: four getters → internal, plus a fold review), TP-2 the issue No. 180 dual-role guard in registerDINAuditor, TP-3 the issue No. 193 two-level S5 (a per-validator timestamp ring alongside the per-slasher GI ring). It also has a status check at 740a613, a sequencing table, coordination with task-plan-051026-2 (PR No. 229), three decisions, and a deferred list.
Functionality — how & why	How: TP-1 changes visibility only, with no storage, event or state-changing changes. TP-2 is one external view call on the coordinator's existing isDINAggregator mapping, placed on the auditor side because aggregator registration always comes first. TP-3 appends storage before __gap and escalates when either ring reaches its threshold. Why: DINTaskAuditor sits in the CI warn band (1,858 B margin) and later work adds bytes. Dual-role registration was ruled not intended (PR No. 182, Judgment call 2). S5 is defined per validator across roles and models but counted per slasher contract. Sizes were re-measured: 22,398 B with TP-1, 22,820 B with the guard alone, 22,500 B together.
Diff vs current develop HEAD	None — new file
Recommended merge proposal	Merge after the amendments in the verification comment. (1) The forwarding PR sets task_021026_19's **Status:** to closed (Discussion No. 216). (2) auditBatches has no test reader, so drop the test-switch step. (3) Note the thin 28 B above the warn line after TP-1 + TP-2, and add plan 2's commitment-check bytes to the budget. (4) Fix the Developer/design/MECHANISM_DESIGN.md path.
Actual merge proposal	Soon
Pending proposal	Decisions 1–3 (separate comment), then forwarding as one Developer/tasks/task_DDMMYY_n.md.
Local merge conflict	No
GitHub merge conflict	No

Verification

Full detail is in the verification comment above. In short: all file:line references resolve at 740a613. The getter reader counts match except auditBatches (0, not 1). Prototype sizes on a real via_ir build: TP-1 −320 B, guard +102 B, together 22,500 B (2,076 B margin). The S5 storage, __gap and the MECHANISM_DESIGN.md discrepancy are confirmed. check_doc_links.py Developer resolves. GitHub CI is green.

Local vs. GitHub agree: both report a clean merge.

[umeradl]

Reviewer decisions: options and recommendations

These are the plan's three "Decisions requested". Each lists the options and a recommendation, using the sizes measured in the verification comment above. @umermjd11, answer per decision; the plan gets amended to match.

Decision 1: TP-1 getter set

Option	Effect
A. The recommended four (auditBatches, dinAuditors, Is_testdataCIDs_Assigned, auditorGIWeight)	Clears the 2,048 B warn band with room for TP-2's guard. All four have no external reader: auditBatches' "1 foundry test" is only a comment (RewardEngine.t.sol:895), and Is_testdataCIDs_Assigned is written but never read. dincli and plan 2 aren't affected.
B. All seven	About twice the saving, but rewardClaimed, testDataDisputes and giRewardSnapshot are exactly what plan 2's claim and dispute commands read, and testDataDisputes/giRewardSnapshot are read by 2 test files each. Replacing them with views gives some of the bytes back and adds churn.
C. A, plus the commitment check from plan 2's verification amendment 5	Plan 2's TP-2 should make setTestDataAssignedFlag check every batch's commitment before AuditTestDataAssigned means anything. That costs DINTaskAuditor bytes too, so the budget should be planned across both plans in one place.

Recommended: A, with C's accounting. Report one size table with TP-1, then TP-1 + TP-2, plus an estimate for plan 2's commitment check. If Is_testdataCIDs_Assigned is still needed after plan 2, keep it internal.

Decision 2: No. 180 dual-role registration

Option	Effect
A. Per-address guard in registerDINAuditor (as proposed)	One external view call (isDINAggregator, already a public mapping on the coordinator). Because aggregator registration (states 6–7) always comes before auditor registration (8–9), the auditor side is the only place a guard can fire. A Sybil with a second stake gets around it, but the attack becomes costly and visible.
B. Document stake cost as the defence and set a non-zero concurrency cap	No contract change, but it depends on a No. 155 value and doesn't block the simple case.

Recommended: A. It matches the PR No. 182 ruling (dual-role not intended) and threat-model Judgment call 2. Include the three tests listed, and flip PR No. 218's test_knownGap_dualRoleRegistration to test_defended_… in whichever PR lands second.

Decision 3: No. 193 cross-model S5

Option	Effect
A. Two-level: keep the per-slasher GI ring, add a per-validator timestamp ring (as proposed)	Per-model behaviour and tests stay the same. The cross-model gap and the S1/S2 split within one model both close. Two new owner-settable params, plus new storage taken from __gap (50 slots at DinValidatorStake.sol:174).
B. Replace with the global timestamp ring only	Simpler, but S5 becomes "per T seconds" for every model, which treats models with different GI cadences differently. It also changes the existing S5 tests' meaning.
C. Accept and document	No code. Row 11 stays a KNOWN GAP and becomes a trust assumption.

Recommended: A. Confirm defaults of 7 days / threshold 6 as placeholders until No. 155. For upgraded proxies, prefer "0 means off" over a reinitializer: Sepolia is DevNet 1.0, nothing on develop is deployed, and the DevNet 2.0 deploy is a fresh DeployPlatform.s.sol run, so initialize sets the defaults. The UpgradeValidation.t.sol storage-layout check must stay green. Keep the MECHANISM_DESIGN.md:87 discrepancy (doc: entire stake + blacklist; code: MIN_STAKE + s5JailDuration jail) as a note, as the plan says. The code really does _applySlash(validator, MIN_STAKE, …) plus _jailInternal(…, s5JailDuration), with a 7-day default.