Clearpool

Description

When the contract is paused (state._isPaused = true), the expected behavior is that sensitive state changes are halted until governance intervention. However, checkpoint() remains callable by any address without pause restrictions. Calling checkpoint() executes _checkpointInterestAndFees(), which updates _exchangeRate, _feesOwedInBase, and _lastAccrualTime based on the current exchange rate.

If the pause was triggered by an out-of-bounds exchange rate in updateExchangeRate, calling checkpoint() during the paused state can still store that invalid rate into contract storage along with associated fee accruals. This undermines the purpose of pausing by committing potentially unsafe values that should remain frozen.

For example:

1. Trigger updateExchangeRate with an out-of-bounds value so the contract pauses.
2. While paused, call checkpoint() from any address.
3. _exchangeRate and accrual data are still updated in storage despite the paused state.

This behavior allows invalid or manipulated data to be persisted, potentially affecting redemptions, pricing, or downstream accounting.

Recommendation

Consider to prevent checkpoint() from executing while the contract is paused, unless called by an explicitly authorized recovery role. This ensures that no invalid exchange rate or accrual data is committed during the paused period, preserving the integrity of the pause mechanism.

Clearpool: Added requiresAuth to restrict the call of this function to controlled addresses. Fixed on commit ID cb325156c14bc83dff16909cf59471e4de31d2d5.

Cantina Managed: Fix verified.

Description

The updateExchangeRate() function stores invalid exchange rates that are subsequently used as the base for all future interest calculations. The function executes _checkpointInterestAndFees() before validation, but always stores the new exchange rate.

When an invalid rate is provided:

1. _checkpointInterestAndFees() executes with the valid old rate
2. Validation fails and the contract pauses
3. The invalid rate is stored
4. Future calls to _checkpointInterestAndFees use this invalid rate as the base

This means any subsequent checkpoint will calculate interest using potentially invalid data, and the error compounds over time.

Recommendation

Consider to enforce pause semantics consistently across all writers:

• Consider to make _checkpointInterestAndFees() a no-op when paused and optionally revert checkpoint() while paused. This requires to remove the the if control pause flow statement of updateExchangeRate() to work.
• Consider to pause-gate any privileged function that indirectly checkpoints or ensure they early-exit without accrual when paused.
• Consider to add tests covering: after a pause, calls to checkpoint() or admin setters do not mutate _exchangeRate, _feesOwedInBase, or _lastAccrualTime until unpaused.

Clearpool: Fixed on commit ID cb325156c14bc83dff16909cf59471e4de31d2d5. The fix erase the pause control flow on updateExchangeRate to let authorized roles to change the exchange rate in case is improperly set on previous interactions. Having the contract paused prevents further operations from happening, but we still allow to store the checkpoint on subsequent calls as it is consider a potential negligible amount that may not disrupt normal operations due to the off-chain monitoring systems in place.

Cantina Managed: Fix verified according to Clearpool team requirements.

Description

Several state-changing paths do not emit dedicated events, reducing observability and making off-chain monitoring and accounting harder:

1. Accrual checkpoints — checkpoint() calls _checkpointInterestAndFees() which mutates _exchangeRate, _feesOwedInBase, and _lastAccrualTime without emitting any event. Accruals triggered indirectly from other functions also leave no checkpoint trace.

2. Share-lock configuration — setShareLockPeriod(uint64) updates shareLockPeriod without emitting an event, despite affecting transfer restrictions (beforeTransfer) and the refundable window used by refundDeposit.

3. Pause via updateExchangeRate — When bounds/time checks fail, updateExchangeRate sets state._isPaused = true but does not emit Paused() (and duplicates pause logic). This creates inconsistent signaling compared to the dedicated pause() function that does emit.

Recommendation

Consider to emit explicit events for these transitions and to reuse existing pause logic to avoid divergence. This would align all pause triggers with consistent on-chain signaling, and improve monitoring of accrual and configuration changes.

Description

setMaxLendingRate only updates the cap and emits MaxLendingRateUpdated. If governance lowers maxLendingRate below the current lendingInfo._lendingRate, the contract continues accruing at the now out-of-policy lending rate until a separate setLendingRate transaction is sent. This creates a window where accruals don’t reflect the newly enforced maximum. Raising the cap is benign.

