Task: Task-Contract Hardening — Ungrindable Subgroup Seed (#151, BL-11), Platform-Resolved Treasury Forwarding (#152), Indexer Lifecycle Events (#153)

ID: task_240926_16 Assigned to: Robbert Abimbola (@robertocarlous) Created: 2026-09-24 Status: Open (assigned) Dates: Sep 24 – Oct 1, 2026 (7 days). This window also covers the follow-on task_17 (to be written), so budget Parts A–C to leave room for it. Repo: https://github.com/InfiniteZeroFoundation/DevNet Base branch: develop — spec written against commit d321c2a (2026-09-24); re-pull before starting regardless of when you begin. Roadmap ref: P3-4.3 (Dispute Resolution Layer — reassignment selector), BL-11 / security-review finding H-2, P3-5.3 (fees/treasury), P4 indexer (issue #24 DIN-Indexer, PR #29) Tracking: Discussion #163 (progress updates, questions, PR links)

Part A: #151 (closes, pending review). Design agreed in the #151 review comment. Developer/BACK_LOG.md BL-11.
Part B: #152 (closes, pending review). Scope agreed in the #152 review comment.
Part C: #153 (closes, pending review — the develop half; the subgraph half lands on PR #29). Scope agreed in the #153 review comment. Indexer-branch consolidation (PR #72 → PR #29): Discussion #162.
Related, not in scope: #156 (aggregation-side shuffle).

Three independent pieces of work, submitted as three separate PRs into develop in order A → B → C. Part C's subgraph half goes onto PR #29 once PR #72 has been folded into it (C0):

All three touch DINTaskCoordinator.sol, but different functions:
- Part A: dispute seed.
- Part B: forfeiture paths.
- Part C: every GIstate assignment, plus the submit/finalize functions.
Parts B and C also touch DINTaskAuditor.sol.
Rebase each PR on develop after the previous one merges, rather than stacking the branches.

If Part A stalls on review, Part C doesn't depend on it and can go ahead. Say so in the tracking discussion before reordering.

Part A — Ungrindable dispute-subgroup seed (#151, BL-11) — PR 1

Context

DINTaskCoordinator._assignFreshSubgroup picks the fresh aggregator subgroup for an upheld S4 dispute by calling _shuffleAddressArray. That function seeds from blockhash(block.number - 1) (foundry/src/DINTaskCoordinator.sol:553-561). resolveDispute is onlyOwner and has no deadline. The model owner can therefore simulate the shuffle off-chain and call resolveDispute(upheld=true) only in a block that produces a subgroup favourable to them. This is BL-11, and it is the dispute-path instance of security-review finding H-2.

Issue #151 proposed a challenger-side commit-reveal. The review on #151 found that this moves the grinding rather than removing it. The challenger knows their own secret and chooses the reveal block, so they can grind the same way. They can also refuse to reveal and get a free re-roll. Part A implements the alternative agreed in that review: a future-block seed fixed at openDispute time. Nobody can predict it when the dispute opens, and once the seed block is mined nobody gets to choose anything.

While resolveDispute/settleRecomputation are both onlyOwner, the owner can still reject an upheld dispute outright. So this is not today's weakest link. It becomes load-bearing once adjudication moves off the owner, and BL-11 requires it before dispute resolution graduates past scaffold.

Scope

A1. Seed-taking shuffle helper

Change _shuffleAddressArray(address[] memory arr) to take an explicit bytes32 seed and derive each swap index from keccak256(abi.encodePacked(seed, i, arr.length)).
Keep the existing autoCreateTier1AndTier2 call site (:498) behaviourally unchanged. Pass it blockhash(block.number - 1) explicitly for now. Replacing that seed is #156's job, and #156 should be able to plug its own seed source into this same helper without touching the dispute path again.
Do not fork a second shuffle function.
DINTaskAuditor.sol:748 has the same pattern and is out of scope. Leave it alone.

A2. Future-block seed on the `Dispute` struct

Add seed state to Dispute: e.g. uint64 seedBlock and bytes32 seed. A zero seed means not locked.
In openDispute, set seedBlock = uint64(block.number) + disputeSeedDelay.
disputeSeedDelay is a new owner/DAO-settable uint64 with a placeholder default in the 5–10 range. Follow the disputeWindow/resolutionWindow pattern and give it a setter with a non-zero guard.
Upgrade safety: DINTaskCoordinator is not behind a proxy today, but check Documentation/technical/storage_layout.md. Update it if appending struct fields or new state variables changes anything recorded there.

A3. Permissionless `lockDisputeSeed`

New lockDisputeSeed(uint _GI, TierKind tierKind, uint batchId), callable by anyone.
Requires an open, not-yet-resolved dispute, no locked seed yet, and block.number > seedBlock.
If blockhash(seedBlock) != 0: store seed = keccak256(abi.encodePacked(blockhash(seedBlock), _GI, tierKind, batchId)) and emit an event.
The 256-block edge: if blockhash(seedBlock) == 0 (more than 256 blocks have passed), re-anchor with seedBlock = block.number + disputeSeedDelay and emit a re-anchor event. Never store or use a zero-derived seed.
- This is not a re-roll lever. The party who dislikes the draw can't stop the challenger or any honest validator from locking it first.

A4. `resolveDispute` / `_assignFreshSubgroup` use the locked seed

resolveDispute(upheld=true) reverts with a new custom error if the seed isn't locked. upheld=false stays callable without a seed, since that path draws no subgroup.
_assignFreshSubgroup passes the dispute's stored seed to _shuffleAddressArray. After this, the outcome of resolveDispute must not depend on the block in which it's called.
Update the NatSpec that currently says "shuffled with the same blockhash-based entropy" (:1343) and the scaffold-wide comment block if it references the old entropy.
Trust caveat, write it down in the NatSpec: on OP Stack, blockhash is sequencer-produced, so this design trusts the sequencer not to grind. That is acceptable for DevNet/testnet. VRF, or multi-party commit-reveal with a slashable non-reveal penalty, is the mainnet-grade follow-up and is out of scope here.

A5. dincli — ABI only

At d321c2a, dincli has no commands for the coordinator's dispute functions (openDispute/resolveDispute/settleRecomputation/expireDispute). Building that CLI surface is out of scope here.
Refresh the bundled ABI in dincli/abis/ if the coordinator ABI changes, and note the missing dispute CLI surface in the PR as a follow-up.

Deliverables (Part A)

Part B — Platform-resolved treasury forwarding (#152) — PR 2

Context

Issue #152 as written is partly stale. PR #141 (issue #43) already forwards two of the three paths it lists:

Frivolous forfeit: DINTaskCoordinator.resolveDispute(upheld=false) burns 50% and forwards 50% (:1199-1210).
Reward share: DINTaskAuditor.settleRewards's 5% share is forwarded (:533-536).
Tests: both are covered by foundry/test/TreasuryForwarding.t.sol (7 tests).

The #152 review found the real gaps are elsewhere.

Stranded funds. Four paths still only count the treasury share in treasuryAccrued and never transfer it, so those tokens are stuck in the task contracts:
- DINTaskCoordinator.settleRecomputation(confirmed=false) (:1292-1293) — also sends the full bond to treasury instead of the 50/50 split every other forfeiture uses (MECHANISM_DESIGN §4 item 1).
- DINTaskAuditor.resolveTestDataDispute, false dispute: the forfeited bond's treasury half (:1477-1482).
- DINTaskAuditor.resolveTestDataDispute, upheld: the model owner's reward-pool penalty's treasury half (:1496-1502).
- DINTaskAuditor.closeExpiredDispute: the expired bond's treasury half (:1518-1524).
- Plus settleRewards with treasuryAddress unset: the 5% share is neither sent nor burned. The coordinator burns in the same situation.
Wrong trust boundary. Both task contracts' setTreasuryAddress is onlyOwner, and the owner of a task contract is the model owner. A model owner can point it at their own wallet and collect the protocol's 5% share, half of every forfeited challenger bond, and half of the penalty levied on themselves when they lose a test-data dispute. Nothing in DeployPlatform.s.sol or dincli ever calls this setter on a task contract, so on a real deployment it's unset unless the one party with a reason to set it wrong sets it.

Scope

B1. Resolve the treasury from the platform

Add slashTreasury() to IDinValidatorStake in foundry/src/DINShared.sol. DinValidatorStake.slashTreasury is platform-owned, and DeployPlatform.s.sol:98 already wires it to the DinTreasury proxy. Both task contracts already hold dinvalidatorStakeContract.
Remove treasuryAddress, setTreasuryAddress, and the auditor's TreasuryAddressUpdated event from both task contracts.
- This deliberately departs from #152's acceptance criterion of a dinTreasury setter plus a TreasuryUpdated event. That criterion is superseded by the #152 review.
If a separate protocol-fee treasury (distinct from the slash destination) is ever wanted, it gets exposed from a platform contract (e.g. DinCoordinator), never a task-level setter. That's out of scope here. Don't build it.

B2. One forwarding helper, used everywhere

One internal helper per contract, e.g. _burnAndForward(uint256 amount):
- burns 50% and sends 50% to dinvalidatorStakeContract.slashTreasury(),
- burns the whole amount if slashTreasury() is unset, mirroring DinValidatorStake.slash() (:646-652).
Duplicate it in each contract or put it in DINShared.sol, whichever fits the existing layout better. Say which in the PR.
Route all six paths through it:
- the two already-wired paths from PR #141,
- settleRecomputation(false),
- the three DINTaskAuditor test-data-dispute paths.
Decision to flag, not guess: the 5% settleRewards share is currently forwarded in full, not burned 50/50. It's a protocol fee, not a forfeiture. Keep it forwarded in full: route it through a forward-only variant of the helper (same unset-treasury burn fallback) rather than the 50/50 one, unless you see a reason otherwise. If you do, raise it on #152 first.
Keep treasuryAccrued as an observability counter if it's still useful. Remove its stale "DinTreasury doesn't exist on develop" NatSpec and the remaining TODO(task_210726_5) comments:
- DINTaskAuditor.sol:54-63
- DINTaskCoordinator.sol:1078-1081
- DINTaskCoordinator.sol:1292

B3. dincli / scripts

Refresh the bundled DINTaskCoordinator/DINTaskAuditor ABIs in dincli/abis/.
Grep dincli/ and foundry/script/ for any setTreasuryAddress / treasuryAddress reference and remove it. None existed at d321c2a; confirm that's still true after re-pulling.

Deliverables (Part B)

Part C — Task-level lifecycle events for the indexer (#153) — PR 3 (+ subgraph commits on PR #29)

Context

The subgraph in PR #29 (feat/din-indexer, still open, head 866521a from 2026-07-14) has six task-level handlers commented out in subgraph/subgraph.yaml, because the events they listen to don't exist on develop. Without them the indexer can't see:

LMS submissions,
T1/T2 aggregation submissions,
which CID won each batch, or the new global model per GI,
GI phase transitions.

Meanwhile dincli's lms.py/aggregation.py keep iterating submissions over RPC (PR #29's Documentation/technical/audits/task-contract-event-audit.md §3.2).

Issue #153 lists only four events and has several errors. The #153 review corrected them, and this part follows that review. The exact event signatures come from audit §4 in PR #29, not the issue text, so the handlers can be uncommented without an ABI mismatch.

Scope

C1. Six events, declared in the contract that emits them

Declare each event in its emitting contract, following the existing convention. Not in DINShared.sol, which has no event declarations. Signatures exactly as audit §4:

Event	Contract / emit site
`GIStateChanged(uint indexed GI, uint8 indexed newState)`	`DINTaskCoordinator`, every state transition (via C2)
`LocalModelSubmitted(uint indexed GI, uint indexed modelIndex, address indexed client, bytes32 modelCID)`	`DINTaskAuditor.submitLocalModel()` (`:700`), after the `lmSubmissions[_GI].push(...)`. `modelIndex` is the pre-push length.
`T1AggregationSubmitted(uint indexed GI, uint indexed batchId, address indexed aggregator, bytes32 cid)`	`DINTaskCoordinator.submitT1Aggregation()`, after the `t1SubmissionCID` write
`T2AggregationSubmitted(uint indexed GI, uint indexed batchId, address indexed aggregator, bytes32 cid)`	`DINTaskCoordinator.submitT2Aggregation()`, after the `t2SubmissionCID` write. `batchId` is always `0` today; keep it for symmetry.
`T1BatchFinalized(uint indexed GI, uint indexed batchId, bytes32 winningCID)`	`finalizeT1Aggregation()`, once per batch, right after `b.finalCID = winningCID` (`:767`)
`T2Finalized(uint indexed GI, bytes32 globalModelCID)`	`finalizeT2Aggregation()`, right after `b.finalCID = winningCID` (`:859`)

C2. `_setGIstate` helper, with the `GIstarted` ordering fix

Replace every GIstate = GIstates.X; assignment in DINTaskCoordinator with an internal _setGIstate(GIstates newState). It assigns the state and emits GIStateChanged(GI, uint8(newState)). This leaves one emit site, so no future transition can be missed.
Ordering bug to avoid: _startGI currently sets GIstate = GIstates.GIstarted before GI++ (:310-311). A mechanical replace would emit GIstarted stamped with the previous GI. Increment GI first, then call _setGIstate(GIstates.GIstarted). Check that no other transition reads GI in a half-updated state.
The constructor/setup transitions (ordinals 0–4) will emit with GI == 0. That's expected; document it in the NatSpec.

C3. Contract size

DINTaskCoordinator is large. Run forge build --sizes before and after, and put both numbers in the PR description. If the contract crosses the 24 KB EIP-170 limit, stop and raise it in the tracking discussion rather than splitting the contract on your own initiative.

C4. dincli

Refresh the bundled DINTaskCoordinator/DINTaskAuditor ABIs in dincli/abis/. Switching lms.py/aggregation.py from RPC iteration to GraphQL is out of scope. It follows once the subgraph half is live.

C0. Consolidate the indexer branch: fold PR #72 into PR #29

This is a prerequisite for C5, not for C1–C4. Agreed in Discussion #162; see that discussion for the full analysis.

Why: PR #72 (feat/din-indexer-65) is stacked on PR #29 and contains all of #29's commits. It adds only two new indexer commits: 2b015f5 (GraphQL-cap docs + update-subgraph-addresses.sh) and 1a2a877 (P3-staking events). Its other 23 commits came in via the feat/p3-staking merge and are already on develop, which is why #72 shows 185 files.
You:
- On your fork: git checkout feat/din-indexer && git merge --ff-only feat/din-indexer-65 && git push.
- Close PR #72 with a "folded into #29" note.
Umer: fast-forward the upstream feat/din-indexer base branch to develop. It's a strict ancestor, 230 commits behind.
You:
- Merge develop into your feat/din-indexer. It was conflict-free at d321c2a; the only non-subgraph/ files left differing are Developer/issues/indexer.md, the event audit doc, dincli/cli/dindao.py (renamed to dinrep.py by PR No. 175 — re-apply your edits there after merging develop), and tests/test_list_pending_requests.py.
- Regenerate subgraph/abis/ from current develop, and re-run graph codegen && graph build.
Not blocking C0, but note it: DinValidatorStake on develop emits events #72 doesn't handle yet:
- UnbondingPeriodUpdated
- SlashTreasuryUpdated
- EncryptionKeyRegistered
- ActiveRegistrationIncremented/Decremented
- the S5/S6 events
Handle them in the same refresh if it's small; otherwise list them in #29's description as a follow-up.

C5. Subgraph half, as commits on PR #29, after PR 3 merges and C0 is done

This lands as further commits on PR #29 (your fork's feat/din-indexer), not as a separate PR and not on develop, because develop has no subgraph/ directory.

Refresh against develop again, since PR 3 will have merged after C0, and regenerate subgraph/abis/ so the six new events are in the ABIs.
Uncomment and implement the six handlers.
Regenerate the uint8 → GraphQL GIstates enum mapping from current foundry/src/DINShared.sol. The July snapshot is off by one from ordinal 14 onward, because LMSevaluationRevealStarted was inserted on 2026-08-27. T1AggregationStarted is now 17, not 16, and the enum has 24 members (0–23). Fix audit §4.1's example and subgraph/docs/daemon-event-schema.md to match.
RewardDeposited is no longer dead code. DINTaskAuditor.depositRewards emits RewardDeposited(uint256 indexed gi, address indexed depositor, uint256 amount) (:488). Don't delete it, as audit §5 proposes. Add a handler for it, and update §5.
ETHTreasuryWithdrawn (commented out under the DinCoordinator data source) doesn't exist and isn't planned. Drop the stub, or repoint it at the events DinTreasury actually emits (EthReceived/EthWithdrawn/ERC20Withdrawn) with a DinTreasury data source. Your call; say which in the PR.

Deliverables (Part C)

Submission

Three PRs from separate branches into develop (A → B → C), plus the Part C subgraph work pushed to PR #29 (after C0):

PR 1 (Part A): references #151 (closes) and BL-11. In the description, call out the chosen disputeSeedDelay default and why, and any storage-layout implications.
PR 2 (Part B): references #152 (closes). In the description, call out where the helper lives (per-contract vs DINShared.sol), and confirm the settleRewards forward-in-full decision.
PR 3 (Part C): references #153. In the description, include the forge build --sizes before/after numbers, and say the subgraph half follows.
PR #29 update (Part C0 + C5): comment on #29 when C0 is done and again when C5 is pushed. Update #29's description to cover #72's folded-in scope, reference #153 (closes), and state the ETHTreasuryWithdrawn decision.

Post each PR link and progress updates in this task's tracking discussion once it's opened.

Notes

Flag every DINTaskCoordinator.sol/DINTaskAuditor.sol/DINShared.sol change (Part C's event additions included, since the _setGIstate refactor touches every state transition) for Umer's review given its cryptoeconomic sensitivity, same as prior tasks in this area (task_100926_12, task_100926_13).
Re-pull develop and re-run forge test right before opening each PR.
If you find a reason to prefer commit-reveal (or VRF) over the future-block seed once you're in the code, raise it on #151 before building it. Don't switch designs silently.
If you find a reason the model owner should control the treasury destination, raise it on #152 before building around it.
Part C's event signatures come from PR #29's audit §4. If you change any of them, change the subgraph handler signatures to match in the same C5 push to PR #29.
#156 (aggregation-side shuffle + T1/T2 commit-reveal) is deliberately not in this task. Part A's seed-taking helper (A1) is the only shared piece.

Navigation Menu

Repository navigation

task_240926_16.md

task_240926_16.md

Task: Task-Contract Hardening — Ungrindable Subgroup Seed (#151, BL-11), Platform-Resolved Treasury Forwarding (#152), Indexer Lifecycle Events (#153)

Part A — Ungrindable dispute-subgroup seed (#151, BL-11) — PR 1

Context

Scope

A1. Seed-taking shuffle helper

A2. Future-block seed on the `Dispute` struct

A3. Permissionless `lockDisputeSeed`

A4. `resolveDispute` / `_assignFreshSubgroup` use the locked seed

A5. dincli — ABI only

Deliverables (Part A)

Part B — Platform-resolved treasury forwarding (#152) — PR 2

Context

Scope

B1. Resolve the treasury from the platform

B2. One forwarding helper, used everywhere

B3. dincli / scripts

Deliverables (Part B)

Part C — Task-level lifecycle events for the indexer (#153) — PR 3 (+ subgraph commits on PR #29)

Context

Scope

C1. Six events, declared in the contract that emits them

C2. `_setGIstate` helper, with the `GIstarted` ordering fix

C3. Contract size

C4. dincli

C0. Consolidate the indexer branch: fold PR #72 into PR #29

C5. Subgraph half, as commits on PR #29, after PR 3 merges and C0 is done

Deliverables (Part C)

Submission

Notes

FilesExpand file tree

task_240926_16.md

Latest commit

History

task_240926_16.md

File metadata and controls

Task: Task-Contract Hardening — Ungrindable Subgroup Seed (#151, BL-11), Platform-Resolved Treasury Forwarding (#152), Indexer Lifecycle Events (#153)

Part A — Ungrindable dispute-subgroup seed (#151, BL-11) — PR 1

Context

Scope

A1. Seed-taking shuffle helper

A2. Future-block seed on the Dispute struct

A3. Permissionless lockDisputeSeed

A4. resolveDispute / _assignFreshSubgroup use the locked seed

A5. dincli — ABI only

Deliverables (Part A)

Part B — Platform-resolved treasury forwarding (#152) — PR 2

Context

Scope

B1. Resolve the treasury from the platform

B2. One forwarding helper, used everywhere

B3. dincli / scripts

Deliverables (Part B)

Part C — Task-level lifecycle events for the indexer (#153) — PR 3 (+ subgraph commits on PR #29)

Context

Scope

C1. Six events, declared in the contract that emits them

C2. _setGIstate helper, with the GIstarted ordering fix

C3. Contract size

C4. dincli

C0. Consolidate the indexer branch: fold PR #72 into PR #29

C5. Subgraph half, as commits on PR #29, after PR 3 merges and C0 is done

Deliverables (Part C)

Submission

Notes

A2. Future-block seed on the `Dispute` struct

A3. Permissionless `lockDisputeSeed`

A4. `resolveDispute` / `_assignFreshSubgroup` use the locked seed

C2. `_setGIstate` helper, with the `GIstarted` ordering fix