Skip to content

Incorrect previewFloatingAssetsAverage Calculation in Previewer.sol #814

Description

@parv1305
Repository exactly/protocol
File contracts/periphery/Previewer.sol
Function previewFloatingAssetsAverage(Market market, uint256 backupEarnings)
Lines 568–582
Bug class Operator-precedence error / incorrect formula reimplementation
Severity Low (view-only helper, no fund-loss path; downstream slippage checks bound the practical impact)
Status Confirmed present, not patched

Summary

Previewer.sol reimplements the protocol's exponential-decay formula for floatingAssetsAverage instead of reusing the correct, canonical implementation already defined in MarketBase.sol. In doing so, it drops a set of parentheses around a ternary expression. Because Solidity's * operator binds tighter than ?:, the elapsed-time term is silently excluded from one of the two branches of the formula. The function returns an incorrect value whenever the "damp down" branch is taken, which is a common, not an edge-case, condition (any time floatingAssets has recently fallen below its own average).


The Correct Formula (reference implementation)

MarketBase.sol — used by Market.sol itself for real, on-chain accounting — computes this correctly:

function previewFloatingAssetsAverage() public view returns (uint256) {
  uint256 memFloatingAssets = floatingAssets;
  uint256 memFloatingAssetsAverage = floatingAssetsAverage;
  uint256 dampSpeedFactor = memFloatingAssets < memFloatingAssetsAverage ? dampSpeedDown : dampSpeedUp;
  uint256 averageFactor = uint256(1e18 - (-int256(dampSpeedFactor * (block.timestamp - lastAverageUpdate))).expWad());
  return memFloatingAssetsAverage.mulWadDown(1e18 - averageFactor) + averageFactor.mulWadDown(memFloatingAssets);
}

Note the two-step structure: the ternary picks a single scalar dampSpeedFactor first, and only then is that scalar multiplied by the elapsed time (block.timestamp - lastAverageUpdate). This is also confirmed identical in InterestRateModel.sol's own copy of the same formula, and matches the live, Etherscan-verified deployed Market contract.


The Buggy Code

// contracts/periphery/Previewer.sol, lines 568–582
function previewFloatingAssetsAverage(Market market, uint256 backupEarnings) internal view returns (uint256) {
  uint256 memFloatingAssets = market.floatingAssets() + backupEarnings;
  uint256 memFloatingAssetsAverage = market.floatingAssetsAverage();
  uint256 averageFactor = uint256(
    1e18 -
      (
        -int256(
          memFloatingAssets < memFloatingAssetsAverage
            ? market.dampSpeedDown()
            : market.dampSpeedUp() * (block.timestamp - market.lastAverageUpdate())
        )
      ).expWad()
  );
  return memFloatingAssetsAverage.mulWadDown(1e18 - averageFactor) + averageFactor.mulWadDown(memFloatingAssets);
}

Root cause

Because * has higher precedence than the ternary ?:, the expression

memFloatingAssets < memFloatingAssetsAverage
  ? market.dampSpeedDown()
  : market.dampSpeedUp() * (block.timestamp - market.lastAverageUpdate())

parses as:

condition ? dampSpeedDown() : (dampSpeedUp() * deltaTime)

The multiplication by deltaTime (block.timestamp - market.lastAverageUpdate()) applies only to the dampSpeedUp() branch. When the condition is true — i.e. memFloatingAssets < memFloatingAssetsAverage, the "damp down" case — the raw, un-scaled dampSpeedDown() rate constant is used directly as the exponent input, as if deltaTime were implicitly 1, regardless of how much time has actually elapsed since lastAverageUpdate.

Minimal fix

Wrap the ternary so the whole selected value is multiplied by deltaTime, mirroring MarketBase.sol:

uint256 averageFactor = uint256(
  1e18 -
    (
      -int256(
        (memFloatingAssets < memFloatingAssetsAverage ? market.dampSpeedDown() : market.dampSpeedUp()) *
          (block.timestamp - market.lastAverageUpdate())
      )
    ).expWad()
);

Where the bug is reachable

previewFloatingAssetsAverage is called from four places inside Previewer.sol, meaning the incorrect value propagates into every one of the following public preview functions:

Call site (line) Public function affected
206 previewDepositAtMaturity(Market, uint256 maturity, uint256 assets)
253 previewBorrowAtMaturity(Market, uint256 maturity, uint256 assets)
392 the internal helper used by previewWithdrawAtMaturity / previewRepayAtMaturity
415 fixedPools(Market) — the pool-by-pool utilization/rate table

All of these are read-only functions intended to show users (typically via the front-end) an accurate preview of the fixed-rate/utilization/yield they'd get before they submit a real transaction.


Impact

  • Incorrect previewed values. Whenever floatingAssets < floatingAssetsAverage (a common state — any time deposits/withdrawals have recently pushed floating assets below their smoothed average, e.g. after net withdrawal pressure), the previewed floatingAssetsAverage is computed using the wrong decay factor. This produces wrong utilization figures and wrong quoted fixed rates/yields in previewDepositAtMaturity, previewBorrowAtMaturity, and fixedPools.
  • No direct fund-loss path. Previewer.sol is a read-only contract; it is never called by Market.sol, Auditor.sol, or any state-changing function. The real on-chain accounting (MarketBase.previewFloatingAssetsAverage) is unaffected and correct.
  • Bounded downstream risk. Every state-changing fixed-pool function on Market.sol (depositAtMaturity, withdrawAtMaturity, borrowAtMaturity, repayAtMaturity) independently enforces a caller-supplied minAssetsRequired/maxAssets bound and reverts with Disagreement() if the real, correctly-computed on-chain outcome doesn't satisfy it. So even a materially wrong preview cannot force a worse-than-authorized execution — worst case is an unnecessary revert, or a user accepting a quote that under/overstates their actual expected rate before signing.
  • UX/trust impact. Front-ends and any off-chain tooling relying on this preview to display APY, expected proceeds, or "you will receive X" figures will show numbers that diverge from what the protocol will actually execute.

Severity Justification: Low

Factor Assessment
Fund loss None — no state-changing path is affected
Exploitability Not adversarially exploitable — no attacker action extracts value; it's a passive miscalculation
Trigger condition Common, not an edge case (any time floating assets dip below their average)
Blast radius Limited to display/preview accuracy in Previewer.sol consumers (front-end, integrators)
Mitigating control On-chain Disagreement() slippage checks in Market.sol bound the practical consequence to failed transactions, not bad executions

Recommendation

  1. Apply the one-line parenthesization fix shown above.
  2. Preferably, refactor Previewer.sol to call market.previewFloatingAssetsAverage() directly (which already exists as a public function on MarketBase.sol) rather than reimplementing the formula a second time — eliminating the possibility of this class of duplication drift entirely.
  3. Add a unit test asserting Previewer.previewFloatingAssetsAverage(...) output matches Market.previewFloatingAssetsAverage() output for cases where floatingAssets < floatingAssetsAverage at varying deltaTime values (the current gap would not be caught by tests that only exercise deltaTime == 0 or the dampSpeedUp branch).

Confirmed by direct comparison against MarketBase.sol (same repository), InterestRateModel.sol (same repository), and the live Etherscan-verified deployed Market contract bytecode, all three of which implement the formula correctly.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions