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

Redundant `_clampUserSums` call in `withdraw()` wastes gas

Author Revealed upon completion

Description

  • Under normal behavior, stakers can call withdraw() to pull their funds out of the pool as long as the agreement has not entered an active-risk state (i.e., riskWindowStart == 0).

  • In ConfidencePool.withdraw(), the code explicitly reverts if riskWindowStart != 0. However, a few lines later, it calls _clampUserSums(msg.sender). Inside _clampUserSums(), the very first check is if (start == 0) return;. Since withdraw() guarantees riskWindowStart is 0, the _clampUserSums() function will always immediately return without modifying any state, needlessly consuming gas for the internal call.

function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
// `riskWindowStart` is the pool's one-way record that risk has materialised;
// gate on it so an upstream registry rewind cannot re-open withdrawals.
if (
riskWindowStart != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
&& state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
) {
revert WithdrawsDisabled();
}
​
uint256 amount = eligibleStake[msg.sender];
if (amount == 0) revert InvalidAmount();
​
// @> Redundant: withdraw() already guarantees riskWindowStart == 0,
// @> and _clampUserSums immediately returns if riskWindowStart == 0.
_clampUserSums(msg.sender);

Risk

Likelihood: High

  • Every single call to withdraw() executes this redundant internal function call.

Impact: Low

  • Stakers waste a small amount of gas per withdraw() execution on a useless internal call and a branch evaluation.

Proof of Concept

function _clampUserSums(address user) private {
uint256 start = riskWindowStart;
if (start == 0) return; // @> ALWAYS true during withdraw()
// ... (unreachable code during withdraw)
}

Recommended Mitigation

Remove the _clampUserSums(msg.sender) call from the withdraw() function to save gas.

uint256 amount = eligibleStake[msg.sender];
if (amount == 0) revert InvalidAmount();
​
- _clampUserSums(msg.sender);
// Withdrawing forfeits the caller's bonus claim: subtract their full contribution from
// the global accumulators so honest stakers' shares aren't diluted by the exiter's
Updates

Lead Judging Commences

inallhonesty Lead Judge 2 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

QA / Informational / Gas

inallhonesty Lead Judge 2 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

QA / Informational / Gas

Support

FAQs

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

Give us feedback!