[robertocarlous]

Reason for revert: PR No. 272 had a non-author approval (robertocarlous, 08:56 UTC Oct 24) but was merged at 14:40 UTC before the required foundry/src second sign-off was obtained — violating MAINTAINING No. 4.2. @abrahamnash had been asked for the second key at 09:01 but had not yet signed off when the merge went in. The develop ruleset only enforces a single peer approval, so the second-key requirement is the merger's responsibility to wait for. The substantive changes in No. 272 are not disputed. This revert restores develop to a reviewable baseline so No. 272 can re-land through the normal process.

What this undoes:

• The three new GIstates entries: AuditSeedLocked, AuditTestDataAssigned, AggSeedLocked
• The per-batch testDataCommitments check inside setTestDataAssignedFlag (the loop that reverts TA_TestDataNotAssigned unless every audit batch has a stored commitment)
• State gating on createAuditorsBatches, setTestDataAssignedFlag, and autoCreateTier1AndTier2 in DINTaskCoordinator
• TC_GINotEnded, TC_SlotsAlreadyReleased custom errors and releaseGIRegistrationSlots state guard
• dincli GI-state mirrors and seed-lock shortcuts added in No. 272
• New LifecycleEvents progress-state tests, including the test_progressStates_oneUnassignedBatchBlocksFlagAndEvaluation regression test

Known downstream effects before No. 272 re-lands:

• setTestDataAssignedFlag reverts to the one-shot boolean with no per-batch check — the bug No. 272 fixed re-opens temporarily.
• PR No. 259 currently relies on TC_GINotEnded and TC_SlotsAlreadyReleased staying on develop (it dropped its own duplicate declarations when it rebased onto No. 272). If this merges first, No. 259 will need those declarations restored before it can merge.

Re-land: PR No. 287.

[Abidoyesimze]

Went through the actual diff rather than the PR metadata, since there's no description here explaining the reason for the revert.

Mechanically this is a clean revert - diffed it against the inverse of #272's merge commit and it's byte-identical, no partial or tampered changes. Checked it out standalone and ran the full suite: clean build, 501 passed, 0 failed.

But the content matters here. The most important thing this undoes is in DINTaskAuditor.sol - setTestDataAssignedFlag goes back to:

if (Is_testdataCIDs_Assigned[_GI]) revert TA_FlagAlreadySet();
Is_testdataCIDs_Assigned[_GI] = flag;
return true;

No check that any batch actually has test data assigned, just a one-shot boolean. That's the exact gap #272 closed (it replaced this with a loop that reverts TA_TestDataNotAssigned unless every audit batch has a stored commitment). Reverting it means startLMsubmissionsEvaluation can succeed again with zero test data distributed to auditors - which is precisely what test_progressStates_oneUnassignedBatchBlocksFlagAndEvaluation was written to catch.

The rest follows from that: DINShared.sol drops the three new GIstates and TA_TestDataNotAssigned/TC_GINotEnded/TC_SlotsAlreadyReleased, DINTaskCoordinator.sol removes the state gating on createAuditorsBatches/setTestDataAssignedFlag/autoCreateTier1AndTier2 and reverts releaseGIRegistrationSlots to string requires, dincli's state mirrors and seed-lock shortcuts disappear, and LifecycleEvents.t.sol loses the new progress-state tests including the regression test above.

One concrete downstream problem: TC_GINotEnded/TC_SlotsAlreadyReleased are gone here. My own PR #259 currently depends on those staying on develop - I specifically dropped my duplicate declarations in favor of #272's copy when I rebased, per the review comment on that PR. If this merges, #259 breaks again.

So: clean revert mechanically, but it reopens a real bug and breaks an already-approved PR, with nothing here saying why. I'd want to understand the reason before this lands.

[Abidoyesimze]

cc: @robertocarlous

[robertocarlous]

@Abidoyesimze — description is now filled in (see PR body above). It covers the reason for the revert, everything that's undone, and the two downstream effects you raised (setTestDataAssignedFlag regression re-opens, PR No. 259 will need TC_GINotEnded/TC_SlotsAlreadyReleased restored). Let me know if that's enough to proceed.

