refactor(sra): strip review-process references and line-number anchors

Code comments: drop 'review B1/S1/S3/#7/Bug A' process wrappers per the
comments/content instructions — keep the design rationale (quarter is a
time property, mirror advances only forward, map/aggregate exclusion).
The B1/S1/S3 numbering collided with the docs' own decision registry
(minLot audit bound / QA-system fixes) — removing the code-side
references resolves the ambiguity.

Docs: replace line-number references (SRAQuarter:269, submitShares:547,
admit:324, ...) with stable file::symbol anchors (test names / invariant
names / function names). Line numbers are a fragile anchor — the id
identity rework shifted them +56-69 lines; symbol anchors never drift.

41 insertions / 41 deletions across 7 files, zero logic changes.

Author: LinZexiao

docs/impl/002-id-identity-test-plan.md

@@ -24,8 +24,8 @@
 ## 3. 关键 slot / 常量（测试侧需要）
 
 ```solidity
-// REGISTRY_SLOT：src/lib/SraStorage.sol:69 已有，值 0xb7fd4b054ced95f43476af93bf71636318271f9e64f7661dc52f0fb4c1a54400（namespace 不变）
-// 但新基线 test/ 下无此常量（早期实现 在 SRARegistry.t.sol:20 定义）——新增测试需自行定义：
+// REGISTRY_SLOT：src/lib/SraStorage.sol 已有，值 0xb7fd4b054ced95f43476af93bf71636318271f9e64f7661dc52f0fb4c1a54400（namespace 不变）
+// 但新基线 test/ 下无此常量（早期实现 在 SRARegistry.t.sol 定义）——新增测试需自行定义：
 bytes32 internal constant REGISTRY_SLOT = 0xb7fd4b054ced95f43476af93bf71636318271f9e64f7661dc52f0fb4c1a54400;
 ```
 
@@ -39,7 +39,7 @@ bytes32 internal constant REGISTRY_SLOT = 0xb7fd4b054ced95f43476af93bf7163631827
 
 ## 4. 新增 5 个行为锁定测试（移植早期实现）
 
-helper 全部复用新基线 `SRATestBase.sol`（`_admit/_postAs/_rollTo/_qEnd/_qPostEnd/_qVerifyEnd/_correctVolume/_fpv/_pair/_registerPairsAs`）+ `SRAShares.t.sol` 内部（`_admitAndPost:267`、`_walletShare:280`、`_sumShares:274`）。早期实现 测试几乎原样可移植。
+helper 全部复用新基线 `SRATestBase.sol`（`_admit/_postAs/_rollTo/_qEnd/_qPostEnd/_qVerifyEnd/_correctVolume/_fpv/_pair/_registerPairsAs`）+ `SRAShares.t.sol` 内部（`SRAShares::_admitAndPost`、`SRAShares::_walletShare`、`SRAShares::_sumShares`）。早期实现 测试几乎原样可移植。
 
 ### 4.1 新增测试清单
 

docs/sra-design.md

@@ -519,13 +519,13 @@ submitShares(Q):
 
 | # | Gap | Added tests (file:line) | Verification point |
 |---|-----|-------------------------|--------------------|
