BattleChain Confidence Pools

Cyfrin

FoundrySolidityLayer 2

7.25 ETH

View all submissions

Previous Next

Submission Details

Impact: low

Likelihood: medium

Stakers lose their entire bonus when a real risk window opens and closes between pool interactions (riskWindowStart never seals)

Author Revealed upon completion

Root + Impact

Description

Describe the normal behavior in one or more sentences

When an agreement passes through an active-risk state (UNDER_ATTACK / PROMOTION_REQUESTED) during the pool's term, the pool is meant to record that risk by sealing riskWindowStart via _observePoolState(). Stakers who held their position through that risk window are then rewarded from the bonus pool through the k=2 time-weighted _bonusShare calculation on resolution.
Explain the specific issue or problem in one or more sentences

Sealing riskWindowStart is lazy: it only happens if some caller invokes a state-touching function (stake / withdraw / pokeRiskWindow) WHILE the registry is in an active-risk state. If a real active-risk window opens and closes between pool interactions (e.g. a short attack while pool activity is low), the first poke lands only after the registry already reached PRODUCTION. _observePoolState then seals riskWindowEnd but leaves riskWindowStart == 0, and pokeRiskWindow() does not revert because its guard only trips when BOTH markers are zero. On resolution _bonusShare returns 0 for every staker, and the full totalBonus sweeps to the sponsor-controlled recoveryAddress. Stakers carried real risk yet receive zero bonus.

Acknowledgement of DESIGN.md §5/§6: the design text discusses the general "no observable risk → bonus pays zero → sweeps to recovery" rule, and frames a missing seal as "nobody interacted with the pool for the entire interval." This report is narrower: the risk window was real and non-trivial, but shorter than the gap between pool interactions, so it closes before the FIRST poke lands — even with an attentive staker set. §6's stated mitigation ("any counterparty can pokeRiskWindow the instant the registry transitions") presumes a party actively watching the registry mempool in real time; for an ordinary staker set that assumption does not hold, and here its failure mode is a total bonus loss for honest stakers rather than the "~zero bias" §7 accepts for the T-anchoring residual. Submitted as Low so the sponsor can decide whether a proactive seal is warranted; if judged an accepted residual, it at least flags that the residual's worst case is 100% bonus forfeiture, not a bounded redistribution.

// ConfidencePool.sol — _bonusShare pays zero whenever the window was never sealed

function _bonusShare(address u, uint256 userEligible) internal view returns (uint256) {

if (snapshotTotalBonus == 0) return 0;

@> if (riskWindowStart == 0) return 0; // real risk that closed before observation => 0 bonus

...

}

// pokeRiskWindow does NOT revert when only riskWindowEnd got sealed, so the

// "no risk observed" state is silently locked in:

function pokeRiskWindow() external {

if (outcome != PoolStates.Outcome.UNRESOLVED) return;

_observePoolState();

@> if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached(); // passes when end!=0

}

Risk

Likelihood:

Reason 1
Occurs whenever the registry's active-risk interval (UNDER_ATTACK -> PRODUCTION) is shorter than the gap between pool interactions, so no caller observes the pool during active risk. This is a plain timing property, not attacker-controlled — a fast attack or a period of low pool activity is enough.
Reason 2
Ordinary stakers are not watching the registry mempool in real time, so the "anyone can pokeRiskWindow at the transition" mitigation does not reliably fire in practice.

Impact:

Impact 1
Stakers who held risk for the full term receive zero bonus on resolution, losing 100% of the bonus they economically earned.
Impact 2
The entire totalBonus is redirected to the sponsor-controlled recoveryAddress instead of the stakers, i.e. a bonus-only transfer of value from stakers to the sponsor. No principal is lost, hence Low.

Proof of Concept

The test deploys the pool through the real ConfidencePoolFactory and uses a MutableAttackRegistry whose state can be advanced. The staker deposits at NEW_DEPLOYMENT; a real UNDER_ATTACK → PRODUCTION window then passes with no pool call in between; pokeRiskWindow() is called only afterwards. The test asserts riskWindowStart() == 0 despite the risk, and that claimExpired() pays the staker principal only while the full bonus sweeps to recoveryAddress. Passes with forge test.

pragma solidity 0.8.26;

import "forge-std/Test.sol";

import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";

import {ConfidencePool} from "../src/ConfidencePool.sol";

import {ConfidencePoolFactory} from "../src/ConfidencePoolFactory.sol";

import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";

import {IBattleChainSafeHarborRegistry} from "@battlechain/interface/IBattleChainSafeHarborRegistry.sol";

import {IAgreement} from "@battlechain/interface/IAgreement.sol";

