docs(backlog,audit): link BL-27..31 to new issues, mark stale findings fixed - #225
Abidoyesimze wants to merge 1 commit into
Conversation
…s fixed BL-27..31 said "no issue yet" -- filed InfiniteZeroFoundation#221 (BL-29, DinFeeRouter fund lock), InfiniteZeroFoundation#222 (BL-30, stalled-GI no recovery), InfiniteZeroFoundation#223 (BL-27+BL-28, dincli model-owner deploy/release-slots gaps), InfiniteZeroFoundation#224 (BL-31 plus the audit doc's M-2/L-4, bundled as "needs a product decision" findings). Added BL-34/BL-35 for M-2/L-4 since they predated the backlog's BL-numbering and had no row of their own. Also closed the loop on the 2026-07 security review (foundry-src-security-review.md): M-3, M-4, L-1, L-5, L-7 are already fixed in current develop (GI-state gate in slashAuditors, a real jail() mechanism, correct stake() CEI order, withdrawFees removed entirely, giRewardPool replacing totalDepositedRewards) but the doc was never updated to say so -- re-verified each against develop @ 740a613 before marking. M-2 and L-4 re-confirmed still open and now point at InfiniteZeroFoundation#224. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Reviewed against Claimed: BL-27..31 had no issue; four new issues now track them: No. 221 (BL-29), No. 222 (BL-30), No. 223 (BL-27 + BL-28), No. 224 (BL-31, plus M-2 / L-4 as BL-34 / BL-35). Verified — exact match. All four issues exist, are open, were filed by the PR author, and their titles match the rows they're linked from:
BL-34 / BL-35 are new rows; neither number existed on Claimed: M-3, M-4, L-1, L-5 and L-7 in the July security review are already fixed on Verified, finding by finding, against
Doc checks: Not independently re-verified: the issue bodies' own analysis (for example "95% of every routed fee is stuck"). The PR only links the issues, and the BL rows' text is unchanged apart from the tracking cell. Every structural claim held up: the four issues exist and match their rows, and M-2, M-3, M-4, L-1 and L-4 are exactly as stated. Two wording fixes before merge:
With those, it's good to merge. Optional nit: M-4's note says " |
Files changed (2) — as of
|
| Field | Value |
|---|---|
| Change | Modified |
| Lines | +7/-5 |
| Diff (what exactly is in this PR) | BL-27..31: the status cell changes from "no issue yet" to "tracked in" No. 223 (BL-27, BL-28), No. 221 (BL-29), No. 222 (BL-30) and No. 224 (BL-31). The rest of each row is unchanged. New BL-34 (M-2, the disableModel kill-switch doesn't reach the task contracts) and BL-35 (L-4, rejectModel doesn't refund the fee), both tracked in No. 224. |
| Functionality — how & why | How: status/tracking cells only, plus two new rows in the existing format. Why: the backlog said "no issue yet" for five confirmed findings that now have issues. M-2 and L-4 predate BL numbering and had no row. Without the links, nobody can tell from the backlog that BL-27..31 are tracked. All four issues exist, are open, and their titles match the rows (see the verification comment). |
Diff vs current develop HEAD |
None — untouched by develop since merge-base |
| Recommended merge proposal | Merge as-is. Optional: BL-34's "only two doc comments in the auditor": the auditor has four comments naming DINModelRegistry (:31, :393, :405, :434) and none reading modelDisabled. The point stands; only the count is off. |
| Actual merge proposal | Soon |
| Pending proposal | None |
| Local merge conflict | No |
| GitHub merge conflict | No |
Documentation/technical/audits/foundry-src-security-review.md
| Field | Value |
|---|---|
| Change | Modified |
| Lines | +10/-4 |
| Diff (what exactly is in this PR) | Status notes: M-2 "still open, No. 224 / BL-34"; M-3 "Fixed — 216527c"; M-4 "Fixed — issue No. 37, aee404f"; L-1 "Fixed"; L-4 linked to No. 224 / BL-35; L-5 "Fixed/obsolete — withdrawFees no longer exists… superseded by setFees/sweepFeesToRouter", linked as [#203](…/pull/203); L-7 "Fixed (task_210726_6 §3)". |
| Functionality — how & why | How: status lines in the same format as the existing H-2 / M-1 / L-2 / L-3 / L-6 fix notes. The finding text stays as the historical record. Why: the review is the baseline new security work checks against, so findings left looking open get re-reported. M-2, M-3, M-4, L-1 and L-4 match the source on develop. |
Diff vs current develop HEAD |
None — untouched by develop since merge-base |
| Recommended merge proposal | Merge with two wording fixes. (1) L-5: No. 203 is an issue, not a PR (/pulls/203 → 404), and it only removed dincli's dead withdraw-fees command. The contract change was 06190e8 "scope DINModelRegistry fee path to ETH-only, accumulate-then-sweep"; cite that instead. (2) L-7: mark it "Partly fixed". The dead-code half is fixed (giRewardPool, RewardDeposited emitted at :487). The stray-ETH half still applies: there's no receive()/fallback() or ETH withdrawal path on develop. Optional: M-4's "jail()" is jailValidator(). |
| Actual merge proposal | Soon |
| Pending proposal | L-5 source attribution and L-7 status, as above (author or merge-time fix). |
| Local merge conflict | No |
| GitHub merge conflict | No |
Verification
Full detail is in the verification comment above. In short: issues No. 221–224 exist and match their rows. M-2, M-3, M-4, L-1 and L-4 match the source on develop. L-5's verdict is right but its source link is wrong. L-7 is only partly fixed. Doc links in Documentation and Developer all resolve. git diff --check is clean. GitHub CI is green.
Local vs. GitHub agree: both report a clean merge; no overlap with what develop gained since the merge-base.
2 participants
Summary
Doc-only follow-up from a codebase/issue scan:
Developer/BACK_LOG.md's BL-27 through BL-31 all said "no issue yet" despite being confirmed, real, unaddressed findings (verified each againstdevelop@740a613before filing — not just taking the backlog's word for it). Filed four issues and linked them back:DinFeeRouter'svalidatorPool/storage/publicGoodsshares accrue with no withdrawal path. Confirmed: onlytreasuryis ever transferred out of the five split buckets; with the default 95/5 split, 95% of every routed fee is stuck today.finalizeT1Aggregation/finalizeT2Aggregationrevert the whole call with no per-batch retry or owner override, andendGI/startGIhave no bypass — checked the full contract for an abort/skip/force function, none exists.dincli/cli/modelownerd/) —dincli model-owner deploysends constructor args missingmodelId_against currentfoundry/srcconstructors (confirmed: 1 arg vs. 2 required, 2 args vs. 3 required — will fail ABI encoding), plus no dincli command ever callsreleaseGIRegistrationSlots.disableModel()still doesn't reach the live task contracts (confirmed: zero references in the coordinator, doc-comments-only in the auditor);rejectModel()'s fee non-refund looks intentional on inspection (consistent withapproveModel()also not touchingfeePaid) but was never documented as such.Added BL-34/BL-35 for M-2/L-4 since they predated the backlog's BL-numbering and had no row of their own, for consistency with how everything else in this doc is tracked.
Also: closed the loop on the 2026-07 security review
While re-verifying BL-31 against current code, found that M-3, M-4, L-1, L-5, L-7 in
Documentation/technical/audits/foundry-src-security-review.mdare already fixed ondevelopbut the doc was never updated to say so:slashAuditors) — fixed in216527c, same batch as C-1/C-2/H-1Jaileddead code) — fixed via P3 staking: DAO-settable floor, per-model stake requirements, concurrent-registration cap, withdrawal queue #37 (aee404f), a realjail()/reactivate path now existsstake()CEI order) — current code updatesactiveStakebefore the external call, correct orderwithdrawFeesno zero-address check) — the function no longer exists at all, superseded bysetFees/sweepFeesToRouter(dincli: remove broken non-proxydinrep deploy, fixadd-slashercrash; drop hardhat daoAdmin shims; bring contract docs in line withfoundry/src#203)totalDepositedRewards/RewardDepositeddead code) — reworked into a real per-GIgiRewardPool, event now emittedEach one re-verified by reading the current source, not just trusting a comment or a git log message. M-2 and L-4 are re-confirmed still open and now point at #224.
Test plan
Doc-only change, no code touched. N/A for
forge test/pytest.