Root + Impact
Description
stake() and contributeBonus() can observe an active-risk or terminal state and then revert, erasing the lifecycle markers that determine bonus allocation.
_observePoolState() is intended to persist the first observed active-risk and terminal timestamps. These markers bound the k=2 bonus window; in particular, riskWindowStart == 0 means no staker receives bonus.
Both deposit paths pass _observePoolState() directly into _assertDepositsAllowed(). In PROMOTION_REQUESTED, PRODUCTION, and CORRUPTED, the observer first writes riskWindowStart or riskWindowEnd, after which the gate reverts StakingClosed. EVM rollback erases the marker and its event together with the rejected deposit.
During PROMOTION_REQUESTED, this can leave riskWindowStart unset until the registry becomes terminal. The subsequent SURVIVED resolution treats the pool as having observed no risk: every staker receives principal only, and the full bonus is sweepable to recoveryAddress. A rejected terminal-state deposit can similarly lose the first riskWindowEnd observation and let a later successful caller choose a different T.
src/ConfidencePool.sol
function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
if (amount == 0) revert InvalidAmount();
_assertDepositsAllowed(_observePoolState());
emit Staked(msg.sender, received);
}
function contributeBonus(uint256 amount) external nonReentrant whenPoolNotPaused {
if (amount == 0) revert InvalidAmount();
_assertDepositsAllowed(_observePoolState());
emit BonusContributed(msg.sender, received);
}
function _assertDepositsAllowed(IAttackRegistry.ContractState state) private pure {
if (
state == IAttackRegistry.ContractState.PROMOTION_REQUESTED
|| state == IAttackRegistry.ContractState.PRODUCTION || state == IAttackRegistry.ContractState.CORRUPTED
) {
revert StakingClosed();
}
}
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
if (riskWindowStart == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}
The permissionless pokeRiskWindow() mitigates the issue only when a separate successful transaction executes before the registry leaves the relevant state. The protocol does not guarantee such a keeper call, and a reverted deposit cannot preserve its own observation.
Risk
Likelihood:
-
This occurs when the first pool calls during PROMOTION_REQUESTED are stake() or contributeBonus() and no separate pokeRiskWindow() transaction persists the marker before the registry becomes terminal.
-
PROMOTION_REQUESTED is an ordinary closing-window state and both affected entrypoints are public, while marker persistence currently depends on an external actor selecting the dedicated observer.
Impact:
-
With riskWindowStart == 0, _bonusShare() returns zero for every staker and the entire bonus can be swept to recoveryAddress.
-
Losing a terminal observation delays riskWindowEnd; a later timestamp changes T and can redistribute the fixed bonus between stakers. Principal accounting remains unaffected in the demonstrated SURVIVED path.
Proof of Concept
Create test/audit/CP005DepositGateObservationRollback.t.sol with the following contents:
pragma solidity 0.8.26;
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
contract CP005DepositGateObservationRollbackTest is BaseConfidencePoolTest {
function test_RevertingPromotionDepositsEraseRiskObservationAndForfeitBonus() external {
uint256 principal = 100 * ONE;
uint256 bonus = 50 * ONE;
_stake(alice, principal);
_contributeBonus(carol, bonus);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PROMOTION_REQUESTED);
token.mint(bob, ONE);
vm.prank(bob);
token.approve(address(pool), ONE);
vm.prank(bob);
vm.expectRevert(IConfidencePool.StakingClosed.selector);
pool.stake(ONE);
token.mint(dave, ONE);
vm.prank(dave);
token.approve(address(pool), ONE);
vm.prank(dave);
vm.expectRevert(IConfidencePool.StakingClosed.selector);
pool.contributeBonus(ONE);
assertEq(pool.riskWindowStart(), 0);
assertFalse(pool.scopeLocked());
vm.warp(block.timestamp + 3 days);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
assertEq(pool.riskWindowStart(), 0, "the active-risk observation was not preserved");
uint256 aliceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.claimSurvived();
assertEq(token.balanceOf(alice) - aliceBefore, principal, "Alice receives principal only");
uint256 recoveryBefore = token.balanceOf(recovery);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(recovery) - recoveryBefore, bonus, "the full bonus is swept");
assertEq(token.balanceOf(address(pool)), 0);
}
}
Run the PoC from the repository root:
-
Execute forge test --offline --match-path test/audit/CP005DepositGateObservationRollback.t.sol -vv.
-
Confirm that the test passes and shows one test passed with zero failures.
Recommended Mitigation
A contract cannot preserve state written in a transaction that later reverts. Reject closed-state deposits through a successful, explicit result so the lifecycle observation remains committed. Emit a rejection event and return whether a deposit was accepted; integrations must check the return value or the existing success events.
src/interfaces/IConfidencePool.sol
+event DepositRejected(address indexed caller, uint8 registryState);
-function stake(uint256 amount) external;
-function contributeBonus(uint256 amount) external;
+function stake(uint256 amount) external returns (bool accepted);
+function contributeBonus(uint256 amount) external returns (bool accepted);
src/ConfidencePool.sol
-function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
+function stake(uint256 amount) external nonReentrant whenPoolNotPaused returns (bool accepted) {
if (amount == 0) revert InvalidAmount();
// ... preliminary checks unchanged ...
- _assertDepositsAllowed(_observePoolState());
+ IAttackRegistry.ContractState state = _observePoolState();
+ if (!_depositsAllowed(state)) {
+ emit DepositRejected(msg.sender, uint8(state));
+ return false;
+ }
// ... transfer and accounting unchanged ...
emit Staked(msg.sender, received);
+ return true;
}
-function contributeBonus(uint256 amount) external nonReentrant whenPoolNotPaused {
+function contributeBonus(uint256 amount) external nonReentrant whenPoolNotPaused returns (bool accepted) {
if (amount == 0) revert InvalidAmount();
// ... preliminary checks unchanged ...
- _assertDepositsAllowed(_observePoolState());
+ IAttackRegistry.ContractState state = _observePoolState();
+ if (!_depositsAllowed(state)) {
+ emit DepositRejected(msg.sender, uint8(state));
+ return false;
+ }
// ... transfer and accounting unchanged ...
emit BonusContributed(msg.sender, received);
+ return true;
}
-function _assertDepositsAllowed(IAttackRegistry.ContractState state) private pure {
+function _depositsAllowed(IAttackRegistry.ContractState state) private pure returns (bool) {
if (
state == IAttackRegistry.ContractState.PROMOTION_REQUESTED
|| state == IAttackRegistry.ContractState.PRODUCTION || state == IAttackRegistry.ContractState.CORRUPTED
) {
- revert StakingClosed();
+ return false;
}
+ return true;
}
When success-on-rejection semantics are unacceptable, the registry or migration layer must expose durable transition history, or an operational process must call pokeRiskWindow() successfully before each transition. Moving the observation earlier inside the same reverting call does not fix rollback.
