From 8caf75d4e80eecf3c68603f12a3979423f2b0ad4 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sat, 15 Aug 2026 12:48:41 +0000 Subject: [PATCH] docs: NoDeployCandidates docstring names its real raise site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `NoDeployCandidates`'s docstring said it is raised from `allSuites`, "which is the only way anything reads the declaration". Both halves are wrong against the source. It is raised from `checkedCandidateSuites`, which `allSuites` merely calls. And `allSuites` is not the only reader: `testSnapshotMatchesSource` reads `checkedCandidateSuites()` directly, while `testSuitesLiveOnEverySupported Network` and `testEveryFrozenSnapshotIsReleased` read `releasedSuites()` directly. That matters more than an ordinary stale comment because the error's whole justification is a totality argument — "there is no reader that answers from an empty one" — so a maintainer checking that argument was sent to the wrong function and told a stronger property than the code has. The file also contradicted itself: `checkedCandidateSuites`'s own docstring 120 lines below already states the correct arrangement. The rationale for one raise site now points at `checkedCandidateSuites` rather than being spelled a second time here, since a claim spelled twice drifting apart is exactly what produced this defect. What is added is the part stated nowhere else: `releasedSuites` is read directly and is untouched by the guard, because a repo with no release is an ordinary state and it is the CANDIDATE the source anchor needs. Two further sites carried the same stale attribution: `candidateSuites` ("which `allSuites` enforces") and `testNoCandidateReverts`, whose docstring said "all three readers" while asserting on four. Closes https://github.com/rainlanguage/rain.deploy/issues/54 Co-Authored-By: Claude Opus 5 (1M context) --- src/abstract/RainDeploySuitesBase.sol | 16 ++++++++++------ test/src/abstract/RainDeploySuitesBase.t.sol | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/abstract/RainDeploySuitesBase.sol b/src/abstract/RainDeploySuitesBase.sol index 24785f5..a32d067 100644 --- a/src/abstract/RainDeploySuitesBase.sol +++ b/src/abstract/RainDeploySuitesBase.sol @@ -26,9 +26,12 @@ error UnknownDeploymentSuite(string requested, string validSuites); /// to declare. When the candidate was a single struct this was true by /// construction; a list has to say it. /// -/// 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` — see there for why the guard sits at +/// that one read rather than at each reader. +/// +/// `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(); /// One deployable unit: a named snapshot of one contract. @@ -139,9 +142,10 @@ abstract contract RainDeploySuitesBase { /// override this, and a repo inherits exactly one declaration. So the list /// is here rather than left to the consumer to assemble. /// - /// MUST NOT be empty, which `allSuites` enforces. A deploy repo always - /// compiles a current source, so there is always something to anchor to — - /// see `NoDeployCandidates` for why an empty list is worse than it looks. + /// MUST NOT be empty, which `checkedCandidateSuites` enforces. A deploy + /// repo always compiles a current source, so there is always something to + /// anchor to — see `NoDeployCandidates` for why an empty list is worse + /// than it looks. /// @return The candidates. function candidateSuites() internal pure virtual returns (DeployCandidate[] memory); diff --git a/test/src/abstract/RainDeploySuitesBase.t.sol b/test/src/abstract/RainDeploySuitesBase.t.sol index 5a619ff..aef4fdc 100644 --- a/test/src/abstract/RainDeploySuitesBase.t.sol +++ b/test/src/abstract/RainDeploySuitesBase.t.sol @@ -145,7 +145,7 @@ contract RainDeploySuitesBaseTest is Test { /// An empty candidate list reads as a repo with nothing left to declare and /// is a repo whose source anchor — the only check that catches a snapshot /// of the wrong contract — has been handed nothing to run over. It is - /// refused rather than tolerated, and refused on all three readers, because + /// refused rather than tolerated, and refused on all four readers, because /// a reader that answers from an empty declaration is a reader through /// which the whole registry can be empty and green. function testNoCandidateReverts() external {