Skip to content

fix(foundry+dincli): commit-then-reveal for T1/T2 aggregation (issue #156 M-1, Part C) #197

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 6 commits into from
Sep 30, 2026
Merged

fix(foundry+dincli): commit-then-reveal for T1/T2 aggregation (issue #156 M-1, Part C) #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bbd8604
feat(foundry): commit-then-reveal for T1/T2 aggregation submissions
Abidoyesimze Sep 29, 2026
9091bf1
test(foundry): cover T1/T2 aggregation commit-reveal and the copy attack
Abidoyesimze Sep 29, 2026
1252d8f
feat(dincli): commit/reveal aggregator commands for T1/T2 submissions
Abidoyesimze Sep 29, 2026
b6627fa
docs: document T1/T2 commit-reveal, mark M-1 aggregation-side fixed
Abidoyesimze Sep 29, 2026
b5d602f
docs(audit): fill in PR #197 link for M-1 aggregation-side fix
Abidoyesimze Sep 29, 2026
b3ff1b9
merge: develop into feat/issue-156-part-c-commit-reveal
Abidoyesimze Sep 30, 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
47 changes: 41 additions & 6 deletions Documentation/public/workflows/model-workflow.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,17 @@ dincli model-owner lms-evaluation close <model_id>

Eligible local models are aggregated hierarchically. Tier 1 (T1) aggregation combines sub-batches, and Tier 2 (T2) aggregation combines the results of T1 into the new global model.

> [!NOTE]
> T1 and T2 aggregation submissions use commit-then-reveal (issue #156
> M-1): `aggregator aggregate-t1/t2 --submit` only **commits** a hidden
> hash of the aggregated CID. The model owner must close the commit
> window and open the reveal window (`aggregation T1/T2 start-reveal`)
> before aggregators can reveal with `aggregator reveal-t1`/`reveal-t2`
> — only revealed CIDs count toward the batch's finalized result. An
> aggregator who commits but never reveals is excluded from finalization
> and is still slashable for a missed submission, exactly as if they had
> never submitted at all.

**Model Owner** generates T1 & T2 batches and starts T1 aggregation:

> [!NOTE]
Expand All @@ -400,41 +411,65 @@ dincli model-owner aggregation create-t1nt2-batches <model_id>
# show t1 and t2 batches
dincli model-owner aggregation show-t1-batches <model_id> --detailed
dincli model-owner aggregation show-t2-batches <model_id> --detailed
# start t1 aggregation
# start t1 aggregation (opens the commit window)
dincli model-owner aggregation T1 start <model_id>
```

**Aggregators** perform T1 aggregation (repeat for each aggregator):
**Aggregators** commit their T1 aggregation (repeat for each aggregator):

```bash
# show the aggregator its assigned t1 batches
dincli aggregator show-t1-batches <model_id> --detailed
# aggregate the assigned t1 batches
# aggregate and commit the assigned t1 batches
dincli aggregator aggregate-t1 <model_id> --submit
```

**Model Owner** opens the T1 reveal window:

```bash
dincli model-owner aggregation T1 start-reveal <model_id>
```

**Aggregators** reveal their committed T1 aggregation (repeat for each aggregator, on the same machine/cache the commit was made from):

```bash
dincli aggregator reveal-t1 <model_id>
```

**Model Owner** closes T1 and starts T2 aggregation:

```bash
# show t1 batches
dincli model-owner aggregation show-t1-batches <model_id> --detailed
# close t1 aggregation
dincli model-owner aggregation T1 close <model_id>
# start t2 aggregation
# start t2 aggregation (opens the commit window)
dincli model-owner aggregation T2 start <model_id>
# show t2 batches
dincli model-owner aggregation show-t2-batches <model_id> --detailed
```

**Aggregators** perform T2 aggregation (repeat for each aggregator):
**Aggregators** commit their T2 aggregation (repeat for each aggregator):

```bash
# show the aggregator its assigned t2 batches
dincli aggregator show-t2-batches <model_id> --detailed
# aggregate the assigned t2 batches
# aggregate and commit the assigned t2 batches
dincli aggregator aggregate-t2 <model_id> --submit
```

**Model Owner** opens the T2 reveal window:

```bash
dincli model-owner aggregation T2 start-reveal <model_id>
```

**Aggregators** reveal their committed T2 aggregation (repeat for each aggregator, on the same machine/cache the commit was made from):

```bash
dincli aggregator reveal-t2 <model_id>
```

**Model Owner** closes T2 aggregation:

```bash
Expand Down
2 changes: 2 additions & 0 deletions Documentation/technical/audits/foundry-src-security-review.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ The caller of these functions (the model owner, since both are gated `onlyOwner`

### M-1. No commit-reveal on aggregation/scoring submissions — "copy the leader" free-riding

**Fixed — auditor scoring: PR #63 (task_210726_6 §2a, predates this report's follow-up numbering). Aggregation-side (`submitT1Aggregation`/`submitT2Aggregation`, described below): [PR #197](https://github.com/InfiniteZeroFoundation/DevNet/pull/197) (issue #156, task_240926_18 Part C).** `commitT1Aggregation`/`revealT1Aggregation` and `commitT2Aggregation`/`revealT2Aggregation` replace the single-shot submit functions; only revealed CIDs count toward finalization, closing the "read every prior submission, then copy the leader" path this finding describes. The commit hash binds `msg.sender` (`keccak256(abi.encode(cid, salt, msg.sender, GI, tierKind, batchId))`), deliberately hardening past this finding's own `keccak256(cid, salt)` recommendation — without the sender binding, a lazy aggregator could copy a peer's *commit hash* itself and reveal the peer's `(cid, salt)` under their own name once the peer reveals, reproducing the same free-riding this fix is meant to close. PR #63's auditor-side commit hash (`keccak256(abi.encodePacked(score, vote, salt))`) has this same unbound-sender weakness and was **not** fixed as part of this PR — see the new issue opened for it, linked from the PR.

**Contracts / functions:** `DINTaskCoordinator.submitT1Aggregation()` / `submitT2Aggregation()` (L520-543, L600-622); `DINTaskAuditor.setAuditScorenEligibility()` (L515-544).

Votes/scores/CIDs are submitted in the clear and tallied by direct value match; there is no commit-then-reveal step. Any participant who is not the first to submit for a given batch/model can read every prior submission from public contract state before deciding what to submit themselves. A lazy or dishonest aggregator can copy another party's already-submitted CID instead of doing the aggregation work, guaranteeing they "match consensus" and avoid `AGG_T1_BAD_CONSENSUS`/`AGG_T2_BAD_CONSENSUS` slashing while contributing nothing. The same applies to an auditor submitting last on `setAuditScorenEligibility` — they can see the running vote tally (`_tryFinalizeEligibility` is invoked after every vote, so intermediate state is observable) and simply match the emerging majority.
Expand Down
58 changes: 33 additions & 25 deletions Documentation/technical/contracts/DINShared.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,18 @@ Value State Name Description
14 LMSevaluationRevealStarted Reveal phase: auditors reveal (score, vote, salt) via `revealAuditScore`; eligibility and median scoring are computed from revealed values only.
15 LMSevaluationClosed Evaluation finalized; approved models identified.
16 T1nT2Bcreated Tier-1 and Tier-2 aggregation batches formed.
17 T1AggregationStarted Tier-1 aggregators can submit their aggregated CIDs.
18 T1AggregationDone Tier-1 finalized; winning CIDs per batch recorded.
19 T2AggregationStarted Tier-2 aggregators can submit their aggregated CIDs.
20 T2AggregationDone Tier-2 finalized; global winning CID recorded.
21 AuditorsSlashed Auditor slashing phase executed.
22 AggregatorsSlashed Aggregator slashing phase executed.
23 GIended GI is complete; system is ready for next GI.
17 T1AggregationStarted Commit phase: Tier-1 aggregators submit hidden CID commitments via `commitT1Aggregation`.
18 T1AggregationRevealStarted Reveal phase: Tier-1 aggregators reveal (cid, salt) via `revealT1Aggregation`; only revealed CIDs count toward finalization.
19 T1AggregationDone Tier-1 finalized; winning CIDs per batch recorded.
20 T2AggregationStarted Commit phase: Tier-2 aggregators submit hidden CID commitments via `commitT2Aggregation`.
21 T2AggregationRevealStarted Reveal phase: Tier-2 aggregators reveal (cid, salt) via `revealT2Aggregation`; only revealed CIDs count toward finalization.
22 T2AggregationDone Tier-2 finalized; global winning CID recorded.
23 AuditorsSlashed Auditor slashing phase executed.
24 AggregatorsSlashed Aggregator slashing phase executed.
25 GIended GI is complete; system is ready for next GI.
```

> **Ordinal note:** `LMSevaluationRevealStarted` (commit-then-reveal auditor scoring, task_210726_6 §2a) sits between `LMSevaluationStarted` and `LMSevaluationClosed` at ordinal 14, shifting every state from `LMSevaluationClosed` onward by +1 relative to the pre-commit-reveal numbering. `dincli/cli/utils.py`'s `states`/`stateDescription` positional mirrors (indexed by this same raw ordinal) have been updated to match — see `dincli/cli/utils.py`'s `states`/`stateDescription` lists.
> **Ordinal note:** `LMSevaluationRevealStarted` (commit-then-reveal auditor scoring, task_210726_6 §2a) sits between `LMSevaluationStarted` and `LMSevaluationClosed` at ordinal 14. `T1AggregationRevealStarted` and `T2AggregationRevealStarted` (commit-then-reveal T1/T2 aggregation, issue #156 M-1, task_240926_18 Part C) sit at ordinals 18 and 21 respectively, immediately after their corresponding commit-phase state — same insert-in-lifecycle-position precedent, not appended. Each insertion shifts every later ordinal by +1 relative to the prior numbering. `dincli/cli/utils.py`'s `states`/`stateDescription` positional mirrors (indexed by this same raw ordinal) have been updated to match — see `dincli/cli/utils.py`'s `states`/`stateDescription` lists.

### 2.2 State Transition Diagram

Expand All @@ -70,39 +72,45 @@ Value State Name Description
[3] AwaitingGenesisModel
│ setGenesisModelIpfsHash()
▼
[4] GenesisModelCreated ◄──────────────────────────────── [23] GIended
[4] GenesisModelCreated ◄──────────────────────────────── [25] GIended
│ startGI() ▲
▼ │ endGI()
[5] GIstarted [22] AggregatorsSlashed
[5] GIstarted [24] AggregatorsSlashed
│ startDINaggregatorsRegistration() ▲
▼ │ slashAggregators()
[6] DINaggregatorsRegistrationStarted [21] AuditorsSlashed
[6] DINaggregatorsRegistrationStarted [23] AuditorsSlashed
│ closeDINaggregatorsRegistration() ▲
▼ │ slashAuditors()
[7] DINaggregatorsRegistrationClosed [20] T2AggregationDone
[7] DINaggregatorsRegistrationClosed [22] T2AggregationDone
│ startDINauditorsRegistration() ▲
▼ │ finalizeT2Aggregation()
[8] DINauditorsRegistrationStarted [19] T2AggregationStarted
[8] DINauditorsRegistrationStarted [21] T2AggregationRevealStarted
│ closeDINauditorsRegistration() ▲
▼ │ startT2Aggregation()
[9] DINauditorsRegistrationClosed [18] T1AggregationDone
▼ │ startT2AggregationReveal()
[9] DINauditorsRegistrationClosed [20] T2AggregationStarted
│ startLMsubmissions() ▲
▼ │ finalizeT1Aggregation()
[10] LMSstarted [17] T1AggregationStarted
▼ │ startT2Aggregation()
[10] LMSstarted [19] T1AggregationDone
│ closeLMsubmissions() ▲
▼ │ startT1Aggregation()
[11] LMSclosed [16] T1nT2Bcreated
▼ │ finalizeT1Aggregation()
[11] LMSclosed [18] T1AggregationRevealStarted
│ createAuditorsBatches() ▲
▼ │ autoCreateTier1AndTier2()
[12] AuditorsBatchesCreated [15] LMSevaluationClosed
▼ │ startT1AggregationReveal()
[12] AuditorsBatchesCreated [17] T1AggregationStarted
│ startLMsubmissionsEvaluation() ▲
▼ │ closeLMsubmissionsEvaluation()
[13] LMSevaluationStarted [14] LMSevaluationRevealStarted
▼ │ startT1Aggregation()
[13] LMSevaluationStarted [16] T1nT2Bcreated
│ (auditors: commitAuditScore, commit phase) ▲
└──────────── startLMsubmissionsEvaluationReveal() ────────┘
(auditors: revealAuditScore, reveal phase)
└──────────── startLMsubmissionsEvaluationReveal() ────────┤ autoCreateTier1AndTier2()
(auditors: revealAuditScore, reveal phase) │
[15] LMSevaluationClosed
▲
│ closeLMsubmissionsEvaluation()
[14] LMSevaluationRevealStarted
```

T1/T2 aggregation submissions follow the same commit-then-reveal shape as LMS evaluation above: `commitT1Aggregation`/`commitT2Aggregation` during the `*AggregationStarted` (commit) state, then the model owner calls `startT1AggregationReveal`/`startT2AggregationReveal` to open `*AggregationRevealStarted`, during which aggregators call `revealT1Aggregation`/`revealT2Aggregation`. Only revealed CIDs are counted by `finalizeT1Aggregation`/`finalizeT2Aggregation`; a committed-but-never-revealed aggregator is excluded from finalization and remains slashable via `slashAggregators`' existing "no submission" (S2) check (issue #156 M-1, task_240926_18 Part C).

---

## 3. Cross-Contract Interfaces
Expand Down
Loading
Loading