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

Reverting observation rolls back the risk-window latch

Author Revealed upon completion

Affected: ConfidencePool.sol:222-227 · ConfidencePool.sol:727-733 · ConfidencePool.sol:784-799

Description

stake() and contributeBonus() observe registry state before enforcing their deposit gate. _observePoolState() writes riskWindowStart and scopeLocked as one-way transitions, then _assertDepositsAllowed() reverts on PROMOTION_REQUESTED, PRODUCTION, or CORRUPTED. EVM atomicity rolls the writes back with the revert, so the pool never persists the first observation.

// L222-227 — stake observes, then may revert on the same tx
_assertDepositsAllowed(_observePoolState());
​
// L727-733 — revert path
if (state == PROMOTION_REQUESTED || state == PRODUCTION || state == CORRUPTED) {
revert StakingClosed();
}

A permissionless caller (Bob) can therefore attempt stake(1 wei) during PROMOTION_REQUESTED and knowingly force the revert; the latches that _observePoolState() would have sealed are lost. Combined with a later legitimate DAO setAttackRegistry migration that reports NOT_DEPLOYED for the agreement, the withdraw() gate re-opens for previously staked capital — even though active risk was already publicly observable on-chain.

Same root cause as M-3, distinct entrypoint: here the failed sealing is triggered by an unprivileged Bob rather than by the observation ordering of a low-traffic pool.

Risk

Likelihood: Requires two events — permissionless reverting deposit (or bonus) call during active risk, plus a later routine DAO registry migration (DESIGN.md §11 treats migration as expected).

Impact: Existing staker exits principal after active-risk was on-chain. Bounded per-staker; aggregate up to totalEligibleStake. No attacker capture — this is a leak vs. the intended commitment invariant, not a direct value transfer to the attacker.

Actor: Bob (revert trigger) is fully permissionless. The migration is the DAO's documented onlyOwner operation.

Recommended Mitigation

When a deposit is blocked, persist the transition instead of reverting the whole transaction. A seal-only-return pattern preserves the one-way observation without accepting the deposit:

function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
if (amount == 0) revert InvalidAmount();
if (amount < minStake) revert BelowMinStake();
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
if (block.timestamp >= expiry) revert StakingClosed();
- _assertDepositsAllowed(_observePoolState());
+ IAttackRegistry.ContractState state = _observePoolState();
+ if (_depositsBlocked(state)) {
+ // observation is durable; drop the caller's deposit silently
+ return;
+ }
// ... rest unchanged
}

Apply the same rule to contributeBonus(). Alternatively, harden the withdraw gate as recommended in M-3 (|| riskWindowEnd != 0) plus store scopeLocked durably via a dedicated sealObservation() no-op path.

Proof of Concept

Save as test/ObserverRevertRollback.t.sol and run forge test --match-contract ObserverRevertRollbackTest -vv.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
​
contract ObserverRevertRollbackTest is BaseConfidencePoolTest {
function testBlockedStakeRollsBackRiskLatchAndMigrationReopensWithdraw() external {
_stake(alice, 100 * ONE);
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PROMOTION_REQUESTED);
​
// Bob's stake observes active risk then reverts; latches never persist.
token.mint(bob, ONE);
vm.startPrank(bob);
token.approve(address(pool), ONE);
vm.expectRevert(IConfidencePool.StakingClosed.selector);
pool.stake(ONE);
vm.stopPrank();
​
assertEq(pool.riskWindowStart(), 0);
assertFalse(pool.scopeLocked());
​
// DAO migrates; new AttackRegistry reads NOT_DEPLOYED for this agreement.
MockAttackRegistry rewound = new MockAttackRegistry();
rewound.setAgreementState(IAttackRegistry.ContractState.NOT_DEPLOYED);
safeHarborRegistry.setAttackRegistry(address(rewound));
​
_withdraw(alice);
assertEq(token.balanceOf(alice), 100 * ONE);
assertEq(pool.eligibleStake(alice), 0);
}
}

Support

FAQs

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

Give us feedback!