import {AgreementDetails, BountyTerms, Contact, Chain, Account} from "@battlechain/types/AgreementTypes.sol";

contract MockToken is ERC20 {

constructor() ERC20("Stake", "STK") {}

function mint(address to, uint256 amount) public { _mint(to, amount); }

}

// Unlike the setExpiry PoC's fixed stub, this registry has a MUTABLE state so the test can drive

// the agreement through NEW_DEPLOYMENT -> UNDER_ATTACK -> PRODUCTION and control exactly when the

// pool is (or is not) allowed to observe active risk.

contract MutableAttackRegistry is IAttackRegistry {

ContractState public state = ContractState.NEW_DEPLOYMENT;

function setState(ContractState s) external { state = s; }

function getAgreementState(address) external view returns (ContractState) { return state; }

function registerDeployment(address, address) external {}

function authorizeAgreementOwner(address, address) external {}

function transferAttackModerator(address, address) external {}

function requestUnderAttack(address) external {}

function requestUnderAttackForUnverifiedContracts(address) external {}

function requestUnderAttackByNonAuthorized(address) external {}

function goToProduction(address) external {}

function promote(address) external {}

function cancelPromotion(address) external {}

function markCorrupted(address) external {}

function approveAttack(address) external {}

function rejectAttackRequest(address, bool) external {}

function instantPromote(address) external {}

function instantCorrupt(address) external {}

function changeRegistryModerator(address) external {}

function setSafeHarborRegistry(address) external {}

function registerContractForExistingAgreement(address) external {}

function unregisterContractForExistingAgreement(address) external {}

function syncNewContracts(address) external {}

function isTopLevelContractUnderAttack(address) external pure returns (bool) { return false; }

function getAttackModerator(address) external pure returns (address) { return address(0); }

function getAgreementForContract(address) external pure returns (address) { return address(0); }

function getAgreementInfo(address) external pure returns (AgreementInfo memory info) { return info; }

function getRegistryModerator() external pure returns (address) { return address(0); }

function getSafeHarborRegistry() external pure returns (address) { return address(0); }

function getAuthorizedOwner(address) external pure returns (address) { return address(0); }

}

contract StubSafeHarborRegistry is IBattleChainSafeHarborRegistry {

address public attackRegistryAddr;

constructor(address _attackRegistry) { attackRegistryAddr = _attackRegistry; }

function getAttackRegistry() external view returns (address) { return attackRegistryAddr; }

function isAgreementValid(address) external pure returns (bool) { return true; }

function setAgreementFactory(address) external {}

function adoptSafeHarbor(address) external {}

function getAgreement(address) external pure returns (address) { return address(0); }

function isChainValid(string calldata) external pure returns (bool) { return true; }

function getAgreementFactory() external pure returns (address) { return address(0); }

}

contract StubAgreement is IAgreement {

address public immutable ownerAddr;

constructor(address _owner) { ownerAddr = _owner; }

function owner() external view returns (address) { return ownerAddr; }

function isContractInScope(address) external pure returns (bool) { return true; }

function extendCommitmentWindow(uint256) external {}

function setProtocolName(string memory) external {}

function setContactDetails(Contact[] memory) external {}

function addOrSetChains(Chain[] memory) external {}

function removeChains(string[] memory) external {}

function addAccounts(string memory, Account[] memory) external {}

function removeAccounts(string memory, string[] memory) external {}

function setBountyTerms(BountyTerms memory) external {}

function setAgreementURI(string memory) external {}

function getCantChangeUntil() external pure returns (uint256) { return 0; }

function getDetails() external pure returns (AgreementDetails memory d) { return d; }

function getProtocolName() external pure returns (string memory) { return ""; }

function getBountyTerms() external pure returns (BountyTerms memory b) { return b; }

function getAgreementURI() external pure returns (string memory) { return ""; }

function getRegistry() external pure returns (address) { return address(0); }

function getChainIds() external pure returns (string[] memory arr) { return arr; }

function getBattleChainCaip2ChainId() external pure returns (string memory) { return ""; }

function getBattleChainScopeAddresses() external pure returns (address[] memory arr) { return arr; }

function getBattleChainScopeCount() external pure returns (uint256) { return 0; }

}

