Skip to content

docs(tasks): add task_061026_20 (contract follow-ups) and task_061026_21 (full-GI alignment); close task_021026_19 #232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
umeradl merged 2 commits into from
Oct 6, 2026
Merged

docs(tasks): add task_061026_20 (contract follow-ups) and task_061026_21 (full-GI alignment); close task_021026_19 #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d95d946
docs(tasks): add task_061026_20 and task_061026_21 (forwarded from ta…
umermjd11 Oct 6, 2026
261e923
docs(tasks): task_061026_21 — PR No. 232 amendment 1: claimDisputeBon…
umermjd11 Oct 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Developer/tasks/task_021026_19.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
**ID:** task_021026_19
**Assigned to:** Umer Majeed (@umermjd11)
**Created:** 2026-10-02
**Status:** Open (assigned)
**Status:** Closed (completed 2026-10-05). All six Parts landed (PRs #211, #212, #214, #215; wiki revision on #207). Closed in [Discussion #216](https://github.com/InfiniteZeroFoundation/DevNet/discussions/216#discussioncomment-18761447)
**Dates:** Oct 2 – Oct 12, 2026 (about 9 working days, per-Part estimates below). Parts A+B and C can start immediately. D and E are gated on earlier PRs merging (see §Sequencing). If those slip, say so early in the tracking discussion.
**Repo:** https://github.com/InfiniteZeroFoundation/DevNet
**Base branch:** `develop` — spec written against commit `84d8c38` (2026-10-02); re-pull before starting regardless of when you begin. `foundry/`, `dincli/`, `tests/` and `.github/` are unchanged from `6ccc28c`, the commit the source plan was verified against.
Expand Down
174 changes: 174 additions & 0 deletions Developer/tasks/task_061026_20.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
# Task: Contract Follow-ups: `DINTaskAuditor` Size (#201 A4), Dual-Role Guard (#180), Cross-Model S5 (#193)

**ID:** task_061026_20
**Assigned to:** Umer Majeed (@umermjd11)
**Created:** 2026-10-06
**Status:** Open (assigned)
**Dates:** Oct 6 – Oct 9, 2026 (about 3 working days; per-Part estimates below). Parts A and C can start right away. Part B goes with Part A (same contract and byte budget). If anything slips, say so early in the tracking discussion.
**Repo:** https://github.com/InfiniteZeroFoundation/DevNet
**Base branch:** `develop`. The spec was written against `a1fcce2` (2026-10-05). The task contracts and `DinValidatorStake` are unchanged since `740a613`, where the source plan was verified. Re-pull before starting.
**Roadmap ref:** P3-6.3a (gas/size), P3-6.3b (audit preparation, open findings), P3-4.2 (penalty tiers, S5)
**Source plan:** `Developer/tasks-plan/umermjd11/task-plan-051026-1.md`, reviewed in [PR #219](https://github.com/InfiniteZeroFoundation/DevNet/pull/219) ([verification review](https://github.com/InfiniteZeroFoundation/DevNet/pull/219#issuecomment-6001172350), [reviewer decisions](https://github.com/InfiniteZeroFoundation/DevNet/pull/219#issuecomment-6001173454), [decisions applied](https://github.com/InfiniteZeroFoundation/DevNet/pull/219#issuecomment-6009815981))
**Companion task:** [task_061026_21](task_061026_21.md) (full-GI alignment). See [Coordination](#coordination-with-task_061026_21).
**Tracking:**
- **Part A:** [#201](https://github.com/InfiniteZeroFoundation/DevNet/issues/201) Part A, item 4 (the `DINTaskAuditor` review). #201 Part B stays open.
- **Part B:** [#180](https://github.com/InfiniteZeroFoundation/DevNet/issues/180) (closes, pending review).
- **Part C:** [#193](https://github.com/InfiniteZeroFoundation/DevNet/issues/193) (closes, pending review).
- [Discussion #230](https://github.com/InfiniteZeroFoundation/DevNet/discussions/230) (progress updates, questions, PR links)

---

## Status check at `a1fcce2`

| Item | State |
|---|---|
| #201 A4 | `DINTaskAuditor` is 22,718 B runtime (1,858 B margin, so the CI gate **warns**). It has 16 public mappings. Four of them either have no reader outside the contract or duplicate an existing view |
| #180 | `registerDINAuditor` (`DINTaskAuditor.sol:667-698`) and `registerDINaggregator` (`DINTaskCoordinator.sol:401-434`) have no cross-role check. The PR #182 review ruled dual-role **not** intended (`Developer/design/adversarial-threat-model.md`, Judgment call 2; Row 6 = KNOWN GAP). Stale NatSpec remains at `DinValidatorStake.sol:121`, `:451` and `:462` |
| #193 | The S5 ring is `_partialSlashGIs[validator][msg.sender]` (`DinValidatorStake.sol:146`; `slashPartial` at `:294-322`), so it is counted per slasher **contract**. That splits S1 (auditor contract) and S2 (coordinator) even within one model. `Developer/design/MECHANISM_DESIGN.md:81` defines S5 per validator, across both roles |

---

## Sequencing

| Part | PR | Touches | Conflicts with | Rule |
|---|---|---|---|---|
| A | PR 1, commit 1 | `DINTaskAuditor.sol` (getter visibility, folds), `dincli/abis/DINTaskAuditor.json`, `DINTaskAuditor.md` §3 | B (same contract and budget); task_061026_21 Part B (`DINTaskAuditor` bytes) | **Start right away** |
| B | PR 1, commit 2 | `DINTaskAuditor.registerDINAuditor`, `IDINTaskCoordinator` + new error (`DINShared.sol`), `DinValidatorStake.sol` NatSpec, tests, threat model Row 6 | A; task_061026_21 Part B (`DINShared.sol` enum) | Same PR as A, so the gate proves both fit |
| C | PR 2 | `DinValidatorStake.sol` (S5 storage + `slashPartial`), S5 tests, deploy-script S5 keys, `DinValidatorStake.md`, `MECHANISM_DESIGN.md`, threat model Row 11 | — | **In parallel with PR 1**, since it's a different contract |

**PR #218** (`P3Adversarial.t.sol`, #154 Part 2) encodes Row 6 and Row 11 as `test_knownGap_dualRoleRegistration` / `test_knownGap_perSlasherS5Evasion`. If it merges first, PR 1 and PR 2 each flip their row's test to `test_defended_…`. Otherwise PR #218 rebases and flips them. No test may be left asserting a closed gap. Row 11's test calls `slashPartial` directly as each task contract, which makes it a good regression check for Part C.

Rebase each PR on `develop` after the previous one merges; no stacked branches.

---

# Part A — #201 Part A item 4: `DINTaskAuditor` size review — PR 1, commit 1

Shrink the contract in place, with no library, split or deploy change. Only **getter visibility** changes: no state-changing function, event or storage slot changes. Prototyped in the plan and re-measured in the PR #219 review (`via_ir`, 200 runs):

| Getter → `internal` | Readers outside the contract | Saving |
|---|---|---|
| `auditBatches` | none (only a comment, `RewardEngine.t.sol:895`). `getAuditorsBatch` already returns the batch | −100 B |
| `dinAuditors` | none. `getDINtaskAuditors` already returns the list | −83 B |
| `Is_testdataCIDs_Assigned` | none. It is written at `:1028` and read only by its own guard at `:1026` | −58 B |
| `auditorGIWeight` | none | −79 B |
| **The four together** (Decision 1) | | **−320 B → 22,398 B** (2,178 B margin) |

**Keep public:** `rewardClaimed`, `testDataDisputes` and `giRewardSnapshot`. task_061026_21's claim and dispute commands read them.

**Scope:**
- Make the four getters `internal`. No test reads them, so no test changes.
- **Fold review: required.** #201 item 4 asks for "the same review" of per-phase duplication that the coordinator got. Measure folds of the near-duplicate dispute and reassignment paths. Commit a fold only if it shrinks the contract: in task_021026_19, `via_ir` made the coordinator's commit/reveal/finalize folds **grow** it (+177 B / +166 B). Report every measurement in the PR, including the ones not taken.
- Regenerate `dincli/abis/DINTaskAuditor.json` (`dincli system dump-abi --artifact foundry/out/DINTaskAuditor.sol/DINTaskAuditor.json --output dincli/abis --official`) and update the `DINTaskAuditor.md` §3 tables.
- Check PR #29's subgraph for reads of the four getters, and post an ABI note there. Don't push to that branch.

**`DINTaskAuditor` budget across both tasks** (Decision 1). Measured on `a1fcce2`:

| Step | Runtime | Margin | Band |
|---|---|---|---|
| `develop` | 22,718 B | 1,858 B | warn |
| + Part A | 22,398 B | 2,178 B | ok |
| + Part B (+102 B) | 22,500 B | 2,076 B | ok (28 B above the warn line) |
| + task_061026_21 Part B's every-batch commitment check (+75 B) | 22,575 B | 2,001 B | **warn** (still far above the 1,024 B fail line) |

There are two ways back out of the warn band: a fold that actually saves bytes, and removing `Is_testdataCIDs_Assigned` once task_061026_21's `AuditTestDataAssigned` coordinator state guards the double-set. Whichever of the two tasks lands second reports the combined size.

## Deliverables (Part A)

- [ ] The four getters are `internal`, with no change to state-changing functions, events or storage layout
- [ ] A size table from **one** build of the implementation, plus every fold measured (kept or not)
- [ ] Bundled ABI regenerated, `DINTaskAuditor.md` §3 updated, ABI note posted on PR #29
- [ ] `cd foundry && npm ci && forge build && python ../.github/scripts/contract_size_gate.py && forge test` is green, including `UpgradeValidation.t.sol`. `pytest -m "not integration"` is green

**Estimate:** 1 day.

---

# Part B — #180: one address can't hold both roles in a GI — PR 1, commit 2

**Decision 2: a per-address guard in `registerDINAuditor`.** A second address with its own stake gets around it (Sybil), but the attack then costs a second stake and is visible on-chain.

- **Where.** Aggregator registration (states 6–7) always comes before auditor registration (8–9) (`DINShared.sol:17-20`), so the auditor side is the only place the guard can fire:
```solidity
if (dintaskcoordinatorContract.isDINAggregator(_GI, msg.sender)) revert TA_DualRoleNotAllowed();
```
Add `isDINAggregator(uint256,address) returns (bool)` to `IDINTaskCoordinator` (`DINShared.sol:110-121`). The coordinator's public mapping `isDINAggregator` (`DINTaskCoordinator.sol:34`) already provides it. Add `TA_DualRoleNotAllowed` to `DINShared.sol`.
- **Cost:** +102 B on `DINTaskAuditor`, measured. The coordinator doesn't change.
- **NatSpec:**
- `DinValidatorStake.sol:451` and `:462` say "(not yet enforced)". Both are enforced now, in the two registration functions.
- `:121` says "decremented at endGI time". The decrement now happens in `releaseGIRegistrationSlots`.
- **Tests** (next to the `StakingEnforcement.t.sol` registration tests):
- an aggregator registering as an auditor in the same GI reverts with `TA_DualRoleNotAllowed`
- the same address registering as an auditor in a different GI succeeds
- a different address with its own stake succeeds, which documents the Sybil limit
- **Docs:**
- threat model Row 6 changes from KNOWN GAP to DEFENDED (with the Sybil note), and its stale line refs are refreshed (`:386` → `:401`, `:660` → `:667`)
- in `DINTaskAuditor.md`, update the registration checks and the §13 caveat
- add a `DINShared.md` error row

## Deliverables (Part B)

- [ ] Guard, interface entry and error added, with the three tests
- [ ] NatSpec fixed at `DinValidatorStake.sol:121`, `:451`, `:462`
- [ ] Threat model Row 6 updated (and PR #218's Row 6 test flipped if it has merged)
- [ ] The gate is green, and the PR shows the A + B size

**Estimate:** 0.5 day.

---

# Part C — #193: S5 escalation across models — PR 2

**Decision 3: a two-level check.**

- **The per-slasher GI ring stays as it is,** with the same per-model escalation and the same tests.
- **Add a per-validator ring of `block.timestamp` values**, `_partialSlashTimes[validator]`, with `s5GlobalWindow` (seconds) and `s5GlobalThreshold`.
- Timestamps only increase, across every slasher, so the ascending-order trim that forced per-contract keying (`DinValidatorStake.sol:136-146`) stays safe.
- `slashPartial` escalates (full `MIN_STAKE` slash + jail) when **either** level reaches its threshold, and then clears both rings.
- This also closes the same-model split: one validator's S1 and S2 misses now add up.

**Scope:**
- **Storage.** Append the new variables before `__gap` (`:174`, 50 slots) and shrink the gap by the slots used. `DinValidatorStakeV2` inherits them. `UpgradeValidation.t.sol` and the `DeployPlatform.t.sol` upgrade tests must stay green.
- **Setter.** `setS5GlobalParams(window, threshold)`, validated like `setS5RecidivismParams` (`:583-594`).
- **Defaults.** `s5GlobalWindow = 7 days` and `s5GlobalThreshold = 6` (2 × `s5RecidivismThreshold`). These are placeholders until #155.
- Set them in `initialize`.
- On an upgraded proxy, **0 means off**; no `reinitializer`. Nothing on `develop` is deployed, and DevNet 2.0 is a fresh `DeployPlatform.s.sol` run.
- Add optional deploy-script env keys `S5_GLOBAL_WINDOW` / `S5_GLOBAL_THRESHOLD`, matching the existing `S5_*` keys. Add them to `DeployPlatform.md` and its env-override tests.
- **Event.** `ValidatorEscalatedS5` gets a level flag, or a sibling event is added, so indexers can tell which level fired.
- **Tests:**
- two task contracts each slash one validator (threshold − 1) times within the window, and the global level escalates
- when the window expires, the global count resets
- the existing S5 tests (`SlashingInvariants.t.sol:488`, `:518`, `:540`) and `test_crossModelGICollision_slashPartialDoesNotUnderflow` (`PR146SlashingRegression.t.sol:321`) stay green
- the default and override deploy tests cover the new keys
- **Docs:**
- `DinValidatorStake.md`
- the `Developer/design/MECHANISM_DESIGN.md` S5 row
- threat model Row 11 (KNOWN GAP → DEFENDED)
- record, but don't change, the separate discrepancy: `MECHANISM_DESIGN.md:87` says S5 is "entire slashable stake + blacklist", while the code does a `MIN_STAKE` slash + a `s5JailDuration` jail (7 days)

## Deliverables (Part C)

- [ ] Two-level S5 with setter, defaults, "0 means off", event and deploy-script keys
- [ ] The new tests, with the existing S5 and upgrade-validation tests green
- [ ] Docs updated (and PR #218's Row 11 test flipped if it has merged)

**Estimate:** 1.5 days.

---

## Coordination with task_061026_21

- **`DINShared.sol`.** Part B adds `isDINAggregator` to `IDINTaskCoordinator`. task_061026_21 Part B edits the `GIstates` enum. The sections differ, and whichever PR merges second rebases.
- **`dincli/abis/DINTaskAuditor.json`.** Both tasks regenerate it. The second to merge regenerates on top.
- **The `DINTaskAuditor` budget** is tracked in Part A's table. Whichever task lands second reports the combined size.
- **The #180 guard and the integration suite.** The suite uses separate accounts for each role (aggregators 11–22, auditors 50–58), so it isn't affected. task_061026_21 adds a negative check once Part B lands.
- **#193** doesn't affect a single-model GI.

---

## Out of scope

- **#201 Part B** (slash reason for committed-but-unrevealed): a mechanism decision tied to #155 / #38.
- **#194, #78, #181, #178:** each needs a design note or a decision first, or is mainnet-grade.
- **dincli, the integration suite and the GI docs:** [task_061026_21](task_061026_21.md).
- **Don't split `DINTaskAuditor`** to win bytes. If the budget can't be met with the levers above, stop and raise it in the tracking discussion.
Loading
Loading