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

Reverting ATTACK_REQUESTED Observation Fails to Persist Scope Lock

Author Revealed upon completion

Description

The pool scope is intended to become immutable once the agreement registry leaves pre-attack staging. pokeRiskWindow() is also documented as a permissionless keeper hook that can observe registry transitions and seal the relevant one-way markers.

The issue is that pokeRiskWindow() calls _observePoolState() and then reverts with RiskWindowNotReached when the registry is ATTACK_REQUESTED. Because the revert rolls back all state changes, the scopeLocked = true write made inside _observePoolState() is not persisted. If the registry later returns to a pre-attack state without any successful pool interaction in between, the owner can still change the scope even though the registry had already left pre-attack staging.

function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
if (
!scopeLocked && state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
) {
// @> This write is rolled back when the caller later reverts
scopeLocked = true;
emit ScopeLocked(block.timestamp);
}
...
}
​
function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return;
_observePoolState();
​
// @> ATTACK_REQUESTED locks no risk-window marker, so the call reverts and undoes scopeLocked
if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached();
}
​
function setPoolScope(address[] calldata accounts) external onlyOwner {
_observePoolState();
if (scopeLocked) revert ScopePostLockImmutable();
_replaceScope(accounts);
}

Risk

Likelihood:

  • This occurs when the agreement enters ATTACK_REQUESTED and the only pool observation during that state is a reverting call such as pokeRiskWindow().

  • This occurs when the registry later returns to NEW_DEPLOYMENT or NOT_DEPLOYED before any successful pool interaction has persisted scopeLocked.

Impact:

  • The owner can change the pool's scope after the agreement already left pre-attack staging, breaking the stated scope immutability point.

  • Existing stakers may have committed funds under one published scope while a different scope is later installed.

Proof of Concept

Add this test to test/unit/ConfidencePool.scope.t.sol:

function testRevertingAttackRequestedObservationDoesNotPersistScopeLock() external {
_seedAgreementScope();
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.ATTACK_REQUESTED);
​
vm.expectRevert(IConfidencePool.RiskWindowNotReached.selector);
pool.pokeRiskWindow();
assertFalse(pool.scopeLocked());
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.NEW_DEPLOYMENT);
pool.setPoolScope(_multiAccountScope());
​
assertFalse(pool.isAccountInScope(DEFAULT_SCOPE_ACCOUNT));
assertTrue(pool.isAccountInScope(ACCOUNT_X));
assertTrue(pool.isAccountInScope(ACCOUNT_Y));
assertTrue(pool.isAccountInScope(ACCOUNT_Z));
}

Run:

forge test --match-test testRevertingAttackRequestedObservationDoesNotPersistScopeLock -vvv

Expected result:

[PASS] testRevertingAttackRequestedObservationDoesNotPersistScopeLock()

Recommended Mitigation

Allow pokeRiskWindow() to succeed when it observes and persists a scope lock, even when no risk-window marker is sealed.

function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return;
+ bool wasScopeLocked = scopeLocked;
_observePoolState();
- if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached();
+ if (riskWindowStart == 0 && riskWindowEnd == 0 && scopeLocked == wasScopeLocked) {
+ revert RiskWindowNotReached();
+ }
}

Alternatively, document that scope locking only persists after a successful pool transaction and does not necessarily happen at the first attempted observation.

Support

FAQs

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

Give us feedback!