contract LazyObservationBonusLossTest is Test {

ConfidencePool pool;

MockToken token;

MutableAttackRegistry attackRegistry;

address sponsor = makeAddr("sponsor");

address recovery = makeAddr("recovery");

address staker = makeAddr("staker");

address bonusDonor = makeAddr("bonusDonor");

function setUp() public {

ConfidencePool poolImpl = new ConfidencePool();

token = new MockToken();

attackRegistry = new MutableAttackRegistry();

StubSafeHarborRegistry safeHarborRegistry = new StubSafeHarborRegistry(address(attackRegistry));

StubAgreement agreement = new StubAgreement(sponsor);

ConfidencePoolFactory factoryImpl = new ConfidencePoolFactory();

bytes memory factoryInit = abi.encodeCall(

ConfidencePoolFactory.initialize, (address(safeHarborRegistry), address(poolImpl), makeAddr("moderator"))

);

ConfidencePoolFactory factory = ConfidencePoolFactory(address(new ERC1967Proxy(address(factoryImpl), factoryInit)));

vm.prank(factory.owner());

factory.setStakeTokenAllowed(address(token), true);

address[] memory accounts = new address[](1);

accounts[0] = address(0x1234);

uint256 expiry = block.timestamp + 60 days;

// recovery address is the sponsor-controlled sweep destination

vm.prank(sponsor);

pool = ConfidencePool(factory.createPool(address(agreement), address(token), expiry, 1 ether, recovery, accounts));

}

/// A real active-risk window opens and closes BETWEEN pool interactions, so `riskWindowStart`

/// is never sealed. The staker held risk for the full term, the agreement survived, yet the

/// entire bonus pool is redirected from the staker to the sponsor's recoveryAddress.

function test_UnobservedRiskWindow_StripsStakerBonus_ToRecovery() public {

// 1. Staker deposits while the registry is still NEW_DEPLOYMENT (deposits allowed; this is

// NOT an active-risk state, so staking does not seal riskWindowStart).

uint256 stakeAmount = 100 ether;

token.mint(staker, stakeAmount);

vm.startPrank(staker);

token.approve(address(pool), stakeAmount);

pool.stake(stakeAmount);

vm.stopPrank();

// A donor funds the bonus pool -- this is the value that should reward the staker.

uint256 bonusAmount = 50 ether;

token.mint(bonusDonor, bonusAmount);

vm.startPrank(bonusDonor);

token.approve(address(pool), bonusAmount);

pool.contributeBonus(bonusAmount);

vm.stopPrank();

assertEq(pool.riskWindowStart(), 0, "sanity: window not sealed after pre-risk deposits");

// 2. A REAL active-risk window opens and closes with NO pool interaction in between.

attackRegistry.setState(IAttackRegistry.ContractState.UNDER_ATTACK);

// ... time passes, attack resolves ...

attackRegistry.setState(IAttackRegistry.ContractState.PRODUCTION);

// 3. First poke happens only AFTER the registry already advanced to PRODUCTION.

// pokeRiskWindow does NOT revert (riskWindowEnd gets sealed), but riskWindowStart stays 0.

pool.pokeRiskWindow();

assertEq(pool.riskWindowStart(), 0, "BUG: risk occurred but riskWindowStart never sealed");

assertGt(pool.riskWindowEnd(), 0, "riskWindowEnd was sealed from the terminal observation");

// 4. Warp past expiry and resolve. PRODUCTION => SURVIVED path; agreement survived the term.

vm.warp(pool.expiry() + 1);

vm.prank(staker);

pool.claimExpired();

// Staker got principal back but ZERO bonus, despite having carried real risk.

assertEq(token.balanceOf(staker), stakeAmount, "staker received principal only -- no bonus");

// 5. The bonus pool sweeps to the sponsor's recoveryAddress instead of the staker.

pool.sweepUnclaimedBonus();

assertEq(token.balanceOf(recovery), bonusAmount, "BUG: full bonus captured by recoveryAddress");

}

Recommended Mitigation

When a terminal state is first observed and no risk window was ever sealed, set riskWindowStart to a conservative lower bound (the observation block). This keeps the bonus flowing to stakers via the amount-weighted globalScore==0 fallback instead of sweeping it to recoveryAddress. Alternatively, explicitly document this "risk-window-shorter-than-poll-interval" case as an accepted residual so integrators can price it in.

function _observePoolState() internal returns (IAttackRegistry.ContractState state) {

state = _getAgreementState();

...

if (riskWindowStart == 0 && _isActiveRiskState(state)) {

_markRiskWindowStart();

}

if (riskWindowEnd == 0 && _isTerminalState(state)) {

+ // A terminal state proves upstream risk existed. If the active-risk window closed

+ // before any pool interaction observed it, seal riskWindowStart to a conservative

+ // lower bound so stakers keep their bonus (amount-weighted via the globalScore==0

+ // fallback) instead of forfeiting it to recoveryAddress.

+ if (riskWindowStart == 0) riskWindowStart = uint32(block.timestamp);

_markRiskWindowEnd();

}

Prize pool breakdown

Total prize

7.25 ETH

nSLOC

589

0 ETH / LOC

High Medium

Low

1.25

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

Support

FAQs

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

Give us feedback!