Recommendation

Consider to align the active lending rate with the new cap in the same transaction when the cap is reduced:

• Finalize accruals up to the change (i.e., checkpoint first).
• Clamp lendingInfo._lendingRate to min(currentRate, newMax).
• Emit a rate-update event to make the adjustment observable.

Alternatively, revert if the new cap is below the current rate unless the call explicitly opts into clamping (e.g., via a parameter), ensuring policy and accrual remain consistent immediately after the cap change.

Summary

when a single request is invalid, instead of skipping it the entire solve will revert.

it is recommended to continue instead.

also noteworthy that transferFrom can also fail so it is best to try-catch around that too.

Description

In AccountantWithRateProviders, the checks for _allowedExchangeRateChangeUpper and _allowedExchangeRateChangeLower use the literal 1e4 as the denominator for basis points calculations. While functionally correct, using a named constant such as BASIS_POINTS improves clarity and maintainability, making it explicit that these values are expressed in basis points.

Recommendation

Consider to replace the raw 1e4 literal with a descriptive constant (e.g., uint256 internal constant BASIS_POINTS = 1e4;) and use it consistently across the codebase for all basis point math. This reduces the risk of misinterpretation and improves code readability.

Description

calculateExchangeRateWithInterest() returns (newRate, interestAccrued), but across its call sites (getRate, getRateSafe, getRateInQuote, previewFeesOwed, and _checkpointInterestAndFees) only newRate is consumed. This implies the function performs extra work (computing interestAccrued) that is not needed for current usage, increasing gas and cognitive load.

Recommendation

Consider to decouple responsibilities so callers compute only what they need. For example, expose a lightweight function that returns just the updated rate, and keep a separate path for callers that require interestAccrued. Align call sites to the minimal interface, reducing redundant calculations and improving readability.

Clearpool: Acknowledged. Decided to keep this value for off-chain and external integration.

Cantina Managed: Acknowledged by Clearpool team.

Description

AtomicQueue prices requests at execution time using the current NAV from the accountant. When there is time between request creation and fulfillment, the exchange rate can drift (e.g., due to continuous interest accrual).

Example scenario:

1. User creates request: “Buy vault shares with 1,000 USDC.”
2. At request time: 1 share = 10 USDC → expects ~100 shares.
3. Later, interest accrual raises NAV: 1 share = 10.50 USDC.
4. On execution, user only receives ~95.2 shares instead of 100.

This creates an expectation gap for flows like “buy shares with USDC,” where appreciating share price means the user ultimately receives fewer shares than anticipated.

In the current Clearpool protocol, the queue is used only for withdrawals (users offer vault shares → receive assets), and the solver role is operated by Clearpool. In this constrained mode, rate drift tends to work in favor of withdrawing users (more assets per share as time passes) and operational risk is reduced by having the protocol as solver. Nevertheless, the mechanism remains sensitive to time-based price movement and would resurface as a user-facing slippage problem if the queue later supports deposits/buy-share intents.

Recommendation

• Consider to encode user price expectations in requests if bi-directional usage is introduced later:

  • Add a per-request minAmountOut (for withdrawals) or maxSharesPerAsset / max acceptable rate (for deposits), enforced at fulfillment.
  • Keep short deadlines to limit drift exposure.

• If the intent is withdraw-only, consider to enforce that at the contract level (restrict allowed offer/want pairs) and document that execution uses real-time NAV so users understand outcomes may improve/change with time.

• For future deposit support, consider UI and solver policies that quote and respect slippage bounds, or snapshot a reference rate at intent time and enforce a tolerance at solve.

Clearpool: Acknowledged. This will be a to-do item if Clearpool supports deposit via AtomicQueue in the future. For now it is not an essential fix.

Cantina Managed: Acknowledged by Clearpool team.

Summary

this (together with the complementary function) only performs bound check, but does not check if _allowedExchangeRateChangeUpper > _allowedExchangeRateChangeLower, basically allowing within-bounds values but in reverse order.