NoDeployCandidates's docstring names the wrong raise site and claims a totality that is not there, contradicting checkedCandidateSuites's own docstring

[thedavidmeister]

Audit finding ABS-06 — dimension 4, severity LOW. Whole-repo audit pass 1 at 440e90b5.

src/abstract/RainDeploySuitesBase.sol:29-32

Problem

/// Raised from `allSuites`, which is the only way anything reads the
/// declaration — `suiteNames` and `suiteByName` both go through it — so there
/// is no reader that answers from an empty one.
error NoDeployCandidates();

Both claims are wrong against the source:

1. It is raised from checkedCandidateSuites (line 161), not from allSuites. allSuites only calls it.
2. allSuites is not the only way anything reads the declaration. RainDeployVerifySnapshot.testSnapshotMatchesSource reads checkedCandidateSuites() directly (RainDeployVerifySnapshot.sol:257); RainDeployVerifyChain.testSuitesLiveOnEverySupportedNetwork (RainDeployVerifyChain.sol:134) and RainDeployVerifySnapshot.testEveryFrozenSnapshotIsReleased (RainDeployVerifySnapshot.sol:265) both read releasedSuites() directly, bypassing allSuites and with it the duplicate-key check.

checkedCandidateSuites's own docstring, 120 lines below, states the correct arrangement and directly contradicts this one: "The ONE place NoDeployCandidates is raised, and the only way anything reads the candidates. allSuites goes through it, and so does the source anchor in RainDeployVerifySnapshot."

This matters more than an ordinary stale comment because the error's entire justification is a totality argument — "there is no reader that answers from an empty one" — and a reader coming to check that argument is pointed at the wrong function and told a stronger property than the code has. Per the org convention that comments describe current behaviour only, a rationale that misdescribes its own enforcement point is a defect in the file.

Proposed fix

Restate the paragraph from the code, and make it agree with checkedCandidateSuites's docstring by pointing at it rather than re-arguing it.

src/abstract/RainDeploySuitesBase.sol, replacing lines 29-31:

-/// Raised from `allSuites`, which is the only way anything reads the
-/// declaration — `suiteNames` and `suiteByName` both go through it — so there
-/// is no reader that answers from an empty one.
+/// Raised from `checkedCandidateSuites`, which is the only way anything reads
+/// the candidates: `allSuites` goes through it, and so does the source anchor
+/// in `RainDeployVerifySnapshot`, which is the reader that matters — a loop
+/// over an empty candidate list passes. One raise site rather than a guard at
+/// each reader, because two spellings of one rule is how the reader that got
+/// the second spelling wrong becomes the one that silently stops asserting.
+///
+/// `releasedSuites` is read directly by the chain group and by the frozen
+/// record check, and is untouched by this: a repo with no release is an
+/// ordinary state, and it is the CANDIDATE that the source anchor needs.
 error NoDeployCandidates();

---

Verification — this finding survived an adversarial refutation pass

Survives. Every factual claim checks out against the source at 440e90b.

(1) Raise site: revert NoDeployCandidates() is at RainDeploySuitesBase.sol:161 inside checkedCandidateSuites (158-164). allSuites (175) merely calls it at 177. The docstring at 29-31 saying "Raised from allSuites" is contradicted verbatim by line 150 in the same file: "The ONE place NoDeployCandidates is raised".

(2) Totality claim: "allSuites ... is the only way anything reads the declaration — suiteNames and suiteByName both go through it" is false three separate ways. RainDeployVerifySnapshot.sol:257 (testSnapshotMatchesSource) reads candidates via checkedCandidateSuites() directly, never through allSuites. RainDeployVerifySnapshot.sol:226 (testSnapshotInternallyConsistent) calls allSuites() directly, so suiteNames/suiteByName are not its only readers either. RainDeployVerifyChain.sol:134 and RainDeployVerifySnapshot.sol:265 read releasedSuites() directly.

Refutation attempts that failed: (a) "raised from allSuites" is not defensible as transitive shorthand, because the sibling docstring 120 lines below states the opposite arrangement explicitly, so the file says two different things about one guard; (b) the test suite does not rescue it — test/src/abstract/RainDeploySuitesBase.t.sol:151-171 testNoCandidateReverts asserts the revert on FOUR readers including externalCheckedCandidateSuites, with its own comment naming that "the source of the refusal itself", corroborating the finding; (c) no CLAUDE.md convention covers or excuses it, and the org convention is that comments describe current behaviour only.

Under-scoped but not wrong: the identical stale attribution appears again at line 142 on candidateSuites — "MUST NOT be empty, which allSuites enforces" — so a fix should cover both sites.

Severity LOW is correct: the guard is real, correct and fully tested, so no production value is at risk; the defect is a safety rationale that points a maintainer at the wrong enforcement point and asserts a stronger property than the code has. Not raised to MEDIUM (no runtime consequence), not dropped to INFO (the prose is load-bearing design rationale that self-contradicts within one file).