Description
-
Normal behavior: riskWindowStart should represent the first observed active-risk moment, and riskWindowEnd should represent the first observed terminal moment. The k=2 bonus formula is intended to reward earlier at-risk capital more heavily and to pay zero bonus when no active-risk window was ever observed.
-
Specific issue: _observePoolState() writes riskWindowStart and riskWindowEnd independently and never enforces riskWindowStart <= riskWindowEnd. A non-continuous trusted registry cutover can make the pool observe terminal state first and active-risk state later. Once the later active observation writes riskWindowStart > riskWindowEnd, the pool exits the zero-bonus no-risk branch and computes bonus using a stale terminal T, allowing bonus resurrection and later-staker weighting inversion.
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
if (riskWindowStart == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}
function _bonusShare(address u, uint256 userEligible) internal view returns (uint256) {
if (snapshotTotalBonus == 0) return 0;
if (riskWindowStart == 0) return 0;
uint256 T = outcomeFlaggedAt;
uint256 userPlus = T * T * userEligible + userSumStakeTimeSq[u];
uint256 userMinus = 2 * T * userSumStakeTime[u];
uint256 userScore = userPlus > userMinus ? userPlus - userMinus : 0;
uint256 plus = T * T * snapshotTotalStaked + snapshotSumStakeTimeSq;
uint256 minus = 2 * T * snapshotSumStakeTime;
uint256 globalScore = plus > minus ? plus - minus : 0;
if (globalScore == 0) {
return Math.mulDiv(userEligible, snapshotTotalBonus, snapshotTotalStaked);
}
return Math.mulDiv(userScore, snapshotTotalBonus, globalScore);
}
Risk
Likelihood:
-
A trusted DAO / Safe Harbor registry owner performs a non-continuous registry cutover after a pool has already observed terminal state from the previous registry.
-
The replacement registry reports the same agreement as active-risk before terminal-equivalent migration continuity has been established.
Impact:
-
Bonus that would have remained unowed and sweepable to recoveryAddress under terminal-first no-risk semantics becomes payable to stakers.
-
In the stronger case, a later staker receives more bonus than the incumbent because the stale terminal timestamp is earlier than the later active-risk start.
Proof of Concept
Bonus resurrection:
function test_DF_LBP_2_4_1_replacementActiveObservationResurrectsBonusEntitlement() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 100 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
pool.pokeRiskWindow();
uint256 terminalObservedAt = pool.riskWindowEnd();
assertEq(pool.riskWindowStart(), 0, "terminal-first path starts in no-risk bonus branch");
assertGt(terminalObservedAt, 0, "terminal observation is locally recorded");
vm.warp(terminalObservedAt + 100);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
assertGt(pool.riskWindowStart(), terminalObservedAt, "replacement active observation starts risk after terminal");
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
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,
200 * ONE,
"later active observation resurrects full bonus entitlement"
);
}
Later-staker inversion:
function test_F9_reversedRiskChronologyRewardsLaterStaker() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 100 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
pool.pokeRiskWindow();
uint256 terminalObservedAt = pool.riskWindowEnd();
vm.warp(terminalObservedAt + 100);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
uint256 startObservedAt = pool.riskWindowStart();
assertGt(startObservedAt, terminalObservedAt, "migration created start after end");
vm.warp(terminalObservedAt + 200);
_stake(bob, 100 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
uint256 aliceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.claimSurvived();
uint256 alicePayout = token.balanceOf(alice) - aliceBefore;
uint256 bobBefore = token.balanceOf(bob);
vm.prank(bob);
pool.claimSurvived();
uint256 bobPayout = token.balanceOf(bob) - bobBefore;
assertGt(bobPayout, alicePayout, "later staker receives larger payout under reversed chronology");
}
Command run:
forge test --match-path test/unit/KiraMigrationFindings.t.sol --match-test 'test_F9_reversedRiskChronologyRewardsLaterStaker|test_DF_LBP_2_4_1_replacementActiveObservationResurrectsBonusEntitlement' -vv
Exact output:
Missing dependencies found. Installing now...
fatal: destination path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/battlechain-safe-harbor-contracts' already exists and is not an empty directory.
fatal: clone of 'https://github.com/Cyfrin/battlechain-safe-harbor-contracts' into submodule path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/battlechain-safe-harbor-contracts' failed
Failed to clone 'lib/battlechain-safe-harbor-contracts'. Retry scheduled
fatal: destination path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/forge-std' already exists and is not an empty directory.
fatal: clone of 'https://github.com/foundry-rs/forge-std' into submodule path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/forge-std' failed
Failed to clone 'lib/forge-std'. Retry scheduled
fatal: destination path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/openzeppelin-contracts' already exists and is not an empty directory.
fatal: clone of 'https://github.com/OpenZeppelin/openzeppelin-contracts' into submodule path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/openzeppelin-contracts' failed
Failed to clone 'lib/openzeppelin-contracts'. Retry scheduled
fatal: destination path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/openzeppelin-contracts-upgradeable' already exists and is not an empty directory.
fatal: clone of 'https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable' into submodule path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/openzeppelin-contracts-upgradeable' failed
Failed to clone 'lib/openzeppelin-contracts-upgradeable'. Retry scheduled
fatal: destination path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/battlechain-safe-harbor-contracts' already exists and is not an empty directory.
fatal: clone of 'https://github.com/Cyfrin/battlechain-safe-harbor-contracts' into submodule path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/battlechain-safe-harbor-contracts' failed
Failed to clone 'lib/battlechain-safe-harbor-contracts' a second time, aborting
Warning: Your project has missing dependencies that could not be installed.
No files changed, compilation skipped
Ran 2 tests for test/unit/KiraMigrationFindings.t.sol:KiraMigrationFindingsTest
[PASS] test_DF_LBP_2_4_1_replacementActiveObservationResurrectsBonusEntitlement() (gas: 531634)
[PASS] test_F9_reversedRiskChronologyRewardsLaterStaker() (gas: 683347)
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 1.85ms (765.39µs CPU time)
Ran 1 test suite in 11.69ms (1.85ms CPU time): 2 tests passed, 0 failed, 0 skipped (2 total tests)
Recommended Mitigation
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
if (
!scopeLocked && state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
) {
scopeLocked = true;
emit ScopeLocked(block.timestamp);
}
- if (riskWindowStart == 0 && _isActiveRiskState(state)) {
+ if (riskWindowEnd != 0 && _isActiveRiskState(state)) {
+ revert InvalidRiskWindowChronology();
+ }
+ if (riskWindowStart == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}
Also reject new deposits after any local terminal observation has been recorded, and require registry migration tooling to preserve per-agreement state continuity before activating a new AttackRegistry pointer.