-| **G1** | `setPricingParams`/`getPricingParams` untested | `SRAQuarter:269` `test_SetPricingParams_UpdatesParams_GetReturns`<br>`SRAQuarter:283` `..._NonOwner_Reverts`<br>`SRAQuarter:291` `..._InvalidParams_Reverts` (priceBand > 10000) | parameter management: update takes effect / gating / invalid params |
-| **G2** | 64-full + submitShares combination untested | `SRAShares:300` `test_SubmitShares_AtFullCapacity_SixtyFourRecipients` | all 64 post → map has exactly 64 recipients (mock MAX_RECIPIENTS boundary), 64-way even split with 1e18/64 each, Σ exact |
-| **G3** | band exact ±20% boundary untested | *(removed with the PRICE_BAND check, FIPs#1275 — no on-chain band arithmetic remains; the tests `SRAQuarter:245...306` were deleted with the mechanism)* | `_checkPriceBand` boundary-inclusive semantics — obsolete (band machinery deleted) |
+| **G1** | `setPricingParams`/`getPricingParams` untested | `SRAQuarter::test_SetPricingParams_UpdatesParams_GetReturns`<br>`SRAQuarter::test_SetPricingParams_NonOwner_Reverts`<br>`SRAQuarter::test_SetPricingParams_InvalidParams_Reverts` (priceBand > 10000) | parameter management: update takes effect / gating / invalid params |
+| **G2** | 64-full + submitShares combination untested | `SRAShares::test_SubmitShares_AtFullCapacity_SixtyFourRecipients` | all 64 post → map has exactly 64 recipients (mock MAX_RECIPIENTS boundary), 64-way even split with 1e18/64 each, Σ exact |
+| **G3** | band exact ±20% boundary untested | *(removed with the PRICE_BAND check, FIPs#1275 — no on-chain band arithmetic remains; the tests were deleted with the mechanism)* | `_checkPriceBand` boundary-inclusive semantics — obsolete (band machinery deleted) |
 | **G4** | MAX_PRICE_PERIODS exactly 32 untested | *(removed with the pricing-period vector, FIPs#1275 — no periods reach the chain; `test_PostVolume_MaxPricePeriods_ExactlyAccepted` deleted)* | on-chain price-period length cap — obsolete |
-| **G5** | multi-quarter share isolation untested | `SRAShares:322` `test_SubmitShares_MultiQuarter_Isolated` | quarter 0 posts A/B → quarter 1 only C posts → quarter 1 map contains only C (no residue), quarter 0 result unaffected |
-| **G6** | failure-path asymmetry | `SRARegistry:328` `test_Replace_AlreadyAdmittedTarget_Reverts` (replace target already admitted)<br>`SRARegistry:344` `test_ReassignBinding_NotAdmittedTarget_Reverts` (target not admitted)<br>`SRARegistry:363` `test_Remove_NotAdmitted_Reverts` (non-orchestrator)<br>`SRARegistry:375` `test_Remove_FrozenOrch_Succeeds` (frozen orchestrator can be removed; implementation does not block) | governance failure branches: errors thrown at the third permissionless execution of the function body |
-| **G7** | no fuzzing | `SRAShares:400` `test_SubmitShares_Fuzz_SumAlwaysExact(uint256,uint256,uint256)` | 3 random usdValues (bounded < 1e30, aligned with the code-enforced MAX_FPV_USD — S3: sampling domain = enforced input domain, not a test-side shrink) → Σ shares always exactly == 1e18 (largest-remainder core invariant, 256 runs) |
+| **G5** | multi-quarter share isolation untested | `SRAShares::test_SubmitShares_MultiQuarter_Isolated` | quarter 0 posts A/B → quarter 1 only C posts → quarter 1 map contains only C (no residue), quarter 0 result unaffected |
+| **G6** | failure-path asymmetry | `SRARegistry::test_Replace_AlreadyAdmittedTarget_Reverts` (replace target already admitted)<br>`SRARegistry::test_ReassignBinding_NotAdmittedTarget_Reverts` (target not admitted)<br>`SRARegistry::test_Remove_NotAdmitted_Reverts` (non-orchestrator)<br>`SRARegistry::test_Remove_FrozenOrch_Succeeds` (frozen orchestrator can be removed; implementation does not block) | governance failure branches: errors thrown at the third permissionless execution of the function body |
+| **G7** | no fuzzing | `SRAShares::test_SubmitShares_Fuzz_SumAlwaysExact(uint256,uint256,uint256)` | 3 random usdValues (bounded < 1e30, aligned with the code-enforced MAX_FPV_USD — S3: sampling domain = enforced input domain, not a test-side shrink) → Σ shares always exactly == 1e18 (largest-remainder core invariant, 256 runs) |
 
 **Implementation issue found**: while writing the G1 tests it was found that the reference updates with each qualifying print (C6 semantics: the last one becomes the new reference) — the "new band applies" test was accordingly changed to directly verify that a value accepted under the old band is rejected after the band change (+20% over-band at band 10%, boundary at band 20%), avoiding reference-update interference with the assertion. (Later superseded by the anchored-reference semantics of deviation-D alignment, §4.3.9.)
 
@@ -696,7 +696,7 @@ Final: SRA deterministic **118/118 Green** (SRAQuarter 44 + SRARegistry 28 + SRA
 
 **S2 — security-claim → code-enforcement map** (§5.1 table): every "Safe"/"Conditionally safe" conclusion now cites the enforcing code point (require / mechanism); a claim without an enforcement reference fails review. Maps all 8 categories (e.g. Integer overflow → the single `MAX_FPV_USD` bound @ postVolume + correctVolume; DoS caps → `MAX_PAIRS(64)` / `MAX_ALLOWLIST(64)` / `MAX_ORCHESTRATORS(64)`).
 
-**S3 — evidence-application-condition annotation**: the fuzz sampling domain `(0,1e30)` is re-annotated as **equal to the code-enforced MAX_FPV_USD** (not a test-side shrink — `SRAShares:400` + `SRAInvariant:255,268`); the largest-remainder algorithm properties are covered by the differential suite (bit-exact fixed cases) + invariant fuzz, with the enforced absolute domain's arithmetic safety independently covered by the §5.5 domain-math bounds (docs §4.3.8 S3 note).
+**S3 — evidence-application-condition annotation**: the fuzz sampling domain `(0,1e30)` is re-annotated as **equal to the code-enforced MAX_FPV_USD** (not a test-side shrink — `SRAShares::test_SubmitShares_Fuzz_SumAlwaysExact` + `SRAInvariant::invariant_NonZeroTotal_ValidShareMap`); the largest-remainder algorithm properties are covered by the differential suite (bit-exact fixed cases) + invariant fuzz, with the enforced absolute domain's arithmetic safety independently covered by the §5.5 domain-math bounds (docs §4.3.8 S3 note).
 
 **S4 — threat model matrix** (§5.13): all 15 external write functions × (malicious orchestrator / compromised owner) → impact → mitigation → sufficiency; every function is closed either by unanimous dual-Safe governance or by code-enforced input bounds + timing gates.
 
@@ -780,9 +780,9 @@ forge coverage --match-contract SRA      # SRA line coverage 100% (branch 67% is
 
 | # | Category | Conclusion | Key basis | Code-enforcement point |
 |---|----------|------------|-----------|------------------------|
-| 1 | Reentrancy | ✅ Safe | no value transfer; the only external call is an fvm precompile with no callback surface | no value transfer (no `payable`/`call`/`transfer` anywhere in `src/ServiceRewardsActor.sol`); the only external call is `FVMRewards.setShares` (fvm precompile, no callback), `submitShares:547` |
-| 2 | Denial of Service (DoS) | ✅ Safe (S13) | all traversals have hard caps (64); replace is an O(1) wallet re-point — no alias chain | `registerPairs` `pairs.length <= MAX_PAIRS(64)` `:262`; `setAdmittedLists` `length <= MAX_ALLOWLIST(64)` `:451`; `admit` `admittedIds.length < MAX_ORCHESTRATORS(64)` `:324`; `postVolume`/`correctVolume` take a single USD value — no period array to traverse (FIPs#1275). Freeze determination is O(1) (stored flag, mirror refactor); the former alias-chain growth point is structurally eliminated (S13) |
-| 3 | Access control | ✅ Safe | governance dual-Safe unanimous + hold; orchestrator self-operations gated; constructor validates Safe proxy | `unanimous`/`unanimousNoHold` modifiers gate every governance method (`admit:324` / `remove:346` / `freeze:369` / `unfreeze:387` / `replace:408` / `reassignBinding:432` / `replaceOwner:443` / `setAdmittedLists:451` / `setPricingParams:480` / `correctVolume:504`); `_veto` requires `msg.sender.isOwner()` (cancelPending:492); constructor `newOwner.isProbablyASafe()` (E2 `:146`) |
+| 1 | Reentrancy | ✅ Safe | no value transfer; the only external call is an fvm precompile with no callback surface | no value transfer (no `payable`/`call`/`transfer` anywhere in `src/ServiceRewardsActor.sol`); the only external call is `FVMRewards.setShares` (fvm precompile, no callback), `submitShares` |
+| 2 | Denial of Service (DoS) | ✅ Safe (S13) | all traversals have hard caps (64); replace is an O(1) wallet re-point — no alias chain | `registerPairs` `pairs.length <= MAX_PAIRS(64)`; `setAdmittedLists` `length <= MAX_ALLOWLIST(64)`; `admit` `admittedIds.length < MAX_ORCHESTRATORS(64)`; `postVolume`/`correctVolume` take a single USD value — no period array to traverse (FIPs#1275). Freeze determination is O(1) (stored flag, mirror refactor); the former alias-chain growth point is structurally eliminated (S13) |
+| 3 | Access control | ✅ Safe | governance dual-Safe unanimous + hold; orchestrator self-operations gated; constructor validates Safe proxy | `unanimous`/`unanimousNoHold` modifiers gate every governance method (`admit` / `remove` / `freeze` / `unfreeze` / `replace` / `reassignBinding` / `replaceOwner` / `setAdmittedLists` / `setPricingParams` / `correctVolume`); `_veto` requires `msg.sender.isOwner()` (`cancelPending`); constructor `newOwner.isProbablyASafe()` (E2) |
 | 4 | Integer overflow | ✅ Safe | 0.8.x checked arithmetic fully on; **input-domain bound enforced at the entries** (single `MAX_FPV_USD=1e30`, audit V3 fix); the enforced absolute domain's arithmetic safety is covered by the §5.5 domain-math bounds (S3: proof premise = code-enforced domain) | the single `MAX_FPV_USD` bound at **both** input entries — `postVolume` and `correctVolume`; `_computeShares` chain: per-orch usd × 1e18 ≤ 1e48, total ≤ 6.4e31 ≪ 2^256 (§5.5); checked arithmetic (0.8.36 default) |
 | 5 | Encoding and boundaries (ABI/CBOR) | ⚠️ Conditionally safe | input side protected by the ABI decoder; output side bounded CBOR; wire contract pending f02 implementation check | input side: Solidity ABI decoder (compile-time, rejects malformed calldata); output side: bounded CBOR in f02 mock (`test/mocks/FVMRewardActor.sol`); wire contract vs real f02 implementation is a protocol-layer premise (no contract-layer code can enforce it) |
 | 6 | Precision issues | ✅ Safe | floor + largest-remainder Σ==1e18; conservation/monotonicity/floor bound covered by the differential suite (bit-exact) + invariant fuzz (SumShares) — the Halmos symbolic checks were removed with the FixedU18 adoption (§5.10) | `_computeShares` largest-remainder method `:683` (Σ shares == SHARE_TOTAL exactly, remainder descending + residue top-ups); shares depend only on USD ratios — no rate arithmetic on chain (FIPs#1275) |

src/ServiceRewardsActor.sol

@@ -169,7 +169,7 @@ contract ServiceRewardsActor is UnanimousGovernance {
         // deployment-time parameter validation, aligned with setPricingParams
         require(priceBand <= BASIS_POINTS, InvalidParameter());
         require(epochsPerQuarter > 0 && postPeriod > 0 && verificationWindow > 0, InvalidParameter());
-        // Review B1: the mirror advances only forward, so the windows must not overlap — a quarter's
+        // The mirror advances only forward, so the windows must not overlap — a quarter's
         // verification window closing after the next quarter has begun would let a governance
         // CorrectVolume target an already-advanced quarter, rewinding activeQ (uint256 intermediate
         // guards the addition against overflow).
@@ -237,7 +237,7 @@ contract ServiceRewardsActor is UnanimousGovernance {
     ///      lastSubmittedQ is a q+1 encoding (0 = none), so "awaiting" ⟺ lastSubmittedQ != latest + 1.
     function _pendingSharesQuarter() internal view returns (bool hasPending, uint64 q) {
         SraStorage.SraStorageQuarter storage qt = _quarter();
-        // The latest bound quarter is a *time* property (review S3 root cause): derive it from
+        // The latest bound quarter is a *time* property: derive it from
         // the clock via _quarterOf, not from the activeQ cache — the cache advances only on
         // writes, so a gap quarter (bound but unwritten) would be missed (activeQ still the
         // previous quarter) and removal would wrongly pass. nowQ > 0 guard mirrors the genesis
@@ -255,7 +255,7 @@ contract ServiceRewardsActor is UnanimousGovernance {
         return (false, 0);
     }
 
-    /// @dev Quarter containing `nowE`, derived from the clock alone (review S3 root cause):
+    /// @dev Quarter containing `nowE`, derived from the clock alone:
     ///      E(q) = ACTIVATION_EPOCH + q * EPOCHS_PER_QUARTER, so the time quarter is a pure
     ///      function of the epoch. Unlike the activeQ mirror cache (which advances only on
     ///      writes), this never lags: a gap quarter with no volume is still a *time* quarter.
@@ -281,14 +281,14 @@ contract ServiceRewardsActor is UnanimousGovernance {
     ///      (backing up and clearing a later quarter's contributions — possible when the windows
     ///      overlap, hence also forbidden at the constructor). q > activeQ (skipping one or more
     ///      quarters with no writes) is allowed: a quarter with no volume is necessarily unwritten
-    ///      (postVolume rejects zero, review S3), and _advanceMirror jumps in one step, keeping
+    ///      (postVolume rejects zero), and _advanceMirror jumps in one step, keeping
     ///      prevFpv = activeQ-1's data (0 for a gap quarter). The window checks bound q above
     ///      (can't write the far future); this guard only rejects rewinds.
     function _assertMirrorWindow(SraStorage.SraStorageQuarter storage qt, uint64 q) internal view {
         require(q >= qt.activeQ, InvalidParameter());
     }
 
-    /// @dev Mirror advance (review S3): the first write of a new quarter (postVolume or correctVolume
+    /// @dev Mirror advance: the first write of a new quarter (postVolume or correctVolume
     ///      with q != activeQ) backs the previous active-quarter contributions up into the previous-
     ///      quarter mirror — exclusion-fixed (frozenAtPostEnd ? 0 : fpv), because the freeze state
     ///      of the previous quarter's E+POST is no longer derivable once the quarter has advanced —
@@ -357,8 +357,8 @@ contract ServiceRewardsActor is UnanimousGovernance {
 
         SraStorage.SraStorageQuarter storage qt = _quarter();
 
-        // Time-correct the mirror cache first (a gap quarter advances on the clock, not on writes
-        // — review S3 root cause), then validate the write target against the corrected cache.
+        // Time-correct the mirror cache first (a gap quarter advances on the clock, not on
+        // writes), then validate the write target against the corrected cache.
         _syncMirror(qt);
         _assertMirrorWindow(qt, q);
         if (qt.activeQ != q) _advanceMirror(qt, q);
@@ -419,7 +419,7 @@ contract ServiceRewardsActor is UnanimousGovernance {
         // the aggregate is a binding snapshot (the read view exposes the bound values directly) and
         // a later removal must not rewrite it. The boundary is binding (not E+POST — freeze's
         // boundary): unlike freeze, removal drops the orchestrator from the admitted list, so the
-        // map and the aggregate must exclude it together for every pre-binding removal (review S1).
+        // map and the aggregate must exclude it together for every pre-binding removal.
         uint64 q = _quarter().activeQ;
         if (!_afterBinding(q) && !o.frozenAtPostEnd && FixedU18.unwrap(o.fpv) > 0) {
             _quarter().totalUsd[q] = _quarter().totalUsd[q] - o.fpv;
@@ -576,8 +576,8 @@ contract ServiceRewardsActor is UnanimousGovernance {
 
         SraStorage.SraStorageQuarter storage qt = _quarter();
 
-        // Time-correct the mirror cache first (gap quarters advance on the clock — review S3
-        // root cause), then validate the write target against the corrected cache. correctVolume
+        // Time-correct the mirror cache first (gap quarters advance on the clock), then
+        // validate the write target against the corrected cache. correctVolume
         // can be the first writer of a quarter (supplying recomputed figures for a quarter nobody
         // posted); the advance backs the previous quarter's data up into prevFpv.
         _syncMirror(qt);

src/lib/SraTypes.sol

@@ -18,7 +18,7 @@ struct Pair {
 /// @notice Quarterly FPV: a single USD-denominated total (FIP-0118 §2.3, FIPs#1275: FIL→USD conversion moved
 ///         off-chain, so the SRA no longer stores pricing periods). `usd` is the face-USD stablecoin volume plus
 ///         the off-chain-converted FIL volume; `usd == 0` means not posted
-///         (PostVolume rejects zero, CorrectVolume(0) clears; per review #7).
+///         (PostVolume rejects zero, CorrectVolume(0) clears).
 /// @dev FixedU18: 18-decimal fixed-point USD (1 USD = 1e18 integer). Adopted per the SWA interface
 ///      (IServiceRewardsActor.aggregatedFPV returns FixedU18) so every USD-consuming computation is
 ///      type-safe against integer/fixed-point mixing (1 vs 1e18 magnitude errors). MAX_FPV_USD(1e30)