fix(task-coordinator): back under EIP-170 with a CI size gate (#201 Part A) + onlyCurrentGI on registerDINaggregator (#206)

.github/scripts/contract_size_gate.py

@@ -0,0 +1,111 @@
+#!/usr/bin/env python3
+"""Enforce an EIP-170 / EIP-3860 size budget on every deployable foundry/src contract.
+
+Run from foundry/ after `forge build` (issue #201 Part A):
+
+    python ../.github/scripts/contract_size_gate.py [--out out]
+
+Reads every foundry/out/<File>.sol/<Contract>.json artifact directly, rather than
+`forge build --sizes`, whose table leaves DINTaskAuditor out. An artifact is
+checked when its metadata compilationTarget is under src/ and its
+deployedBytecode is non-empty (interfaces and abstract contracts have none;
+test and script contracts aren't under src/). Linked libraries would show
+unresolved `__$...$__` placeholders; none exist today, and the gate counts them
+at their 20-byte linked size.
+
+Fails when any runtime margin under EIP-170 is below FAIL_MARGIN, or any initcode
+exceeds EIP-3860. Warns, without failing, when a runtime margin is below
+WARN_MARGIN, so a contract approaching the budget shows up in every PR's log.
+Fails closed if no artifacts are found.
+"""
+from __future__ import annotations
+
+import argparse
+import json
+import re
+import sys
+from pathlib import Path
+
+EIP170 = 24_576          # max runtime bytecode (bytes)
+EIP3860 = 49_152         # max initcode (bytes)
+FAIL_MARGIN = 1_024      # required runtime headroom under EIP170 (issue #201, PR #209 Decision 1)
+WARN_MARGIN = 2_048      # headroom below which CI warns
+
+PLACEHOLDER = re.compile(r"__\$[0-9a-fA-F]{34}\$__")
+
+
+def code_size(obj: str) -> int:
+    hexstr = obj[2:] if obj.startswith("0x") else obj
+    # Each unlinked-library placeholder is 40 hex chars, the size of the address it becomes.
+    hexstr = PLACEHOLDER.sub("0" * 40, hexstr)
+    return len(hexstr) // 2
+
+
+def compilation_target(artifact: dict) -> str | None:
+    metadata = artifact.get("metadata") or {}
+    if isinstance(metadata, str):
+        try:
+            metadata = json.loads(metadata)
+        except json.JSONDecodeError:
+            return None
+    target = (metadata.get("settings") or {}).get("compilationTarget") or {}
+    return next(iter(target), None)
+
+
+def collect(out_dir: Path) -> list[tuple[str, str, int, int]]:
+    rows = []
+    for path in sorted(out_dir.glob("*.sol/*.json")):
+        try:
+            artifact = json.loads(path.read_text())
+        except (OSError, json.JSONDecodeError) as exc:
+            print(f"::error::cannot read {path}: {exc}")
+            sys.exit(1)
+        source = compilation_target(artifact)
+        if not source or not source.startswith("src/"):
+            continue
+        runtime = code_size((artifact.get("deployedBytecode") or {}).get("object", ""))
+        if runtime == 0:
+            continue
+        initcode = code_size((artifact.get("bytecode") or {}).get("object", ""))
+        rows.append((path.stem, source, runtime, initcode))
+    return rows
+
+
+def main() -> int:
+    parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
+    parser.add_argument("--out", default="out", help="forge artifact directory (default: out)")
+    args = parser.parse_args()
+
+    out_dir = Path(args.out)
+    rows = collect(out_dir)
+    if not rows:
+        print(f"::error::no deployable src/ artifacts found under {out_dir}/ -- run `forge build` first")
+        return 1
+
+    print(f"EIP-170 runtime limit {EIP170} B (fail below {FAIL_MARGIN} B margin, warn below {WARN_MARGIN} B); "
+          f"EIP-3860 initcode limit {EIP3860} B")
+    print(f"{'contract':<28} {'runtime':>8} {'margin':>8} {'initcode':>9}  status")
+    failed = False
+    for name, source, runtime, initcode in sorted(rows, key=lambda r: EIP170 - r[2]):
+        margin = EIP170 - runtime
+        status = "ok"
+        if margin < FAIL_MARGIN:
+            status = "FAIL"
+            failed = True
+            print(f"::error file=foundry/{source}::{name} runtime {runtime} B leaves {margin} B under EIP-170; "
+                  f"the budget requires >= {FAIL_MARGIN} B")
+        elif margin < WARN_MARGIN:
+            status = "warn"
+            print(f"::warning file=foundry/{source}::{name} runtime {runtime} B leaves only {margin} B under "
+                  f"EIP-170 (warn below {WARN_MARGIN} B)")
+        if initcode > EIP3860:
+            status = "FAIL"
+            failed = True
+            print(f"::error file=foundry/{source}::{name} initcode {initcode} B exceeds EIP-3860 ({EIP3860} B)")
+        print(f"{name:<28} {runtime:>8} {margin:>8} {initcode:>9}  {status}")
+
+    return 1 if failed else 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

.github/workflows/ci.yml

@@ -61,6 +61,13 @@ jobs:
         working-directory: foundry
         run: forge build
 
+      # EIP-170/EIP-3860 budget for every foundry/src contract: fails below a
+      # 1,024 B runtime margin, warns below 2,048 B. Reads artifacts directly
+      # because `forge build --sizes` omits DINTaskAuditor. Issue #201 Part A.
+      - name: contract size gate
+        working-directory: foundry
+        run: python3 ../.github/scripts/contract_size_gate.py
+
       - name: forge test
         working-directory: foundry
         run: forge test

Developer/CONTRIBUTING.md

@@ -47,6 +47,11 @@ For issue-specific contributor packets, review questions, and curated reading li
   dincli system dump-abi --artifact foundry/out/<Contract>.sol/<Contract>.json --output dincli/abis --official
   ```
   `dincli/abis/*.json` is the only ABI dincli has for the platform contracts (`DinCoordinator`, `DinToken`, `DinValidatorStake`, `DINModelRegistry`, `DinFeeRouter`), and the fallback for `DINTaskCoordinator`/`DINTaskAuditor` when a model's manifest doesn't supply custom `task_contracts` artifacts. A stale bundle means dincli calls functions that no longer exist (`AttributeError`) or decodes events/structs against the wrong layout. No network or wallet needed to run it. See [issue #177](https://github.com/InfiniteZeroFoundation/DevNet/issues/177) for what drifting looks like when this is skipped.
+- **Contract size budget.** Every deployable `foundry/src` contract must keep at least **1,024 B** of runtime headroom under EIP-170's 24,576 B. Initcode must stay under EIP-3860's 49,152 B. CI enforces both with `.github/scripts/contract_size_gate.py`, and warns when a contract's margin drops below **2,048 B**. Both thresholds are constants at the top of the script (issue #201). Check locally after `forge build`:
+  ```
+  cd foundry && python3 ../.github/scripts/contract_size_gate.py
+  ```
+  `foundry/anvil.sh` lifts the limit on the local chain, so a local deploy is not a size check. A PR that pushes a contract under the 1,024 B fail line needs a size reduction in the same PR. Don't split a contract without raising it with a maintainer first.
 
 ## Code Standards
 

Documentation/technical/contracts/DINTaskCoordinator.md

@@ -55,10 +55,12 @@ Auditor registration, client submissions, scoring and rewards live in the paired
 |----------|-------------|
 | `dinAggregators[gi]`, `isDINAggregator[gi][addr]` | Registration list and flag |
 | `registrationSlotsReleased[gi]` | Guard for `releaseGIRegistrationSlots` |
-| `tier1Batches[gi]`, `tier2Batches[gi]` | Batch arrays (`Tier1Batch` / `Tier2Batch`); T2 always has at most one batch, id 0 |
+| `tier1Batches[gi]`, `tier2Batches[gi]` | Batch arrays (`Tier1Batch` / `Tier2Batch`); T2 always has at most one batch, id 0. Internal: read through `getTier1Batch` / `getTier2Batch` |
 | `isTier1Aggregator`, `isTier2Aggregator` | Batch assignment flags (not public) |
 | `t1CommitHash`, `t1Committed` (and `t2*`) | Commit hash and commit flag per `[gi][batchId][aggregator]` |
 | `t1SubmissionCID`, `t1Submitted`, `t1Votes` (and `t2*`) | Revealed CID, revealed flag and per-CID vote count. Written only by the reveal functions; `t*Submitted` stays `false` for an aggregator who committed but never revealed |
+
+The batch arrays and the ten `t1*`/`t2*` maps are `internal`, to keep the contract under EIP-170 (issue #201 Part A). Read per-aggregator state through `getAggregatorSubmission(gi, tier, batchId, aggregator)`, which returns `(committed, commitHash, submitted, cid, votes)`. `votes` is the vote count of that aggregator's own revealed CID, because the vote maps are keyed by CID. It is 0 before the reveal. A vote count for an arbitrary CID is no longer readable from outside the contract.
 | `tier1FinalizedAt`, `tier2FinalizedAt` | Finalization timestamps (dispute window start) |
 | `tier2Score[gi]` | Owner-recorded quality score for the T2 model |
 | `aggregatorWeight[gi][addr]`, `totalAggregatorWeight[gi]` | Reward weights (one unit per aggregator per finalized batch) |
@@ -153,7 +155,7 @@ The three reveal-opening calls (`startLMsubmissionsEvaluationReveal`, `startT1Ag
 Requires `_GI == GI + 1` and a funded reward pool: `dinTaskAuditorContract.giRewardPool(_GI) > 0`, otherwise `TC_GIRewardPoolNotFunded`. The two-argument overload also calls `updatePassScore`. Then `GI++`.
 
 ### 6.2 `registerDINaggregator`
-In `DINaggregatorsRegistrationStarted` only. There is no `onlyCurrentGI` modifier: the checks and the registration list use the `_GI` the caller passes. Checks in order: `isValidatorActive` (`TC_AggregatorNotActive`), not already registered, fewer than 300 registered (`TC_RegistrationCapReached`), stake ≥ `getModelStakeMin(modelId)` when non-zero (`TC_StakeBelowModelFloor`), and — when `maxConcurrentRegistrationsPerStakeUnit > 0` — `activeRegistrationCount < (stake / minStake) × cap` (`TC_ConcurrentRegistrationCapReached`). Then records the aggregator, calls `incrementActiveRegistration`, emits `DINValidatorRegistered`.
+In `DINaggregatorsRegistrationStarted` only, and `onlyCurrentGI`: `_GI` must be the current GI (`TC_WrongGI`), as for `DINTaskAuditor.registerDINAuditor` (issue #206). Checks in order: `isValidatorActive` (`TC_AggregatorNotActive`), not already registered, fewer than 300 registered (`TC_RegistrationCapReached`), stake ≥ `getModelStakeMin(modelId)` when non-zero (`TC_StakeBelowModelFloor`), and — when `maxConcurrentRegistrationsPerStakeUnit > 0` — `activeRegistrationCount < (stake / minStake) × cap` (`TC_ConcurrentRegistrationCapReached`). Then records the aggregator, calls `incrementActiveRegistration`, emits `DINValidatorRegistered`.
 
 ### 6.3 Batch-Assignment Seed Lock
 
@@ -268,17 +270,17 @@ DINTaskAuditor → DINTaskCoordinator: GI, GIstate, aggregatorWeight (at claim t
 
 Earlier findings from the [foundry/src security review](../audits/foundry-src-security-review.md) should be read alongside these. Several are now fixed: unbounded registration (capped at 300), zero-CID collision (`TC_ZeroCID`), missing quorum (`TC_InsufficientSubmissions`), grindable shuffle (H-2, §6.3) and copy-the-leader aggregation (M-1, §6.5).
 
-- **No. 1 — Runtime bytecode is over the EIP-170 limit.** With commit-reveal aggregation merged, this contract compiles to 24,585 bytes, 9 over the 24,576-byte limit, so it cannot be deployed to Optimism Sepolia or any other real chain from `develop`. `foundry/anvil.sh` starts anvil with `--code-size-limit 4294967295`, and neither `forge test` nor CI checks contract size, so the local devnet and a green test run do not show the problem. Tracked in issue #201 (Part A), which also sets a size budget for every contract.
+- **No. 1 — Little room under EIP-170.** Issue #201 Part A brought this contract back under the 24,576-byte limit. It folded the T1/T2 slash loops into `_slashBatch`, and replaced the public batch and per-aggregator getters with `getAggregatorSubmission`. CI now runs `.github/scripts/contract_size_gate.py`. The gate fails any `foundry/src` contract with less than 1,024 bytes of margin, and warns below 2,048 bytes. This contract sits in the warning band, so the next feature added here may need another size reduction first. `foundry/anvil.sh` still lifts the limit locally, so a local deploy is not a size check; the CI gate is.
 - **No. 2 — Selective non-reveal is cheaper than being wrong.** Reveals land one at a time. A committed aggregator who sees that peers' CIDs differ from their own can withhold the reveal and take the S2 liveness slash (30% of `minStake` by default) instead of the full-`minStake` bad-consensus slash. The same trade-off exists for auditors. Whether committed-but-unrevealed gets its own reason code and fraction is open in issue #201 (Part B).
 - **No. 3 — `expireDispute` can slash an idle fresh subgroup that had no on-chain way to act.** The fresh subgroup cannot submit a recomputation to this contract, so the only thing preventing the timeout slash is the owner calling `settleRecomputation` in time. If the owner does nothing, anyone can slash three uninvolved validators a full `minStake` each.
 - **No. 4 — `modelId` is fixed at construction, but assigned at registry approval.** The registry assigns the ID only when it approves the request, which requires this contract to already exist. The deployer must predict the ID. A wrong guess silently applies another model's stake floor (or none).
 - **No. 5 — Plurality with three aggregators.** Two colluding aggregators in a batch win the vote; nothing verifies the aggregated model itself. The dispute path (§7) is the only recourse, and the owner adjudicates it.
 - **No. 6 — No recovery from a stalled GI.** If a batch never reaches the reveal quorum, `finalizeT1Aggregation` / `finalizeT2Aggregation` keep reverting and there is no owner path to skip the batch or abort the GI.
-- **No. 7 — Stale NatSpec:** `slashAggregators` says the slash is always `minStake()` (no-reveal is now the S2 fraction). Comments around the dispute scaffold still say `DinTreasury` "doesn't exist on develop yet" (forfeitures are already forwarded). Several parameters are described as "DAO-settable"; they are `onlyOwner`, i.e. set by the model owner.
+- **No. 7 — Stale NatSpec:** comments around the dispute scaffold still say `DinTreasury` "doesn't exist on develop yet" (forfeitures are already forwarded). Several parameters are described as "DAO-settable"; they are `onlyOwner`, i.e. set by the model owner.
 - **No. 8 — Leftovers:** `networkFeeFloor` is stored but not enforced. `setTestDataAssignedFlag` gates nothing: evaluation can start without test data being assigned. `releaseGIRegistrationSlots` uses string `require` messages, unlike the rest of the contract.
 - **No. 9 — Not upgradeable:** a bug in a model's task contracts requires redeploying them and re-registering the model.
 - **No. 10 — dincli lags this contract:** `dincli model-owner deploy task-coordinator` still calls the older one-argument constructor (no `modelId`). `dincli aggregator aggregate-t2` names its working directory, worker job and container after the last T1 batch id, not the T2 batch id (issue #202, Part 2); the on-chain commit is unaffected.
-- **No. 11 — `registerDINaggregator` doesn't check the GI.** Unlike `DINTaskAuditor.registerDINAuditor`, it has no `onlyCurrentGI`: while any GI's registration window is open, a validator can register for a future GI (§6.2). Up to 300 addresses can fill GI N+1's list during GI N's window, which locks out honest registrants and hands the attacker every T1/T2 batch. Registering for an already-released past GI leaks the caller's own concurrent-registration slot. dincli always passes the current GI. Tracked in issue #206; the fix (add `onlyCurrentGI`) has to fit within the EIP-170 budget from No. 1.
+- **No. 11 — Fixed: `registerDINaggregator` now checks the GI.** It used to have no `onlyCurrentGI`, so during GI N's window a validator could register for GI N+1. Up to 300 addresses could fill that list, which locked out honest registrants and handed the attacker every T1/T2 batch. Registering for a released past GI also leaked the caller's own slot. Fixed by adding `onlyCurrentGI` (issue #206); it cost 9 bytes inside the No. 1 budget.
 
 ---
 
@@ -297,3 +299,5 @@ Earlier findings from the [foundry/src security review](../audits/foundry-src-se
 - `modelId` constructor argument.
 - Locked batch-assignment seeds for auditor and T1/T2 batches (`lockAuditSeed`, `lockAggSeed`; issue #156 H-2, PR #191).
 - Commit-then-reveal T1/T2 aggregation with a sender-bound commit hash (`commitT*Aggregation`, `revealT*Aggregation`, `startT*AggregationReveal`; issue #156 M-1, PR #197). Adds states `T1AggregationRevealStarted` (18) and `T2AggregationRevealStarted` (21), shifting later ordinals.
+- Back under EIP-170 (issue #201 Part A). The `slashAggregators` T1/T2 loops are folded into `_slashBatch`, with unchanged slashes and events. `tier1Batches` / `tier2Batches` and the ten `t1*`/`t2*` per-aggregator maps are now `internal`, and the new `getAggregatorSubmission` view replaces their getters. This is a view-ABI change; dincli and the bundled ABI were updated with it.
+- `registerDINaggregator` gains `onlyCurrentGI`: registering for any GI other than the current one reverts with `TC_WrongGI` (issue #206).

Documentation/technical/testing/dincli-testing-guide.md

@@ -10,7 +10,7 @@ The chain backend and platform deploy script are chosen by
 chain ID 1337 on `http://127.0.0.1:8545`.
 
 > [!WARNING]
-> `foundry/anvil.sh` starts Anvil with `--code-size-limit 4294967295`, so the local chain accepts contracts above the 24,576-byte EIP-170 limit. `DINTaskCoordinator` is currently over that limit (issue #201), so a green local run does not show that a contract can be deployed to a real chain.
+> `foundry/anvil.sh` starts Anvil with `--code-size-limit 4294967295`, so the local chain accepts contracts above the 24,576-byte EIP-170 limit. A green local run therefore does not show that a contract can be deployed to a real chain. CI enforces the limit instead, with `.github/scripts/contract_size_gate.py` (issue #201).
 
 ---