fix(oracles): UniswapV3Oracle quote-feed circuit breaker + setTwapPeriod cardinality recheck

src/oracles/UniswapV3Oracle.sol

@@ -59,6 +59,18 @@ interface IERC20Decimals {
     function decimals() external view returns (uint8);
 }
 
+/// @title IChainlinkAggregatorBounds
+/// @notice Circuit-breaker bounds exposed by Chainlink's underlying off-chain aggregator. Mirrors
+///         the same interface in {ChainlinkOracle}: a consumer proxy forwards reads to its current
+///         `aggregator()`, which carries immutable `minAnswer`/`maxAnswer` bounds. During an extreme
+///         dislocation the aggregator clamps its reported answer to a bound, so a price sitting at a
+///         bound is a saturated (untrustworthy) reading rather than a true market price.
+interface IChainlinkAggregatorBounds {
+    function aggregator() external view returns (address);
+    function minAnswer() external view returns (int192);
+    function maxAnswer() external view returns (int192);
+}
+
 /// @title UniswapV3Oracle
 /// @notice Price oracle using Uniswap V3 TWAP
 /// @dev Uses time-weighted average prices from Uniswap V3 pools
@@ -133,6 +145,12 @@ contract UniswapV3Oracle is IPriceOracle, IPriceOracleAdmin, Ownable {
     /// @notice Required time the sequencer must have been up before prices are accepted.
     uint256 public sequencerGracePeriod;
 
+    /// @notice Tokens with a configured pool, tracked so {setTwapPeriod} can re-grow every pool's
+    ///         observation buffer when the period is raised (F2). Append-only; entries whose feed was
+    ///         later removed are skipped at use (`poolConfigs[token].pool == 0`).
+    address[] private _configuredTokens;
+    mapping(address => bool) private _tokenTracked;
+
     // ═══════════════════════════════════════════════════════════════════════════
     // ERRORS
     // ═══════════════════════════════════════════════════════════════════════════
@@ -147,6 +165,10 @@ contract UniswapV3Oracle is IPriceOracle, IPriceOracleAdmin, Ownable {
     ///         so the TWAP would degenerate toward manipulable spot. `have` < `need`.
     error InsufficientObservationCardinality(address pool, uint16 have, uint16 need);
 
+    /// @notice Quote-token Chainlink answer sits at the aggregator's min/maxAnswer circuit-breaker
+    ///         bound, i.e. the feed is saturated and the value is not a trustworthy market price.
+    error AnswerOutOfBounds(address token, int256 answer, int192 minAnswer, int192 maxAnswer);
+
     event SequencerUptimeFeedConfigured(address indexed feed, uint256 gracePeriod);
 
     // ═══════════════════════════════════════════════════════════════════════════
@@ -289,6 +311,12 @@ contract UniswapV3Oracle is IPriceOracle, IPriceOracleAdmin, Ownable {
             quoteTokenFeeds[quoteToken] = quoteFeed;
         }
 
+        // F2: remember the token so a later setTwapPeriod can re-validate/grow this pool's buffer.
+        if (!_tokenTracked[token]) {
+            _tokenTracked[token] = true;
+            _configuredTokens.push(token);
+        }
+
         emit PriceFeedConfigured(token, pool);
     }
 
@@ -319,10 +347,23 @@ contract UniswapV3Oracle is IPriceOracle, IPriceOracleAdmin, Ownable {
     }
 
     /// @notice Set TWAP observation period
+    /// @dev F2: a longer period needs a deeper observation ring buffer. `configurePool` grows the
+    ///      buffer for the period in effect at config time, but raising the period here previously
+    ///      skipped that check — an already-configured pool could then no longer span the window and
+    ///      `observe()` would revert (price unavailable). Re-ensure (and opportunistically grow)
+    ///      every configured pool for the new period; reverts `InsufficientObservationCardinality`
+    ///      if a pool cannot be grown to support `period`, so an unsupportable period is rejected.
     /// @param period New TWAP period in seconds
     function setTwapPeriod(uint32 period) external onlyOwner {
         require(period > 0, "Invalid period");
         twapPeriod = period;
+
+        uint256 len = _configuredTokens.length;
+        for (uint256 i = 0; i < len; i++) {
+            address pool = poolConfigs[_configuredTokens[i]].pool;
+            if (pool == address(0)) continue; // feed since removed
+            _ensureObservationCardinality(IUniswapV3Pool(pool));
+        }
     }
 
     /// @notice Configure the L2 sequencer uptime feed. Set to `address(0)` to disable on L1.
@@ -383,6 +424,12 @@ contract UniswapV3Oracle is IPriceOracle, IPriceOracleAdmin, Ownable {
             if (answeredInRound < roundId) revert StalePrice(token, updatedAt, maxAge);
             if (block.timestamp - updatedAt > maxAge) revert StalePrice(token, updatedAt, maxAge);
 
+            // F1: circuit breaker — reject a quote answer clamped to the aggregator's min/maxAnswer
+            // bound. A saturated quote feed during an extreme dislocation would otherwise propagate a
+            // pinned (floor/ceiling) value through the TWAP→USD conversion, mis-pricing the asset
+            // (under-/over-valuing exposure and slashing). Parity with ChainlinkOracle.
+            _checkAnswerBounds(token, quoteFeed, answer);
+
             uint8 feedDecimals = AggregatorV3Interface(quoteFeed).decimals();
             if (feedDecimals < 18) {
                 // forge-lint: disable-next-line(unsafe-typecast)
@@ -416,6 +463,32 @@ contract UniswapV3Oracle is IPriceOracle, IPriceOracleAdmin, Ownable {
         data.isValid = true;
     }
 
+    /// @dev Revert when `answer` is pinned to the quote feed's underlying circuit-breaker bound.
+    ///      Bounds live on the proxy's current `aggregator()`, not the proxy, so resolve it first.
+    ///      Feeds that do not expose these getters are skipped via try/catch — the sign/staleness
+    ///      checks at the call site still apply. Identical logic to {ChainlinkOracle._checkAnswerBounds}.
+    function _checkAnswerBounds(address token, address feed, int256 answer) internal view {
+        try IChainlinkAggregatorBounds(feed).aggregator() returns (address agg) {
+            if (agg == address(0)) return;
+            try IChainlinkAggregatorBounds(agg).minAnswer() returns (int192 minAnswer) {
+                try IChainlinkAggregatorBounds(agg).maxAnswer() returns (int192 maxAnswer) {
+                    if (minAnswer < maxAnswer && (answer <= minAnswer || answer >= maxAnswer)) {
+                        revert AnswerOutOfBounds(token, answer, minAnswer, maxAnswer);
+                    }
+                } catch { }
+            } catch { }
+        } catch {
+            // Some proxies expose minAnswer()/maxAnswer() directly without aggregator().
+            try IChainlinkAggregatorBounds(feed).minAnswer() returns (int192 minAnswer) {
+                try IChainlinkAggregatorBounds(feed).maxAnswer() returns (int192 maxAnswer) {
+                    if (minAnswer < maxAnswer && (answer <= minAnswer || answer >= maxAnswer)) {
+                        revert AnswerOutOfBounds(token, answer, minAnswer, maxAnswer);
+                    }
+                } catch { }
+            } catch { }
+        }
+    }
+
     /// @dev Reverts if the sequencer feed (when configured) reports the L2 sequencer
     ///      as down or recently restarted within `sequencerGracePeriod`. No-op on L1.
     function _requireSequencerUp() internal view {

test/audit/medlow/Oracles.t.sol

@@ -195,7 +195,11 @@ contract ChainlinkOracleBoundsTest is Test {
 
         vm.expectRevert(
             abi.encodeWithSelector(
-                ChainlinkOracle.AnswerOutOfBounds.selector, address(token), int256(1000e8), int192(1000e8), int192(9000e8)
+                ChainlinkOracle.AnswerOutOfBounds.selector,
+                address(token),
+                int256(1000e8),
+                int192(1000e8),
+                int192(9000e8)
             )
         );
         oracle.getPrice(address(token));
@@ -208,7 +212,11 @@ contract ChainlinkOracleBoundsTest is Test {
 
         vm.expectRevert(
             abi.encodeWithSelector(
-                ChainlinkOracle.AnswerOutOfBounds.selector, address(token), int256(9000e8), int192(1000e8), int192(9000e8)
+                ChainlinkOracle.AnswerOutOfBounds.selector,
+                address(token),
+                int256(9000e8),
+                int192(1000e8),
+                int192(9000e8)
             )
         );
         oracle.getPrice(address(token));
@@ -461,3 +469,122 @@ contract UniswapV3OracleZeroPriceTest is Test {
         oracle.getPriceData(address(token));
     }
 }
+
+// ─────────────────────────────────────────────────────────────────────────────
+// UniswapV3Oracle: quote-feed min/maxAnswer circuit breaker (F1, medium)
+// ─────────────────────────────────────────────────────────────────────────────
+
+contract UniswapV3OracleQuoteBoundsTest is Test {
+    UniswapV3Oracle internal oracle;
+    MockCardinalityPool internal pool;
+    MockToken internal token; // token0, 6 dec
+    MockToken internal quote; // token1, 18 dec
+    MockBoundedAggregator internal quoteFeed;
+
+    function setUp() public {
+        vm.warp(1_000_000);
+        token = new MockToken(6);
+        quote = new MockToken(18);
+        pool = new MockCardinalityPool(address(token), address(quote), 200, true);
+        pool.setTick(0);
+        quoteFeed = new MockBoundedAggregator(8);
+        quoteFeed.setRound(1, 1);
+        quoteFeed.setData(1800e8, block.timestamp);
+        oracle = new UniswapV3Oracle(address(quote));
+        oracle.configurePool(address(token), address(pool), address(quoteFeed), false);
+    }
+
+    /// F1: a quote answer pinned to the aggregator's minAnswer floor is rejected (parity with
+    /// ChainlinkOracle). Pre-fix the Uniswap quote-feed path had no bounds check, so a saturated
+    /// quote feed would propagate a pinned floor/ceiling through the TWAP→USD conversion.
+    function test_F1_RevertWhenQuoteAnswerAtMinBound() public {
+        quoteFeed.setBounds(int192(1000e8), int192(9000e8));
+        quoteFeed.setData(1000e8, block.timestamp);
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                UniswapV3Oracle.AnswerOutOfBounds.selector,
+                address(token),
+                int256(1000e8),
+                int192(1000e8),
+                int192(9000e8)
+            )
+        );
+        oracle.getPrice(address(token));
+    }
+
+    function test_F1_RevertWhenQuoteAnswerAtMaxBound() public {
+        quoteFeed.setBounds(int192(1000e8), int192(9000e8));
+        quoteFeed.setData(9000e8, block.timestamp);
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                UniswapV3Oracle.AnswerOutOfBounds.selector,
+                address(token),
+                int256(9000e8),
+                int192(1000e8),
+                int192(9000e8)
+            )
+        );
+        oracle.getPrice(address(token));
+    }
+
+    /// A quote answer strictly inside [min, max] still prices normally.
+    function test_F1_QuoteAnswerWithinBoundsAccepted() public {
+        quoteFeed.setBounds(int192(1000e8), int192(9000e8));
+        quoteFeed.setData(1800e8, block.timestamp);
+        assertGt(oracle.getPrice(address(token)), 0);
+    }
+}
+
+// ─────────────────────────────────────────────────────────────────────────────
+// UniswapV3Oracle: setTwapPeriod re-validates observation cardinality (F2, medium)
+// ─────────────────────────────────────────────────────────────────────────────
+
+contract UniswapV3OracleSetTwapCardinalityTest is Test {
+    UniswapV3Oracle internal oracle;
+    MockToken internal token; // 6 dec
+    MockToken internal quote; // 18 dec
+    MockBoundedAggregator internal quoteFeed;
+
+    function setUp() public {
+        vm.warp(1_000_000);
+        token = new MockToken(6);
+        quote = new MockToken(18);
+        quoteFeed = new MockBoundedAggregator(8);
+        quoteFeed.setRound(1, 1);
+        quoteFeed.setData(1800e8, block.timestamp);
+        oracle = new UniswapV3Oracle(address(quote));
+    }
+
+    /// F2: raising the TWAP period re-grows an already-configured (growable) pool's observation
+    /// buffer to span the new window. Pre-fix setTwapPeriod skipped the cardinality check entirely.
+    function test_F2_SetTwapPeriodGrowsConfiguredPool() public {
+        MockCardinalityPool growable = new MockCardinalityPool(address(token), address(quote), 200, true);
+        growable.setTick(0);
+        // Configures fine at the default 1800s period (needs ceil(1800/12)=150 <= 200).
+        oracle.configurePool(address(token), address(growable), address(quoteFeed), false);
+
+        // Raise to 36000s -> needs ceil(36000/12) = 3000 slots.
+        oracle.setTwapPeriod(36_000);
+
+        (,,, uint16 card, uint16 cardNext,,) = growable.slot0();
+        assertGe(uint256(card >= cardNext ? card : cardNext), 3000, "pool grown to cover the raised period");
+    }
+
+    /// F2: raising the period beyond a non-growable pool's buffer is rejected, instead of silently
+    /// leaving a pool whose `observe()` over the new window would revert (price unavailable).
+    function test_F2_SetTwapPeriodRevertsWhenPoolCannotGrow() public {
+        MockCardinalityPool fixedPool = new MockCardinalityPool(address(token), address(quote), 200, false);
+        fixedPool.setTick(0);
+        oracle.configurePool(address(token), address(fixedPool), address(quoteFeed), false);
+
+        vm.expectRevert(
+            abi.encodeWithSelector(
+                UniswapV3Oracle.InsufficientObservationCardinality.selector,
+                address(fixedPool),
+                uint16(200),
+                uint16(3000)
+            )
+        );
+        oracle.setTwapPeriod(36_000);
+    }
+}