BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: low

_clampUserSums Called Before Zero-Stake Validation in withdraw() and claimSurvived()

Author Revealed upon completion

Root + Impact

Description

  • The withdraw() and claimSurvived() functions both call _clampUserSums(msg.sender) before verifying whether the caller has any eligible stake. When a user with zero balance calls either function, the contract executes an unnecessary storage read and comparison in _clampUserSums, then reverts shortly after at the stake validation check.

// --->> src/ConfidencePool.sol:288-318 — withdraw()
function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
if (riskWindowStart != 0 || ...) {
revert WithdrawsDisabled();
}
uint256 amount = eligibleStake[msg.sender];
if (amount == 0) revert InvalidAmount();
_clampUserSums(msg.sender); // --->> Executes before any benefit is realized
sumStakeTime -= userSumStakeTime[msg.sender];
// ...
}
// --->> src/ConfidencePool.sol:382-405 — claimSurvived()
function claimSurvived() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED) revert OutcomeNotSet();
if (hasClaimed[msg.sender]) revert InvalidAmount();
uint256 userEligible = eligibleStake[msg.sender];
if (userEligible == 0) revert InvalidAmount();
_clampUserSums(msg.sender); // --->> Same issue — called after zero check
// ...
}
The _clampUserSums function performs a storage load (riskWindowStart, eligibleStake[u]) and a comparison on every call. When the user has no stake, these operations execute uselessly because the subsequent zero-balance check will revert the transaction anyway.

Risk

Likelihood:

  • Any user (or contract) that calls withdraw() or claimSurvived() without previously staking, or after a prior claim/withdrawal has zeroed their eligibleStake, triggers the unnecessary computation

  • Third-party integrations or batch scripts that iterate over user lists to check claim eligibility will hit this path repeatedly

Impact:

  • Each unnecessary _clampUserSums call wastes approximately 2,600 gas (two SLOAD operations at 2,100 gas each for cold slots, or 200 gas for warm slots) plus comparison logic

  • Across many zero-stake calls (e.g., a frontend checking claim status for all users), cumulative gas waste becomes material

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import "forge-std/Test.sol";
contract ZeroStakeGasWaste is Test {
// After deploying and initializing a pool:
// 1. User A has never staked (eligibleStake[A] = 0)
// 2. Pool is in SURVIVED state with riskWindowStart != 0
function test_withdraw_wastes_gas_before_revert() public {
// User A calls withdraw() with no stake
// Expected: revert with InvalidAmount
// Actual: _clampUserSums executes first, then reverts
// Gas trace shows:
// - _clampUserSums: 2x SLOAD (riskWindowStart, eligibleStake) = ~4200 gas
// - Then revert at "if (amount == 0) revert InvalidAmount()"
// The SLOAD operations are wasted — the revert happens immediately after
}
function test_claimSurvived_wastes_gas_before_revert() public {
// Same pattern: _clampUserSums runs, then zero-check reverts
// Gas waste: ~4200 gas per call
}
}
To measure the difference, add a counter to _clampUserSums in a test fork and compare gas usage with the check moved earlier.

Recommended Mitigation

Move the zero-stake validation before _clampUserSums in both functions:
// withdraw()
function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
if (riskWindowStart != 0 || ...) {
revert WithdrawsDisabled();
}
+ uint256 amount = eligibleStake[msg.sender];
+ if (amount == 0) revert InvalidAmount();
+
- uint256 amount = eligibleStake[msg.sender];
- if (amount == 0) revert InvalidAmount();
-
- _clampUserSums(msg.sender);
+ _clampUserSums(msg.sender);
sumStakeTime -= userSumStakeTime[msg.sender];
// ...
}
// claimSurvived()
function claimSurvived() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED) revert OutcomeNotSet();
if (hasClaimed[msg.sender]) revert InvalidAmount();
- uint256 userEligible = eligibleStake[msg.sender];
- if (userEligible == 0) revert InvalidAmount();
-
- _clampUserSums(msg.sender);
+ uint256 userEligible = eligibleStake[msg.sender];
+ if (userEligible == 0) revert InvalidAmount();
+
+ _clampUserSums(msg.sender);
// ...
}
The fix reorders existing statements without changing semantics — _clampUserSums is only meaningful when eligibleStake > 0, so the early return is safe.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!