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

Stakers locked into an active attack lose their entire bonus when the risk window is never sealed

Author Revealed upon completion

Root + Impact

Description

A staker is meant to lose the exit and start earning the bonus at the same moment: docs/DESIGN.md §9 says a staker "only forfeits the exit option once risk has actually materialised, which is exactly when they begin earning the risk premium."

In code these are two different signals. withdraw closes on the live UNDER_ATTACK state, but the bonus only accrues once riskWindowStart is sealed, which needs a transaction that succeeds while the registry is active-risk (stake / contributeBonus / pokeRiskWindow). A locked-in staker's own withdraw seals the flag and then reverts on the next line, so the revert rolls the seal back. Result: a staker locked in through a real attack that survives can receive zero bonus, and the whole bonus pool is swept to the sponsor's recoveryAddress.

// src/ConfidencePool.sol
function withdraw() external nonReentrant {
IAttackRegistry.ContractState state = _observePoolState();
if (
riskWindowStart != 0
@> || (state != IAttackRegistry.ContractState.NOT_DEPLOYED // exit closes on the LIVE
@> && state != IAttackRegistry.ContractState.NEW_DEPLOYMENT // state, even while
@> && state != IAttackRegistry.ContractState.ATTACK_REQUESTED) // riskWindowStart == 0
) {
@> revert WithdrawsDisabled(); // revert rolls back the seal _observePoolState() just made
}
...
}
​
function _bonusShare(address u, uint256 userEligible) internal view returns (uint256) {
if (snapshotTotalBonus == 0) return 0;
@> if (riskWindowStart == 0) return 0; // bonus gated on the PERSISTED latch: a different signal
...
}

Risk

Likelihood:

  • Occurs whenever the registry passes through an active-risk state and no stake / contributeBonus / pokeRiskWindow lands during that interval, i.e. the default for a passive pool (locked-in stakers cannot withdraw, and staking more only dilutes their share).

  • Occurs when the active-risk interval is short, e.g. a DAO instantPromote from UNDER_ATTACK straight to PRODUCTION in one transaction, so a watching staker gets no block in which to poke.

Impact:

  • Winning stakers who bore the full risk get none of the k=2 bonus (_bonusShare returns 0 for all).

  • sweepUnclaimedBonus then sends the entire bonus pool to the sponsor's recoveryAddress (with riskWindowStart == 0 only principal is reserved), reassigning participants' yield to the party that funded it.

Proof of Concept

PoC_LockedInStakerNoBonus.t.sol (in this folder) drops into test/unit/. Two tests share the same path (NEW_DEPLOYMENT -> UNDER_ATTACK -> PRODUCTION), stake, and bonus; a single pokeRiskWindow during the attack is the only difference and it flips the reward between zero and the full bonus.

function testBug_LockedInWinningStakerGetsNoBonus() external {
_stake(alice, STAKE);
_contributeBonus(carol, BONUS);
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK); // alice locked in
​
vm.prank(alice);
vm.expectRevert(IConfidencePool.WithdrawsDisabled.selector);
pool.withdraw();
assertEq(pool.riskWindowStart(), 0); // her reverted exit did not seal; nobody else observed
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION); // attack survived
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
uint256 aliceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.claimSurvived();
assertEq(token.balanceOf(alice) - aliceBefore - STAKE, 0); // zero bonus for the locked-in winner
​
uint256 recoveryBefore = token.balanceOf(recovery);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(recovery) - recoveryBefore, BONUS); // full bonus swept to sponsor
}
forge test --match-path 'test/unit/PoC_LockedInStakerNoBonus.t.sol' -vv
[PASS] testBug_LockedInWinningStakerGetsNoBonus()
[PASS] testControl_OnePokeDuringAttackSecuresTheBonus() // one poke -> alice gets the full bonus

Recommended Mitigation

Make the observation that closes the exit commit the seal instead of discarding it. _observePoolState() already seals riskWindowStart on an active-risk read; just stop withdraw from reverting that seal away.

function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
+ bool sealedBefore = riskWindowStart != 0;
IAttackRegistry.ContractState state = _observePoolState();
+ // If this observation freshly sealed the window, the exit just closed and the premium just
+ // started. Persist the seal by returning, not reverting: a revert rolls it back and strips
+ // the now-locked-in staker of the bonus (DESIGN.md §9).
+ if (!sealedBefore && riskWindowStart != 0) {
+ emit WithdrawBlocked(msg.sender);
+ return;
+ }
if (
riskWindowStart != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
&& state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
) {
revert WithdrawsDisabled();
}

This preserves the genuine §5 "no observable risk" case (a jump straight to a terminal state never seals riskWindowStart, so it still reverts and pays no bonus) while closing the gap for a demonstrably locked-in staker. WithdrawBlocked(address) is a new event so the caller learns the exit was closed; a plain return also works.

Support

FAQs

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

Give us feedback!