From bbd8604c365f26508ef5b6de89a807d7570567e0 Mon Sep 17 00:00:00 2001 From: Abidoyesimze Date: Tue, 29 Sep 2026 01:05:25 +0100 Subject: [PATCH 1/5] feat(foundry): commit-then-reveal for T1/T2 aggregation submissions submitT1Aggregation/submitT2Aggregation accepted a plaintext CID in a single transaction, letting any aggregator who isn't first to submit in their batch read every prior submission from public state and copy it instead of doing the aggregation work (issue #156 M-1). Replace with commitT1Aggregation/revealT1Aggregation and the T2 equivalents, mirroring PR #63's auditor-side commit-reveal shape with one deliberate hardening: the commit hash binds msg.sender (plus GI, tier, batchId) -- keccak256(abi.encode(cid, salt, msg.sender, GI, tierKind, batchId)) -- rather than issue #156's own keccak256(cid, salt) proposal. 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 hash has this identical weakness and is deliberately left unfixed here -- tracked separately in #192. GIstates gains T1AggregationRevealStarted/T2AggregationRevealStarted, inserted immediately after their corresponding commit-phase state (same lifecycle-position precedent as LMSevaluationRevealStarted, not appended). t1Submitted/t2Submitted and t1SubmissionCID/t2SubmissionCID are now written at reveal time only; a committed-but-never-revealed aggregator simply never sets them, so slashAggregators()'s existing "no submission" (S2) check needed no changes to keep working. finalizeT1Aggregation/finalizeT2Aggregation now gate on the reveal state instead of the commit state; their body is otherwise unchanged. Co-Authored-By: Claude Sonnet 5 --- foundry/src/DINShared.sol | 60 +++++++++-- foundry/src/DINTaskCoordinator.sol | 168 +++++++++++++++++++++++++---- 2 files changed, 202 insertions(+), 26 deletions(-) diff --git a/foundry/src/DINShared.sol b/foundry/src/DINShared.sol index bd6b817..6ec840d 100644 --- a/foundry/src/DINShared.sol +++ b/foundry/src/DINShared.sol @@ -36,12 +36,21 @@ enum GIstates { LMSevaluationClosed, // 15 T1nT2Bcreated, // 16 T1AggregationStarted, // 17 - T1AggregationDone, // 18 - T2AggregationStarted, // 19 - T2AggregationDone, // 20 - AuditorsSlashed, // 21 - AggregatorsSlashed, // 22 - GIended // 23 + // Reveal phase for commit-then-reveal T1/T2 aggregation submissions + // (issue #156 M-1, task_240926_18 Part C). Same 2026-08-27 precedent as + // LMSevaluationRevealStarted above: inserted immediately after each + // commit phase rather than appended, so dincli/cli/utils.py's `states`/ + // `stateDescription` mirrors and Documentation/technical/contracts/ + // DINShared.md §2.1 have been updated to match, shifting every + // subsequent entry by +1 (T1) then +1 again (T2). + T1AggregationRevealStarted, // 18 + T1AggregationDone, // 19 + T2AggregationStarted, // 20 + T2AggregationRevealStarted, // 21 + T2AggregationDone, // 22 + AuditorsSlashed, // 23 + AggregatorsSlashed, // 24 + GIended // 25 } // ───────────────────────────────────────────────────────────────────────────── @@ -295,7 +304,8 @@ error TC_BatchNotFound(); error TC_OnlyOneTier2Batch(); /// @dev GI state does not permit starting T1 aggregation. error TC_NotReadyForT1Aggregation(); -/// @dev T1 aggregation phase has not been started. +/// @dev T1 aggregation commit phase has not been started (see +/// commitT1Aggregation, issue #156 M-1). error TC_T1AggregationNotStarted(); /// @dev The batch index or aggregator assignment is invalid. error TC_InvalidBatch(); @@ -309,7 +319,8 @@ error TC_NoSubmissions(); error TC_NotReadyToFinalizeT1(); /// @dev GI state does not permit starting T2 aggregation. error TC_NotReadyForT2Aggregation(); -/// @dev T2 aggregation phase has not been started. +/// @dev T2 aggregation commit phase has not been started (see +/// commitT2Aggregation, issue #156 M-1). error TC_T2AggregationNotStarted(); /// @dev The T2 batch has not received its submission yet. error TC_NotReadyToFinalizeT2(); @@ -396,3 +407,36 @@ error TC_InvalidSlashFraction(); error TC_DisputeSeedNotLocked(); error TC_DisputeSeedBlockNotMined(); error TC_DisputeSeedAlreadyLocked(); + +// ───────────────────────────────────────────────────────────────────────────── +// Custom errors — commit-then-reveal T1/T2 aggregation (issue #156 M-1, +// task_240926_18 Part C, DINTaskCoordinator). Mirrors task_210726_6 §2a's +// auditor-side commit-reveal errors above, with the sender-bound hash +// hardening described on commitT1Aggregation/commitT2Aggregation's NatSpec. +// ───────────────────────────────────────────────────────────────────────────── + +/// @dev GI state does not permit starting the T1 aggregation reveal phase. +error TC_T1RevealCannotBeStarted(); +/// @dev GI state does not permit revealing a T1 aggregation submission at this time. +error TC_T1RevealPhaseNotOpen(); +/// @dev This aggregator has already committed a T1 aggregation CID for this batch. +error TC_T1AlreadyCommitted(); +/// @dev The T1 commit hash must not be zero. +error TC_T1EmptyCommitHash(); +/// @dev This aggregator has not committed a T1 aggregation CID for this batch. +error TC_T1NoCommitFound(); +/// @dev The revealed (cid, salt) does not hash to the stored T1 commitment. +error TC_T1RevealHashMismatch(); + +/// @dev GI state does not permit starting the T2 aggregation reveal phase. +error TC_T2RevealCannotBeStarted(); +/// @dev GI state does not permit revealing a T2 aggregation submission at this time. +error TC_T2RevealPhaseNotOpen(); +/// @dev This aggregator has already committed a T2 aggregation CID for this batch. +error TC_T2AlreadyCommitted(); +/// @dev The T2 commit hash must not be zero. +error TC_T2EmptyCommitHash(); +/// @dev This aggregator has not committed a T2 aggregation CID for this batch. +error TC_T2NoCommitFound(); +/// @dev The revealed (cid, salt) does not hash to the stored T2 commitment. +error TC_T2RevealHashMismatch(); diff --git a/foundry/src/DINTaskCoordinator.sol b/foundry/src/DINTaskCoordinator.sol index 7b769b6..a881922 100644 --- a/foundry/src/DINTaskCoordinator.sol +++ b/foundry/src/DINTaskCoordinator.sol @@ -54,11 +54,18 @@ contract DINTaskCoordinator is Ownable, ReentrancyGuardTransient { mapping(uint => mapping(uint => mapping(address => bool))) isTier1Aggregator; // Audit & voting maps GI ➜ batchId ➜ validator ➜ … + // t1SubmissionCID/t1Submitted are written at REVEAL time only (issue #156 + // M-1, task_240926_18 Part C commit-then-reveal) -- a committed-but- + // never-revealed aggregator leaves t1Submitted false, which is exactly + // what slashAggregators()'s existing "no submission" (S2) check already + // reads, so that loop needed no changes for the commit-reveal split. mapping(uint => mapping(uint => mapping(address => bytes32))) public t1SubmissionCID; mapping(uint => mapping(uint => mapping(address => bool))) public t1Submitted; mapping(uint => mapping(uint => mapping(bytes32 => uint))) public t1Votes; // CID ➜ votes + mapping(uint => mapping(uint => mapping(address => bytes32))) public t1CommitHash; + mapping(uint => mapping(uint => mapping(address => bool))) public t1Committed; struct Tier2Batch { uint batchId; @@ -76,6 +83,8 @@ contract DINTaskCoordinator is Ownable, ReentrancyGuardTransient { mapping(uint => mapping(uint => mapping(address => bool))) public t2Submitted; mapping(uint => mapping(uint => mapping(bytes32 => uint))) public t2Votes; + mapping(uint => mapping(uint => mapping(address => bytes32))) public t2CommitHash; + mapping(uint => mapping(uint => mapping(address => bool))) public t2Committed; /// @notice Per-aggregator count of finalized T1/T2 batches they were /// assigned to in a GI, one increment per (aggregator, @@ -237,7 +246,9 @@ contract DINTaskCoordinator is Ownable, ReentrancyGuardTransient { /// @notice Emitted on every GI state transition. /// @dev GI is 0 during constructor/setup transitions (ordinals 0–4); expected. event GIStateChanged(uint indexed GI, uint8 indexed newState); + event T1AggregationCommitted(uint indexed GI, uint indexed batchId, address indexed aggregator, bytes32 commitHash); event T1AggregationSubmitted(uint indexed GI, uint indexed batchId, address indexed aggregator, bytes32 cid); + event T2AggregationCommitted(uint indexed GI, uint indexed batchId, address indexed aggregator, bytes32 commitHash); event T2AggregationSubmitted(uint indexed GI, uint indexed batchId, address indexed aggregator, bytes32 cid); event T1BatchFinalized(uint indexed GI, uint indexed batchId, bytes32 winningCID); event T2Finalized(uint indexed GI, bytes32 globalModelCID); @@ -727,7 +738,8 @@ contract DINTaskCoordinator is Ownable, ReentrancyGuardTransient { } /// @notice Transitions GI state to T1AggregationStarted, opening the - /// Tier-1 submission window for assigned aggregators. + /// Tier-1 commit window for assigned aggregators + /// (commitT1Aggregation). /// @param _GI Current GI index. function startT1Aggregation( uint _GI @@ -737,20 +749,79 @@ contract DINTaskCoordinator is Ownable, ReentrancyGuardTransient { _setGIstate(GIstates.T1AggregationStarted); } - /// @notice Submits an aggregation result CID for a Tier-1 batch. - /// @dev Caller must be an assigned, active aggregator who has not already submitted. - /// Votes are tallied per CID; the majority CID is selected at finalization. + /// @notice Phase 1 of commit-then-reveal Tier-1 aggregation: lock in a + /// hidden aggregation CID. + /// @dev Caller must be an assigned, active aggregator who has not already + /// committed. `commitHash` must equal keccak256(abi.encode(cid, + /// salt, msg.sender, GI, TierKind.Tier1, batchId)) for the values + /// revealed later -- binding the committer's own address (and GI/ + /// tier/batchId) into the hash, unlike PR #63's auditor-side + /// commitHash, closes the "replay a peer's commit hash and reveal + /// their own (cid, salt) under it after they reveal" free-riding + /// path (issue #156 M-1). The contract cannot and does not validate + /// this at commit time -- that's the point. /// @param _GI Current GI index. /// @param _batchId Tier-1 batch index. - /// @param _aggregationCID IPFS CID of the aggregated model weights, encoded as bytes32. - function submitT1Aggregation( + /// @param commitHash keccak256(abi.encode(cid, salt, msg.sender, GI, TierKind.Tier1, batchId)). + function commitT1Aggregation( uint _GI, uint _batchId, - bytes32 _aggregationCID + bytes32 commitHash ) external onlyCurrentGI(_GI) { if (GIstate != GIstates.T1AggregationStarted) revert TC_T1AggregationNotStarted(); if (_batchId >= tier1Batches[_GI].length) revert TC_InvalidBatch(); + if (!isTier1Aggregator[_GI][_batchId][msg.sender]) + revert TC_NotBatchAggregator(); + if (!dinvalidatorStakeContract.isValidatorActive(msg.sender)) { + revert TC_AggregatorNotActive(); + } + if (commitHash == bytes32(0)) revert TC_T1EmptyCommitHash(); + if (t1Committed[_GI][_batchId][msg.sender]) + revert TC_T1AlreadyCommitted(); + + t1CommitHash[_GI][_batchId][msg.sender] = commitHash; + t1Committed[_GI][_batchId][msg.sender] = true; + + emit T1AggregationCommitted(_GI, _batchId, msg.sender, commitHash); + } + + /// @notice Closes the T1 commit window and opens the reveal window so + /// assigned aggregators can call revealT1Aggregation. + /// @dev Must run strictly after commits close and before any reveal is + /// accepted -- see revealT1Aggregation's GIstate gate. + /// @param _GI Current GI index. + function startT1AggregationReveal( + uint _GI + ) external onlyOwner onlyCurrentGI(_GI) { + if (GIstate != GIstates.T1AggregationStarted) + revert TC_T1RevealCannotBeStarted(); + _setGIstate(GIstates.T1AggregationRevealStarted); + } + + /// @notice Phase 2 of commit-then-reveal: reveal the (cid, salt) behind a + /// prior commitment and have it counted. + /// @dev Reverts unless the caller committed for this (GI, batchId) and the + /// revealed values hash to that commitment. Open only while + /// GIstate == T1AggregationRevealStarted, strictly after the commit + /// window has been closed by the model owner. An aggregator who + /// committed but never reveals simply never sets t1Submitted, so + /// they're excluded from finalization and remain slashable via the + /// existing slashAggregators() "no submission" (S2) check -- no + /// special-casing needed for the non-reveal case. + /// @param _GI Current GI index. + /// @param _batchId Tier-1 batch index. + /// @param _aggregationCID IPFS CID of the aggregated model weights, encoded as bytes32. + /// @param salt Arbitrary value chosen at commit time to prevent hash pre-image search. + function revealT1Aggregation( + uint _GI, + uint _batchId, + bytes32 _aggregationCID, + bytes32 salt + ) external onlyCurrentGI(_GI) { + if (GIstate != GIstates.T1AggregationRevealStarted) + revert TC_T1RevealPhaseNotOpen(); + if (_batchId >= tier1Batches[_GI].length) revert TC_InvalidBatch(); // Verify sender is an assigned aggregator if (!isTier1Aggregator[_GI][_batchId][msg.sender]) @@ -758,10 +829,17 @@ contract DINTaskCoordinator is Ownable, ReentrancyGuardTransient { if (!dinvalidatorStakeContract.isValidatorActive(msg.sender)) { revert TC_AggregatorNotActive(); } + if (!t1Committed[_GI][_batchId][msg.sender]) revert TC_T1NoCommitFound(); if (t1Submitted[_GI][_batchId][msg.sender]) revert TC_AlreadySubmitted(); if (_aggregationCID == bytes32(0)) revert TC_ZeroCID(); + bytes32 expectedHash = keccak256( + abi.encode(_aggregationCID, salt, msg.sender, _GI, TierKind.Tier1, _batchId) + ); + if (expectedHash != t1CommitHash[_GI][_batchId][msg.sender]) + revert TC_T1RevealHashMismatch(); + t1Submitted[_GI][_batchId][msg.sender] = true; t1SubmissionCID[_GI][_batchId][msg.sender] = _aggregationCID; emit T1AggregationSubmitted(_GI, _batchId, msg.sender, _aggregationCID); @@ -770,14 +848,14 @@ contract DINTaskCoordinator is Ownable, ReentrancyGuardTransient { t1Votes[_GI][_batchId][_aggregationCID]++; } - /// @notice Closes the Tier-1 submission window and selects the majority CID + /// @notice Closes the Tier-1 reveal window and selects the majority CID /// for every batch. /// @dev Iterates all T1 batches; reverts on the first batch that has no submissions. /// @param _GI Current GI index. function finalizeT1Aggregation( uint _GI ) external onlyOwner onlyCurrentGI(_GI) { - if (GIstate != GIstates.T1AggregationStarted) + if (GIstate != GIstates.T1AggregationRevealStarted) revert TC_NotReadyToFinalizeT1(); Tier1Batch[] storage batches = tier1Batches[_GI]; @@ -825,7 +903,8 @@ contract DINTaskCoordinator is Ownable, ReentrancyGuardTransient { _setGIstate(GIstates.T1AggregationDone); } - /// @notice Opens the Tier-2 aggregation submission window. + /// @notice Opens the Tier-2 aggregation commit window + /// (commitT2Aggregation). /// @param _GI Current GI index. function startT2Aggregation( uint _GI @@ -835,30 +914,83 @@ contract DINTaskCoordinator is Ownable, ReentrancyGuardTransient { _setGIstate(GIstates.T2AggregationStarted); } - /// @notice Submits an aggregation result CID for the Tier-2 batch. - /// @dev _batchId must be 0. Caller must be an assigned, active aggregator who - /// has not already submitted. + /// @notice Phase 1 of commit-then-reveal Tier-2 aggregation: lock in a + /// hidden aggregation CID. + /// @dev Same sender-bound commit-hash hardening as commitT1Aggregation + /// (issue #156 M-1); see that function's NatSpec. /// @param _GI Current GI index. /// @param _batchId Must be 0. - /// @param _aggregationCID IPFS CID of the final aggregated model, encoded as bytes32. - function submitT2Aggregation( + /// @param commitHash keccak256(abi.encode(cid, salt, msg.sender, GI, TierKind.Tier2, batchId)). + function commitT2Aggregation( uint _GI, uint _batchId, - bytes32 _aggregationCID + bytes32 commitHash ) external onlyCurrentGI(_GI) { if (GIstate != GIstates.T2AggregationStarted) revert TC_T2AggregationNotStarted(); if (_batchId != 0) revert TC_OnlyOneTier2Batch(); + if (!isTier2Aggregator[_GI][_batchId][msg.sender]) + revert TC_NotBatchAggregator(); + if (!dinvalidatorStakeContract.isValidatorActive(msg.sender)) { + revert TC_AggregatorNotActive(); + } + if (commitHash == bytes32(0)) revert TC_T2EmptyCommitHash(); + if (t2Committed[_GI][_batchId][msg.sender]) + revert TC_T2AlreadyCommitted(); + + t2CommitHash[_GI][_batchId][msg.sender] = commitHash; + t2Committed[_GI][_batchId][msg.sender] = true; + + emit T2AggregationCommitted(_GI, _batchId, msg.sender, commitHash); + } + + /// @notice Closes the T2 commit window and opens the reveal window so + /// assigned aggregators can call revealT2Aggregation. + /// @param _GI Current GI index. + function startT2AggregationReveal( + uint _GI + ) external onlyOwner onlyCurrentGI(_GI) { + if (GIstate != GIstates.T2AggregationStarted) + revert TC_T2RevealCannotBeStarted(); + _setGIstate(GIstates.T2AggregationRevealStarted); + } + + /// @notice Phase 2 of commit-then-reveal: reveal the (cid, salt) behind a + /// prior commitment and have it counted. + /// @dev _batchId must be 0. Same non-reveal handling as revealT1Aggregation + /// (see its NatSpec) -- a committed-but-never-revealed aggregator + /// simply never sets t2Submitted, and remains slashable via + /// slashAggregators()'s existing "no submission" (S2) check. + /// @param _GI Current GI index. + /// @param _batchId Must be 0. + /// @param _aggregationCID IPFS CID of the final aggregated model, encoded as bytes32. + /// @param salt Arbitrary value chosen at commit time to prevent hash pre-image search. + function revealT2Aggregation( + uint _GI, + uint _batchId, + bytes32 _aggregationCID, + bytes32 salt + ) external onlyCurrentGI(_GI) { + if (GIstate != GIstates.T2AggregationRevealStarted) + revert TC_T2RevealPhaseNotOpen(); + if (_batchId != 0) revert TC_OnlyOneTier2Batch(); if (!isTier2Aggregator[_GI][_batchId][msg.sender]) revert TC_NotBatchAggregator(); if (!dinvalidatorStakeContract.isValidatorActive(msg.sender)) { revert TC_AggregatorNotActive(); } + if (!t2Committed[_GI][_batchId][msg.sender]) revert TC_T2NoCommitFound(); if (t2Submitted[_GI][_batchId][msg.sender]) revert TC_AlreadySubmitted(); if (_aggregationCID == bytes32(0)) revert TC_ZeroCID(); + bytes32 expectedHash = keccak256( + abi.encode(_aggregationCID, salt, msg.sender, _GI, TierKind.Tier2, _batchId) + ); + if (expectedHash != t2CommitHash[_GI][_batchId][msg.sender]) + revert TC_T2RevealHashMismatch(); + t2Submitted[_GI][_batchId][msg.sender] = true; t2SubmissionCID[_GI][_batchId][msg.sender] = _aggregationCID; emit T2AggregationSubmitted(_GI, _batchId, msg.sender, _aggregationCID); @@ -867,13 +999,13 @@ contract DINTaskCoordinator is Ownable, ReentrancyGuardTransient { t2Votes[_GI][_batchId][_aggregationCID]++; } - /// @notice Closes the Tier-2 submission window and selects the majority CID. + /// @notice Closes the Tier-2 reveal window and selects the majority CID. /// @dev Reverts if the Tier-2 batch has received no submissions. /// @param _GI Current GI index. function finalizeT2Aggregation( uint _GI ) external onlyOwner onlyCurrentGI(_GI) { - if (GIstate != GIstates.T2AggregationStarted) + if (GIstate != GIstates.T2AggregationRevealStarted) revert TC_NotReadyToFinalizeT2(); Tier2Batch[] storage batches = tier2Batches[_GI]; From 9091bf1fba944f0b1a726d32ba78958401edd3d6 Mon Sep 17 00:00:00 2001 From: Abidoyesimze Date: Tue, 29 Sep 2026 01:05:45 +0100 Subject: [PATCH 2/5] test(foundry): cover T1/T2 aggregation commit-reveal and the copy attack AggregatorCommitReveal.t.sol (new): the actual property M-1's fix is about -- test_copyAttack_replayingPeerCommitHash_cannotReveal proves an aggregator who copies a peer's commit hash verbatim cannot reveal the peer's (cid, salt) under their own address, since the hash the contract expects for their own reveal is computed with their own address baked in. Plus reveal-before-window/without-commit/wrong-cid/ wrong-salt/double-commit/double-reveal reverts, finalize-before-reveal gating, a commit-without-reveal S2-slash PoC, and a full-GI walk through both new reveal states (asserting GIstate at each step). The other 8 files gain commit+reveal (and, for the 0-T2-batch cases, a startT2AggregationReveal call) ahead of what were direct submitT1Aggregation/submitT2Aggregation calls, and GasSimulation.t.sol's dedicated submit-gas benchmarks split into separate commit/reveal measurements (revealT1Aggregation is what now does the vote-counting write the old single-shot submit did). Co-Authored-By: Claude Sonnet 5 --- foundry/test/AggregatorCommitReveal.t.sol | 516 +++++++++++++++++++++ foundry/test/DisputeResolution.t.sol | 66 ++- foundry/test/GasSimulation.t.sol | 139 ++++-- foundry/test/LifecycleEvents.t.sol | 124 ++++- foundry/test/PR146SlashingRegression.t.sol | 18 +- foundry/test/RewardEngine.t.sol | 31 +- foundry/test/SecurityFindings.t.sol | 32 +- foundry/test/StakingEnforcement.t.sol | 16 +- foundry/test/TreasuryForwarding.t.sol | 36 +- 9 files changed, 893 insertions(+), 85 deletions(-) create mode 100644 foundry/test/AggregatorCommitReveal.t.sol diff --git a/foundry/test/AggregatorCommitReveal.t.sol b/foundry/test/AggregatorCommitReveal.t.sol new file mode 100644 index 0000000..cdd5aaa --- /dev/null +++ b/foundry/test/AggregatorCommitReveal.t.sol @@ -0,0 +1,516 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.28; + +// ───────────────────────────────────────────────────────────────────────────── +// Contract-level tests for issue #156 M-1 (task_240926_18 Part C, aggregation +// side): commit-then-reveal for T1/T2 aggregation submissions. Mirrors +// AuditorCommitReveal.t.sol's structure, with one addition specific to this +// side's hardening -- the commit hash binds msg.sender (GI, tier, batchId +// too), unlike PR #63's auditor-side hash, so a lazy aggregator can't copy a +// peer's commit hash and reveal the peer's (cid, salt) under their own name. +// Run: forge test --match-contract AggregatorCommitRevealTest -vv +// ───────────────────────────────────────────────────────────────────────────── + +import {Test} from "forge-std/Test.sol"; +import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; + +import {DinToken} from "../src/DinToken.sol"; +import {DinCoordinator} from "../src/DinCoordinator.sol"; +import {DinValidatorStake} from "../src/DinValidatorStake.sol"; +import {DINModelRegistry} from "../src/DINModelRegistry.sol"; +import {DINTaskCoordinator} from "../src/DINTaskCoordinator.sol"; +import {DINTaskAuditor} from "../src/DINTaskAuditor.sol"; +import {GIstates} from "../src/DINShared.sol"; + +contract AggregatorCommitRevealTest is Test { + DinToken tokenImpl; + DinCoordinator coordinatorImpl; + DinValidatorStake stakeImpl; + DINModelRegistry registryImpl; + + DinToken token; + DinCoordinator coordinator; + DinValidatorStake stake; + DINModelRegistry registry; + + DINTaskCoordinator tc; + DINTaskAuditor ta; + + address admin = makeAddr("admin"); + address modelOwner = makeAddr("modelOwner"); + address auditor1 = makeAddr("auditor1"); + address auditor2 = makeAddr("auditor2"); + address auditor3 = makeAddr("auditor3"); + address client1 = makeAddr("client1"); + address client2 = makeAddr("client2"); + address client3 = makeAddr("client3"); + address agg1 = makeAddr("agg1"); + address agg2 = makeAddr("agg2"); + address agg3 = makeAddr("agg3"); + address agg4 = makeAddr("agg4"); + address agg5 = makeAddr("agg5"); + address agg6 = makeAddr("agg6"); + + // Fixed salt for every commit in this suite -- same rationale as + // AuditorCommitReveal.t.sol's TEST_SALT (secrecy isn't what these tests + // exercise; nothing here reads salt before the matching reveal call). + bytes32 constant TEST_SALT = bytes32(uint256(0xC0FFEE)); + bytes32 constant CID_A = bytes32(uint256(0xA1)); + bytes32 constant CID_B = bytes32(uint256(0xB2)); + + function _deployPlatform() internal { + vm.startPrank(admin); + + tokenImpl = new DinToken(); + TransparentUpgradeableProxy tokenProxy = new TransparentUpgradeableProxy( + address(tokenImpl), + admin, + abi.encodeCall(DinToken.initialize, ()) + ); + token = DinToken(address(tokenProxy)); + + coordinatorImpl = new DinCoordinator(); + TransparentUpgradeableProxy coordinatorProxy = new TransparentUpgradeableProxy( + address(coordinatorImpl), + admin, + abi.encodeCall(DinCoordinator.initialize, (address(token))) + ); + coordinator = DinCoordinator(address(coordinatorProxy)); + + token.setCoordinator(address(coordinator)); + + stakeImpl = new DinValidatorStake(); + TransparentUpgradeableProxy stakeProxy = new TransparentUpgradeableProxy( + address(stakeImpl), + admin, + abi.encodeCall( + DinValidatorStake.initialize, + (address(token), address(coordinator)) + ) + ); + stake = DinValidatorStake(address(stakeProxy)); + + coordinator.updateValidatorStakeContract(address(stake)); + + registryImpl = new DINModelRegistry(); + TransparentUpgradeableProxy registryProxy = new TransparentUpgradeableProxy( + address(registryImpl), + admin, + abi.encodeCall(DINModelRegistry.initialize, (address(stake))) + ); + registry = DINModelRegistry(address(registryProxy)); + + vm.stopPrank(); + } + + function _fundAndStake(address who) internal { + vm.deal(who, 1 ether); + vm.prank(who); + coordinator.depositAndMint{value: 0.001 ether}(); + vm.startPrank(who); + token.approve(address(stake), type(uint256).max); + stake.stake(10 ether); // MIN_STAKE + vm.stopPrank(); + } + + function _fundDinBalance(address who, uint256 dinAmount) internal { + uint256 ethNeeded = (dinAmount * 1e18) / (1_000_000 * 1e18) + 1; + vm.deal(who, ethNeeded + 1 ether); + vm.prank(who); + coordinator.depositAndMint{value: ethNeeded}(); + vm.prank(who); + token.approve(address(ta), type(uint256).max); + } + + function _deployTaskPair() internal { + vm.startPrank(modelOwner); + tc = new DINTaskCoordinator(address(stake), 1); + ta = new DINTaskAuditor(address(stake), address(tc), 1); + tc.setDINTaskAuditorContract(address(ta)); + vm.stopPrank(); + + vm.startPrank(admin); + coordinator.addSlasherContract(address(tc)); + coordinator.addSlasherContract(address(ta)); + vm.stopPrank(); + + vm.startPrank(modelOwner); + tc.setDINTaskCoordinatorAsSlasher(); + tc.setDINTaskAuditorAsSlasher(); + tc.setGenesisModelIpfsHash(bytes32(uint256(1))); + ta.setDinToken(address(token)); + vm.stopPrank(); + _fundDinBalance(modelOwner, 1 ether); + vm.startPrank(modelOwner); + ta.depositRewards(1, 1 ether); + tc.startGI(1); + vm.stopPrank(); + } + + /// @dev Drives the GI to T1AggregationStarted (the T1 commit-phase state) + /// with 6 registered aggregators (3 T1 + 3 T2 per + /// T1_AGGREGATORS_PER_BATCH), 3 auditors, and 3 approved models + /// (T1_MODELS_PER_BATCH), WITHOUT committing or revealing any T1 + /// aggregation -- callers drive commit/reveal themselves. + function _runToT1AggregationStarted() internal { + _deployPlatform(); + _deployTaskPair(); + + _fundAndStake(auditor1); + _fundAndStake(auditor2); + _fundAndStake(auditor3); + _fundAndStake(agg1); + _fundAndStake(agg2); + _fundAndStake(agg3); + _fundAndStake(agg4); + _fundAndStake(agg5); + _fundAndStake(agg6); + + vm.startPrank(modelOwner); + tc.startDINaggregatorsRegistration(1); + vm.stopPrank(); + vm.prank(agg1); tc.registerDINaggregator(1); + vm.prank(agg2); tc.registerDINaggregator(1); + vm.prank(agg3); tc.registerDINaggregator(1); + vm.prank(agg4); tc.registerDINaggregator(1); + vm.prank(agg5); tc.registerDINaggregator(1); + vm.prank(agg6); tc.registerDINaggregator(1); + + vm.startPrank(modelOwner); + tc.closeDINaggregatorsRegistration(1); + tc.startDINauditorsRegistration(1); + vm.stopPrank(); + + vm.prank(auditor1); ta.registerDINAuditor(1); + vm.prank(auditor2); ta.registerDINAuditor(1); + vm.prank(auditor3); ta.registerDINAuditor(1); + + vm.startPrank(modelOwner); + tc.closeDINauditorsRegistration(1); + tc.startLMsubmissions(1); + vm.stopPrank(); + + vm.prank(client1); ta.submitLocalModel(bytes32(uint256(100)), 1); + vm.prank(client2); ta.submitLocalModel(bytes32(uint256(200)), 1); + vm.prank(client3); ta.submitLocalModel(bytes32(uint256(300)), 1); + + vm.startPrank(modelOwner); + tc.closeLMsubmissions(1); + tc.createAuditorsBatches(1); + tc.setTestDataAssignedFlag(1, true); + tc.startLMsubmissionsEvaluation(1); + vm.stopPrank(); + + (, address[] memory batchAuditors, uint[] memory modelIdxs, ) = ta.getAuditorsBatch(1, 0); + bytes32 scoreCommit = keccak256(abi.encodePacked(uint256(100), true, TEST_SALT)); + for (uint i = 0; i < batchAuditors.length; i++) { + for (uint m = 0; m < modelIdxs.length; m++) { + vm.prank(batchAuditors[i]); + ta.commitAuditScore(1, 0, modelIdxs[m], scoreCommit); + } + } + + vm.prank(modelOwner); + tc.startLMsubmissionsEvaluationReveal(1); + + for (uint i = 0; i < batchAuditors.length; i++) { + for (uint m = 0; m < modelIdxs.length; m++) { + vm.prank(batchAuditors[i]); + ta.revealAuditScore(1, 0, modelIdxs[m], 100, true, TEST_SALT); + } + } + + vm.startPrank(modelOwner); + tc.closeLMsubmissionsEvaluation(1); + tc.autoCreateTier1AndTier2(1); + tc.startT1Aggregation(1); + vm.stopPrank(); + } + + // ───────────────────────────────────────────────────────────────────── + // Commit-then-reveal helpers. `_openT1RevealPhase` is the model-owner + // action closing T1 commits and opening T1 reveals. + // ───────────────────────────────────────────────────────────────────── + + function _t1CommitHash(address who, bytes32 cid, bytes32 salt, uint batchId) internal pure returns (bytes32) { + return keccak256(abi.encode(cid, salt, who, uint(1), DINTaskCoordinator.TierKind.Tier1, batchId)); + } + + function _commitT1(address who, uint batchId, bytes32 cid) internal { + vm.prank(who); + tc.commitT1Aggregation(1, batchId, _t1CommitHash(who, cid, TEST_SALT, batchId)); + } + + function _revealT1(address who, uint batchId, bytes32 cid) internal { + vm.prank(who); + tc.revealT1Aggregation(1, batchId, cid, TEST_SALT); + } + + function _openT1RevealPhase() internal { + vm.prank(modelOwner); + tc.startT1AggregationReveal(1); + } + + // ───────────────────────────────────────────────────────────────────── + // Correctness and anti-copying properties. + // ───────────────────────────────────────────────────────────────────── + + function test_commitReveal_happyPath_cidCountedAfterReveal() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + + _commitT1(t1aggs[0], 0, CID_A); + _openT1RevealPhase(); + _revealT1(t1aggs[0], 0, CID_A); + + assertTrue(tc.t1Submitted(1, 0, t1aggs[0])); + assertEq(tc.t1SubmissionCID(1, 0, t1aggs[0]), CID_A); + } + + /// @dev issue #156 M-1's actual hardening: the commit hash binds + /// msg.sender, so an aggregator who copies a peer's *commit hash* + /// verbatim (observable on-chain the moment the peer commits) + /// cannot later reveal the peer's (cid, salt) under their own + /// address -- the hash the contract recomputes at reveal time + /// includes the revealer's own address, which won't match what the + /// copier stored. Without this binding (issue #156's original + /// `keccak256(cid, salt)` proposal), this reveal would succeed. + function test_copyAttack_replayingPeerCommitHash_cannotReveal() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + address honest = t1aggs[0]; + address copier = t1aggs[1]; + + bytes32 honestCommitHash = _t1CommitHash(honest, CID_A, TEST_SALT, 0); + vm.prank(honest); + tc.commitT1Aggregation(1, 0, honestCommitHash); + + // Copier submits the exact same commit hash bytes as their own commit. + vm.prank(copier); + tc.commitT1Aggregation(1, 0, honestCommitHash); + + _openT1RevealPhase(); + + // Honest party can reveal their own (cid, salt) fine. + _revealT1(honest, 0, CID_A); + + // Copier tries to reveal the same (cid, salt) they saw the honest + // party commit to -- reverts, because the hash the contract expects + // for the copier's own reveal includes the copier's address, not + // the honest party's. + vm.prank(copier); + vm.expectRevert(); // TC_T1RevealHashMismatch + tc.revealT1Aggregation(1, 0, CID_A, TEST_SALT); + } + + function test_reveal_beforeRevealPhase_reverts() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + + _commitT1(t1aggs[0], 0, CID_A); + + // Still in the commit phase (T1AggregationStarted) -- reveal must + // not be accepted yet, the entire point of the two-phase split. + vm.prank(t1aggs[0]); + vm.expectRevert(); // TC_T1RevealPhaseNotOpen + tc.revealT1Aggregation(1, 0, CID_A, TEST_SALT); + } + + function test_reveal_withoutPriorCommit_reverts() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + + _openT1RevealPhase(); + + vm.prank(t1aggs[0]); + vm.expectRevert(); // TC_T1NoCommitFound + tc.revealT1Aggregation(1, 0, CID_A, TEST_SALT); + } + + function test_reveal_wrongCID_reverts() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + + _commitT1(t1aggs[0], 0, CID_A); + _openT1RevealPhase(); + + vm.prank(t1aggs[0]); + vm.expectRevert(); // TC_T1RevealHashMismatch + tc.revealT1Aggregation(1, 0, CID_B, TEST_SALT); + } + + function test_reveal_wrongSalt_reverts() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + + _commitT1(t1aggs[0], 0, CID_A); + _openT1RevealPhase(); + + vm.prank(t1aggs[0]); + vm.expectRevert(); // TC_T1RevealHashMismatch + tc.revealT1Aggregation(1, 0, CID_A, bytes32(uint256(999))); + } + + function test_commit_duringCommitPhase_twiceReverts() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + + _commitT1(t1aggs[0], 0, CID_A); + + vm.prank(t1aggs[0]); + vm.expectRevert(); // TC_T1AlreadyCommitted + tc.commitT1Aggregation(1, 0, _t1CommitHash(t1aggs[0], CID_B, TEST_SALT, 0)); + } + + function test_commit_zeroHashReverts() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + + vm.prank(t1aggs[0]); + vm.expectRevert(); // TC_T1EmptyCommitHash + tc.commitT1Aggregation(1, 0, bytes32(0)); + } + + function test_reveal_twiceReverts() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + + _commitT1(t1aggs[0], 0, CID_A); + _openT1RevealPhase(); + _revealT1(t1aggs[0], 0, CID_A); + + vm.prank(t1aggs[0]); + vm.expectRevert(); // TC_AlreadySubmitted + tc.revealT1Aggregation(1, 0, CID_A, TEST_SALT); + } + + function test_finalize_beforeRevealStarted_reverts() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + + _commitT1(t1aggs[0], 0, CID_A); + _commitT1(t1aggs[1], 0, CID_A); + + // Still T1AggregationStarted (commit phase) -- finalize now requires + // T1AggregationRevealStarted. + vm.prank(modelOwner); + vm.expectRevert(); // TC_NotReadyToFinalizeT1 + tc.finalizeT1Aggregation(1); + } + + /// @dev A committed-but-never-revealed aggregator must be excluded from + /// finalization exactly like a non-participant, and remain + /// slashable via the existing slashAggregators() "no submission" + /// (S2) check -- no special-casing needed for the non-reveal case. + function test_commitButNeverReveal_isS2Slashed() public { + _runToT1AggregationStarted(); + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + assertEq(t1aggs.length, 3, "sanity: T1 batch should have 3 aggregators"); + + // All 3 commit, but only 2 ever reveal -- quorum (2 of 3) still met. + _commitT1(t1aggs[0], 0, CID_A); + _commitT1(t1aggs[1], 0, CID_A); + _commitT1(t1aggs[2], 0, CID_A); + + _openT1RevealPhase(); + _revealT1(t1aggs[0], 0, CID_A); + _revealT1(t1aggs[1], 0, CID_A); + // t1aggs[2] committed but never reveals. + + vm.prank(modelOwner); + tc.finalizeT1Aggregation(1); + assertTrue(tc.t1Submitted(1, 0, t1aggs[0])); + assertFalse(tc.t1Submitted(1, 0, t1aggs[2]), "committed but never revealed -- excluded like a non-participant"); + + // Run T2 to completion too (the fixture's other 3 aggregators form a + // real T2 batch, not a trivial empty one) so slashAuditors/ + // slashAggregators become callable. + vm.prank(modelOwner); + tc.startT2Aggregation(1); + (, address[] memory t2aggs, , ) = tc.getTier2Batch(1, 0); + for (uint i = 0; i < t2aggs.length; i++) { + vm.prank(t2aggs[i]); + tc.commitT2Aggregation( + 1, 0, + keccak256(abi.encode(CID_B, TEST_SALT, t2aggs[i], uint(1), DINTaskCoordinator.TierKind.Tier2, uint(0))) + ); + } + vm.prank(modelOwner); + tc.startT2AggregationReveal(1); + for (uint i = 0; i < t2aggs.length; i++) { + vm.prank(t2aggs[i]); + tc.revealT2Aggregation(1, 0, CID_B, TEST_SALT); + } + + vm.startPrank(modelOwner); + tc.finalizeT2Aggregation(1); + tc.slashAuditors(1); + vm.stopPrank(); + + uint256 stakeBefore = stake.getStake(t1aggs[2]); + vm.prank(modelOwner); + tc.slashAggregators(1); + uint256 stakeAfter = stake.getStake(t1aggs[2]); + + assertLt(stakeAfter, stakeBefore, "committed-but-never-revealed aggregator must be S2-slashed"); + // The 2 who revealed and matched consensus keep their full stake. + assertEq(stake.getStake(t1aggs[0]), 10 ether); + assertEq(stake.getStake(t1aggs[1]), 10 ether); + } + + // ───────────────────────────────────────────────────────────────────── + // Full-GI walk through the new T1/T2 reveal states. + // ───────────────────────────────────────────────────────────────────── + + function test_fullGI_walksThroughT1AndT2RevealStates() public { + _runToT1AggregationStarted(); + assertEq(uint8(tc.GIstate()), uint8(GIstates.T1AggregationStarted)); + + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + _commitT1(t1aggs[0], 0, CID_A); + _commitT1(t1aggs[1], 0, CID_A); + _commitT1(t1aggs[2], 0, CID_A); + + _openT1RevealPhase(); + assertEq(uint8(tc.GIstate()), uint8(GIstates.T1AggregationRevealStarted)); + + _revealT1(t1aggs[0], 0, CID_A); + _revealT1(t1aggs[1], 0, CID_A); + _revealT1(t1aggs[2], 0, CID_A); + + vm.prank(modelOwner); + tc.finalizeT1Aggregation(1); + assertEq(uint8(tc.GIstate()), uint8(GIstates.T1AggregationDone)); + + vm.prank(modelOwner); + tc.startT2Aggregation(1); + assertEq(uint8(tc.GIstate()), uint8(GIstates.T2AggregationStarted)); + + (, address[] memory t2aggs, , ) = tc.getTier2Batch(1, 0); + assertEq(t2aggs.length, 3, "sanity: remaining 3 aggregators form the T2 batch"); + for (uint i = 0; i < t2aggs.length; i++) { + vm.prank(t2aggs[i]); + tc.commitT2Aggregation( + 1, 0, + keccak256(abi.encode(CID_B, TEST_SALT, t2aggs[i], uint(1), DINTaskCoordinator.TierKind.Tier2, uint(0))) + ); + } + + vm.prank(modelOwner); + tc.startT2AggregationReveal(1); + assertEq(uint8(tc.GIstate()), uint8(GIstates.T2AggregationRevealStarted)); + + for (uint i = 0; i < t2aggs.length; i++) { + vm.prank(t2aggs[i]); + tc.revealT2Aggregation(1, 0, CID_B, TEST_SALT); + } + + vm.prank(modelOwner); + tc.finalizeT2Aggregation(1); + assertEq(uint8(tc.GIstate()), uint8(GIstates.T2AggregationDone)); + + (, , bool finalized, bytes32 finalCID) = tc.getTier2Batch(1, 0); + assertTrue(finalized); + assertEq(finalCID, CID_B); + } +} diff --git a/foundry/test/DisputeResolution.t.sol b/foundry/test/DisputeResolution.t.sol index 6161ef4..263ec31 100644 --- a/foundry/test/DisputeResolution.t.sol +++ b/foundry/test/DisputeResolution.t.sol @@ -242,15 +242,67 @@ contract DisputeResolutionTest is Test { vm.stopPrank(); (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); - for (uint i = 0; i < t1aggs.length; i++) { - vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, 0, WINNING_CID); - } + _commitAndRevealT1(t1aggs, 1, 0, WINNING_CID); vm.prank(modelOwner); tc.finalizeT1Aggregation(1); } + /// @dev issue #156 M-1: commits each of `aggs` to `cid` for T1 batch + /// `batchId`, opens the reveal window, then reveals every commit. + /// Small helper (not inlined at each call site) so fixtures like + /// _runToT1Finalized below stay small -- the same reasoning as + /// _lockAuditSeedNow/_lockAggSeedNow just above avoided a solc + /// via_ir ICE in this file previously. + function _commitAndRevealT1( + address[] memory aggs, + uint gi, + uint batchId, + bytes32 cid + ) internal { + for (uint i = 0; i < aggs.length; i++) { + bytes32 commitHash = keccak256( + abi.encode(cid, TEST_SALT, aggs[i], gi, DINTaskCoordinator.TierKind.Tier1, batchId) + ); + vm.prank(aggs[i]); + tc.commitT1Aggregation(gi, batchId, commitHash); + } + + vm.prank(modelOwner); + tc.startT1AggregationReveal(gi); + + for (uint i = 0; i < aggs.length; i++) { + vm.prank(aggs[i]); + tc.revealT1Aggregation(gi, batchId, cid, TEST_SALT); + } + } + + /// @dev Same as `_commitAndRevealT1`, but each aggregator can reveal a + /// different CID (per-aggregator dissent) -- `cids[i]` must line up + /// with `aggs[i]`. + function _commitAndRevealT1Dissenting( + address[] memory aggs, + uint gi, + uint batchId, + bytes32[] memory cids + ) internal { + for (uint i = 0; i < aggs.length; i++) { + bytes32 commitHash = keccak256( + abi.encode(cids[i], TEST_SALT, aggs[i], gi, DINTaskCoordinator.TierKind.Tier1, batchId) + ); + vm.prank(aggs[i]); + tc.commitT1Aggregation(gi, batchId, commitHash); + } + + vm.prank(modelOwner); + tc.startT1AggregationReveal(gi); + + for (uint i = 0; i < aggs.length; i++) { + vm.prank(aggs[i]); + tc.revealT1Aggregation(gi, batchId, cids[i], TEST_SALT); + } + } + /// @dev Same as `_runToT1Finalized`, except one of the disputed batch's /// 3 assigned aggregators submits a different CID than the other /// two. The majority CID (WINNING_CID, 2 votes) still finalizes the @@ -344,13 +396,13 @@ contract DisputeResolutionTest is Test { // dissenter) votes a different CID. WINNING_CID still wins 2-1. (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); dissenter = t1aggs[2]; + bytes32[] memory cids = new bytes32[](t1aggs.length); for (uint i = 0; i < t1aggs.length; i++) { - bytes32 cid = t1aggs[i] == dissenter + cids[i] = t1aggs[i] == dissenter ? bytes32(uint256(0xBAD)) : WINNING_CID; - vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, 0, cid); } + _commitAndRevealT1Dissenting(t1aggs, 1, 0, cids); vm.prank(modelOwner); tc.finalizeT1Aggregation(1); diff --git a/foundry/test/GasSimulation.t.sol b/foundry/test/GasSimulation.t.sol index c920cb1..ba66e0e 100644 --- a/foundry/test/GasSimulation.t.sol +++ b/foundry/test/GasSimulation.t.sol @@ -193,25 +193,54 @@ contract GasSimulationTest is Test { _completeEvalAndOpenT1(1); } - /// @dev From T1AggregationStarted: has 2 aggregators per T1 batch submit (quorum met), - /// 1 per batch doesn't submit (will be slashed). Runs through to T2AggregationDone. - /// Post-H-1 worst case: minimum quorum (2 of 3) required to reach finalization. + /// @dev issue #156 M-1: commits `who` to `cid` for T1 batch `batchId`, + /// immediately (commit window only -- caller opens the reveal + /// window and calls revealT1Aggregation separately). + function _commitT1(address who, uint gi, uint batchId, bytes32 cid) internal { + bytes32 commitHash = keccak256( + abi.encode(cid, TEST_SALT, who, gi, DINTaskCoordinator.TierKind.Tier1, batchId) + ); + vm.prank(who); + tc.commitT1Aggregation(gi, batchId, commitHash); + } + + function _commitT2(address who, uint gi, uint batchId, bytes32 cid) internal { + bytes32 commitHash = keccak256( + abi.encode(cid, TEST_SALT, who, gi, DINTaskCoordinator.TierKind.Tier2, batchId) + ); + vm.prank(who); + tc.commitT2Aggregation(gi, batchId, commitHash); + } + + /// @dev From T1AggregationStarted: has 2 aggregators per T1 batch commit+reveal + /// (quorum met), 1 per batch doesn't (will be slashed). Runs through to + /// T2AggregationDone. Post-H-1 worst case: minimum quorum (2 of 3) + /// required to reach finalization. function _runT1T2WorstCase(uint gi) internal { bytes32 cid = bytes32("agreed_cid"); uint t1Cnt = tc.tier1BatchCount(gi); for (uint b = 0; b < t1Cnt; b++) { (, address[] memory bAggs,,,) = tc.getTier1Batch(gi, b); - vm.prank(bAggs[0]); tc.submitT1Aggregation(gi, b, cid); - vm.prank(bAggs[1]); tc.submitT1Aggregation(gi, b, cid); // quorum met (2 of 3) - // bAggs[2] intentionally does not submit → will be slashed (1 per batch) + _commitT1(bAggs[0], gi, b, cid); + _commitT1(bAggs[1], gi, b, cid); // quorum met (2 of 3) + // bAggs[2] intentionally does not commit → will be slashed (1 per batch) + } + tc.startT1AggregationReveal(gi); + for (uint b = 0; b < t1Cnt; b++) { + (, address[] memory bAggs,,,) = tc.getTier1Batch(gi, b); + vm.prank(bAggs[0]); tc.revealT1Aggregation(gi, b, cid, TEST_SALT); + vm.prank(bAggs[1]); tc.revealT1Aggregation(gi, b, cid, TEST_SALT); } tc.finalizeT1Aggregation(gi); tc.startT2Aggregation(gi); (, address[] memory t2Aggs,,) = tc.getTier2Batch(gi, 0); - vm.prank(t2Aggs[0]); tc.submitT2Aggregation(gi, 0, cid); - vm.prank(t2Aggs[1]); tc.submitT2Aggregation(gi, 0, cid); // quorum met (2 of 3) - // t2Aggs[2] does not submit → will be slashed + _commitT2(t2Aggs[0], gi, 0, cid); + _commitT2(t2Aggs[1], gi, 0, cid); // quorum met (2 of 3) + // t2Aggs[2] does not commit → will be slashed + tc.startT2AggregationReveal(gi); + vm.prank(t2Aggs[0]); tc.revealT2Aggregation(gi, 0, cid, TEST_SALT); + vm.prank(t2Aggs[1]); tc.revealT2Aggregation(gi, 0, cid, TEST_SALT); tc.finalizeT2Aggregation(gi); tc.setTier2Score(gi, 85); } @@ -220,34 +249,69 @@ contract GasSimulationTest is Test { // SCENARIO 1 — Aggregation submissions // ───────────────────────────────────────────────────────────────────────── - /// @dev Per-aggregator cost of submitting a T1 CID (cold storage paths). - function test_gas_s1_submitT1Aggregation_cold() public { + // issue #156 M-1: submitT1Aggregation/submitT2Aggregation's single write + // split into commitT1Aggregation (writes a hash only, no vote counter + // touch) + revealT1Aggregation (the actual CID write + vote-counter + // increment -- structurally what the old single-shot submit did). Gas + // below is measured for both steps rather than folding commit's cost + // into what used to be one call; Developer/design/gas-simulation- + // network-fee.md's fee-floor sizing still reflects the pre-split, + // single-tx numbers and needs re-measurement against these two-tx + // figures separately (out of scope for this PR). + + /// @dev Per-aggregator cost of committing a T1 CID hash (cold storage paths). + function test_gas_s1_commitT1Aggregation_cold() public { _setupToT1Open(3); (, address[] memory bAggs,,,) = tc.getTier1Batch(1, 0); uint before = gasleft(); - vm.prank(bAggs[0]); tc.submitT1Aggregation(1, 0, bytes32("cid_a")); - console.log("[GAS][S1] submitT1Aggregation (1st call, cold SSTORE):", before - gasleft()); + _commitT1(bAggs[0], 1, 0, bytes32("cid_a")); + console.log("[GAS][S1] commitT1Aggregation (1st call, cold SSTORE):", before - gasleft()); } - /// @dev Second aggregator submitting to the same batch — vote counter increment is warm. - function test_gas_s1_submitT1Aggregation_warm() public { + /// @dev Per-aggregator cost of revealing a T1 CID (cold storage paths). + function test_gas_s1_revealT1Aggregation_cold() public { _setupToT1Open(3); (, address[] memory bAggs,,,) = tc.getTier1Batch(1, 0); - vm.prank(bAggs[0]); tc.submitT1Aggregation(1, 0, bytes32("cid_a")); + _commitT1(bAggs[0], 1, 0, bytes32("cid_a")); + tc.startT1AggregationReveal(1); uint before = gasleft(); - vm.prank(bAggs[1]); tc.submitT1Aggregation(1, 0, bytes32("cid_a")); // same CID → warm counter - console.log("[GAS][S1] submitT1Aggregation (2nd call, warm vote counter):", before - gasleft()); + vm.prank(bAggs[0]); tc.revealT1Aggregation(1, 0, bytes32("cid_a"), TEST_SALT); + console.log("[GAS][S1] revealT1Aggregation (1st call, cold SSTORE):", before - gasleft()); } - function test_gas_s1_finalizeT1_3batches() public { + /// @dev Second aggregator revealing to the same batch — vote counter increment is warm. + function test_gas_s1_revealT1Aggregation_warm() public { _setupToT1Open(3); - for (uint b = 0; b < 3; b++) { - (, address[] memory a,,,) = tc.getTier1Batch(1, b); - vm.prank(a[0]); tc.submitT1Aggregation(1, b, bytes32("cid")); - vm.prank(a[1]); tc.submitT1Aggregation(1, b, bytes32("cid")); // quorum needs 2 + (, address[] memory bAggs,,,) = tc.getTier1Batch(1, 0); + _commitT1(bAggs[0], 1, 0, bytes32("cid_a")); + _commitT1(bAggs[1], 1, 0, bytes32("cid_a")); + tc.startT1AggregationReveal(1); + vm.prank(bAggs[0]); tc.revealT1Aggregation(1, 0, bytes32("cid_a"), TEST_SALT); + + uint before = gasleft(); + vm.prank(bAggs[1]); tc.revealT1Aggregation(1, 0, bytes32("cid_a"), TEST_SALT); // same CID → warm counter + console.log("[GAS][S1] revealT1Aggregation (2nd call, warm vote counter):", before - gasleft()); + } + + function _commitRevealT1Batches(uint gi, uint batchCount) internal { + for (uint b = 0; b < batchCount; b++) { + (, address[] memory a,,,) = tc.getTier1Batch(gi, b); + _commitT1(a[0], gi, b, bytes32("cid")); + _commitT1(a[1], gi, b, bytes32("cid")); // quorum needs 2 + } + tc.startT1AggregationReveal(gi); + for (uint b = 0; b < batchCount; b++) { + (, address[] memory a,,,) = tc.getTier1Batch(gi, b); + vm.prank(a[0]); tc.revealT1Aggregation(gi, b, bytes32("cid"), TEST_SALT); + vm.prank(a[1]); tc.revealT1Aggregation(gi, b, bytes32("cid"), TEST_SALT); } + } + + function test_gas_s1_finalizeT1_3batches() public { + _setupToT1Open(3); + _commitRevealT1Batches(1, 3); uint before = gasleft(); tc.finalizeT1Aggregation(1); console.log("[GAS][S1] finalizeT1Aggregation (3 batches, 3 agg/batch):", before - gasleft()); @@ -255,11 +319,7 @@ contract GasSimulationTest is Test { function test_gas_s1_finalizeT1_5batches() public { _setupToT1Open(5); - for (uint b = 0; b < 5; b++) { - (, address[] memory a,,,) = tc.getTier1Batch(1, b); - vm.prank(a[0]); tc.submitT1Aggregation(1, b, bytes32("cid")); - vm.prank(a[1]); tc.submitT1Aggregation(1, b, bytes32("cid")); // quorum needs 2 - } + _commitRevealT1Batches(1, 5); uint before = gasleft(); tc.finalizeT1Aggregation(1); console.log("[GAS][S1] finalizeT1Aggregation (5 batches, 3 agg/batch):", before - gasleft()); @@ -267,32 +327,27 @@ contract GasSimulationTest is Test { function test_gas_s1_finalizeT1_10batches() public { _setupToT1Open(10); - for (uint b = 0; b < 10; b++) { - (, address[] memory a,,,) = tc.getTier1Batch(1, b); - vm.prank(a[0]); tc.submitT1Aggregation(1, b, bytes32("cid")); - vm.prank(a[1]); tc.submitT1Aggregation(1, b, bytes32("cid")); // quorum needs 2 - } + _commitRevealT1Batches(1, 10); uint before = gasleft(); tc.finalizeT1Aggregation(1); console.log("[GAS][S1] finalizeT1Aggregation (10 batches, 3 agg/batch):", before - gasleft()); } - /// @dev Per-T2-aggregator cost of submitting a T2 CID. - function test_gas_s1_submitT2Aggregation() public { + /// @dev Per-T2-aggregator cost of revealing a T2 CID. + function test_gas_s1_revealT2Aggregation() public { _setupToT1Open(3); // Complete T1 first — quorum requires 2 of 3 per batch - for (uint b = 0; b < tc.tier1BatchCount(1); b++) { - (, address[] memory a,,,) = tc.getTier1Batch(1, b); - vm.prank(a[0]); tc.submitT1Aggregation(1, b, bytes32("cid")); - vm.prank(a[1]); tc.submitT1Aggregation(1, b, bytes32("cid")); - } + _commitRevealT1Batches(1, tc.tier1BatchCount(1)); tc.finalizeT1Aggregation(1); tc.startT2Aggregation(1); (, address[] memory t2Aggs,,) = tc.getTier2Batch(1, 0); + _commitT2(t2Aggs[0], 1, 0, bytes32("cid_t2")); + tc.startT2AggregationReveal(1); + uint before = gasleft(); - vm.prank(t2Aggs[0]); tc.submitT2Aggregation(1, 0, bytes32("cid_t2")); - console.log("[GAS][S1] submitT2Aggregation (cold SSTORE):", before - gasleft()); + vm.prank(t2Aggs[0]); tc.revealT2Aggregation(1, 0, bytes32("cid_t2"), TEST_SALT); + console.log("[GAS][S1] revealT2Aggregation (cold SSTORE):", before - gasleft()); } // ───────────────────────────────────────────────────────────────────────── diff --git a/foundry/test/LifecycleEvents.t.sol b/foundry/test/LifecycleEvents.t.sol index 207f944..26490f0 100644 --- a/foundry/test/LifecycleEvents.t.sol +++ b/foundry/test/LifecycleEvents.t.sol @@ -6,8 +6,9 @@ pragma solidity ^0.8.28; // - GIStateChanged emitted by _setGIstate with correct GI and ordinal // - GI ordering fix: GIstarted fires for GI N, not GI N-1 // - LocalModelSubmitted emitted by DINTaskAuditor.submitLocalModel -// - T1AggregationSubmitted emitted by submitT1Aggregation -// - T2AggregationSubmitted emitted by submitT2Aggregation +// - T1AggregationSubmitted emitted by revealT1Aggregation (issue #156 M-1 +// split submitT1Aggregation into commitT1Aggregation + revealT1Aggregation) +// - T2AggregationSubmitted emitted by revealT2Aggregation (same split) // - T1BatchFinalized emitted per batch in finalizeT1Aggregation // - T2Finalized emitted in finalizeT2Aggregation // Run: forge test --match-contract LifecycleEventsTest -vv @@ -70,12 +71,14 @@ contract LifecycleEventsTest is Test { uint8 constant GI_LMS_EVAL_CLOSED = 15; uint8 constant GI_T1T2_CREATED = 16; uint8 constant GI_T1_AGG_STARTED = 17; - uint8 constant GI_T1_AGG_DONE = 18; - uint8 constant GI_T2_AGG_STARTED = 19; - uint8 constant GI_T2_AGG_DONE = 20; - uint8 constant GI_AUDITORS_SLASHED = 21; - uint8 constant GI_AGGREGATORS_SLASHED = 22; - uint8 constant GI_ENDED = 23; + uint8 constant GI_T1_AGG_REVEAL_STARTED = 18; + uint8 constant GI_T1_AGG_DONE = 19; + uint8 constant GI_T2_AGG_STARTED = 20; + uint8 constant GI_T2_AGG_REVEAL_STARTED = 21; + uint8 constant GI_T2_AGG_DONE = 22; + uint8 constant GI_AUDITORS_SLASHED = 23; + uint8 constant GI_AGGREGATORS_SLASHED = 24; + uint8 constant GI_ENDED = 25; function setUp() public { vm.startPrank(admin); @@ -248,6 +251,24 @@ contract LifecycleEventsTest is Test { vm.stopPrank(); } + /// @dev issue #156 M-1: commits `who` to `cid` for T1/T2 batch `batchId`. + /// Does not open the reveal window itself. + function _commitT1(address who, uint gi, uint batchId, bytes32 cid) internal { + bytes32 commitHash = keccak256( + abi.encode(cid, TEST_SALT, who, gi, DINTaskCoordinator.TierKind.Tier1, batchId) + ); + vm.prank(who); + tc.commitT1Aggregation(gi, batchId, commitHash); + } + + function _commitT2(address who, uint gi, uint batchId, bytes32 cid) internal { + bytes32 commitHash = keccak256( + abi.encode(cid, TEST_SALT, who, gi, DINTaskCoordinator.TierKind.Tier2, batchId) + ); + vm.prank(who); + tc.commitT2Aggregation(gi, batchId, commitHash); + } + // ── GIStateChanged ordering fix ─────────────────────────────────────────── /// @dev Verifies that GIstarted fires with the new GI index (N), not N-1. @@ -270,22 +291,35 @@ contract LifecycleEventsTest is Test { _advanceToT1(1); uint256 t1Count = tc.tier1BatchCount(1); + for (uint b = 0; b < t1Count; b++) { + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); + for (uint i = 0; i < t1aggs.length; i++) { + _commitT1(t1aggs[i], 1, b, CID_A); + } + } + vm.prank(modelOwner); tc.startT1AggregationReveal(1); for (uint b = 0; b < t1Count; b++) { (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); for (uint i = 0; i < t1aggs.length; i++) { vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, b, CID_A); + tc.revealT1Aggregation(1, b, CID_A, TEST_SALT); } } vm.prank(modelOwner); tc.finalizeT1Aggregation(1); vm.prank(modelOwner); tc.startT2Aggregation(1); try tc.getTier2Batch(1, 0) returns (uint, address[] memory t2aggs, bool, bytes32) { + for (uint i = 0; i < t2aggs.length; i++) { + _commitT2(t2aggs[i], 1, 0, CID_B); + } + vm.prank(modelOwner); tc.startT2AggregationReveal(1); for (uint i = 0; i < t2aggs.length; i++) { vm.prank(t2aggs[i]); - tc.submitT2Aggregation(1, 0, CID_B); + tc.revealT2Aggregation(1, 0, CID_B, TEST_SALT); } - } catch {} + } catch { + vm.prank(modelOwner); tc.startT2AggregationReveal(1); + } vm.prank(modelOwner); tc.finalizeT2Aggregation(1); vm.prank(modelOwner); tc.slashAuditors(1); @@ -335,10 +369,13 @@ contract LifecycleEventsTest is Test { (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); address firstAgg = t1aggs[0]; + _commitT1(firstAgg, 1, 0, CID_A); + vm.prank(modelOwner); tc.startT1AggregationReveal(1); + vm.expectEmit(true, true, true, true, address(tc)); emit DINTaskCoordinator.T1AggregationSubmitted(1, 0, firstAgg, CID_A); vm.prank(firstAgg); - tc.submitT1Aggregation(1, 0, CID_A); + tc.revealT1Aggregation(1, 0, CID_A, TEST_SALT); } // ── T2AggregationSubmitted ──────────────────────────────────────────────── @@ -348,11 +385,18 @@ contract LifecycleEventsTest is Test { _advanceToT1(1); uint256 t1Count = tc.tier1BatchCount(1); + for (uint b = 0; b < t1Count; b++) { + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); + for (uint i = 0; i < t1aggs.length; i++) { + _commitT1(t1aggs[i], 1, b, CID_A); + } + } + vm.prank(modelOwner); tc.startT1AggregationReveal(1); for (uint b = 0; b < t1Count; b++) { (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); for (uint i = 0; i < t1aggs.length; i++) { vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, b, CID_A); + tc.revealT1Aggregation(1, b, CID_A, TEST_SALT); } } vm.prank(modelOwner); tc.finalizeT1Aggregation(1); @@ -360,10 +404,13 @@ contract LifecycleEventsTest is Test { (uint bId, address[] memory t2aggs, , ) = tc.getTier2Batch(1, 0); + _commitT2(t2aggs[0], 1, bId, CID_B); + vm.prank(modelOwner); tc.startT2AggregationReveal(1); + vm.expectEmit(true, true, true, true, address(tc)); emit DINTaskCoordinator.T2AggregationSubmitted(1, bId, t2aggs[0], CID_B); vm.prank(t2aggs[0]); - tc.submitT2Aggregation(1, bId, CID_B); + tc.revealT2Aggregation(1, bId, CID_B, TEST_SALT); } // ── T1BatchFinalized ────────────────────────────────────────────────────── @@ -377,11 +424,18 @@ contract LifecycleEventsTest is Test { uint256 t1Count = tc.tier1BatchCount(1); assertEq(t1Count, 2, "fixture must produce two Tier-1 batches"); + for (uint b = 0; b < t1Count; b++) { + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); + for (uint i = 0; i < t1aggs.length; i++) { + _commitT1(t1aggs[i], 1, b, CID_A); + } + } + vm.prank(modelOwner); tc.startT1AggregationReveal(1); for (uint b = 0; b < t1Count; b++) { (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); for (uint i = 0; i < t1aggs.length; i++) { vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, b, CID_A); + tc.revealT1Aggregation(1, b, CID_A, TEST_SALT); } } @@ -401,20 +455,31 @@ contract LifecycleEventsTest is Test { _advanceToT1(1); uint256 t1Count = tc.tier1BatchCount(1); + for (uint b = 0; b < t1Count; b++) { + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); + for (uint i = 0; i < t1aggs.length; i++) { + _commitT1(t1aggs[i], 1, b, CID_A); + } + } + vm.prank(modelOwner); tc.startT1AggregationReveal(1); for (uint b = 0; b < t1Count; b++) { (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); for (uint i = 0; i < t1aggs.length; i++) { vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, b, CID_A); + tc.revealT1Aggregation(1, b, CID_A, TEST_SALT); } } vm.prank(modelOwner); tc.finalizeT1Aggregation(1); vm.prank(modelOwner); tc.startT2Aggregation(1); (, address[] memory t2aggs, , ) = tc.getTier2Batch(1, 0); + for (uint i = 0; i < t2aggs.length; i++) { + _commitT2(t2aggs[i], 1, 0, CID_B); + } + vm.prank(modelOwner); tc.startT2AggregationReveal(1); for (uint i = 0; i < t2aggs.length; i++) { vm.prank(t2aggs[i]); - tc.submitT2Aggregation(1, 0, CID_B); + tc.revealT2Aggregation(1, 0, CID_B, TEST_SALT); } vm.expectEmit(true, false, false, true, address(tc)); @@ -535,11 +600,23 @@ contract LifecycleEventsTest is Test { vm.prank(modelOwner); tc.startT1Aggregation(1); uint256 t1Count = tc.tier1BatchCount(1); + for (uint b = 0; b < t1Count; b++) { + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); + for (uint i = 0; i < t1aggs.length; i++) { + _commitT1(t1aggs[i], 1, b, CID_A); + } + } + + // T1AggregationRevealStarted (issue #156 M-1) + vm.expectEmit(true, true, false, false, address(tc)); + emit DINTaskCoordinator.GIStateChanged(1, GI_T1_AGG_REVEAL_STARTED); + vm.prank(modelOwner); tc.startT1AggregationReveal(1); + for (uint b = 0; b < t1Count; b++) { (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); for (uint i = 0; i < t1aggs.length; i++) { vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, b, CID_A); + tc.revealT1Aggregation(1, b, CID_A, TEST_SALT); } } @@ -554,9 +631,18 @@ contract LifecycleEventsTest is Test { vm.prank(modelOwner); tc.startT2Aggregation(1); (, address[] memory t2aggs, , ) = tc.getTier2Batch(1, 0); + for (uint i = 0; i < t2aggs.length; i++) { + _commitT2(t2aggs[i], 1, 0, CID_B); + } + + // T2AggregationRevealStarted (issue #156 M-1) + vm.expectEmit(true, true, false, false, address(tc)); + emit DINTaskCoordinator.GIStateChanged(1, GI_T2_AGG_REVEAL_STARTED); + vm.prank(modelOwner); tc.startT2AggregationReveal(1); + for (uint i = 0; i < t2aggs.length; i++) { vm.prank(t2aggs[i]); - tc.submitT2Aggregation(1, 0, CID_B); + tc.revealT2Aggregation(1, 0, CID_B, TEST_SALT); } // T2AggregationDone diff --git a/foundry/test/PR146SlashingRegression.t.sol b/foundry/test/PR146SlashingRegression.t.sol index c55139a..346abe7 100644 --- a/foundry/test/PR146SlashingRegression.t.sol +++ b/foundry/test/PR146SlashingRegression.t.sol @@ -223,14 +223,28 @@ contract PR146SlashingRegressionTest is Test { bytes32 realCID = bytes32(uint256(0xC1D)); for (uint i = 0; i < t1aggs.length; i++) { if (t1aggs[i] == agg3) continue; + bytes32 commitHash = keccak256( + abi.encode(realCID, TEST_SALT, t1aggs[i], uint(1), DINTaskCoordinator.TierKind.Tier1, uint(0)) + ); vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, 0, realCID); + tc.commitT1Aggregation(1, 0, commitHash); + } + + vm.startPrank(modelOwner); + tc.startT1AggregationReveal(1); + vm.stopPrank(); + + for (uint i = 0; i < t1aggs.length; i++) { + if (t1aggs[i] == agg3) continue; + vm.prank(t1aggs[i]); + tc.revealT1Aggregation(1, 0, realCID, TEST_SALT); } vm.startPrank(modelOwner); tc.finalizeT1Aggregation(1); tc.startT2Aggregation(1); - tc.finalizeT2Aggregation(1); // trivial: 0 T2 batches at exactly 3 aggregators + tc.startT2AggregationReveal(1); // trivial: 0 T2 batches at exactly 3 aggregators + tc.finalizeT2Aggregation(1); vm.stopPrank(); } diff --git a/foundry/test/RewardEngine.t.sol b/foundry/test/RewardEngine.t.sol index ec0c494..ab05022 100644 --- a/foundry/test/RewardEngine.t.sol +++ b/foundry/test/RewardEngine.t.sol @@ -243,14 +243,26 @@ contract RewardEngineTest is Test { (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); bytes32 realCID = bytes32(uint256(0xC1D)); for (uint i = 0; i < t1aggs.length; i++) { + bytes32 commitHash = keccak256( + abi.encode(realCID, TEST_SALT, t1aggs[i], uint(1), DINTaskCoordinator.TierKind.Tier1, uint(0)) + ); vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, 0, realCID); + tc.commitT1Aggregation(1, 0, commitHash); + } + + vm.prank(modelOwner); + tc.startT1AggregationReveal(1); + + for (uint i = 0; i < t1aggs.length; i++) { + vm.prank(t1aggs[i]); + tc.revealT1Aggregation(1, 0, realCID, TEST_SALT); } vm.startPrank(modelOwner); tc.finalizeT1Aggregation(1); tc.startT2Aggregation(1); - tc.finalizeT2Aggregation(1); // trivial: 0 T2 batches at exactly 3 aggregators + tc.startT2AggregationReveal(1); // trivial: 0 T2 batches at exactly 3 aggregators + tc.finalizeT2Aggregation(1); tc.slashAuditors(1); tc.slashAggregators(1); vm.stopPrank(); @@ -967,14 +979,27 @@ contract RewardEngineTest is Test { vm.stopPrank(); (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); + bytes32 t1cid = bytes32(uint256(0xC1D)); + for (uint i = 0; i < t1aggs.length; i++) { + bytes32 commitHash = keccak256( + abi.encode(t1cid, TEST_SALT, t1aggs[i], uint(1), DINTaskCoordinator.TierKind.Tier1, uint(0)) + ); + vm.prank(t1aggs[i]); + tc.commitT1Aggregation(1, 0, commitHash); + } + + vm.prank(modelOwner); + tc.startT1AggregationReveal(1); + for (uint i = 0; i < t1aggs.length; i++) { vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, 0, bytes32(uint256(0xC1D))); + tc.revealT1Aggregation(1, 0, t1cid, TEST_SALT); } vm.startPrank(modelOwner); tc.finalizeT1Aggregation(1); tc.startT2Aggregation(1); + tc.startT2AggregationReveal(1); tc.finalizeT2Aggregation(1); tc.slashAuditors(1); tc.slashAggregators(1); diff --git a/foundry/test/SecurityFindings.t.sol b/foundry/test/SecurityFindings.t.sol index 988f1d0..bde5af9 100644 --- a/foundry/test/SecurityFindings.t.sol +++ b/foundry/test/SecurityFindings.t.sol @@ -319,12 +319,22 @@ contract SecurityFindingsTest is Test { (, address[] memory t1aggs,,,) = tc.getTier1Batch(1, 0); assertEq(t1aggs.length, 3, "sanity: T1 batch should have 3 aggregators"); - // C-2 regression: bytes32(0) is now rejected at submit time with TC_ZeroCID. - // Before the fix this call succeeded and permanently bricked finalizeT1Aggregation - // (TC_NoSubmissions at finalize, GI stuck forever). + // C-2 regression: bytes32(0) is now rejected at reveal time with TC_ZeroCID + // (issue #156 M-1 moved the actual CID write from submit to reveal). Before + // the original fix this call succeeded and permanently bricked + // finalizeT1Aggregation (TC_NoSubmissions at finalize, GI stuck forever). + bytes32 commitHash = keccak256( + abi.encode(bytes32(0), TEST_SALT, t1aggs[0], uint(1), DINTaskCoordinator.TierKind.Tier1, uint(0)) + ); + vm.prank(t1aggs[0]); + tc.commitT1Aggregation(1, 0, commitHash); + + vm.prank(modelOwner); + tc.startT1AggregationReveal(1); + vm.prank(t1aggs[0]); vm.expectRevert(TC_ZeroCID.selector); - tc.submitT1Aggregation(1, 0, bytes32(0)); + tc.revealT1Aggregation(1, 0, bytes32(0), TEST_SALT); } // ───────────────────────────────────────────────────────────────────── @@ -453,9 +463,20 @@ contract SecurityFindingsTest is Test { (, address[] memory t1aggs,,,) = tc.getTier1Batch(1, 0); bytes32 realCID = bytes32(uint256(0xC1D)); + for (uint i = 0; i < t1aggs.length; i++) { + bytes32 commitHash = keccak256( + abi.encode(realCID, TEST_SALT, t1aggs[i], uint(1), DINTaskCoordinator.TierKind.Tier1, uint(0)) + ); + vm.prank(t1aggs[i]); + tc.commitT1Aggregation(1, 0, commitHash); + } + + vm.prank(modelOwner); + tc.startT1AggregationReveal(1); + for (uint i = 0; i < t1aggs.length; i++) { vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, 0, realCID); + tc.revealT1Aggregation(1, 0, realCID, TEST_SALT); } vm.startPrank(modelOwner); @@ -467,6 +488,7 @@ contract SecurityFindingsTest is Test { // trivially. Not the subject of this measurement (H-1's aggregator- // side loops scale with *aggregator* count, held fixed here at the // minimum needed to reach slashAuditors()). + tc.startT2AggregationReveal(1); tc.finalizeT2Aggregation(1); vm.stopPrank(); } diff --git a/foundry/test/StakingEnforcement.t.sol b/foundry/test/StakingEnforcement.t.sol index 93b8072..d5d0896 100644 --- a/foundry/test/StakingEnforcement.t.sol +++ b/foundry/test/StakingEnforcement.t.sol @@ -388,14 +388,26 @@ contract StakingEnforcementTest is Test { (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, 0); bytes32 realCID = bytes32(uint256(0xC1D)); for (uint i = 0; i < t1aggs.length; i++) { + bytes32 t1CommitHash = keccak256( + abi.encode(realCID, salt, t1aggs[i], uint(1), DINTaskCoordinator.TierKind.Tier1, uint(0)) + ); vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, 0, realCID); + tc.commitT1Aggregation(1, 0, t1CommitHash); + } + + vm.prank(modelOwner); + tc.startT1AggregationReveal(1); + + for (uint i = 0; i < t1aggs.length; i++) { + vm.prank(t1aggs[i]); + tc.revealT1Aggregation(1, 0, realCID, salt); } vm.startPrank(modelOwner); tc.finalizeT1Aggregation(1); tc.startT2Aggregation(1); - tc.finalizeT2Aggregation(1); // trivial: 0 T2 batches at exactly 3 aggregators + tc.startT2AggregationReveal(1); // trivial: 0 T2 batches at exactly 3 aggregators + tc.finalizeT2Aggregation(1); tc.slashAuditors(1); tc.slashAggregators(1); tc.endGI(1); diff --git a/foundry/test/TreasuryForwarding.t.sol b/foundry/test/TreasuryForwarding.t.sol index f792c60..580ad6c 100644 --- a/foundry/test/TreasuryForwarding.t.sol +++ b/foundry/test/TreasuryForwarding.t.sol @@ -201,13 +201,26 @@ contract TreasuryForwardingTest is Test { tc.startT1Aggregation(1); vm.stopPrank(); - // Submit for every T1 batch so finalizeT1Aggregation doesn't revert. + // Commit+reveal for every T1 batch so finalizeT1Aggregation doesn't revert. uint256 t1Count = tc.tier1BatchCount(1); + bytes32 t1cid = bytes32(uint256(0xC1D)); for (uint b = 0; b < t1Count; b++) { (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); for (uint i = 0; i < t1aggs.length; i++) { + bytes32 commitHash = keccak256( + abi.encode(t1cid, TEST_SALT, t1aggs[i], uint(1), DINTaskCoordinator.TierKind.Tier1, b) + ); vm.prank(t1aggs[i]); - tc.submitT1Aggregation(1, b, bytes32(uint256(0xC1D))); + tc.commitT1Aggregation(1, b, commitHash); + } + } + vm.prank(modelOwner); + tc.startT1AggregationReveal(1); + for (uint b = 0; b < t1Count; b++) { + (, address[] memory t1aggs, , , ) = tc.getTier1Batch(1, b); + for (uint i = 0; i < t1aggs.length; i++) { + vm.prank(t1aggs[i]); + tc.revealT1Aggregation(1, b, t1cid, TEST_SALT); } } @@ -216,13 +229,26 @@ contract TreasuryForwardingTest is Test { tc.startT2Aggregation(1); vm.stopPrank(); - // Submit T2 aggregation if a T2 batch was created (6-agg case). + // Commit+reveal T2 aggregation if a T2 batch was created (6-agg case). + bytes32 t2cid = bytes32(uint256(0xC2D)); try tc.getTier2Batch(1, 0) returns (uint, address[] memory t2aggs, bool, bytes32) { for (uint i = 0; i < t2aggs.length; i++) { + bytes32 commitHash = keccak256( + abi.encode(t2cid, TEST_SALT, t2aggs[i], uint(1), DINTaskCoordinator.TierKind.Tier2, uint(0)) + ); vm.prank(t2aggs[i]); - tc.submitT2Aggregation(1, 0, bytes32(uint256(0xC2D))); + tc.commitT2Aggregation(1, 0, commitHash); } - } catch {} + vm.prank(modelOwner); + tc.startT2AggregationReveal(1); + for (uint i = 0; i < t2aggs.length; i++) { + vm.prank(t2aggs[i]); + tc.revealT2Aggregation(1, 0, t2cid, TEST_SALT); + } + } catch { + vm.prank(modelOwner); + tc.startT2AggregationReveal(1); + } vm.startPrank(modelOwner); tc.finalizeT2Aggregation(1); From 1252d8fe0f94bd23efa12f864bca7b32732efa11 Mon Sep 17 00:00:00 2001 From: Abidoyesimze Date: Tue, 29 Sep 2026 01:06:08 +0100 Subject: [PATCH 3/5] feat(dincli): commit/reveal aggregator commands for T1/T2 submissions aggregator aggregate-t1/aggregate-t2 --submit now commits a hidden CID hash instead of submitting the plaintext CID directly, caching (cid, salt) locally the same way auditor.py's commit-reveal flow does (never regenerated on retry). New aggregator reveal-t1/reveal-t2 commands read that cache and call revealT1Aggregation/ revealT2Aggregation once the model owner opens the reveal window via the new model-owner aggregation T1/T2 start-reveal commands. aggregation T1/T2 close's GIstate gate moves from *AggregationStarted to *AggregationRevealStarted to match. _agg_commit_hash computes keccak256(abi.encode(cid, salt, msg.sender, GI, tierKind, batchId)) via eth_abi.encode (standard, non-packed ABI encoding) + Web3.keccak, matching commitT1Aggregation/ commitT2Aggregation's NatSpec exactly -- verified against golden hashes computed independently via `cast abi-encode` + `cast keccak` (tests/test_aggregator_commit_hash.py), not by comparing the implementation to itself. cli/utils.py's states/stateDescription GIstates mirrors gain T1AggregationRevealStarted/T2AggregationRevealStarted in lifecycle position, and drop a stale duplicate append of LMSevaluationRevealStarted that had been sitting at the end of both lists since before that insertion precedent was established. Regenerate dincli/abis/DINTaskCoordinator.json (full dump-abi regen) for the new commit/reveal functions, storage getters, and events. Co-Authored-By: Claude Sonnet 5 --- dincli/abis/DINTaskCoordinator.json | 346 +++++++++++++++++++++++++- dincli/cli/aggregator.py | 158 +++++++++++- dincli/cli/modelownerd/aggregation.py | 70 +++++- dincli/cli/utils.py | 21 +- tests/test_aggregator_commit_hash.py | 70 ++++++ 5 files changed, 632 insertions(+), 33 deletions(-) create mode 100644 tests/test_aggregator_commit_hash.py diff --git a/dincli/abis/DINTaskCoordinator.json b/dincli/abis/DINTaskCoordinator.json index 786a025..43bcf2b 100644 --- a/dincli/abis/DINTaskCoordinator.json +++ b/dincli/abis/DINTaskCoordinator.json @@ -190,6 +190,52 @@ "outputs": [], "stateMutability": "nonpayable" }, + { + "type": "function", + "name": "commitT1Aggregation", + "inputs": [ + { + "name": "_GI", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "_batchId", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "commitHash", + "type": "bytes32", + "internalType": "bytes32" + } + ], + "outputs": [], + "stateMutability": "nonpayable" + }, + { + "type": "function", + "name": "commitT2Aggregation", + "inputs": [ + { + "name": "_GI", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "_batchId", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "commitHash", + "type": "bytes32", + "internalType": "bytes32" + } + ], + "outputs": [], + "stateMutability": "nonpayable" + }, { "type": "function", "name": "createAuditorsBatches", @@ -825,6 +871,62 @@ "outputs": [], "stateMutability": "nonpayable" }, + { + "type": "function", + "name": "revealT1Aggregation", + "inputs": [ + { + "name": "_GI", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "_batchId", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "_aggregationCID", + "type": "bytes32", + "internalType": "bytes32" + }, + { + "name": "salt", + "type": "bytes32", + "internalType": "bytes32" + } + ], + "outputs": [], + "stateMutability": "nonpayable" + }, + { + "type": "function", + "name": "revealT2Aggregation", + "inputs": [ + { + "name": "_GI", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "_batchId", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "_aggregationCID", + "type": "bytes32", + "internalType": "bytes32" + }, + { + "name": "salt", + "type": "bytes32", + "internalType": "bytes32" + } + ], + "outputs": [], + "stateMutability": "nonpayable" + }, { "type": "function", "name": "s2SlashFractionBps", @@ -1144,6 +1246,19 @@ "outputs": [], "stateMutability": "nonpayable" }, + { + "type": "function", + "name": "startT1AggregationReveal", + "inputs": [ + { + "name": "_GI", + "type": "uint256", + "internalType": "uint256" + } + ], + "outputs": [], + "stateMutability": "nonpayable" + }, { "type": "function", "name": "startT2Aggregation", @@ -1159,49 +1274,74 @@ }, { "type": "function", - "name": "submitT1Aggregation", + "name": "startT2AggregationReveal", "inputs": [ { "name": "_GI", "type": "uint256", "internalType": "uint256" + } + ], + "outputs": [], + "stateMutability": "nonpayable" + }, + { + "type": "function", + "name": "t1CommitHash", + "inputs": [ + { + "name": "", + "type": "uint256", + "internalType": "uint256" }, { - "name": "_batchId", + "name": "", "type": "uint256", "internalType": "uint256" }, { - "name": "_aggregationCID", + "name": "", + "type": "address", + "internalType": "address" + } + ], + "outputs": [ + { + "name": "", "type": "bytes32", "internalType": "bytes32" } ], - "outputs": [], - "stateMutability": "nonpayable" + "stateMutability": "view" }, { "type": "function", - "name": "submitT2Aggregation", + "name": "t1Committed", "inputs": [ { - "name": "_GI", + "name": "", "type": "uint256", "internalType": "uint256" }, { - "name": "_batchId", + "name": "", "type": "uint256", "internalType": "uint256" }, { - "name": "_aggregationCID", - "type": "bytes32", - "internalType": "bytes32" + "name": "", + "type": "address", + "internalType": "address" } ], - "outputs": [], - "stateMutability": "nonpayable" + "outputs": [ + { + "name": "", + "type": "bool", + "internalType": "bool" + } + ], + "stateMutability": "view" }, { "type": "function", @@ -1290,6 +1430,64 @@ ], "stateMutability": "view" }, + { + "type": "function", + "name": "t2CommitHash", + "inputs": [ + { + "name": "", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "", + "type": "address", + "internalType": "address" + } + ], + "outputs": [ + { + "name": "", + "type": "bytes32", + "internalType": "bytes32" + } + ], + "stateMutability": "view" + }, + { + "type": "function", + "name": "t2Committed", + "inputs": [ + { + "name": "", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "", + "type": "uint256", + "internalType": "uint256" + }, + { + "name": "", + "type": "address", + "internalType": "address" + } + ], + "outputs": [ + { + "name": "", + "type": "bool", + "internalType": "bool" + } + ], + "stateMutability": "view" + }, { "type": "function", "name": "t2SubmissionCID", @@ -1931,6 +2129,37 @@ ], "anonymous": false }, + { + "type": "event", + "name": "T1AggregationCommitted", + "inputs": [ + { + "name": "GI", + "type": "uint256", + "indexed": true, + "internalType": "uint256" + }, + { + "name": "batchId", + "type": "uint256", + "indexed": true, + "internalType": "uint256" + }, + { + "name": "aggregator", + "type": "address", + "indexed": true, + "internalType": "address" + }, + { + "name": "commitHash", + "type": "bytes32", + "indexed": false, + "internalType": "bytes32" + } + ], + "anonymous": false + }, { "type": "event", "name": "T1AggregationSubmitted", @@ -1987,6 +2216,37 @@ ], "anonymous": false }, + { + "type": "event", + "name": "T2AggregationCommitted", + "inputs": [ + { + "name": "GI", + "type": "uint256", + "indexed": true, + "internalType": "uint256" + }, + { + "name": "batchId", + "type": "uint256", + "indexed": true, + "internalType": "uint256" + }, + { + "name": "aggregator", + "type": "address", + "indexed": true, + "internalType": "address" + }, + { + "name": "commitHash", + "type": "bytes32", + "indexed": false, + "internalType": "bytes32" + } + ], + "anonymous": false + }, { "type": "event", "name": "T2AggregationSubmitted", @@ -2413,11 +2673,71 @@ "name": "TC_T1AggregationNotStarted", "inputs": [] }, + { + "type": "error", + "name": "TC_T1AlreadyCommitted", + "inputs": [] + }, + { + "type": "error", + "name": "TC_T1EmptyCommitHash", + "inputs": [] + }, + { + "type": "error", + "name": "TC_T1NoCommitFound", + "inputs": [] + }, + { + "type": "error", + "name": "TC_T1RevealCannotBeStarted", + "inputs": [] + }, + { + "type": "error", + "name": "TC_T1RevealHashMismatch", + "inputs": [] + }, + { + "type": "error", + "name": "TC_T1RevealPhaseNotOpen", + "inputs": [] + }, { "type": "error", "name": "TC_T2AggregationNotStarted", "inputs": [] }, + { + "type": "error", + "name": "TC_T2AlreadyCommitted", + "inputs": [] + }, + { + "type": "error", + "name": "TC_T2EmptyCommitHash", + "inputs": [] + }, + { + "type": "error", + "name": "TC_T2NoCommitFound", + "inputs": [] + }, + { + "type": "error", + "name": "TC_T2RevealCannotBeStarted", + "inputs": [] + }, + { + "type": "error", + "name": "TC_T2RevealHashMismatch", + "inputs": [] + }, + { + "type": "error", + "name": "TC_T2RevealPhaseNotOpen", + "inputs": [] + }, { "type": "error", "name": "TC_TaskAuditorContractCannotBeSet", diff --git a/dincli/cli/aggregator.py b/dincli/cli/aggregator.py index e374564..016d897 100644 --- a/dincli/cli/aggregator.py +++ b/dincli/cli/aggregator.py @@ -1,9 +1,12 @@ +import json +import secrets from pathlib import Path import time from typing import Optional import typer from rich.table import Table from web3 import Web3 +from eth_abi import encode as abi_encode from dincli.cli.dintoken import (buy_dintokens, read_din_per_eth_rate, read_dintoken_stake, stake_dintokens) from dincli.cli.utils import (CACHE_DIR, MIN_STAKE, build_and_send_tx, @@ -25,6 +28,49 @@ dintoken_app = typer.Typer(help="Commands for DIN Token in DIN.") app.add_typer(dintoken_app, name="dintoken") +# DINTaskCoordinator.TierKind ordinals (Tier1=0, Tier2=1) -- must match the +# Solidity enum exactly, since it's ABI-encoded into the commit hash. +TIER1 = 0 +TIER2 = 1 + + +# Local persistence for the commit-then-reveal T1/T2 aggregation flow +# (issue #156 M-1): the (cid, salt) committed at commit time must be +# reproduced exactly at reveal time, so it's cached to disk between the two +# commands rather than recomputed -- recomputing would also require +# re-running the aggregation worker a second time. Mirrors auditor.py's +# commit-store pattern for the auditor-side commit-reveal flow. +def _agg_commit_store_path(model_base_dir: Path, tier: int, gi: int, batch_id: int) -> Path: + tier_name = "t1" if tier == TIER1 else "t2" + return model_base_dir / "aggregations" / "commits" / f"{tier_name}_gi_{gi}_batch_{batch_id}.json" + + +def _save_agg_commit(model_base_dir: Path, tier: int, gi: int, batch_id: int, cid_bytes32: bytes, salt: bytes) -> None: + path = _agg_commit_store_path(model_base_dir, tier, gi, batch_id) + path.parent.mkdir(parents=True, exist_ok=True) + with open(path, "w") as f: + json.dump({"cid": cid_bytes32.hex(), "salt": salt.hex()}, f) + + +def _load_agg_commit(model_base_dir: Path, tier: int, gi: int, batch_id: int) -> Optional[dict]: + path = _agg_commit_store_path(model_base_dir, tier, gi, batch_id) + if not path.exists(): + return None + with open(path) as f: + data = json.load(f) + return {"cid": bytes.fromhex(data["cid"]), "salt": bytes.fromhex(data["salt"])} + + +def _agg_commit_hash(cid_bytes32: bytes, salt: bytes, sender: str, gi: int, tier: int, batch_id: int) -> bytes: + """keccak256(abi.encode(cid, salt, msg.sender, GI, tierKind, batchId)) -- + must match DINTaskCoordinator.commitT1Aggregation/commitT2Aggregation's + NatSpec exactly, including plain (non-packed) ABI encoding.""" + encoded = abi_encode( + ["bytes32", "bytes32", "address", "uint256", "uint8", "uint256"], + [cid_bytes32, salt, Web3.to_checksum_address(sender), gi, tier, batch_id], + ) + return Web3.keccak(encoded) + @dintoken_app.command(help="Buy DINTokens where amount is ETH to exchange for DINTokens") def buy( @@ -335,23 +381,75 @@ def aggregate_t1( if submit: try: aggregated_cid_bytes32 = Web3.to_bytes(hexstr=get_bytes32_from_cid(aggregated_cid)) + salt = secrets.token_bytes(32) + commit_hash = _agg_commit_hash(aggregated_cid_bytes32, salt, account.address, curr_GI, TIER1, bid) time.sleep(2) build_and_send_tx( ctx, - taskCoordinator_contract.functions.submitT1Aggregation(curr_GI, bid, aggregated_cid_bytes32), - f"Submitting T1 aggregation CID for T1 batch {bid} with aggregated CID {aggregated_cid}", - "Aggregation CID submitted.", - "Could not submit aggregation CID.", + taskCoordinator_contract.functions.commitT1Aggregation(curr_GI, bid, commit_hash), + f"Committing T1 aggregation CID for T1 batch {bid}", + f"T1 aggregation committed for batch {bid}! Reveal after the model owner opens the reveal window (`dincli aggregator reveal-t1`).", + "Could not commit aggregation CID.", exit_on_failure=False ) + # Only cache locally once the commit tx is known to have been + # attempted -- reveal is a no-op without this file. + _save_agg_commit(model_base_dir, TIER1, curr_GI, bid, aggregated_cid_bytes32, salt) except Exception as e: - console.print(f"[bold red]✗ Could not submit aggregation CID. Error: {e}[/bold red]") + console.print(f"[bold red]✗ Could not commit aggregation CID. Error: {e}[/bold red]") raise typer.Exit(1) if not found_batch: console.print(f"[yellow]No T1 batches found for aggregator {account.address}[/yellow]") + +@app.command("reveal-t1", help="Reveal a previously committed T1 aggregation CID") +def reveal_t1( + ctx: typer.Context, + model_id: int = typer.Argument(..., help="Model ID"), + gi: int = typer.Option(None, "--gi", help="Global iteration number"), + batch_id: int = typer.Option(None, "--batch", help="Batch ID"), +): + effective_network, w3, account, console = ctx.obj.get_en_w3_account_console(model_id) + + taskCoordinator_contract = ctx.obj.get_deployed_din_task_coordinator_contract(True, model_id) + + curr_GI, GIstate = ctx.obj.get_current_gi_and_state(taskCoordinator_contract) + + ref_gi = ctx.obj.validate_gi_ET_curr_GI(gi, curr_GI) + ctx.obj.validate_GIstate_ET_given_GIstate(GIstate, "T1AggregationRevealStarted", "Can not reveal T1 aggregation at this time") + + model_base_dir = ctx.obj.get_model_base_dir(model_id) + t1_batches_count = taskCoordinator_contract.functions.tier1BatchCount(curr_GI).call() + + found_any = False + for bid in range(t1_batches_count): + if batch_id is not None and bid != batch_id: + continue + + commit = _load_agg_commit(model_base_dir, TIER1, curr_GI, bid) + if commit is None: + continue + + found_any = True + console.print(f"[bold green]Revealing T1 aggregation for batch {bid}[/bold green]") + + try: + build_and_send_tx( + ctx, + taskCoordinator_contract.functions.revealT1Aggregation(curr_GI, bid, commit["cid"], commit["salt"]), + f"Revealing T1 aggregation for batch {bid}", + f"T1 aggregation revealed for batch {bid}!", + f"T1 aggregation reveal failed for batch {bid}!", + exit_on_failure=False + ) + except Exception as e: + console.print(f"[bold red]✗ Error revealing T1 aggregation for batch {bid}: {e}[/bold red]") + + if not found_any: + console.print("[yellow]No local T1 commits found to reveal.[/yellow]") + @app.command("aggregate-t2", help="Aggregate T2 batches") def aggregate_t2( @@ -488,18 +586,58 @@ def aggregate_t2( if submit: try: aggregated_cid_bytes32 = Web3.to_bytes(hexstr=get_bytes32_from_cid(aggregated_cid)) + salt = secrets.token_bytes(32) + commit_hash = _agg_commit_hash(aggregated_cid_bytes32, salt, account.address, curr_GI, TIER2, i) build_and_send_tx( ctx, - taskCoordinator_contract.functions.submitT2Aggregation(curr_GI, i, aggregated_cid_bytes32), - f"Submitting T2 aggregation CID for T2 batch {bid} with aggregated CID {aggregated_cid}", - "Aggregation CID submitted.", - "Could not submit aggregation CID.", + taskCoordinator_contract.functions.commitT2Aggregation(curr_GI, i, commit_hash), + f"Committing T2 aggregation CID for T2 batch {bid}", + f"T2 aggregation committed for batch {bid}! Reveal after the model owner opens the reveal window (`dincli aggregator reveal-t2`).", + "Could not commit aggregation CID.", exit_on_failure=False ) + _save_agg_commit(model_base_dir, TIER2, curr_GI, i, aggregated_cid_bytes32, salt) except Exception as e: - console.print(f"[bold red]✗ Could not submit aggregation CID. Error: {e}[/bold red]") + console.print(f"[bold red]✗ Could not commit aggregation CID. Error: {e}[/bold red]") raise typer.Exit(1) if not found_batch: console.print(f"[yellow]No T2 batches found for aggregator {account.address} in GI {curr_GI}[/yellow]") + + +@app.command("reveal-t2", help="Reveal a previously committed T2 aggregation CID") +def reveal_t2( + ctx: typer.Context, + model_id: int = typer.Argument(..., help="Model ID"), + gi: int = typer.Option(None, "--gi", help="Global iteration number"), +): + effective_network, w3, account, console = ctx.obj.get_en_w3_account_console(model_id) + + taskCoordinator_contract = ctx.obj.get_deployed_din_task_coordinator_contract(True, model_id) + + curr_GI, GIstate = ctx.obj.get_current_gi_and_state(taskCoordinator_contract) + + ref_gi = ctx.obj.validate_gi_ET_curr_GI(gi, curr_GI) + ctx.obj.validate_GIstate_ET_given_GIstate(GIstate, "T2AggregationRevealStarted", "Can not reveal T2 aggregation at this time") + + model_base_dir = ctx.obj.get_model_base_dir(model_id) + + commit = _load_agg_commit(model_base_dir, TIER2, curr_GI, 0) + if commit is None: + console.print("[yellow]No local T2 commit found to reveal.[/yellow]") + return + + console.print("[bold green]Revealing T2 aggregation[/bold green]") + + try: + build_and_send_tx( + ctx, + taskCoordinator_contract.functions.revealT2Aggregation(curr_GI, 0, commit["cid"], commit["salt"]), + "Revealing T2 aggregation", + "T2 aggregation revealed!", + "T2 aggregation reveal failed!", + exit_on_failure=False + ) + except Exception as e: + console.print(f"[bold red]✗ Error revealing T2 aggregation: {e}[/bold red]") diff --git a/dincli/cli/modelownerd/aggregation.py b/dincli/cli/modelownerd/aggregation.py index 3b611dd..9ee48bd 100644 --- a/dincli/cli/modelownerd/aggregation.py +++ b/dincli/cli/modelownerd/aggregation.py @@ -181,6 +181,39 @@ def start_t1_aggregation( console.print(f"[red]Error: Tier 1 Aggregation started transaction failed[/red] {e}") raise typer.Exit(1) +@t1_app.command("start-reveal") +def start_t1_aggregation_reveal( + ctx: typer.Context, + model_id: int = typer.Argument(..., help="Model ID"), + gi: int = typer.Option(None, "--gi", help="Global iteration number"), +): + """Close the T1 commit window and open the reveal window (commit-then-reveal aggregation, issue #156 M-1).""" + effective_network, w3, account, console = ctx.obj.get_en_w3_account_console(model_id) + + task_coordinator_Contract = ctx.obj.get_deployed_din_task_coordinator_contract(True, model_id) + + curr_GI, GIstate = ctx.obj.get_current_gi_and_state(task_coordinator_Contract) + + ref_gi = ctx.obj.validate_gi_ET_curr_GI(gi, curr_GI) + ctx.obj.validate_GIstate_ET_given_GIstate(GIstate, "T1AggregationStarted", "Can not start Tier 1 aggregation reveal at this time.") + + console.print(f"[bold green]Starting Tier 1 Aggregation reveal window[/bold green]") + + try: + tx_receipt = build_and_send_tx( + ctx, + task_coordinator_Contract.functions.startT1AggregationReveal(ref_gi), + "Starting Tier 1 Aggregation reveal", + "Tier 1 Aggregation reveal started", + "Tier 1 Aggregation reveal start transaction failed", + exit_on_failure=False + ) + console.print(f"[dim]Tx hash: {tx_receipt.transactionHash.hex()}[/dim]") + except Exception as e: + console.print(f"[red]Error: Tier 1 Aggregation reveal start transaction failed[/red] {e}") + raise typer.Exit(1) + + @t1_app.command("close") def close_t1_aggregation( ctx: typer.Context, @@ -194,7 +227,7 @@ def close_t1_aggregation( curr_GI, GIstate = ctx.obj.get_current_gi_and_state(task_coordinator_Contract) ref_gi = ctx.obj.validate_gi_ET_curr_GI(gi, curr_GI) - ctx.obj.validate_GIstate_ET_given_GIstate(GIstate, "T1AggregationStarted","Can not close Tier 1 aggregation at this time.") + ctx.obj.validate_GIstate_ET_given_GIstate(GIstate, "T1AggregationRevealStarted","Can not close Tier 1 aggregation at this time.") console.print(f"[bold green]Finalizing Tier 1 Aggregation[/bold green]") @@ -243,6 +276,39 @@ def start_t2_aggregation( raise typer.Exit(1) +@t2_app.command("start-reveal") +def start_t2_aggregation_reveal( + ctx: typer.Context, + model_id: int = typer.Argument(..., help="Model ID"), + gi: int = typer.Option(None, "--gi", help="Global iteration number"), +): + """Close the T2 commit window and open the reveal window (commit-then-reveal aggregation, issue #156 M-1).""" + effective_network, w3, account, console = ctx.obj.get_en_w3_account_console(model_id) + + task_coordinator_Contract = ctx.obj.get_deployed_din_task_coordinator_contract(True, model_id) + + curr_GI, GIstate = ctx.obj.get_current_gi_and_state(task_coordinator_Contract) + + ref_gi = ctx.obj.validate_gi_ET_curr_GI(gi, curr_GI) + ctx.obj.validate_GIstate_ET_given_GIstate(GIstate, "T2AggregationStarted", "Can not start Tier 2 aggregation reveal at this time.") + + console.print(f"[bold green]Starting Tier 2 Aggregation reveal window[/bold green]") + + try: + tx_receipt = build_and_send_tx( + ctx, + task_coordinator_Contract.functions.startT2AggregationReveal(ref_gi), + "Starting Tier 2 Aggregation reveal", + "Tier 2 Aggregation reveal started", + "Tier 2 Aggregation reveal start transaction failed", + exit_on_failure=False + ) + console.print(f"[dim]Tx hash: {tx_receipt.transactionHash.hex()}[/dim]") + except Exception as e: + console.print(f"[red]Error: Tier 2 Aggregation reveal start transaction failed[/red] {e}") + raise typer.Exit(1) + + @t2_app.command("close") def close_t2_aggregation( ctx: typer.Context, @@ -263,7 +329,7 @@ def close_t2_aggregation( curr_GI, GIstate = ctx.obj.get_current_gi_and_state(task_coordinator_Contract) ref_gi = ctx.obj.validate_gi_ET_curr_GI(gi, curr_GI) - ctx.obj.validate_GIstate_ET_given_GIstate(GIstate, "T2AggregationStarted", "Can not close Tier 2 aggregation at this time.") + ctx.obj.validate_GIstate_ET_given_GIstate(GIstate, "T2AggregationRevealStarted", "Can not close Tier 2 aggregation at this time.") console.print(f"[bold green]Finalizing Tier 2 Aggregation[/bold green]") try: diff --git a/dincli/cli/utils.py b/dincli/cli/utils.py index 9be9fae..08f1f8f 100644 --- a/dincli/cli/utils.py +++ b/dincli/cli/utils.py @@ -656,13 +656,14 @@ def save_din_info(data: dict): "LM submissions evaluation closed", "T1nT2B created", "T1B aggregation started", + "T1B aggregation reveal started", "T1B aggregation done", "T2B aggregation started", + "T2B aggregation reveal started", "T2B aggregation done", "Auditors slashed", "Validators slashed", "GI ended", - "LM submissions evaluation reveal started" ] states = [ @@ -684,20 +685,24 @@ def save_din_info(data: dict): "LMSevaluationClosed", "T1nT2Bcreated", "T1AggregationStarted", + # issue #156 M-1 (task_240926_18 Part C): inserted in lifecycle + # position, same 2026-08-27 precedent LMSevaluationRevealStarted + # above set -- this list is a positional mirror of DINShared.sol's + # GIstates enum, and that enum's own comment explains why insertion + # (not appending) is required to keep this mirror in sync. A stale + # duplicate append of LMSevaluationRevealStarted previously sat at + # the end of this list from before that precedent was applied here; + # removed as part of this same fix. + "T1AggregationRevealStarted", "T1AggregationDone", "T2AggregationStarted", + "T2AggregationRevealStarted", "T2AggregationDone", "AuditorsSlashed", "AggregatorsSlashed", "GIended", - # Appended, not inserted where it chronologically belongs (between - # LMSevaluationStarted and LMSevaluationClosed) -- this list is a - # positional mirror of DINShared.sol's GIstates enum, which appends - # this member for the same reason (see the enum's own comment). - # Inserting here would desync every state index below GIended. - "LMSevaluationRevealStarted" ] - + GIstate_to_index = {state: idx for idx, state in enumerate(states)} diff --git a/tests/test_aggregator_commit_hash.py b/tests/test_aggregator_commit_hash.py new file mode 100644 index 0000000..0ceb7d2 --- /dev/null +++ b/tests/test_aggregator_commit_hash.py @@ -0,0 +1,70 @@ +"""Tests for dincli.cli.aggregator._agg_commit_hash (issue #156 M-1). + +commitT1Aggregation/commitT2Aggregation on DINTaskCoordinator require +commitHash == keccak256(abi.encode(cid, salt, msg.sender, GI, tierKind, +batchId)). This must match byte-for-byte, or every `aggregator reveal-t1`/ +`reveal-t2` call ever sent would revert with TC_T1RevealHashMismatch / +TC_T2RevealHashMismatch. Golden values below were computed independently via +`cast abi-encode` + `cast keccak` (not by importing this module's own +encoding logic), so a field-order or type mistake here would be caught +rather than silently agreeing with itself. +""" +from web3 import Web3 + +from dincli.cli.aggregator import TIER1, TIER2, _agg_commit_hash + + +def test_agg_commit_hash_matches_cast_golden_vector_tier1(): + cid = Web3.to_bytes(hexstr="0x00000000000000000000000000000000000000000000000000000000000000a1") + salt = Web3.to_bytes(hexstr="0x0000000000000000000000000000000000000000000000000000000000c0ffee") + sender = "0x1234567890123456789012345678901234567890" + gi = 1 + batch_id = 0 + + result = _agg_commit_hash(cid, salt, sender, gi, TIER1, batch_id) + + assert result.hex() == "e3d4a8c4ae8d553d5b31a514ea2e99c01ce29e6d0e4949daa9506af57153aa70" + + +def test_agg_commit_hash_matches_cast_golden_vector_tier2(): + cid = Web3.to_bytes(hexstr="0x00000000000000000000000000000000000000000000000000000000000000b2") + salt = Web3.to_bytes(hexstr="0x000000000000000000000000000000000000000000000000000000000000dead") + sender = "0xabcdefabcdefabcdefabcdefabcdefabcdefabcd" + gi = 7 + batch_id = 3 + + result = _agg_commit_hash(cid, salt, sender, gi, TIER2, batch_id) + + assert result.hex() == "66a96e7747bc1c4c5a3ca9e051d24e8be0ceff00b21b9ca3be6740db92b59cf0" + + +def test_agg_commit_hash_changes_with_sender(): + """The entire point of binding msg.sender into the hash (issue #156 M-1, + hardening #156's own `keccak256(cid, salt)` proposal) is that two + different senders committing the same (cid, salt) get different hashes + -- otherwise one could replay the other's commit hash and reveal under + it. Confirm the Python side actually varies with sender, not just the + Solidity side.""" + cid = Web3.to_bytes(hexstr="0x00000000000000000000000000000000000000000000000000000000000000a1") + salt = Web3.to_bytes(hexstr="0x0000000000000000000000000000000000000000000000000000000000c0ffee") + gi, batch_id = 1, 0 + + hash_a = _agg_commit_hash(cid, salt, "0x1234567890123456789012345678901234567890", gi, TIER1, batch_id) + hash_b = _agg_commit_hash(cid, salt, "0xabcdefabcdefabcdefabcdefabcdefabcdefabcd", gi, TIER1, batch_id) + + assert hash_a != hash_b + + +def test_agg_commit_hash_changes_with_tier(): + """Same cid/salt/sender/gi/batchId, different tier -> different hash. + Without this, a T1 commit could be replayed as a T2 reveal (or the + domain-separation the enum provides would be a no-op).""" + cid = Web3.to_bytes(hexstr="0x00000000000000000000000000000000000000000000000000000000000000a1") + salt = Web3.to_bytes(hexstr="0x0000000000000000000000000000000000000000000000000000000000c0ffee") + sender = "0x1234567890123456789012345678901234567890" + gi, batch_id = 1, 0 + + hash_t1 = _agg_commit_hash(cid, salt, sender, gi, TIER1, batch_id) + hash_t2 = _agg_commit_hash(cid, salt, sender, gi, TIER2, batch_id) + + assert hash_t1 != hash_t2 From b6627fa236f465923af34d3c5c8a6b096cb5ce89 Mon Sep 17 00:00:00 2001 From: Abidoyesimze Date: Tue, 29 Sep 2026 01:06:23 +0100 Subject: [PATCH 4/5] docs: document T1/T2 commit-reveal, mark M-1 aggregation-side fixed model-workflow.md: walk through the new commit/reveal split for T1 and T2 aggregation, including the model-owner start-reveal step between commit and close. DINShared.md: update the GIstates ordinal table and lifecycle diagram for T1AggregationRevealStarted/T2AggregationRevealStarted; note the commit-then-reveal shape mirrors the existing LMS evaluation section. foundry-src-security-review.md: mark M-1's aggregation-side half fixed (PR number filled in once opened), and note the auditor-side half (PR #63) is deliberately left unfixed, tracked in the new #192. Co-Authored-By: Claude Sonnet 5 --- .../public/workflows/model-workflow.md | 47 +++++++++++++-- .../audits/foundry-src-security-review.md | 2 + .../technical/contracts/DINShared.md | 58 +++++++++++-------- 3 files changed, 76 insertions(+), 31 deletions(-) diff --git a/Documentation/public/workflows/model-workflow.md b/Documentation/public/workflows/model-workflow.md index 7908a56..8771b19 100644 --- a/Documentation/public/workflows/model-workflow.md +++ b/Documentation/public/workflows/model-workflow.md @@ -369,6 +369,17 @@ dincli model-owner lms-evaluation close 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: ```bash @@ -377,19 +388,31 @@ dincli model-owner aggregation create-t1nt2-batches # show t1 and t2 batches dincli model-owner aggregation show-t1-batches --detailed dincli model-owner aggregation show-t2-batches --detailed -# start t1 aggregation +# start t1 aggregation (opens the commit window) dincli model-owner aggregation T1 start ``` -**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 --detailed -# aggregate the assigned t1 batches +# aggregate and commit the assigned t1 batches dincli aggregator aggregate-t1 --submit ``` +**Model Owner** opens the T1 reveal window: + +```bash +dincli model-owner aggregation T1 start-reveal +``` + +**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 Owner** closes T1 and starts T2 aggregation: ```bash @@ -397,21 +420,33 @@ dincli aggregator aggregate-t1 --submit dincli model-owner aggregation show-t1-batches --detailed # close t1 aggregation dincli model-owner aggregation T1 close -# start t2 aggregation +# start t2 aggregation (opens the commit window) dincli model-owner aggregation T2 start # show t2 batches dincli model-owner aggregation show-t2-batches --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 --detailed -# aggregate the assigned t2 batches +# aggregate and commit the assigned t2 batches dincli aggregator aggregate-t2 --submit ``` +**Model Owner** opens the T2 reveal window: + +```bash +dincli model-owner aggregation T2 start-reveal +``` + +**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 Owner** closes T2 aggregation: ```bash diff --git a/Documentation/technical/audits/foundry-src-security-review.md b/Documentation/technical/audits/foundry-src-security-review.md index c2232a4..0d2f9eb 100644 --- a/Documentation/technical/audits/foundry-src-security-review.md +++ b/Documentation/technical/audits/foundry-src-security-review.md @@ -149,6 +149,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 #XXX](https://github.com/InfiniteZeroFoundation/DevNet/pull/XXX) (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. diff --git a/Documentation/technical/contracts/DINShared.md b/Documentation/technical/contracts/DINShared.md index c28f7f2..84ddcef 100644 --- a/Documentation/technical/contracts/DINShared.md +++ b/Documentation/technical/contracts/DINShared.md @@ -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 @@ -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 From b5d602fa4c2fd232e6c2e396e3470fdf57197289 Mon Sep 17 00:00:00 2001 From: Abidoyesimze Date: Tue, 29 Sep 2026 01:07:43 +0100 Subject: [PATCH 5/5] docs(audit): fill in PR #197 link for M-1 aggregation-side fix Follow-up to the previous commit's placeholder now that the PR exists. Co-Authored-By: Claude Sonnet 5 --- Documentation/technical/audits/foundry-src-security-review.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/technical/audits/foundry-src-security-review.md b/Documentation/technical/audits/foundry-src-security-review.md index 0d2f9eb..b45f179 100644 --- a/Documentation/technical/audits/foundry-src-security-review.md +++ b/Documentation/technical/audits/foundry-src-security-review.md @@ -149,7 +149,7 @@ 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 #XXX](https://github.com/InfiniteZeroFoundation/DevNet/pull/XXX) (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. +**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).