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

`InvalidAmount` Error Used for "Already Claimed" Guard

Author Revealed upon completion

Root + Impact

Description

The claimSurvived() and claimExpired() functions correctly prevent double-claiming via the hasClaimed[msg.sender] state variable. When a staker attempts to claim a second time, the transaction reverts — which is correct behavior. However, the revert reason emitted is InvalidAmount(), which implies the caller passed an incorrect numerical parameter. The actual failure is a state conflict: the caller has already claimed their stake. The InvalidAmount error is overloaded across at least 4 distinct failure modes in the contract, making debugging and off-chain error handling imprecise.

// ConfidencePool.sol — claimSurvived() line 384
function claimSurvived() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED) revert OutcomeNotSet();
@> if (hasClaimed[msg.sender]) revert InvalidAmount(); // <-- NOT an amount issue
uint256 userEligible = eligibleStake[msg.sender];
@> if (userEligible == 0) revert InvalidAmount(); // <-- NOT an amount issue (user passed none)
// ...
}
​
// ConfidencePool.sol — claimExpired() line 578
function claimExpired() external nonReentrant {
// ...
@> if (hasClaimed[msg.sender]) revert InvalidAmount(); // <-- same misleading error
// ...
}

Risk

Likelihood: High

  • Any staker who calls claimSurvived() or claimExpired() twice triggers this revert path

  • Front-end applications parsing revert reasons will display "Invalid Amount" to the user, when the real issue is "Already Claimed"

  • Error handling middleware that categorizes reverts by selector will conflate "already claimed" with "zero amount" and "no stake" — three semantically distinct failures sharing one selector

Impact: Low

  • No funds are lost or locked — the guard correctly prevents double-claims

  • Off-chain systems (indexers, dApps, monitoring) receive misleading error signals

  • Degraded developer experience when debugging failed transactions on block explorers

Proof of Concept

File: L2-InvalidAmount-AlreadyClaimed.poc.t.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
/// @notice PoC for L-2: `InvalidAmount` error used for "already claimed" guard
contract L2_InvalidAmountAlreadyClaimed_POC is BaseConfidencePoolTest {
function testPOC_L2_invalidAmountForAlreadyClaimed_claimSurvived() external {
_stake(alice, 10 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
// First claim succeeds
vm.prank(alice);
pool.claimSurvived();
​
// Second claim reverts with InvalidAmount — NOT AlreadyClaimed or similar
vm.prank(alice);
vm.expectRevert(IConfidencePool.InvalidAmount.selector);
pool.claimSurvived();
}
​
function testPOC_L2_invalidAmountForAlreadyClaimed_claimExpired() external {
_stake(alice, 10 * ONE);
vm.warp(pool.expiry());
​
// First claim resolves + claims
vm.prank(alice);
pool.claimExpired();
​
// Second claim reverts with InvalidAmount — misleading error
vm.prank(alice);
vm.expectRevert(IConfidencePool.InvalidAmount.selector);
pool.claimExpired();
}
}

Run with:

forge test --match-path 'L2-InvalidAmount-AlreadyClaimed.poc.t.sol' -vv

Recommended Mitigation

Introduce a dedicated AlreadyClaimed error and use it exclusively for the hasClaimed[msg.sender] guards in claimSurvived() and claimExpired(). The existing InvalidAmount error is retained for actual amount-validation failures (zero amount passed, below minStake, zero eligible stake in withdraw()), preserving backward compatibility for off-chain systems that already handle those cases. The new error is purely additive — existing callers who only check InvalidAmount continue to see it for amount-related reverts, while systems that want precise error handling can differentiate the "already claimed" case.

// In IConfidencePool.sol and ConfidencePool.sol:
+ error AlreadyClaimed();
​
// claimSurvived() line 384:
- if (hasClaimed[msg.sender]) revert InvalidAmount();
+ if (hasClaimed[msg.sender]) revert AlreadyClaimed();
​
// claimExpired() line 578:
- if (hasClaimed[msg.sender]) revert InvalidAmount();
+ if (hasClaimed[msg.sender]) revert AlreadyClaimed();
​
* if (hasClaimed\[msg.sender]) revert InvalidAmount();
​
- if (hasClaimed\[msg.sender]) revert AlreadyClaimed();
​

Support

FAQs

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

Give us feedback!