ConfidencePool — QA Report (Low / Informational / Gas)

Curated low-severity, informational and gas findings for src/ConfidencePool.sol /
src/ConfidencePoolFactory.sol. Each is code-grounded with file:line. None is a High/Medium (those
were assessed separately). Items are grouped by category.

Low

L-01: contributeBonus() does not latch expiryLocked, so a sponsor can still move expiry after real bonus capital is committed

Location: src/ConfidencePool.sol:266-285 (contributeBonus) vs 229-231 (stake latch); setExpiry gate 622-623

stake() latches the one-way expiryLocked flag (229-231: if (!expiryLocked) expiryLocked = true;), permanently freezing expiry (setExpiry reverts ExpiryLocked at 623). contributeBonus() transfers real ERC20 into the pool and increments totalBonus (276-282) but never touches expiryLocked. So while expiry is still mutable (no stake yet), a sponsor can accept bonus donations and then call setExpiry to change the deadline governing how long that donated capital is locked. DESIGN.md 10 frames the latch purely around the first stake / staker reliance and does not carve out bonus contributors, so the asymmetry is undocumented. Reproduced in test/wf/r3_input-validation.t.sol::test_contributeBonus_doesNotLockExpiry (PASS): carol contributes 100e18 bonus, expiryLocked stays false, sponsor pushes expiry out ~10 years, then a 1-token stake latches it.

Impact: A bonus contributor's donated funds can have their effective lockup/resolution deadline changed by the sponsor after the contribution, since only stake() freezes expiry. No theft (bonus is a donation), but the deadline a contributor relied on when donating is not protected the way a staker's is. outcomeFlaggedAt==expiry (569) confirms expiry genuinely feeds the EXPIRED-path bonus weighting, so the impact is concrete.

Recommendation: Latch expiryLocked in contributeBonus() as well (mirror 229-231), or explicitly document in DESIGN.md 10 that bonus contributors are intentionally unprotected against expiry changes before the first stake.

---

L-02: A claim can succeed while paying zero bonus to a dust-sized staker

Location: src/ConfidencePool.sol:719 (share computation via Math.mulDiv), 589/603-605 (claimExpired payout), 391/404 (claimSurvived payout)