[Abidoyesimze]

Reason checks out - process revert for the missing non-author sign-off (MAINTAINING No. 1), not a content dispute. Both downstream effects I raised are explicitly acknowledged in the description. Mechanically clean (verified byte-identical against the inverse of #272's merge, full suite green standalone). Approving.

[umeradl]

Retroactive review: this PR merged without the standard three comments, so they're posted here after the fact. Same format as din-pr-review.

Reviewed against develop. Branch head 4b26f06 = the revert commit 82b8494 plus a sync merge of develop (55c4b52). git merge-tree --write-tree 55c4b52 82b8494 is clean (exit 0) and produces tree 60cc7c1. That is the tree of the PR head 4b26f06 and also of the final merge ee99619, so CI ran on the exact tree that landed. CI on 4b26f06: CI OK green. Results: Solidity forge build + contract size gate + forge test (39 suites, 516 passed, 0 failed), Python pytest -m "not integration" (501 passed, 133 deselected), Docs. Reproduced locally (git-level only, since CI covers build/tests): exact-inverse checks, tree equality, storage/upgrade impact, and the downstream PR No. 259.

Claimed: a clean revert of PR No. 272 (merge commit c1dba19), with the substantive changes not disputed.

Verified — exact inverse, nothing else: git diff 82b8494^ 82b8494 and git diff c1dba19 c1dba19^1 are byte-identical (cmp: no difference). Stronger check on what actually landed: I started from develop just before the merge (ee99619^1 = 55c4b52), applied the inverse of No. 272 in a scratch index, and wrote the tree. It gives 60cc7c1, the same as ee99619^{tree}. So develop after this PR is develop before it minus exactly No. 272, and nothing else. The changes that No. 264 (one-shot dispute comments/docs) and No. 236 made in the meantime to the four overlapping files (DINTaskCoordinator.sol, DisputeResolution.t.sol, DINShared.md, DINTaskCoordinator.md) are still in place.

Claimed: removes the three GIstates entries AuditSeedLocked, AuditTestDataAssigned and AggSeedLocked.

Verified — gone from the enum: GIstates on develop is back to AuditorsBatchesCreated = 12, LMSevaluationStarted = 13 … GIended = 25 (was 28). AuditSeedLocked/AggSeedLocked still appear on develop, but only as the events that predate No. 272 (DINTaskCoordinator.sol:245-247), not as states. The dincli/cli/utils.py positional mirrors (states / stateDescription) shrink to match: three entries removed, and "Aggregators slashed" goes back to "Validators slashed". lms_evaluation.py gates on AuditorsBatchesCreated again.

Claimed: removes the per-batch testDataCommitments check in setTestDataAssignedFlag.

Verified — back to the one-shot boolean: DINTaskAuditor.sol:1033-1046 on develop checks GIstate == AuditorsBatchesCreated, flag, then Is_testdataCIDs_Assigned[_GI] → TA_FlagAlreadySet, with no loop over auditBatches. TA_TestDataNotAssigned and test_progressStates_oneUnassignedBatchBlocksFlagAndEvaluation are gone (git grep on origin/develop: 0 hits). As @Abidoyesimze's first review pointed out, startLMsubmissionsEvaluation can again start with no test data distributed. The PR body acknowledges this.

Claimed: removes TC_GINotEnded/TC_SlotsAlreadyReleased, and releaseGIRegistrationSlots goes back to string requires.

Verified: both errors have 0 hits in foundry/src on develop. StakingEnforcement.t.sol goes back to vm.expectRevert(bytes("slots already released")).

Claimed (downstream): PR No. 259 will need those two declarations restored.

Verified — already handled: No. 259's current head 6493426 declares both errors itself (DINShared.sol, 1 hit each). It is clean against develop in git merge-tree (exit 0), and its own CI is green.

Storage / upgrade safety (not covered by CI): the revert puts mapping(uint256 => bool) internal Is_testdataCIDs_Assigned back into DINTaskAuditor (line 262), and enum ordinals shift by up to 3. DINTaskAuditor/DINTaskCoordinator are plain Ownable per-model contracts, not proxied, and nothing on develop is deployed, so there is no layout to preserve. Documentation/technical/storage_layout.md doesn't cover the task contracts and isn't touched. Size gate: DINTaskCoordinator is 23,497 B (1,079 B margin, warn) and DINTaskAuditor is 21,290 B, both under EIP-170.

Not independently re-verified: I didn't re-run the build/tests locally. The CI run above ran on the identical tree, which MAINTAINING No. 4.2 counts as evidence.

---

Every checkable claim holds up. This is a mechanically exact process revert. No. 272 had the peer approval but was merged without the foundry/src second sign-off (MAINTAINING No. 4.2), not without a non-author approval as this PR's description says with both downstream effects acknowledged, and No. 259 has since absorbed its own. Two process notes for next time:

• A PR touching foundry/src/** needs the second sign-off from @umeradl or @abrahamnash on top of the peer approval (MAINTAINING No. 4.2). Reverts are no exception (No. 4.7: "goes through a PR and peer approval like anything else").
• Re-land: PR No. 287 (No. 4.7). Until it merges, the setTestDataAssignedFlag gap is live on develop.

Edited 2026-10-26: corrected the revert reason. No. 272 did have a non-author approval (@robertocarlous, 08:56 UTC Oct 24, carried over to d8dbd1f). What it lacked was the foundry/src second sign-off (MAINTAINING No. 4.2), which GitHub doesn't enforce.

[umeradl]

Files changed (25) — as of 4b26f06 (PR head)

Retroactive: posted after the merge, so the predictions below are checked against what actually landed. Comment 3 follows.

Diffed against merge-base 55c4b52 (develop just before the merge). The branch was cut at c1dba19 (No. 272's merge). Since then develop picked up No. 264 (7b8afe0, one-shot dispute docs/comments) and No. 236 (55c4b52). They overlap this PR in 4 files: DINTaskCoordinator.sol, DisputeResolution.t.sol, DINShared.md and DINTaskCoordinator.md. All 4 are disjoint hunks. The author's sync merge 4b26f06 absorbed them with no conflicts. GitHub agrees: PR shows MERGED. Live mergeable/mergeStateStatus are no longer reported after the merge, but the develop ruleset requires strict up-to-date, and the head tree equals the merge tree (60cc7c1). A local git merge-tree --write-tree 55c4b52 82b8494 dry run is clean (exit 0) and produces the same tree.

Shared Why for every block below: No. 272 was merged without a non-author approval (MAINTAINING No. 1), so this restores develop to its pre-272 baseline and No. 272 re-lands through review. Every file's diff is the exact inverse of its No. 272 hunk (cmp byte-identical, see the verification comment above).

foundry/src/DINShared.sol

Field	Value
Change	Modified
Lines	+19/-31
Diff (what exactly is in this PR)	Removes AuditSeedLocked, AuditTestDataAssigned and AggSeedLocked from GIstates and renumbers the comments (GIended 28 → 25). Removes TA_TestDataNotAssigned, TC_GINotEnded and TC_SlotsAlreadyReleased. Restores TA_FlagAlreadySet and TC_EvalPhaseNotClosed.
Functionality — how & why	How: GIstates is the uint8 enum both task contracts gate on and dincli mirrors by raw ordinal. Removing the three progress states shifts every later ordinal back by up to 3. The errors are the revert selectors used by the restored checks in the two contracts below. Why: shared (see above).
Diff vs current develop HEAD	None. Untouched by No. 264/236.
Recommended merge proposal	Merged as-is. Exact inverse, CI green on identical tree.
Actual merge proposal	Soon
Pending proposal	Re-land No. 272's states in a new PR (MAINTAINING No. 4.7).
Local merge conflict	No
GitHub merge conflict	No

foundry/src/DINTaskAuditor.sol

Field	Value
Change	Modified
Lines	+8/-12
Diff (what exactly is in this PR)	createAuditorsBatches gates on LMSclosed again (was AuditSeedLocked). setTestDataAssignedFlag drops the every-batch testDataCommitments loop and goes back to the one-shot Is_testdataCIDs_Assigned[_GI] → TA_FlagAlreadySet guard. The Is_testdataCIDs_Assigned mapping is restored (line 262).
Functionality — how & why	How: setTestDataAssignedFlag (:1033-1046) now checks the coordinator state is AuditorsBatchesCreated, flag == true and that the flag isn't already set, then sets it. It no longer checks that each audit batch has a stored test-data commitment, so startLMsubmissionsEvaluation can follow with zero test data distributed. Restoring the mapping changes storage, but the contract is plain Ownable (not proxied) and nothing is deployed. Why: shared. The reopened gap is the known downstream cost (PR body; @Abidoyesimze's first review).
Diff vs current develop HEAD	None. Untouched by No. 264/236.
Recommended merge proposal	Merged as-is. Size gate: 21,290 B runtime, 3,286 B margin.
Actual merge proposal	Soon
Pending proposal	Every-batch check returns with No. 272's re-land.
Local merge conflict	No
GitHub merge conflict	No

foundry/src/DINTaskCoordinator.sol

Field	Value
Change	Modified
Lines	+17/-27
Diff (what exactly is in this PR)	lockAuditSeed/lockAggSeed no longer _setGIstate to the seed-locked states. createAuditorsBatches gates on LMSclosed + auditSeed != 0 (TC_LMEvalCannotBeStarted / TC_AuditSeedNotLocked). setTestDataAssignedFlag no longer moves to AuditTestDataAssigned, and startLMsubmissionsEvaluation gates on AuditorsBatchesCreated. autoCreateTier1AndTier2 gates on LMSevaluationClosed + aggSeed != 0. releaseGIRegistrationSlots goes back to require(giEnded, "GI has not ended") / require(!registrationSlotsReleased[_GI], "slots already released").
Functionality — how & why	How: seed locking becomes a side value again (checked as seed != 0) instead of a state transition. That removes the three intermediate states from the owner-driven phase sequence, so the GI goes LMSclosed → AuditorsBatchesCreated → LMSevaluationStarted and LMSevaluationClosed → T1nT2Bcreated as before No. 272. Why: shared.
Diff vs current develop HEAD	No. 264 added one-shot-dispute comments at :1390 (openDispute) and :1538 (resolveDispute). These are disjoint from this PR's hunks, and both coexist in the merged file.
Recommended merge proposal	Merged as-is (union with No. 264's comments). Size gate: 23,497 B runtime, 1,079 B margin (warn, under EIP-170).
Actual merge proposal	Soon
Pending proposal	None beyond the re-land.
Local merge conflict	No
GitHub merge conflict	No

dincli/abis/DINTaskAuditor.json, dincli/abis/DINTaskCoordinator.json

Field	Value
Change	Modified
Lines	+5/-5, +5/-10
Diff (what exactly is in this PR)	Error entries follow the Solidity change: TA_FlagAlreadySet back and TA_TestDataNotAssigned out. TC_EvalPhaseNotClosed back and TC_GINotEnded/TC_SlotsAlreadyReleased out. The surrounding entries reorder to match.
Functionality — how & why	How: these bundled ABIs are what DinContext falls back to when a manifest doesn't supply task_contracts, so dincli decodes the restored revert selectors. Why: keep the bundled ABIs in step with foundry/src.
Diff vs current develop HEAD	None.
Recommended merge proposal	Merged as-is. These are the exact pre-272 regenerated ABIs.
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

dincli/cli/utils.py, dincli/cli/modelownerd/aggregation.py, dincli/cli/modelownerd/auditor_batches.py, dincli/cli/modelownerd/lms_evaluation.py

Field	Value
Change	Modified
Lines	+1/-9, +8/-10, +8/-10, +1/-1
Diff (what exactly is in this PR)	utils.py: the positional states/stateDescription mirrors lose the three entries, and "Aggregators slashed" goes back to "Validators slashed". aggregation.py/auditor_batches.py: the "skip validation if already in *SeedLocked" shortcut is removed. They validate LMSevaluationClosed/LMSclosed, then always call ensure_batch_seed_locked. lms_evaluation.py: gates on AuditorsBatchesCreated again.
Functionality — how & why	How: GIstateToStr indexes the mirror list by the raw enum ordinal, so it has to shrink exactly when the enum does, or every later state would print wrong. The phase commands pre-check the on-chain state before sending a tx. Why: shared, and these mirror the contract revert one-for-one.
Diff vs current develop HEAD	None.
Recommended merge proposal	Merged as-is. pytest 501 passed in CI.
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

Documentation/technical/contracts/DINShared.md, DINTaskAuditor.md, DINTaskCoordinator.md

Field	Value
Change	Modified
Lines	+58/-65, +4/-5, +18/-20
Diff (what exactly is in this PR)	Enum table, error tables, function descriptions and changelog lines go back to the pre-272 text.
Functionality — how & why	How: docs only. Documentation/ describes develop, so it has to follow the code back. Why: shared.
Diff vs current develop HEAD	No. 264 edited the TC_DisputeAlreadyOpen row in DINShared.md and added two lines to DINTaskCoordinator.md. These are disjoint hunks and both coexist.
Recommended merge proposal	Merged as-is (union with No. 264). Docs job green.
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

Developer/tasks/task_061026_21.md

Field	Value
Change	Modified
Lines	+7/-9
Diff (what exactly is in this PR)	Drops No. 272's "Re-checked at eb8085d" note and its line-reference refresh. References go back to pre-272 positions.
Functionality — how & why	How: spec text only. Why: shared. It was part of No. 272's merge commit, so the exact revert takes it too.
Diff vs current develop HEAD	None.
Recommended merge proposal	Merged as-is.
Actual merge proposal	Soon
Pending proposal	The re-land PR should redo the line-ref refresh against its own base.
Local merge conflict	No
GitHub merge conflict	No

foundry/test/LifecycleEvents.t.sol

Field	Value
Change	Modified
Lines	+27/-180
Diff (what exactly is in this PR)	Removes the progress-state tests, including test_progressStates_oneUnassignedBatchBlocksFlagAndEvaluation, and the _submitAndCloseLMS/_advanceToEvalClosed helpers. Restores the hard-coded GI_* ordinal constants (12–25) and the inline lockAggSeed/setTestDataAssignedFlag steps.
Functionality — how & why	How: the restored tests drive the GI through the pre-272 sequence and assert GIstateChanged events against the old ordinals. Why: shared. The removed regression test guarded exactly the gap that reopens.
Diff vs current develop HEAD	None.
Recommended merge proposal	Merged as-is. forge 516/516 in CI.
Actual merge proposal	Soon
Pending proposal	The regression test returns with the re-land.
Local merge conflict	No
GitHub merge conflict	No

foundry/test/utils/PlaceholderTestData.sol + 10 test files (AggregatorCommitReveal, AuditorCommitReveal, DisputeResolution, GasSimulation, PR146SlashingRegression, RewardEngine, ScoringValidation, SecurityFindings, StakingEnforcement, TreasuryForwarding .t.sol)

Field	Value
Change	Deleted (helper) / Modified
Lines	-29 (helper); -2 to -7 each; StakingEnforcement +1/-4
Diff (what exactly is in this PR)	Deletes the assignPlaceholderTestData helper and removes its import and calls. StakingEnforcement also goes back to vm.expectRevert(bytes("slots already released")).
Functionality — how & why	How: No. 272's every-batch check made setTestDataAssignedFlag revert unless each batch had a commitment, so every lifecycle test had to assign placeholder test data first. With the check gone, the setup step isn't needed. Why: shared.
Diff vs current develop HEAD	No. 264 added tests to DisputeResolution.t.sol. These are disjoint and coexist.
Recommended merge proposal	Merged as-is.
Actual merge proposal	Soon
Pending proposal	Helper returns with the re-land.
Local merge conflict	No
GitHub merge conflict	No

Verification

CI run 40305896167 on 4b26f06 (tree identical to the merge): forge build + size gate + forge test 516/516, pytest 501 passed, Docs green. Exact-inverse and tree-equality checks are in the verification comment above.

Local vs. GitHub agree: the local dry run is clean, the head tree equals the merge tree, and GitHub merged with no conflict.

[umeradl]

Actual outcome — PR No. 273 merged (no fixes / deviation)

This supersedes the pre-merge review comment above with what actually happened when this PR was resolved against develop.

Three commits from this PR, all on origin/develop:

1. ee99619: the real merge of this PR (merge commit, not squash). Path A: merged on GitHub by @Abidoyesimze after their approval of 4b26f06. 0 conflicts, as predicted. GitHub agrees: PR shows MERGED.
2. 82b8494: the revert itself (revert of No. 272's merge c1dba19, byte-identical inverse).
3. 4b26f06: the author's sync merge of develop (55c4b52, i.e. No. 264 + No. 236) into the branch. It had no conflicts, and its tree (60cc7c1) is identical to ee99619's.

No review-fix or deviation commit was needed.

Files unchanged from the PR (25 of 25)

All 25 files landed exactly as the revert authored them. The 4 files that No. 264 also touched (foundry/src/DINTaskCoordinator.sol, foundry/test/DisputeResolution.t.sol, Documentation/technical/contracts/DINShared.md, DINTaskCoordinator.md) carry No. 264's disjoint hunks alongside the revert. Net effect on develop: ee99619 = 55c4b52 minus exactly No. 272 (tree check in the verification comment).

Verification

• PR CI run 40305896167 on 4b26f06: Solidity (build, size gate, forge 516/516), Python (pytest 501 passed), Docs. All green.
• develop push run 40324161806 on ee99619: green.

Local vs. GitHub agree: no conflicts predicted and none occurred.

Follow-ups:

• No. 272 (task_061026_21 Parts A+B) re-lands as PR No. 287 (MAINTAINING No. 4.7). Until it merges, setTestDataAssignedFlag doesn't check per-batch test data on develop.
• PR No. 259 already restored TC_GINotEnded/TC_SlotsAlreadyReleased on its own branch (head 6493426, CI green, clean against develop).

---

@Abidoyesimze, thanks for the careful review here. Your first pass (byte-identical inverse check, flagging the reopened gap and the No. 259 break) is exactly the depth we want. Going forward, please run the generalised skills from .claude/skills/ (merged in No. 271; setup in .claude/scripts/README.md) on every PR you review or merge:

• din-pr-review: runs the peer check and posts comments 1 (verification) and 2 (per-file merge proposal) before you approve.
• din-pr-merge (path A): merges, then fills the Actual merge proposal rows and posts comment 3 (outcome). As the merger, those rows on the table above are yours to fill (MAINTAINING No. 4.3).
• Labels: Reviewed after comments 1–2, then merged after the merge (No. 4.8).

Also, any PR touching foundry/src/** needs the second sign-off from @umeradl or @abrahamnash before merge, reverts included (No. 4.2 / No. 4.7). Please request it rather than merging on one approval. GitHub's ruleset only enforces the single peer approval, so this is on the merger. That's exactly how No. 272 got in: it was peer-approved, but @robertocarlous merged it 5 hours after asking @abrahamnash for the second key, without getting it.

Edited 2026-10-26: corrected the revert reason. No. 272 did have a non-author approval (@robertocarlous, 08:56 UTC Oct 24, carried over to d8dbd1f). What it lacked was the foundry/src second sign-off (MAINTAINING No. 4.2), which GitHub doesn't enforce.

[umeradl]

@robertocarlous, a correction to the reason in this PR's description and in my comments above (now edited). No. 272 did have a non-author approval: yours, at 08:56 UTC on Oct 24, which GitHub carried over to d8dbd1f. What was missing was the foundry/src second sign-off (MAINTAINING No. 4.2). You asked @abrahamnash for it at 09:01, but the merge went in at 14:40 before he'd signed off. The develop ruleset only enforces the single peer approval, so for contract PRs the merger has to wait for the second key. Re-land: PR No. 287.