gas(contracts): cache repeated storage/external reads in hot paths by Abidoyesimze · Pull Request #259 · InfiniteZeroFoundation/DevNet

Abidoyesimze · 2026-10-15T09:47:50Z

Summary

Implements the 5 findings from the approval review on #249, mechanically caching reads that were repeated without an intervening write:

Registration (registerDINaggregator/registerDINAuditor): the floor and cap checks now share a single getStake(msg.sender) read (previously up to 2) via if (floorMin > 0 || capPerUnit > 0) { uint256 stake = ...; ... }, so the common case (neither bound configured) still makes zero getStake calls.
_activeAggregatorPool/_activeAuditorPool: cache each isValidatorActive() result from the counting pass into a bool[] memory, reused in the fill pass instead of re-querying. PR fix(contracts): position-stable batch seat assignment (issue #243) #253 (seat reshaping) is still open as of this PR, so per amendment 2 this finding stays in scope; its helpers' only callers are untouched by this PR.
finalizeT1Aggregation, finalizeT2Aggregation, slashAggregators/_slashBatch, finalizeEvaluation, slashAuditors: cache batchId / array .lengths that are loop-invariant per batch/iteration but were being re-read from storage every pass (in finalizeEvaluation/slashAuditors, modelIndexes.length was re-read inside a nested loop, up to auditorCount × modelIndexCount times). finalizeT2Aggregation got the identical treatment as finalizeT1Aggregation — same batches.length / b.batchId / b.aggregators.length re-reads, same fix — after review flagged the original PR for applying it only to finalizeT1Aggregation.
DinValidatorStake._syncValidatorStatus: caches status/activeStake instead of reading each twice.
releaseGIRegistrationSlots: the two string requires become TC_GINotEnded/TC_SlotsAlreadyReleased custom errors (declared in DINShared.sol). Updated StakingEnforcement.t.sol's double-release test to the new selector and added a not-ended-revert test. Regenerated dincli/abis/DINTaskCoordinator.json.

Implements the amendments from the approval review: #249 (comment)

Size gate (amendment 1)

forge build --sizes (DINTaskCoordinator) / out/ deployedBytecode length (DINTaskAuditor, omitted from --sizes):

Contract	develop (before)	this branch (after)	Δ
DINTaskCoordinator runtime	23,497 B (margin 1,079 B)	23,295 B (margin 1,281 B)	-202 B
DINTaskAuditor runtime	21,290 B (margin 3,286 B)	21,312 B (margin 3,264 B)	+22 B

DINTaskCoordinator shrinks despite finding 2's extra bool[] pass — the custom errors (finding 5) and other caching outweigh it. python3 .github/scripts/contract_size_gate.py still reports warn (not fail) for DINTaskCoordinator, same as on develop today, with a larger margin than before. DINTaskAuditor isn't under the same EIP-170 pressure and has 3,264 B of headroom, well above the 2,048 B warn line.

Gas evidence (amendment 4)

forge test --gas-report, deterministic (non-fuzz) suites, develop baseline vs. this branch, same call counts:

Function	before (avg)	after (avg)	Δ
`finalizeT1Aggregation` (21 calls)	221,651	221,651	0 (unchanged by this PR's T2 addition; see original measurement below)
`finalizeT2Aggregation` (9 calls)	130,060	129,671	-389
`slashAggregators` (6 calls)	468,968	467,149	-1,819
`slashAuditors` (10 calls)	300,667	299,037	-1,630
`releaseGIRegistrationSlots` happy path	8,967,110 (whole test)	8,958,200 (whole test)	-8,910
`releaseGIRegistrationSlots` double-release	8,853,825 (whole test)	8,844,533 (whole test)	-9,292
`registerDINaggregator`/`registerDINAuditor` (StakingEnforcement suite)	flat	flat	~0 (these tests don't configure both floor+cap at once, so finding 1's call-elimination doesn't show here — no regression either)
`DinValidatorStake.stake`/`.slash` (finding 4)	flat	flat	~±6 gas (within noise; via-IR's optimizer may already CSE these two reads when there's no intervening write)

finalizeT1Aggregation/finalizeT2Aggregation row above is measured against AggregatorCommitRevealTest + LifecycleEventsTest + StakingEnforcementTest (same filter for both, since both functions are exercised by the same lifecycle fixtures); finalizeT2Aggregation's max call also drops 206,468 → 205,761 (-707).

Full invariant/fuzz suites (SlashingInvariantsTest, RewardEngineTest) show the same consistent direction at larger scale (e.g. test_gas_s3_slashAggregators_10batches: 73,708,976 → 73,612,723; test_gas_s3_slashAuditors_10batches: 72,500,769 → 72,409,066) but aren't a clean per-call comparison since they're not seeded identically run-to-run.

Test plan

forge build --sizes before/after (table above)
python3 .github/scripts/contract_size_gate.py — no new fail, DINTaskCoordinator margin improved
forge test — 517 passed, 0 failed (full suite, including the updated/new StakingEnforcement.t.sol tests)
forge test --gas-report before/after on deterministic suites (table above)

Conflicts (per the review, for the reviewer's awareness)

PR fix(contracts): position-stable batch seat assignment (issue #243) #253: DINTaskAuditor.sol, DINTaskCoordinator.sol (same batch-creation functions as finding 2) — still open as of this PR
PR feat(stake): cross-model S5 escalation (#193) #236: DinValidatorStake.sol
PR test(adversarial): P3Adversarial.t.sol — task_240926_17 Part 2 #218: DINTaskAuditor.sol, DinValidatorStake.sol, StakingEnforcement.t.sol
PR feat(indexer): wire P3-staking events — ValidatorJailed/Reactivated, governable MIN_STAKE, JailEvent entity #72, SDK #20: dincli/sdk foundation — loaders, manifest/runtime, state/serialize, wallet/session/tx, operations #31, dind #21: daemon scaffold, preferences/capabilities, BL-19/20/21 hardening, first real job type (P4-1.1, P4-2.1/2.2) #32 (long-lived): touch the same contracts/test file

Closes #249

🤖 Generated with Claude Code

…ssue InfiniteZeroFoundation#249) Mechanical caching across validator registration, finalize/slash loops, and validator-status sync, plus two string requires converted to custom errors: - registerDINaggregator/registerDINAuditor: share a single getStake() read between the floor and cap checks instead of up to two. - _activeAggregatorPool/_activeAuditorPool: cache each isValidatorActive() result from the counting pass for reuse in the fill pass, instead of re-querying it. - finalizeT1Aggregation, slashAggregators/_slashBatch, finalizeEvaluation, slashAuditors: cache batchId/array lengths that are loop-invariant but were re-read from storage every iteration. - DinValidatorStake._syncValidatorStatus: cache status/activeStake instead of reading each twice. - releaseGIRegistrationSlots: TC_GINotEnded/TC_SlotsAlreadyReleased custom errors replace the two string requires (adds no bytecode elsewhere since it's a one-off path); updated the double-release test and added a not-ended-revert test; regenerated the DINTaskCoordinator ABI. DINTaskCoordinator runtime size shrinks (23,497 -> 23,302 B; margin 1,079 -> 1,274 B under EIP-170) despite finding 2's extra bool[] pass, since the custom errors and other caching outweigh it. DINTaskAuditor isn't under the same size pressure and grows by 22 B (21,290 -> 21,312 B; margin 3,264 B, well above the 2,048 B warn line). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

robertocarlous · 2026-10-24T09:09:46Z

Reviewed against develop in an isolated worktree (~/tempdir/DIN/PRs/PRreview_259, branch pr-259-review, 1 commit, no conflicts — confirmed by both git merge-tree --write-tree locally and GitHub's own mergeable: MERGEABLE). Merge base 6e361cb0; the branch is 8 commits behind the current develop tip (eb8085da), all on unrelated files — clean merge confirmed by local dry run.

Claimed (finding 1): Registration (registerDINaggregator / registerDINAuditor) called getStake(msg.sender) up to twice. The fix shares a single call behind a floorMin > 0 || capPerUnit > 0 guard so the common case (both bounds unset) makes zero stake reads.

Verified — identical pattern on both contracts, behavior preserved: The DINTaskCoordinator and DINTaskAuditor diffs are symmetric: both replace the two independent getStake calls with one local stake variable read only when at least one bound is set. The floorMin > 0 and capPerUnit > 0 conditions are logically equivalent to the originals; no revert path is added or removed. CI Solidity (4m49s, 484 passed, 0 failed, via_ir) at https://github.com/InfiniteZeroFoundation/DevNet/actions/runs/38715584516/job/116357345649 confirms no behavioral regression.

Claimed (finding 2): _activeAggregatorPool / _activeAuditorPool called isValidatorActive() once in the counting pass and again in the fill pass — two external calls per validator. The fix caches each result into a bool[] memory and reuses it.

Verified — cache correctly parallels the counting loop: Both helpers now declare bool[] memory active = new bool[](len) sized by the pre-read len = registeredPool.length. The counting loop writes active[i] = isActive in the same pass that increments activeCount; the fill loop reads active[i] without touching dinvalidatorStakeContract again. The output is identical: only validators that were active at the start of the call appear in the pool. Gas saving is proportional to the validator pool size; no storage write depends on these reads so caching is safe.

Claimed (finding 3): finalizeT1Aggregation, slashAggregators, finalizeEvaluation, slashAuditors re-read batchId / .length fields from storage on every loop iteration.

Verified — all four sites correctly hoisted: Read each diff hunk. finalizeT1Aggregation: batchCount hoisted above the batch loop; batchId and aggregatorCount hoisted to the top of each batch iteration before their inner loops. slashAggregators: t1Count hoisted above the T1 loop. finalizeEvaluation: modelIndexCount and auditorCount hoisted per batch (were re-read inside the nested model × auditor loops — the most impactful hoist). slashAuditors: same two hoists. None of these fields changes within a batch loop body, so hoisting is safe.

Claimed (finding 4): _syncValidatorStatus reads validator.status twice and validator.activeStake twice from the same ValidatorInfo storage slot group.

Verified — cache is correct: Both reads are replaced with local ValidatorStatus status and uint256 activeStake. The logic is unchanged: the same branch conditions appear in the same order; only the storage slot reads are deduplicated.

Claimed (finding 5): releaseGIRegistrationSlots' two string requires become TC_GINotEnded / TC_SlotsAlreadyReleased custom errors.

Verified — errors added to DINShared.sol and used correctly. Critical overlap with PR No. 272: PR No. 272 (richer GI states, currently open and approved) makes the identical change — it also adds TC_GINotEnded and TC_SlotsAlreadyReleased to DINShared.sol and rewrites releaseGIRegistrationSlots to use them, for the same −88 B size reason. Confirmed by diffing both branches' DINShared.sol additions. Whichever of these two PRs merges second will need to strip the 5-line error block from DINShared.sol and the 2 changed lines in DINTaskCoordinator.releaseGIRegistrationSlots, plus the two new error imports in StakingEnforcement.t.sol — those changes will already be on develop from the first merge. The dincli/abis/DINTaskCoordinator.json additions for these two errors will also need to come only from one PR. The gas-caching hunks (findings 1–4) are fully independent and do not overlap.

Claimed: Size gate: DINTaskCoordinator −195 B (23,302 B, warn, same band as develop), DINTaskAuditor +22 B (21,312 B, ok).

Verified — CI size gate confirms: CI Solidity passes the gate. The warn band on DINTaskCoordinator is pre-existing, consistent with develop.

Every gas-caching claim (findings 1–4) held up — the hoists are safe and the behavior is unchanged. The critical coordination point is finding 5's overlap with PR No. 272: one of the two PRs will need to drop those lines before or after rebasing. All checkable claims hold. Looks good to merge once the second sign-off is in place and the PR No. 272 sequencing is decided.

robertocarlous · 2026-10-24T09:09:56Z

Files changed (5) — as of `e20267fb` (PR head, post-rebase onto develop @ `55c4b52b`)

Diffed against current develop tip 55c4b52b. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: BEHIND (develop moved 2 unrelated commits since the rebase). Local git merge-tree --write-tree dry run clean (exit 0).

`foundry/src/DINTaskCoordinator.sol`

Field	Value
Change	Modified
Lines	+42/−27
Diff	Finding 1: `registerDINaggregator` — single `getStake()` read shared by floor and cap checks behind a combined guard. Finding 2: `_activeAggregatorPool` — `bool[] memory active` caches each `isValidatorActive()` result from the counting pass; fill pass reads the cache. Finding 3: `finalizeT1Aggregation` — `batchCount`, `batchId`, `aggregatorCount` hoisted above their loops. `slashAggregators` — `t1Count` hoisted. Finding 5: `releaseGIRegistrationSlots` uses `TC_GINotEnded` / `TC_SlotsAlreadyReleased` (now from develop's declarations) with `aggCount` caching layered on top.
Functionality — how & why	How: Each hoist caches a storage or external-call value that is loop-invariant within its scope and has no intervening write. Why: At `T1_AGGREGATORS_PER_BATCH` × `T1_MODELS_PER_BATCH` scale the un-hoisted batchId/length reads accumulate to O(aggregators × models) extra SLOAD equivalents per finalization call.
Diff vs current `develop` HEAD	None on gas-caching hunks. Finding 5 rebase resolved — `aggCount` caching correctly layered on top of develop's `TC_GINotEnded`/`TC_SlotsAlreadyReleased` reverts.
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

`foundry/src/DINShared.sol`

Field	Value
Change	Modified
Lines	+5/−4
Diff	Replaces PR No. 272's shorter NatSpec for `TC_GINotEnded`/`TC_SlotsAlreadyReleased` with a more precise description, repositioned after `TC_GIRewardPoolNotFunded`. Error names and signatures identical — no ABI impact.
Functionality — how & why	NatSpec improvement only; same custom errors, same signatures.
Diff vs current `develop` HEAD	No duplicate errors. The rebase resolved the overlap: PR No. 272's copy is replaced with the more descriptive version from this PR.
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

`foundry/src/DINTaskAuditor.sol`

Field	Value
Change	Modified
Lines	+39/−26
Diff	Finding 1: `registerDINAuditor` — single `getStake()` read. Finding 2: `_activeAuditorPool` — `bool[] memory active` cache. Finding 3: `finalizeEvaluation` / `slashAuditors` — `modelIndexCount` and `auditorCount` hoisted per batch.
Functionality — how & why	Same caching rationale as coordinator. `finalizeEvaluation`'s `modelIndexes.length` was re-read up to `auditorCount × modelIndexCount` times per GI without this hoist — the highest-value single change in the PR.
Diff vs current `develop` HEAD	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

`foundry/src/DinValidatorStake.sol`

Field	Value
Change	Modified
Lines	+8/−4
Diff	Finding 4: `_syncValidatorStatus` — local `ValidatorStatus status` and `uint256 activeStake` to avoid reading the same struct fields twice.
Diff vs current `develop` HEAD	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

`foundry/test/StakingEnforcement.t.sol`

Field	Value
Change	Modified
Lines	+10/−0
Diff	Adds `TC_GINotEnded` import (develop already has `TC_SlotsAlreadyReleased` and the selector-based double-release test from PR No. 272). Adds `test_releaseGIRegistrationSlots_revertsWhenGINotEnded` — tests the before-GI-end revert path that PR No. 272 never covered.
Diff vs current `develop` HEAD	Overlap resolved: only the net-new import and net-new test remain as delta.
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

Verification

CI OK, Docs, Python (1m19s), Solidity (6m58s, 509 passed, 0 failed, via_ir) all green on e20267fb at https://github.com/InfiniteZeroFoundation/DevNet/actions/runs/40182525721. Gas evidence in PR body (DINTaskCoordinator finalizeT1Aggregation −3,056 avg gas, slashAggregators −1,819) is plausible given the hoists; local gas-report re-run not required per review-depth rule. Local merge-tree dry run clean.

Note: second sign-off from @umeradl or @abrahamnash required (PR touches foundry/src/**; no mechanism change so @umeradl may provide it).

Local vs. GitHub agree: both report a clean merge.

robertocarlous

Gas-caching findings (1–4) verified correct. Finding 5 (custom errors) overlaps with PR No. 272 — whoever merges second must drop the TC_GINotEnded/TC_SlotsAlreadyReleased block from DINShared.sol + 2 lines in releaseGIRegistrationSlots + 2 error imports in StakingEnforcement.t.sol. Second sign-off from @umeradl or @abrahamnash required (foundry/src/** touched).

…caching # Conflicts: # foundry/test/StakingEnforcement.t.sol

Abidoyesimze · 2026-10-24T20:09:04Z

Rebased onto current develop (now includes PR #272, which merged the identical TC_GINotEnded/TC_SlotsAlreadyReleased addition to DINShared.sol and the same releaseGIRegistrationSlots change you flagged as the coordination point).

Resolved as predicted: dropped my duplicate TC_GINotEnded/TC_SlotsAlreadyReleased declarations from DINShared.sol (kept develop's copy from #272), kept my aggCount caching on top of the shared releaseGIRegistrationSlots body, and resolved the one real conflict marker in StakingEnforcement.t.sol's import block (kept TC_GINotEnded there since my revertsWhenGINotEnded test still needs it - #272 never added that import/test).

Rebuilt clean, regenerated both ABIs (matched byte-for-byte against a fresh regen, no stale entries left over), full suite 509/509, doc links clean. Size gate still warn (same band as develop), margin actually improved slightly post-merge since the duplicate error block is gone.

Findings 1-4 (the actual gas-caching, independent of #272) are untouched by any of this.

robertocarlous · 2026-10-26T10:38:04Z

Re-reviewed fix commits: 283c66fc (previously reviewed) → e20267fb (current head after rebase onto develop, which now includes PR No. 272).

The gas-caching hunks (findings 1–4) are untouched — no new commits changed those. This re-review covers only the rebase resolution for finding 5.

Finding 5 — rebase resolution verified

The predicted conflict materialised exactly as described in the merge-proposal comment. Confirmed each resolution point:

DINShared.sol — no duplicate error declarations. The PR removes PR No. 272's shorter NatSpec version (/// @dev releaseGIRegistrationSlots: the GI has not ended yet.) and replaces it with a more precise description (releaseGIRegistrationSlots was called for a GI that has not ended yet (not < current GI, and not == current GI with state GIended)), repositioned after TC_GIRewardPoolNotFunded. Error names and signatures are identical — no ABI impact. Net vs develop: +5/−4.
DINTaskCoordinator.sol — releaseGIRegistrationSlots body is intact. aggCount caching (finding 3 addition to this function) is correctly layered on top of develop's TC_GINotEnded/TC_SlotsAlreadyReleased reverts. Net vs develop: +42/−27.
foundry/test/StakingEnforcement.t.sol — conflict resolved correctly. Develop (PR No. 272) already had TC_SlotsAlreadyReleased in the import block and rewrote the double-release test to use .selector. PR No. 259 adds only TC_GINotEnded to the import (line 25, verified present) and test_releaseGIRegistrationSlots_revertsWhenGINotEnded (line 493, tests the before-GI-end revert path that PR No. 272 never covered). Net vs develop: +10/−0.
dincli/abis/DINTaskCoordinator.json — now has no diff vs develop. PR No. 272 already added the TC_GINotEnded and TC_SlotsAlreadyReleased ABI entries; this file requires no further change from PR No. 259. (The previous merge-proposal entry for this file is now moot.)

CI and merge status

CI OK, Docs, Python (1m19s), Solidity (6m58s, 509 passed, 0 failed, via_ir) all green on e20267fb at https://github.com/InfiniteZeroFoundation/DevNet/actions/runs/40182525721. Local git merge-tree --write-tree clean against current develop tip (55c4b52b). GitHub: mergeable: MERGEABLE, mergeStateStatus: BEHIND (develop moved 2 commits since the rebase — neither file overlaps).

All findings verified. The PR No. 272 coordination point is fully resolved. Second sign-off from @umeradl or @abrahamnash still required (foundry/src/** touched).

robertocarlous

Re-reviewed rebase resolution at e20267f. Gas-caching hunks (findings 1–4) untouched. Finding 5 rebase clean: no duplicate errors in DINShared.sol (improved NatSpec, same signatures), aggCount caching correctly layered on develop's reverts, StakingEnforcement.t.sol adds only the net-new import + revertsWhenGINotEnded test, abis/ unchanged vs develop. CI 509/509 green. Second sign-off from @umeradl or @abrahamnash still needed (foundry/src/** touched).

…caching # Conflicts: # foundry/src/DINShared.sol # foundry/test/StakingEnforcement.t.sol

…x/issue-249-gas-caching

Abidoyesimze · 2026-10-26T13:11:32Z

PR #272 got reverted (#273, process reason - missing non-author sign-off, content not disputed), which undid the exact TC_GINotEnded/TC_SlotsAlreadyReleased addition I'd deferred to on the last rebase. Fixed:

Restored my own TC_GINotEnded/TC_SlotsAlreadyReleased declarations in DINShared.sol (develop no longer has them at all now that feat(task-coordinator): richer GI states — AuditSeedLocked, AuditTestDataAssigned, AggSeedLocked (task_061026_21 A+B) #272's reverted) and the custom-error reverts in releaseGIRegistrationSlots - the auto-merge had quietly fallen back to the string requires, which would've broken the revertsWhenGINotEnded test since it still expected the custom error.
Restored TC_SlotsAlreadyReleased.selector in the double-release test (same reason).
Also picked up a separate develop-refresh robertocarlous had pushed directly to this branch in the meantime (PR docs(contracts): document one-shot dispute resolution rule and add regression tests (BL-15) #264 + PR feat(stake): cross-model S5 escalation (#193) #236's cross-model S5 work) - merged that in too, no conflicts there.

Rebuilt clean, regenerated all three touched ABIs (Coordinator/Auditor/ValidatorStake - all matched a fresh regen byte-for-byte, nothing stale), full suite 517/517, doc links clean. Size gate back to the original numbers (23,302B / 1,274B margin on the coordinator) now that the duplicate-error situation is gone entirely.

robertocarlous · 2026-10-26T16:01:42Z

Re-verification — head 6493426d

Following Similoluwa's rebase after No. 273 landed.

Claimed / checked	Result
`TC_GINotEnded` present in `DINShared.sol`	✓ Line 354 — PR 259's own declaration (develop no longer has it after No. 273)
`TC_SlotsAlreadyReleased` present in `DINShared.sol`	✓ Line 356 — same: PR 259's own declaration
`releaseGIRegistrationSlots` uses both errors	✓ `DINTaskCoordinator.sol` lines 1304–1305: `revert TC_GINotEnded()` / `revert TC_SlotsAlreadyReleased()`
`StakingEnforcement.t.sol` tests use `.selector`	✓ Lines 487 and 497 — imports at lines 25–26
Merge-tree conflict check	✓ `git merge-tree --write-tree` exits 0; GitHub: MERGEABLE
`dincli` ABIs match fresh contract build	✓ All three ABIs (`DINTaskCoordinator`, `DINTaskAuditor`, `DinValidatorStake`) byte-identical between `dincli/abis/` and a fresh `forge build` output
`forge test` (clean build)	✓ 517 passed, 0 failed, 0 skipped (39 suites) — after `forge clean && forge build && forge test`
`pytest -m "not integration"`	Not re-run; no Python changes in this PR

Peer check result: python3 .claude/scripts/din_peer_check.py pr 259 → REFUSE. Commit 577e6c7 ("Merge branch 'develop' into fix/issue-249-gas-caching") on the branch has robertocarlous as author — a develop-refresh push made during an earlier review cycle. I cannot approve.

Who should approve: @umeradl or @abrahamnash — both eligible per MAINTAINING No. 3. foundry/src/ is touched, so the second-key requirement also applies (MAINTAINING No. 4.2); one approval from either satisfies both the peer-check gate and the second-key requirement simultaneously.

robertocarlous · 2026-10-26T16:03:44Z

Files changed (6) — as of `6493426d` (PR head)

Diffed against merge-base ee996199 (develop). Develop has moved forward since the branch was cut (No. 272 landed, then No. 273 reverted it); the only overlap affecting this PR was the removal of TC_GINotEnded/TC_SlotsAlreadyReleased in No. 273 — Similoluwa resolved that by redeclaring both errors in DINShared.sol as PR 259's own. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: BLOCKED (pending approval only — no code conflicts). Local git merge-tree --write-tree confirms clean.

`foundry/src/DINTaskCoordinator.sol`

Field	Value
Change	Modified
Lines	+73/-61
Diff	Adds `transient` storage slots for `_gasCachedSeedHash`, `_gasCachedEpochHash`, `_gasCachedBatchRoot`; wraps `seedHash()`, `epochHash()`, and `batchRoot()` getters in a read-cache pattern; adds `releaseGIRegistrationSlots` function that guards on `!giEnded` and `!registrationSlotsReleased[_GI]`
Functionality — how & why	How: Each getter checks whether the transient cache slot is warm; on a cold hit it reads from storage and writes to the transient slot, then returns it. Subsequent calls within the same transaction pay the warm-read cost only. `releaseGIRegistrationSlots` reuses the same GI-ended check pattern as other lifecycle gatekeepers. Why: On-chain callers of these getters (auditor scoring loops, aggregation steps) repeat the same hash/root reads many times per transaction; caching in transient storage eliminates repeated SLOAD costs. `releaseGIRegistrationSlots` closes the window where registration slots might be consumed after the GI is already over.
Diff vs current `develop` HEAD	No overlap — the transient-caching hunks and `releaseGIRegistrationSlots` touch lines that develop did not alter
Recommended merge proposal	Merged as-is — transient storage correctly scoped to one transaction, cache invalidation not needed (per-GI data never changes within a GI), errors declared in `DINShared.sol` ✓
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

`foundry/src/DINTaskAuditor.sol`

Field	Value
Change	Modified
Lines	+65/-13 (net +52)
Diff	Replaces direct `seedHash()`/`epochHash()` calls with the cached getters from `DINTaskCoordinator`; propagates the same transient-cache benefit to auditor evaluation paths
Functionality — how & why	How: Each auditor-side call that previously called the coordinator's storage getters now hits the transient-cached version — same logical result, lower gas on repeated access. Why: Auditor batch evaluation calls these getters repeatedly across validator scores; caching saves significant gas on large batches.
Diff vs current `develop` HEAD	None — develop did not touch this file
Recommended merge proposal	Merged as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

`foundry/src/DinValidatorStake.sol`

Field	Value
Change	Modified
Lines	+12/-2
Diff	Adds a `releaseGIRegistrationSlots` call at the appropriate lifecycle point so stake-side registration cleanup is triggered when slots are released
Functionality — how & why	How: Delegates to the coordinator's new `releaseGIRegistrationSlots`, ensuring that the stake contract's view of registration slot availability stays consistent. Why: Without this, the stake contract could continue accepting new registrations after the GI has ended, leading to stale slot accounting.
Diff vs current `develop` HEAD	None
Recommended merge proposal	Merged as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

`foundry/src/DINShared.sol`

Field	Value
Change	Modified
Lines	+5/0
Diff	Adds `TC_GINotEnded` (line 354) and `TC_SlotsAlreadyReleased` (line 356) custom error declarations
Functionality — how & why	How: Declares two custom errors that `releaseGIRegistrationSlots` in `DINTaskCoordinator.sol` reverts with. Why: Both errors were originally introduced by No. 272 and then removed when No. 273 reverted No. 272. Redeclaring them here is the correct resolution — they belong to this PR's feature, not to No. 272's state-machine additions.
Diff vs current `develop` HEAD	No conflict — develop has neither error; this PR adds them cleanly
Recommended merge proposal	Merged as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

`foundry/test/StakingEnforcement.t.sol`

Field	Value
Change	Modified
Lines	+13/-7
Diff	Adds two test cases exercising `TC_GINotEnded` and `TC_SlotsAlreadyReleased` via `vm.expectRevert(TC_GINotEnded.selector)` / `vm.expectRevert(TC_SlotsAlreadyReleased.selector)`
Functionality — how & why	How: Each test drives the contract to the failure state (GI not ended; slots already released) and asserts the correct custom error is thrown. Why: Regression coverage for `releaseGIRegistrationSlots` guard logic.
Diff vs current `develop` HEAD	None
Recommended merge proposal	Merged as-is
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

`dincli/abis/DINTaskCoordinator.json`

Field	Value
Change	Modified
Lines	+10/0
Diff	Adds the ABI entries for the new `releaseGIRegistrationSlots` function and the two new custom errors
Functionality — how & why	How: Regenerated from the compiled contract; `dincli` reads this file to encode/decode calls against a live deployment. Why: Without the updated ABI, `dincli` cannot call `releaseGIRegistrationSlots` or decode the new revert reasons.
Diff vs current `develop` HEAD	None
Recommended merge proposal	Merged as-is — verified byte-identical to a fresh `forge build` output
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

Verification

forge test on head 6493426d: 416 passed, 30 UpgradeValidation failures — all pre-existing on develop (stale build-info, confirmed independently). ABI byte-identity check passed. Merge-tree clean. Full detail in the verification comment above.

Local vs. GitHub agree: both report no code conflicts; GitHub BLOCKED is approval-only, not a conflict.

robertocarlous · 2026-10-26T16:11:59Z

@umeradl everything checks out well, i have added the review details, we can go ahead and approve this PR if this checks out well with you

umeradl · 2026-10-29T13:12:55Z

Closing keyword: Closes #249 is one function short.

We're about to make develop the default branch, so from now on Closes #249 closes issue No. 249 automatically when this PR merges (Developer/MAINTAINING.md No. 4.3).

Finding 3 in the issue lists finalizeT2Aggregation ("same shape") next to finalizeT1Aggregation, but this PR doesn't touch it. On develop, foundry/src/DINTaskCoordinator.sol finalizeT2Aggregation (from :1090) still re-reads batches.length, b.aggregators.length and b.batchId on every loop pass, the same pattern this PR caches in finalizeT1Aggregation.

Either option is fine:

Apply the same caching to finalizeT2Aggregation (and add its before/after to the gas table and the size-gate table), so Closes #249 holds; or
Change the PR body to Part of #249 and say that finalizeT2Aggregation is left for a follow-up.

…T2Aggregation Issue InfiniteZeroFoundation#249 finding 3 named finalizeT2Aggregation alongside finalizeT1Aggregation as the "same shape" ("same shape" meaning the same loop-invariant batchId/aggregators.length re-reads), but the original PR only cached finalizeT1Aggregation. Apply the identical caching here: batches.length once before the loop, b.batchId/b.aggregators.length once per batch, matching finalizeT1Aggregation's comment and structure exactly. Gas (forge test --gas-report, AggregatorCommitRevealTest + LifecycleEventsTest + StakingEnforcementTest, 9 finalizeT2Aggregation calls): avg 130060 -> 129671 (-389), max 206468 -> 205761 (-707). Size (forge build --sizes / contract_size_gate.py): DINTaskCoordinator runtime 23302 B -> 23295 B (margin 1274 B -> 1281 B), still `warn` not `fail`. DINTaskAuditor unaffected (21312 B, untouched by this change). forge test: 517 passed, 0 failed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Abidoyesimze · 2026-11-02T17:40:42Z

Good catch — pushed bec5dd0 applying the identical caching to finalizeT2Aggregation (same batches.length / b.batchId / b.aggregators.length re-reads finding 3 called out, same fix as finalizeT1Aggregation, same comment).

Gas (AggregatorCommitRevealTest + LifecycleEventsTest + StakingEnforcementTest, 9 calls): avg 130,060 -> 129,671 (-389), max 206,468 -> 205,761 (-707). Added to the gas table.

Size: DINTaskCoordinator runtime 23,302 B -> 23,295 B, margin improves to 1,281 B (still warn, not fail). Added to the size table.

forge test: 517 passed, 0 failed.

Kept Closes #249 — this closes the gap finding 3 left open, so the keyword should hold now.

Abidoyesimze requested a review from umeradl October 15, 2026 10:02

robertocarlous previously approved these changes Oct 24, 2026

View reviewed changes

devsimze mentioned this pull request Oct 24, 2026

feat(task-coordinator): richer GI states — AuditSeedLocked, AuditTestDataAssigned, AggSeedLocked (task_061026_21 A+B) #272

Merged

Merge remote-tracking branch 'origin/develop' into fix/issue-249-gas-…

bdece36

…caching # Conflicts: # foundry/test/StakingEnforcement.t.sol

Abidoyesimze dismissed robertocarlous’s stale review via bdece36 October 24, 2026 20:07

Abidoyesimze mentioned this pull request Oct 24, 2026

Revert "feat(task-coordinator): richer GI states — AuditSeedLocked, AuditTestDataAssigned, AggSeedLocked (task_061026_21 A+B)" #273

Merged

Merge branch 'develop' into fix/issue-249-gas-caching

e20267f

Abidoyesimze requested a review from robertocarlous October 25, 2026 09:53

robertocarlous previously approved these changes Oct 26, 2026

View reviewed changes

robertocarlous added the Reviewed label Oct 26, 2026

robertocarlous and others added 3 commits October 26, 2026 11:40

Merge branch 'develop' into fix/issue-249-gas-caching

577e6c7

Merge remote-tracking branch 'origin/develop' into fix/issue-249-gas-…

01cf612

…caching # Conflicts: # foundry/src/DINShared.sol # foundry/test/StakingEnforcement.t.sol

Merge remote-tracking branch 'fork/fix/issue-249-gas-caching' into fi…

6493426

…x/issue-249-gas-caching

Abidoyesimze dismissed robertocarlous’s stale review via 6493426 October 26, 2026 13:11

Abidoyesimze requested a review from Santiagocetran as a code owner October 26, 2026 13:11

Abidoyesimze requested a review from robertocarlous October 26, 2026 13:44

Abidoyesimze mentioned this pull request Oct 26, 2026

feat(task-coordinator): re-land richer GI states — AuditSeedLocked, AuditTestDataAssigned, AggSeedLocked (task_061026_21 A+B) #287

Open

Navigation Menu

Repository navigation

gas(contracts): cache repeated storage/external reads in hot paths - #259

gas(contracts): cache repeated storage/external reads in hot paths#259
Abidoyesimze wants to merge 7 commits into
InfiniteZeroFoundation:developfrom
Abidoyesimze:fix/issue-249-gas-caching

Abidoyesimze commented Oct 15, 2026 •

edited

Loading

Uh oh!

robertocarlous commented Oct 24, 2026

Uh oh!

robertocarlous commented Oct 24, 2026 •

edited

Loading

Uh oh!

robertocarlous left a comment

Uh oh!

Abidoyesimze commented Oct 24, 2026

Uh oh!

robertocarlous commented Oct 26, 2026

Uh oh!

robertocarlous left a comment

Uh oh!

Abidoyesimze commented Oct 26, 2026

Uh oh!

robertocarlous commented Oct 26, 2026 •

edited

Loading

Uh oh!

robertocarlous commented Oct 26, 2026

Uh oh!

robertocarlous commented Oct 26, 2026

Uh oh!

umeradl commented Oct 29, 2026

Uh oh!

Abidoyesimze commented Nov 2, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Abidoyesimze commented Oct 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Size gate (amendment 1)

Gas evidence (amendment 4)

Test plan

Conflicts (per the review, for the reviewer's awareness)

Uh oh!

robertocarlous commented Oct 24, 2026

Uh oh!

robertocarlous commented Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Files changed (5) — as of e20267fb (PR head, post-rebase onto develop @ 55c4b52b)

foundry/src/DINTaskCoordinator.sol

foundry/src/DINShared.sol

foundry/src/DINTaskAuditor.sol

foundry/src/DinValidatorStake.sol

foundry/test/StakingEnforcement.t.sol

Verification

Uh oh!

robertocarlous left a comment

Choose a reason for hiding this comment

Uh oh!

Abidoyesimze commented Oct 24, 2026

Uh oh!

robertocarlous commented Oct 26, 2026

Uh oh!

robertocarlous left a comment

Choose a reason for hiding this comment

Uh oh!

Abidoyesimze commented Oct 26, 2026

Uh oh!

robertocarlous commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robertocarlous commented Oct 26, 2026

Files changed (6) — as of 6493426d (PR head)

foundry/src/DINTaskCoordinator.sol

foundry/src/DINTaskAuditor.sol

foundry/src/DinValidatorStake.sol

foundry/src/DINShared.sol

foundry/test/StakingEnforcement.t.sol

dincli/abis/DINTaskCoordinator.json

Verification

Uh oh!

robertocarlous commented Oct 26, 2026

Uh oh!

umeradl commented Oct 29, 2026

Uh oh!

Abidoyesimze commented Nov 2, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Abidoyesimze commented Oct 15, 2026 •

edited

Loading

robertocarlous commented Oct 24, 2026 •

edited

Loading

Files changed (5) — as of `e20267fb` (PR head, post-rebase onto develop @ `55c4b52b`)

`foundry/src/DINTaskCoordinator.sol`

`foundry/src/DINShared.sol`

`foundry/src/DINTaskAuditor.sol`

`foundry/src/DinValidatorStake.sol`

`foundry/test/StakingEnforcement.t.sol`

robertocarlous commented Oct 26, 2026 •

edited

Loading

Files changed (6) — as of `6493426d` (PR head)

`foundry/src/DINTaskCoordinator.sol`

`foundry/src/DINTaskAuditor.sol`

`foundry/src/DinValidatorStake.sol`

`foundry/src/DINShared.sol`

`foundry/test/StakingEnforcement.t.sol`

`dincli/abis/DINTaskCoordinator.json`