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

Redundant Pause Modifiers Duplicate Inherited OpenZeppelin Logic

Author Revealed upon completion

Description

  • ConfidencePool inherits OpenZeppelin Pausable, which already provides whenNotPaused and whenPaused.

  • The pool redeclares equivalent custom modifiers: whenPoolNotPaused and whenPoolPaused. These perform the same state checks as the inherited modifiers, only with different custom errors.

  • This adds unnecessary source complexity and bytecode surface, and creates inconsistent pause error behavior between ConfidencePool and ConfidencePoolFactory, which already uses the inherited whenNotPaused.

// src/ConfidencePool.sol
​
@> modifier whenPoolNotPaused() {
@> if (paused()) revert PoolPaused();
@> _;
@> }
​
// aderyn-ignore-next-line(modifier-used-only-once)
@> modifier whenPoolPaused() {
@> if (!paused()) revert PoolNotPaused();
@> _;
@> }
​
function stake(uint256 amount) external nonReentrant whenPoolNotPaused { ... }
​
function contributeBonus(uint256 amount) external nonReentrant whenPoolNotPaused { ... }
​
function pause() external onlyOwner whenPoolNotPaused {
_pause();
}
​
function unpause() external onlyOwner whenPoolPaused {
_unpause();
}

OpenZeppelin already provides the same guard behavior:

// lib/openzeppelin-contracts/contracts/utils/Pausable.sol
​
@> modifier whenNotPaused() {
@> _requireNotPaused();
@> _;
@> }
​
@> modifier whenPaused() {
@> _requirePaused();
@> _;
@> }

Relevant files:

Risk

Likelihood:

  • This occurs on every function using whenPoolNotPaused or whenPoolPaused.

  • The duplicated modifiers are currently used by stake, contributeBonus, pause, and unpause.

Impact:

  • Slightly larger deployed bytecode and unnecessary custom logic.

  • Inconsistent pause revert selectors across the system: the factory uses OpenZeppelin pause errors, while the pool uses PoolPaused / PoolNotPaused.

  • More maintenance burden because future pause behavior changes would need to account for both custom pool modifiers and inherited OpenZeppelin modifiers.

Proof of Concept

// Both guards enforce the same condition.
​
modifier whenPoolNotPaused() {
if (paused()) revert PoolPaused();
_;
}
​
// Equivalent inherited OpenZeppelin behavior:
modifier whenNotPaused() {
_requireNotPaused(); // reverts when paused() == true
_;
}

The same applies to the paused-only path:

modifier whenPoolPaused() {
if (!paused()) revert PoolNotPaused();
_;
}
​
// Equivalent inherited OpenZeppelin behavior:
modifier whenPaused() {
_requirePaused(); // reverts when paused() == false
_;
}

Recommended Mitigation

Use the inherited OpenZeppelin modifiers directly unless the custom revert selectors are intentionally part of the public error interface.

- modifier whenPoolNotPaused() {
- if (paused()) revert PoolPaused();
- _;
- }
-
- modifier whenPoolPaused() {
- if (!paused()) revert PoolNotPaused();
- _;
- }
​
- function stake(uint256 amount) external nonReentrant whenPoolNotPaused {
+ function stake(uint256 amount) external nonReentrant whenNotPaused {
​
- function contributeBonus(uint256 amount) external nonReentrant whenPoolNotPaused {
+ function contributeBonus(uint256 amount) external nonReentrant whenNotPaused {
​
- function pause() external onlyOwner whenPoolNotPaused {
+ function pause() external onlyOwner whenNotPaused {
​
- function unpause() external onlyOwner whenPoolPaused {
+ function unpause() external onlyOwner whenPaused {

Support

FAQs

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

Give us feedback!