_bonusShare floors via Math.mulDiv (719), so a staker whose weighted entitlement is worth less than 1 wei of snapshotTotalBonus receives bonusShare == 0. The claim path does not special-case a zero bonus: it still marks hasClaimed[msg.sender]=true, emits ClaimSurvived/ClaimExpired with bonusShare=0, and pays only principal. Reproduced in test/wf/r3_rounding-dust-ux.t.sol::test_smallStaker_getsZeroBonus (whale 1e6*1e18 + bob 1e18, 100 wei bonus -> bob's share floors to 0, claim succeeds, hasClaimed=true). DESIGN.md discusses dust only at the aggregate/pool level (10), never at the per-staker level.

Impact: A small staker in a whale-dominated pool with a modest bonus can earn literally zero bonus while larger stakers are paid; the fractional entitlement becomes part of the swept remainder. Loss bounded to sub-wei dust, no invariant broken, but a user staking exactly minStake into a whale pool should know their k=2 bonus can round to nothing.

Recommendation: Document the floor behavior for small stakers in DESIGN.md 7 so depositors understand the k=2 bonus can round to zero for dust-relative stakes; optionally expose a view that previews the effective share before committing.

---

L-03: claimExpired() reverts InvalidOutcome for non-triggering SURVIVED stakers, forcing a silent switch to claimSurvived()

Location: src/ConfidencePool.sol:514-516 (guard) and 557-560 (auto-resolve to SURVIVED)

When the registry is terminal-PRODUCTION at the first post-expiry call, claimExpired() auto-resolves the pool to SURVIVED (557-560) and pays only the triggering caller. Every other survived staker who then calls the same claimExpired() entrypoint hits the guard outcome != UNRESOLVED && outcome != EXPIRED (514) and reverts InvalidOutcome (515). The outcome is in fact valid (SURVIVED); the staker must instead call claimSurvived() (gated on outcome==SURVIVED, 382-384). The error name misdescribes the state and there is no on-chain signal telling the staker which claim entrypoint applies. The EXPIRED branch does not have this wrinkle (514 permits outcome==EXPIRED), so the asymmetry is between the two auto-resolution branches. Reproduced in test/wf/r3_error-handling-edge.t.sol::test_claimExpired_reverts_InvalidOutcome_for_nontriggering_survived_staker (PASS).

Impact: UX/observability: a staker following the natural 'call claimExpired after expiry' flow gets a confusing revert (InvalidOutcome) despite being owed funds under a valid SURVIVED resolution. Funds are recoverable via claimSurvived(); no loss.

Recommendation: Either route non-triggering claimants transparently (have claimExpired forward to the claimSurvived path when outcome==SURVIVED), or revert with a dedicated, descriptive error such as UseClaimSurvived() so wallets/integrators can surface the correct next action.

---

Informational

I-01: Overloaded InvalidAmount() custom error signals several distinct failure conditions (already-claimed vs nothing-to-claim vs zero-amount)

Location: src/ConfidencePool.sol:223 (stake amount==0), 303 (withdraw: nothing staked), 384 & 387 (claimSurvived: already-claimed AND nothing-to-claim), 578 (claimExpired: already-claimed)

The single custom error InvalidAmount() is reverted for semantically different conditions: a zero stake amount (223), a withdraw with zero eligible stake (303), a repeat claim after hasClaimed (384, 578), and a claim with zero eligible stake (387). For the double-claim case nothing about an amount is invalid - the amount was valid and already paid. The contract otherwise uses precise dedicated errors for analogous situations (e.g. BountyAlreadyClaimed at 434, OutcomeAlreadySet at 327), so the overload is inconsistent with its own error vocabulary. Reproduced in test/wf/r3_error-handling-edge.t.sol::test_double_claim_reverts_with_misleading_InvalidAmount.

Impact: Purely observability/UX: front-ends and integrators cannot map the revert selector to a precise cause, and the name 'InvalidAmount' is actively misleading for the already-claimed case where no amount was supplied. No fund impact.

Recommendation: Introduce distinct errors (e.g. AlreadyClaimed(), NothingToClaim(), NothingStaked()) and use them at the respective sites; keep InvalidAmount() only for genuine invalid-amount inputs (stake with amount==0).

---

I-02: OutcomeFlagged does not index the attacker address or the outcome enum (only the moderator)

Location: src/interfaces/IConfidencePool.sol:12 (declaration); src/ConfidencePool.sol:378 (manual emit), 550/560/570 (auto-resolution emits)

event OutcomeFlagged(address indexed moderator, PoolStates.Outcome outcome, bool goodFaith, address attacker) indexes only moderator. The two fields consumers would naturally filter on are unindexed: attacker (the named good-faith-CORRUPTED bounty recipient) and outcome (the resolution type). The same event is also emitted from the auto-resolution path (550/560/570) always with moderator == address(0), so filtering by the resolution moment or by attacker is impossible without scanning and decoding every OutcomeFlagged log.

Impact: Off-chain tooling (a would-be attacker's monitoring, and stakers watching for a CORRUPTED resolution that names an attacker) cannot subscribe by topic on attacker or outcome; they must fetch and decode all OutcomeFlagged events. Observability only.

Recommendation: Mark attacker (and, if desired, outcome) as indexed: event OutcomeFlagged(address indexed moderator, PoolStates.Outcome indexed outcome, bool goodFaith, address indexed attacker).

---

I-06: Interface getters riskWindowStart/riskWindowEnd/pokeRiskWindow omit the expiry cap, contradicting the contract's own storage-variable NatSpec

Location: src/interfaces/IConfidencePool.sol:98-106 and 111-114 vs src/ConfidencePool.sol:806-807, 823-827

The interface documents riskWindowStart/riskWindowEnd as 'Timestamp of the first observation ... in an active-risk state' (98-106) and pokeRiskWindow as sealing them 'at block.timestamp' (111-114). The implementation caps the stored value at expiry: _markRiskWindowStart does if (t > expiry) t = expiry; (806-807) and _markRiskWindowEnd the same (823-827). So a seal observed after expiry stores expiry, not the observation block.timestamp. The contract's own storage-variable NatSpec correctly documents 'Capped at expiry', so the interface doc is the outlier.

Impact: An off-chain indexer/integrator trusting the interface docstring would treat these getters as the raw first-observation timestamp and could mis-interpret a value clamped to expiry, mis-modeling the bonus window bounds. Documentation-only.

Recommendation: Add 'capped at expiry' to the riskWindowStart, riskWindowEnd, and pokeRiskWindow interface NatSpec to match the storage-variable docs and the _markRiskWindow* implementation.

---

I-07: Interface withdraw NatSpec ties availability solely to current registry state, omitting the one-way riskWindowStart != 0 latch that keeps it disabled after a registry rewind

Location: src/interfaces/IConfidencePool.sol:126-129 vs src/ConfidencePool.sol:293-300

The interface @dev states withdraw is 'Allowed only while the registry is in a pre-attack state (NOT_DEPLOYED, NEW_DEPLOYMENT, or ATTACK_REQUESTED); reverts ... once the agreement is UNDER_ATTACK or beyond' - implying availability is a pure function of the current registry state. The code (293-300) reverts WithdrawsDisabled if riskWindowStart != 0 OR the state is not pre-attack. Thus once the risk window has ever been sealed, withdraw stays permanently disabled even if the registry later rewinds to ATTACK_REQUESTED - a scenario the contract comment (291-292) and DESIGN 11 explicitly contemplate. The underlying one-way behavior is intended/documented; only the interface docstring is wrong for the rewind case.

Impact: An integrator reading only the interface could wrongly conclude withdraw re-opens if the registry returns to a pre-attack state; in reality the riskWindowStart latch keeps it closed. Documentation-only.

Recommendation: Add to the withdraw interface NatSpec that withdraw is additionally gated on the one-way riskWindowStart != 0 latch, so a registry rewind to a pre-attack state does not re-open it.

---

I-09: RiskWindow / ScopeLock observation events omit the triggering registry state (survival vs breach not distinguishable from logs)

Location: src/ConfidencePool.sol:816 (RiskWindowStarted), 828 (RiskWindowEnded), 791 (ScopeLocked); src/interfaces/IConfidencePool.sol:29-38

RiskWindowStarted/RiskWindowEnded/ScopeLocked each carry only a timestamp. RiskWindowEnded fires when the registry is first observed terminal - either PRODUCTION (survived) or CORRUPTED (breached), see _isTerminalState (837-839) - but the terminal state is not in the event and is not derivable from any other pool log at that block (the resolving flagOutcome/claimExpired may be much later and can even diverge, e.g. registry CORRUPTED but moderator flags SURVIVED per DESIGN 8). The pool does not even persist the registry ContractState in storage, so an indexer cannot recover it later via a view call - only via an archive-node historical read against the separate IAttackRegistry. RiskWindowStarted likewise does not say whether UNDER_ATTACK or PROMOTION_REQUESTED opened it.

Impact: A log-only indexer cannot tell, at RiskWindowEnded time, whether the underlying agreement survived (PRODUCTION) or was breached (CORRUPTED) - the single most staker-relevant fact - nor which active-risk state opened the window. Pure observability gap.

Recommendation: Include the observed IAttackRegistry.ContractState in these events, e.g. RiskWindowEnded(uint256 timestamp, IAttackRegistry.ContractState state), so consumers can reconstruct the registry transition without archive-node state queries.

---

I-11: Bonus rounding dust is unrecoverable until EVERY staker claims; one non-claiming staker strands it permanently

Location: src/ConfidencePool.sol:482-493 (sweepUnclaimedBonus reserve logic)

sweepUnclaimedBonus reserves totalEligibleStake plus (when riskWindowStart!=0) snapshotTotalBonus-claimedBonus (483-487). While ANY staker has not yet claimed, totalEligibleStake != 0 so the full outstanding bonus stays reserved and amount = freeBalance-reserved <= 0 -> NothingToSweep (493). Each claim decrements totalEligibleStake by exactly the claimer's userEligible and increments claimedBonus by their floor-divided share, pinning freeBalance-reserved at 0 until the last claimant exits. Consequently the rounding remainder (from finding L-02) and an abandoning staker's bonus can only be released after the LAST eligible staker claims; if a single staker abandons the pool (lost key, disinterest), the remainder is permanently locked - recoverable by no one, including recoveryAddress. DESIGN.md documents dust only in the CORRUPTED-bounty (12) and generic-sweep (10) contexts, never this SURVIVED/EXPIRED last-claimer gating.

Impact: The rounding remainder is not lost to an attacker but can become permanently stranded whenever one staker never claims. Magnitude sub-wei-to-few-wei per pool; dust-recoverability/observability note.

Recommendation: Accept and document that dust recovery is gated on all stakers claiming, or track a separate distributable-bonus counter so the pure rounding remainder (never owed to any specific non-claimer) can be swept independently of outstanding principal reserves.

---

I-12: Scope array in initialize()/setPoolScope() has no maximum-length bound (defense-in-depth)

Location: src/ConfidencePool.sol:749-776 (_replaceScope), reached via initialize:214 and setPoolScope:640

_replaceScope iterates accounts (arbitrary address[] calldata length) doing a storage write and an external IAgreement.isContractInScope call per element (764-772), with no upper bound on accounts.length. A sufficiently large scope array can exceed the block gas limit, making initialize (thus createPool) or setPoolScope unexecutable for that array. The impact is self-limited (only the pool creator/owner supplies the array and only their own pool is affected), so this is a missing defense-in-depth bound, not an external DoS. DESIGN.md 8 describes scope semantics/locking but has no array-size/gas-limit discussion.

Impact: An owner/creator can brick pool creation or a scope update for their own pool by passing an oversized array; no cross-user impact. Low likelihood, self-inflicted.

Recommendation: Add a sane MAX_SCOPE_ACCOUNTS upper bound and revert with a dedicated error when accounts.length exceeds it, to fail fast and cheaply.

---

I-13: pokeRiskWindow() has inconsistent 'nothing happened' semantics: silent no-op when resolved vs revert RiskWindowNotReached when unreached

Location: src/ConfidencePool.sol:649-658 (no-op return at 652; revert at 657)

pokeRiskWindow() is the permissionless mechanism to seal the risk-window markers. When nothing can be sealed it behaves in two opposite ways: if the pool is already resolved it returns silently (652), but if the registry simply never reached an active-risk/terminal state it reverts RiskWindowNotReached (657). RiskWindowStarted/RiskWindowEnded fire only on a real seal (inside _markRiskWindowStart, 816/828), so a successful poke tx is indistinguishable from a no-op success without reading storage. DESIGN.md (149-162) explains why permissionless poking is outcome-neutral/safe but never addresses this silent-return-vs-revert asymmetry.

Impact: Observability only. An off-chain sealer cannot tell from a successful pokeRiskWindow() tx whether a marker was actually sealed (absence of an event must be inferred), and the two no-progress paths behave oppositely, making seal-state monitoring fragile.

Recommendation: Make the two no-progress paths consistent (both revert with distinct errors, or both return a bool indicating whether a marker was sealed), or document that callers must watch RiskWindowStarted/RiskWindowEnded rather than tx success.

---

Gas

G-01: _replaceScope copies the entire _scopeAccounts storage array into memory solely to clear the membership mapping

Location: src/ConfidencePool.sol:753-758

address[] memory old = _scopeAccounts; (753) performs a full storage-to-memory copy of the dynamic scope array (1 SLOAD for length + 1 SLOAD + 1 MSTORE per element + memory allocation/expansion), yet old is used solely to iterate and set isAccountInScope[old[i]] = false in the immediately-following loop, after which delete _scopeAccounts (758) discards it anyway. Iterating the storage array directly with a cached length reads the same element slots but eliminates all n MSTOREs, the memory expansion, and the allocation. The via-IR/optimizer does not rewrite a storage-array-to-memory copy into in-place iteration, so the saving is real under the contest compiler settings.

Impact: Wasted gas on every scope replacement (initialize() and setPoolScope()), scaling O(n) with the number of scope accounts: ~n memory stores plus quadratic memory-expansion cost, none needed since only the mapping is being cleared. No correctness impact.

Recommendation: Drop the old memory copy; cache uint256 n = _scopeAccounts.length; then loop isAccountInScope[_scopeAccounts[i]] = false;, and delete _scopeAccounts; afterward (the clearing loop must run before the delete).

---

G-02: Three sub-word state variables each waste a full storage slot (expiry, scopeLocked, outcomeFlaggedAt)

Location: src/ConfidencePool.sol:69 (expiry), 78 (scopeLocked), 127 (outcomeFlaggedAt)

forge inspect ConfidencePool storage-layout confirms expiry (uint32) occupies slot 8 alone (28 bytes unused), scopeLocked (bool) slot 11 alone (31 bytes unused), and outcomeFlaggedAt (uint32) slot 20 alone (28 bytes unused), because each is declared between full-word neighbours. These plus the two uint32s already packed in slot 30 (_firstGoodFaithCorruptedAt + corruptedClaimDeadline, 24 bytes free) total only ~17 bytes of small fields spread across 4 slots. solc never reorders storage, so declaring the sub-word fields adjacently to pack them into one 32-byte slot must be done in source; it survives via-IR/optimizer. This is an EIP-1167 clone implementation (no pre-existing per-clone state), so reordering is safe.

Impact: Up to ~3 extra storage slots are allocated and cold-touched over a pool's lifecycle (each first zero->nonzero SSTORE ~20k, each cold SLOAD ~2.1k). Consolidating removes those redundant slots.

Recommendation: Group the sub-word fields (expiry, scopeLocked, outcomeFlaggedAt, _firstGoodFaithCorruptedAt, corruptedClaimDeadline) into adjacent declarations so they share one slot; re-run forge inspect ... storage-layout to confirm the reduced slot count.

---

G-03: Redundant storage reads of totalEligibleStake / riskWindowStart in sweepUnclaimedBonus

Location: src/ConfidencePool.sol:483, 484, 499 (totalEligibleStake); 485, 499 (riskWindowStart)

sweepUnclaimedBonus reads storage variable totalEligibleStake three times (483 condition, 484 assignment, 499 condition) and riskWindowStart twice (485, 499) within a single non-branching flow, without caching. These are warm SLOADs after the first read but still ~100 gas each and trivially cacheable to memory locals.

Impact: Minor, deterministic gas overspend (~2-3 redundant warm SLOADs) on every sweepUnclaimedBonus call. No correctness impact.

Recommendation: Cache uint256 tes = totalEligibleStake; and uint256 rws = riskWindowStart; once at function entry and reuse the locals in the subsequent comparisons and the assignment.

---

G-04: initialize writes the zero-default value to outcome (redundant SSTORE on a freshly-deployed clone)

Location: src/ConfidencePool.sol:212

outcome = PoolStates.Outcome.UNRESOLVED; assigns the enum's first member, value 0 (PoolStates.sol declares UNRESOLVED first). initialize runs on a freshly-deployed EIP-1167 clone whose storage is already zero, and no other field in outcome's slot (slot 19, shared with attacker/goodFaith/expiryLocked) is written during initialize, so this is a redundant write of the default value. Because initialize is compiled without knowledge that the clone's storage is zero, the optimizer keeps the SSTORE.

Impact: Every createPool pays for a needless SSTORE of the zero default with no effect on state.

Recommendation: Remove the outcome = PoolStates.Outcome.UNRESOLVED; assignment (the storage default already equals UNRESOLVED), or keep it only if desired for explicitness, accepting the gas cost.

---

G-05: claimCorrupted() bad-faith path performs a redundant zero write to bountyClaimed

Location: src/ConfidencePool.sol:419-421

In the bad-faith CORRUPTED sweep, line 420 executes bountyClaimed = bountyEntitlement;. For any bad-faith CORRUPTED resolution, bountyEntitlement is 0 (flagOutcome at 362 assigns entitlement only when willBeGoodFaithCorrupted; the claimExpired auto-CORRUPTED path never sets it). bountyClaimed's other writers (445, 466) are gated on goodFaith being true, so bountyClaimed is also 0 in this state - line 420 is an SSTORE of the value the slot already holds, a redundant write on every bad-faith sweep. Harmless to accounting but pays needless gas and can read as load-bearing to a maintainer.

Impact: Wasted gas on each bad-faith claimCorrupted call (a warm SSTORE of an unchanged value). No correctness impact.

Recommendation: Drop the assignment in the bad-faith branch (both are already 0 there), or add a comment clarifying it is a defensive no-op if kept for uniformity.

---