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

withdraw() calls _clampUserSums() which is provably always a no-op, wasting ~330-390 gas per call

Author Revealed upon completion

Root + Impact

Description

  • _clampUserSums(u) exists to promote a staker's pre-risk deposit timestamps up to riskWindowStart once the active-risk window has opened, so their k=2 bonus score doesn't wrongly credit pre-risk calendar time. It is a no-op whenever riskWindowStart == 0 (window never opened yet).

  • withdraw() is only reachable when riskWindowStart == 0 -- the preceding guard reverts WithdrawsDisabled() for every other case. By the time execution reaches _clampUserSums(msg.sender) inside withdraw(), riskWindowStart == 0 is already guaranteed, so the call always hits _clampUserSums's own early return and never has any effect. It costs real gas (two SLOADs plus internal-call dispatch) on every single withdraw() for zero behavioral effect.

function withdraw() external nonReentrant {
...
@> if (riskWindowStart != 0 || (state != NOT_DEPLOYED && ...)) revert WithdrawsDisabled();
uint256 amount = eligibleStake[msg.sender];
if (amount == 0) revert InvalidAmount();
​
@> _clampUserSums(msg.sender); // riskWindowStart == 0 guaranteed here -> always no-op
...
}
​
function _clampUserSums(address u) internal {
uint256 start = riskWindowStart;
uint256 stake_ = eligibleStake[u];
@> if (start == 0 || stake_ == 0) return; // the only branch ever reachable from withdraw()
...
}

Risk

Likelihood:

  • Reason 1 Every successful withdraw() call reaches this dead call -- it's not conditional on any external state, it's structurally unreachable code within this one call site regardless of who calls it or when.

  • Reason 2 Any pool with active stakers who withdraw before risk materializes pays this cost on every single withdrawal, for the entire lifetime of every pool the factory ever creates.

Impact:

  • Impact 1 Modest but permanent gas overhead on a frequently-used user-facing function, for zero benefit -- pure waste compounding across every pool and every withdrawal, forever.

  • Impact 2 No fund-safety impact -- purely a gas/efficiency issue, correctly scoped as Low rather than a correctness finding.

Proof of Concept

Verified at three independent levels, all agreeing on the same order of magnitude:
​
1. Local mocks, independent stacks per pool variant (rules out warm/cold cross-contamination): 379-391 gas saved, both call orders, byte-for-byte identical post-withdraw state.
​
2. Fork simulation against the live BattleChain testnet registry (chainId 627) using vm.snapshotState()/revertToState() so both measurements start from an identical restored EVM access-list state: exactly 379 gas saved.
​
3. A REAL signed broadcast (forge script --broadcast) against the live chain -- two fresh real Agreements registered via the real AgreementFactory, real ConfidencePool/ConfidencePoolPatched clones deployed, staked and withdrawn with real transactions:
- Original withdraw() tx 0xa4d6e4053a7f5a03920f9c1ded5fe39bf2702a7feecdf5bb0e5dcbea3383d111 (block 17052): gasUsed = 454752
- Patched withdraw() tx 0xeeaf91b69127144e1d3aef23d1065ec7f436d98311fb9c19fe446a2f4041f74f (block 17053): gasUsed = 454418
- Diff: 334 gas saved, confirmed via real on-chain transaction receipts, publicly verifiable at explorer.testnet.battlechain.com.
​
function test_PoC_withdrawGasCost_originalVsPatched_identicalOutcome() external {
uint256 amount = 500 * ONE;
​
tokenA.mint(alice, amount);
vm.prank(alice); tokenA.approve(address(original), amount);
vm.prank(alice); original.stake(amount);
​
tokenB.mint(alice, amount);
vm.prank(alice); tokenB.approve(address(patched), amount);
vm.prank(alice); patched.stake(amount);
​
vm.prank(alice);
uint256 gasBeforeOriginal = gasleft();
original.withdraw();
uint256 gasUsedOriginal = gasBeforeOriginal - gasleft();
​
vm.prank(alice);
uint256 gasBeforePatched = gasleft();
patched.withdraw();
uint256 gasUsedPatched = gasBeforePatched - gasleft();
​
// Local: 379-391 gas saved. Live-testnet real broadcast: 334 gas saved
// (tx 0xa4d6e405...3d111 vs 0xeeaf91b6...1f74f, blocks 17052/17053).
assertGt(gasUsedOriginal, gasUsedPatched);
}

Recommended Mitigation

Remove the dead call entirely -- it can never do anything at this call site.
​
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

Support

FAQs

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

Give us feedback!