Skip to content

task_021026_19.md

Latest commit

 

History

History
274 lines (203 loc) · 21.4 KB

task_021026_19.md

File metadata and controls

274 lines (203 loc) · 21.4 KB
Raw

Task: P3 Deploy Blocker + Open Security/dincli Findings — EIP-170 Size Gate (#201 A), onlyCurrentGI (#206), Sender-Bound Audit Commit (#192), Auditor Commit Retry (#202), Test-Data Dispute Drain (#205), Wiki DIN-Rep (#207)

ID: task_021026_19 Assigned to: Umer Majeed (@umermjd11) Created: 2026-10-02 Status: Closed (completed 2026-10-05). All six Parts landed (PRs #211, #212, #214, #215; wiki revision on #207). Closed in Discussion #216 Dates: Oct 2 – Oct 12, 2026 (about 9 working days, per-Part estimates below). Parts A+B and C can start immediately. D and E are gated on earlier PRs merging (see §Sequencing). If those slip, say so early in the tracking discussion. Repo: https://github.com/InfiniteZeroFoundation/DevNet Base branch: develop — spec written against commit 84d8c38 (2026-10-02); re-pull before starting regardless of when you begin. foundry/, dincli/, tests/ and .github/ are unchanged from 6ccc28c, the commit the source plan was verified against. Roadmap ref: P3-6.3a (gas/size, deploy blocker), P3-6.3b (audit preparation, open findings), P3-4.3 (dispute resolution), P3-SCR (auditor commit-reveal) Source plan: task-plan-021026-1.md (reviewed and merged in PR #209; verification review, reviewer decisions) Tracking:

  • Parts A + B: #201 Part A (Part B of the issue stays open; see Out of scope) and #206 (closes, pending review). #201 is a hard blocker per the #201 decision comment.
  • Part C: #192 (closes, pending review). Scope agreed in the #192 approval review (amendments 1–6).
  • Part D: #202 (closes, pending review).
  • Part E: #205 (closes, pending review). Design agreed in PR #209 Decision 3.
  • Part F: #207 (closes once the wiki revision is linked).
  • Discussion #216 (progress updates, questions, PR links)

Status check at 84d8c38

None of the six is implemented on develop:

Issue State at 84d8c38
#201 A DINTaskCoordinator is 24,585 B runtime, 9 B over EIP-170. DINTaskAuditor is 22,567 B (margin 2,009 B). Both figures come from foundry/out/*.json deployedBytecode, via_ir, 200 runs. ci.yml runs plain forge build/forge test (:60-66) with no size check, and forge build --sizes leaves DINTaskAuditor out of its table. foundry/anvil.sh:6 lifts the code-size limit without saying why
#206 registerDINaggregator(uint _GI) public (DINTaskCoordinator.sol:397) has no onlyCurrentGI (the modifier is at :246). The auditor side has it (DINTaskAuditor.sol:660)
#192 revealAuditScore checks keccak256(abi.encodePacked(score, vote, salt)) (DINTaskAuditor.sol:1160). The formula is also in the NatSpec at :244, :1080, :1091. dincli uses Web3.solidity_keccak(["uint256","bool","bytes32"], …) (dincli/cli/auditor.py:534)
#202 evaluate_lms makes a fresh salt for every LM on every run (auditor.py:533) and calls _save_commit unconditionally after the send (:550). aggregate_t2 reads bid from getTier2Batch (aggregator.py:525), then rebinds it in the T1 loop (:547)
#205 openTestDataDispute (DINTaskAuditor.sol:1461) and resolveTestDataDispute (:1495) have no caller check. disputeBondAmount is 0 at deploy (:283). test_resolveDispute_upheld_returnsBondAndPenalises (EncryptedTestData.t.sol:319) encodes the attack as intended behaviour
#207 The wiki DIN-Representative page still describes dincli dindao, three proxies, set-admin, withdraw, and a kill switch the task contracts don't read

Sequencing

Part PR Touches Conflicts with Rule
A PR 1, commit 1 DINTaskCoordinator.sol (getters, slashAggregators), .github/workflows/ci.yml + new .github/scripts/ gate, foundry/anvil.sh, Developer/CONTRIBUTING.md, dincli/cli/aggregator.py, dincli/cli/modelownerd/aggregation.py, dincli/abis/DINTaskCoordinator.json, tests/test_aggregator_commit_retry.py, coordinator foundry tests, DINTaskCoordinator.md B (same contract and byte budget), D (aggregator.py) Start immediately.
B PR 1, commit 2 DINTaskCoordinator.registerDINaggregator + tests A Same PR as A. It can't merge alone: it would take develop to 24,594 B and fail the gate.
C PR 2 DINTaskAuditor.revealAuditScore + NatSpec, dincli/cli/auditor.py, the foundry tests that commit scores, DINTaskAuditor.md, DINShared.md D (auditor.py), E (DINTaskAuditor.sol, tests) Start immediately, in parallel with PR 1. It touches a different contract.
D PR 3 dincli/cli/auditor.py evaluate_lms, dincli/cli/aggregator.py aggregate_t2, new pytest files C (hash helper), A (aggregator.py getter switch) After PR 1 and PR 2 merge. Part D2 could go earlier on its own if A hasn't touched aggregate_t2.
E PR 4 DINTaskAuditor test-data dispute functions + default bond, EncryptedTestData.t.sol, DINTaskAuditor.md §13 C (same contract and test files) After PR 2 merges, rebased.
F — (wiki) GitHub wiki only — Anytime.

Rebase each PR on develop after the previous one merges, rather than stacking branches. Long-running drafts PR #31/#32 touch DINTaskAuditor.sol and dincli/cli/auditor.py; they rebase on these changes, not the other way round (#192 approval review, Conflicts).

Part A — #201 Part A: EIP-170 budget, CI gate, DINTaskCoordinator under budget — PR 1, commit 1

Design: shrink inside the same contract

Don't add a library, a second contract, or a deploy step. All three levers below were prototyped twice against 6ccc28c: once for the source plan, and once independently in the PR #209 review. Runtime bytes, via_ir, optimizer_runs = 200:

Lever Plan prototype Review prototype
A1a. Fold slashAggregators' duplicated T1/T2 loops (:1119-1195) into one internal _slashBatch(GI, batchId, aggregators, finalCID, minStakeAmt, s2Amount, bool t2) −471 B −515 B
A1b. Make the 10 per-aggregator getters internal and add one view (below) −355 B −355 B
A1c. Make the tier1Batches/tier2Batches auto-getters internal. getTier1Batch/getTier2Batch already cover them — −214 B (both)
Combined 23,542 B (margin 1,034 B) 23,498 B (margin 1,078 B)
Combined + Part B 23,551 B (margin 1,025 B) 23,507 B (margin 1,069 B)

Rejected (measured):

  • folding T1/T2 commit…/reveal… into shared internals: +177 B, because via_ir inlines them
  • folding finalize… the same way: +166 B
  • lowering optimizer_runs to 1: only −76 B

Don't retry these.

A1a — _slashBatch:

  • Keep the reason codes as bytes32: AGG_T1_NO_SUBMISSION, AGG_T2_NO_SUBMISSION, AGG_T1_BAD_CONSENSUS, AGG_T2_BAD_CONSENSUS.
  • Keep every AggregatorSlashed event argument, the S2 partial slash with its s2Amount > 0 guard, and the full-minStake bad-consensus slash.
  • Keep the T1-then-T2 order.
  • Behaviour must be identical, and the existing slashing tests must pass unchanged.

A1b — getAggregatorSubmission:

function getAggregatorSubmission(uint _GI, TierKind tier, uint _batchId, address aggregator)
    external view
    returns (bool committed, bytes32 commitHash, bool submitted, bytes32 cid, uint votes);
  • It replaces the public getters t1/t2 × SubmissionCID, Submitted, Votes, CommitHash, Committed. The storage stays as it is; only the getters go.
  • votes is the vote count for that aggregator's own revealed CID (tNVotes[GI][batchId][cid]), and 0 before a reveal. t1Votes/t2Votes are keyed by CID (:66,85), so per-arbitrary-CID lookup goes away. Nothing calls it today (PR #209 Decision 4).
  • Callers to move:
    • dincli: dincli/cli/aggregator.py:213,265,322,532 and dincli/cli/modelownerd/aggregation.py:109,154. These are the only Python callers.
    • pytest: tests/test_aggregator_commit_retry.py:53, which mocks t1Committed/t2Committed by name.
    • foundry: AggregatorCommitReveal.t.sol:299,300,599,600 (t1Submitted, t1SubmissionCID), plus any other reader forge build flags.
  • Subgraph: PR #29's mappings only .bind() DINModelRegistry and never call these getters. Only its bundled ABI goes stale, so post a note on PR #29 with the ABI change. Don't push to that branch.

The margin after Part B is thin (1,025–1,069 B against a 1,024 B fail line). Measure these two extra candidates and report them in the PR even if you don't take them:

  • fold tier1FinalizedAt/tier2FinalizedAt into a view. TreasuryForwarding.t.sol:275 reads tier1FinalizedAt
  • merge tier-duplicated custom errors (TC_T1/T2EmptyCommitHash, …AlreadyCommitted, …NoCommitFound, …RevealHashMismatch) where the function already implies the tier

Scope

  • A2. CI size gate. Add a step right after forge build in .github/workflows/ci.yml, backed by a script in .github/scripts/ (next to check_doc_links.py). The script:
    • reads every foundry/out/<File>.sol/<Contract>.json for deployable contracts under foundry/src directly, not the --sizes table, which omits DINTaskAuditor. Skip interfaces, libraries and abstract contracts (empty deployedBytecode)
    • prints a table of contract, runtime size, margin and initcode size
    • fails if any runtime margin is below 1,024 B, and warns (doesn't fail) below 2,048 B (PR #209 Decision 1). Both thresholds are named constants at the top of the script
    • fails if any initcode exceeds EIP-3860's 49,152 B
  • A3. foundry/anvil.sh: keep --code-size-limit, but add a comment saying it hides EIP-170 locally, and that the CI gate is what enforces the limit.
  • A4. Developer/CONTRIBUTING.md: record both thresholds, how to run the gate locally, and that a PR touching either task contract must keep the gate green.
  • A5. Docs + ABI:
    • update the state-variable and view tables in Documentation/technical/contracts/DINTaskCoordinator.md
    • regenerate dincli/abis/DINTaskCoordinator.json in the same PR (Decision 4)

Deliverables (Part A)

  • A1a–c implemented, with no change to state-changing functions, events or storage layout
  • A per-change and combined size table, from one build of the actual implementation, plus the two extra candidates' measured sizes
  • The gate is in CI. The PR shows it failing on a deliberately oversized build and printing the 2,048 B warning
  • dincli call sites and tests/test_aggregator_commit_retry.py moved. pytest -m "not integration" is green
  • cd foundry && npm ci && forge build && forge test is green across the full suite, including UpgradeValidation.t.sol
  • anvil.sh comment, CONTRIBUTING.md thresholds, DINTaskCoordinator.md and the bundled ABI updated
  • ABI note posted on PR #29

Estimate: 3 days.

Part B — #206: onlyCurrentGI on registerDINaggregator — PR 1, commit 2

Ship it as a separate commit in PR 1, so the security fix can be reviewed and reverted on its own (PR #209 Decision 2).

  • Add onlyCurrentGI(_GI) to registerDINaggregator (DINTaskCoordinator.sol:397), matching registerDINAuditor (DINTaskAuditor.sol:660).
  • Regression tests:
    • registerDINaggregator(GI + 1) and registerDINaggregator(GI - 1) revert with TC_WrongGI
    • the current-GI path still registers and increments activeRegistrationCount
  • Cost: +9 B (measured). The gate must still pass with A + B together.
  • Update DINTaskCoordinator.md §10 if it lists this as a caveat.

Deliverables (Part B)

  • Modifier added, with the three tests above
  • The gate is green, and the PR shows the final size with B included

Estimate: 0.5 day.

Part C — #192: sender-bound auditor commit hash — PR 2

Implements amendments 1–6 of the #192 approval review:

  1. Contract:
    • revealAuditScore checks keccak256(abi.encode(score, vote, salt, msg.sender, gi, batchId, modelIndex)) instead of abi.encodePacked(score, vote, salt) (DINTaskAuditor.sol:1160).
    • Update the formula in the NatSpec at :244, :1080 (commitAuditScore @dev) and :1091.
    • Function signatures and the bytes32 commitHash parameter don't change, so the bundled ABI stays the same. Confirm that.
  2. dincli:
    • Add _audit_commit_hash(score, vote, salt, sender, gi, batch_id, model_index) in dincli/cli/auditor.py, modelled on _agg_commit_hash (dincli/cli/aggregator.py:65).
    • It uses eth_abi.encode(["uint256","bool","bytes32","address","uint256","uint256","uint256"], …) + Web3.keccak, with the auditor's own account address.
    • Use it at :534. reveal_lms sends the stored (score, vote, salt) and doesn't recompute the hash, so it's unchanged.
  3. Per-auditor hashing in every test that commits scores. Most tests compute one commitHash before the auditor loop, which only works while the hash is sender-independent.
    • Put the formula in one shared test helper.
    • Use it in GasSimulation.t.sol, LifecycleEvents.t.sol, PR146SlashingRegression.t.sol, SecurityFindings.t.sol, StakingEnforcement.t.sol, TreasuryForwarding.t.sol, DisputeResolution.t.sol, RewardEngine.t.sol, AggregatorCommitReveal.t.sol (its auditor phase also calls commitAuditScore), and the _commitScore helpers in AuditorCommitReveal.t.sol and ScoringValidation.t.sol.
    • Line numbers have moved since the approval review; find the call sites by grep.
  4. Regression tests in AuditorCommitReveal.t.sol:
    • auditor B commits A's hash, then reveals A's (score, vote, salt): TA_RevealHashMismatch
    • a hash built for model X doesn't reveal for model Y, or for another batch or GI
    • the honest path still finalizes
  5. Docs: update Documentation/technical/contracts/DINTaskAuditor.md:123,199 and DINShared.md:216,401 (including the TA_RevealHashMismatch row).
  6. Size: report DINTaskAuditor before and after in the PR, using the Part A gate once it has merged.

hardhat/contracts/DINTaskAuditor.sol is a stale mirror, so updating it is optional.

Deliverables (Part C)

  • Contract hash, NatSpec, dincli helper, the shared test helper and every affected test updated
  • The three regression tests
  • Docs updated. Bundled ABI confirmed unchanged
  • forge test and pytest -m "not integration" green. DINTaskAuditor size reported

Estimate: 2 days (most of it is test churn).

Part D — #202: dincli auditor commit retry + aggregate-t2 batch id — PR 3

Starts after PR 1 and PR 2 merge. It uses Part C's _audit_commit_hash, and Part A's getter switch in aggregator.py.

D1. Auditor commit retry (evaluate_lms, dincli/cli/auditor.py:530-553)

This is the auditor-side copy of PR #197's aggregator fix. Mirror aggregate_t1/aggregate_t2:

  • Before evaluating an LM, read hasCommittedLM(GI, batchId, account, modelIndex) (a public mapping, DINTaskAuditor.sol:257). If it's already committed, skip that LM and leave its commit cache untouched.
  • Write the commit cache before sending the commit tx. build_and_send_tx(..., exit_on_failure=False) returns None both on a revert and on a failed receipt wait for a tx that still mines, so saving only on success can lose the preimage.
  • Tests, following tests/test_aggregator_commit_retry.py:
    • an already-committed LM leaves its cache byte-identical and sends no tx
    • on a failed send, the cache existed at send time and hashes, via _audit_commit_hash, to exactly the commit that was sent

D2. aggregate_t2 stale batch id (dincli/cli/aggregator.py)

  • Rename the inner T1 loop's unpacked variables (:547), e.g. (t1_bid, _, _, _, t1_cid). Then bid from getTier2Batch (:525) is what names the T2 models path, the worker job (aggregator_t2_gi_{GI}_batch_{bid}), the container, and the log lines.
  • Switch the batch_id truthiness checks to is not None, as in aggregate_t1.
  • Test: with tier1BatchCount > 1, the job and container names passed to write_worker_job/run_worker_container carry the T2 batch id.

Deliverables (Part D)

  • D1 skip-if-committed and cache-before-send, with both tests
  • D2 rename and is not None checks, with the naming test
  • pytest -m "not integration" green

Estimate: 1 day.

Part E — #205: test-data dispute drain — PR 4

Starts after PR 2 merges (same contract and test files).

Design (agreed, PR #209 Decision 3: #205 options 1 + 2)

  • Only the model owner's reveal can resolve.
    • resolveTestDataDispute becomes owner-only.
    • A reveal that matches the stored commitment clears the dispute, and the bond is forfeited as today.
    • A reveal that doesn't match upholds it.
  • Silence loses. If the owner doesn't reveal within disputeWindowBlocks, closeExpiredDispute (:1540) upholds the dispute instead of forfeiting the bond: bond back, disputePenaltyBps penalty via _burnAndForward, batch set to pendingReassignment.
  • Only batch auditors can open. openTestDataDispute requires isBatchAuditor(gi, batchId, msg.sender) (a public mapping, DINTaskAuditor.sol:230).
  • Non-zero default bond.
    • disputeBondAmount defaults to 100 * 1e18 DIN, matching DINTaskCoordinator.disputeBond (DINTaskCoordinator.sol:149).
    • It stays settable through the existing setDisputeBondAmount (:1429). Revisit the value alongside #155's tokenomics values.
  • Out of scope: option 3 (committing keccak256(K) on its own) goes to #181/#38. The owner still judges disputes about their own test data, which is the trust assumption #181 tracks; say so in the NatSpec.

Tests

  • a non-owner resolveTestDataDispute reverts
  • an owner reveal with the true K and plaintext hash clears the dispute
  • the owner staying silent past the window upholds it via closeExpiredDispute (bond returned, penalty applied, reassignment pending)
  • a non-batch-auditor openTestDataDispute reverts
  • the deployed default disputeBondAmount == 100 * 1e18
  • rewrite test_resolveDispute_upheld_returnsBondAndPenalises and test_resolveDispute_upheld_blocksFurtherOpen (EncryptedTestData.t.sol:319,341), which currently encode the attack
  • the #205 failure scenario: an outsider with junk K, repeated after each reassignment, can no longer reduce giRewardPool

Deliverables (Part E)

  • Owner-only resolve, silence-upholds expiry, batch-auditor-only open, 100 * 1e18 default bond
  • All tests above. forge test green
  • DINTaskAuditor.md §13 No. 1 caveat replaced with the new flow. Any dincli command calling these functions updated, with pytest green
  • DINTaskAuditor size before and after in the PR. The Part A gate must stay green; Parts C and E both grow this contract

Estimate: 2 days.

Part F — #207: wiki DIN-Representative page — wiki edit, no PR

Rewrite the DIN-Representative wiki page against Documentation/public/roles/dinrep.md, as merged in PR #204. Cover every row of #207's table:

  • dincli dindao … → dincli dinrep …, with the current command names (approve-registration-request, reject-registration-request, approve-manifest-update, reject-manifest-update, …)
  • the command-reference link points to roles/dinrep.md (roles/dindao.md doesn't exist)
  • platform deploy: seven proxies (adding DinTreasury, DinToken, DinFeeRouter, DinEmission), deployed by foundry/script/DeployPlatform.s.sol, then dincli system import-deployments
  • fees: DinCoordinator.withdraw() is gone. Faucet and registry ETH go to DinFeeRouter via sweep-fees
  • there's no set-admin. Ownership is transferOwnership on each contract
  • minting: depositAndMint() and DinCoordinator.mintEmission() (callable only by DinEmission), both under mintCap. The owner sets dinPerEth, mintCap and retireFaucet
  • DIN-DAO governance is deferred to post-mainnet (DD-3)
  • kill switch: the task contracts don't read modelDisabled; disabling only blocks manifest updates

Keep the wiki conventions: blob/develop/... links, no assignee names or week-level dates. Link the wiki revision on #207.

Estimate: 0.5 day.

Out of scope

  • #201 Part B (the slash reason for committed-but-unrevealed), #193, #194 and #180: each needs a mechanism decision or design note first.
  • #181, #178: mainnet-grade.
  • Everything else is listed in the source plan's Deferred table.
  • Don't split either task contract to win bytes. If the Part A gate can't be met with the levers above plus the two extra candidates, stop and raise it in the tracking discussion.