[VPD-349] PriceDeviationSentinel Contract by web3rover · Pull Request #24 · VenusProtocol/venus-periphery

web3rover · 2025-12-08T04:44:18Z

Summary

Add DeviationSentinel contract that monitors price deviations between ResilientOracle and SentinelOracle, automatically pausing borrow/supply and zeroing collateral factors when large
deviations are detected, and restoring them when prices normalize
Add SentinelOracle, PancakeSwapOracle, and UniswapOracle as on-chain price sources for deviation comparison
Add comprehensive unit tests, fork tests, and deployment scripts for BSC testnet

Problem

Price manipulation or oracle lag can leave markets exposed. The DeviationSentinel provides an automated circuit breaker that a trusted keeper can trigger when oracle prices diverge beyond a
configured threshold.

How It Works

Deviation detected → Sentinel stores current CF/LT in marketStates, sets CF to 0, and pauses supply/borrow
Deviation resolves → Sentinel restores stored CF/LT and unpauses actions
Protection against stale state → If governance creates new e-mode pools or changes CF while the market is paused, the restore logic skips uninitialized pools (storedCF == 0 && currentCF != 0) and avoids restoring LT = 0 to prevent liquidation risk

DeviationSentinel State Conflicts with Governance Actions

A conflict arises when governance (VIP) needs to modify a market's parameters (CF, LT, pause state, or e-mode configuration) while DeviationSentinel has an active deviation state for that market.

Scenario A — Governance changes CF without calling `resetMarketState`

Deviation detected → Sentinel stores CF = 80%, sets CF = 0, pauses supply
Governance VIP sets CF = 60% (new risk parameter) directly on the comptroller
Deviation resolves → Sentinel restores CF = 80% (the stale stored value)
Result: The governance-intended CF = 60% is silently overwritten with the old 80% value — a security risk where the protocol operates with a higher CF than governance approved

Scenario B — Governance calls `resetMarketState` but doesn't handle pause state

Deviation detected → Sentinel stores CF = 80%, sets CF = 0, pauses supply
Governance calls resetMarketState → clears all stored state (borrowPaused = false, cfModifiedAndSupplyPaused = false, stored CFs cleared)
Governance VIP sets CF = 60%
Deviation resolves → Sentinel sees borrowPaused = false and cfModifiedAndSupplyPaused = false, so it does nothing
Result: Supply remains paused with no automated recovery path via the Sentinel if governance didn't manually unpause

Scenario C — E-mode changes (add/remove pool or market)

Sentinel has stored CFs for pool IDs [1, 2, 3]
Governance creates a new e-mode pool (ID 4) or removes market from pool 2
On restore, Sentinel iterates corePoolId() to lastPoolId() — it may try to restore a CF for a pool the market is no longer in, or skip a newly added pool, leading to incorrect state

Required Governance Procedure

When governance needs to intervene on a market with an active deviation state, the VIP must follow this exact order:

resetMarketState(market) — Clears all stored state (pause flags, stored CFs/LTs). Note: this clears the sentinel's tracking but does not unpause on the comptroller
Make all protocol changes (new CF/LT values, e-mode pool changes)
Unpause supply/borrow on the comptroller manually — Since resetMarketState only clears sentinel state, governance must explicitly unpause the actions that the sentinel had paused
handleDeviation(market) — Re-evaluates the market against the current oracle prices with a clean slate

Why This Procedure Works in Every Scenario

By explicitly unpausing and then calling handleDeviation, the sentinel captures the fresh governance-intended state:

Deviation still exists at VIP execution → handleDeviation stores the NEW CF values (set by governance in step 3), sets CF to 0, and re-pauses the market. When the deviation resolves later,
the sentinel restores the correct governance-intended CF. ✓
Deviation already resolved by VIP execution → handleDeviation is a no-op since no deviation is detected. The market stays unpaused with the new CF values. ✓
E-mode pool changes (Scenario C) → handleDeviation iterates the updated pool list (corePoolId() to lastPoolId()) and stores the correct new pool configuration. ✓

The market being re-paused when a deviation still exists is the correct behavior — the sentinel should continue protecting the market, just with the updated parameters.

Contracts Added

Contract	Description
`DeviationSentinel`	Core sentinel logic — deviation detection, pause/unpause, CF store/restore
`SentinelOracle`	Aggregates multiple oracle sources for sentinel price comparison
`PancakeSwapOracle`	TWAP oracle using PancakeSwap V3 pools
`UniswapOracle`	TWAP oracle using Uniswap V3 pools

Test Plan

Unit tests for DeviationSentinel (deviation handling, CF restore, new pool protection, LT=0 edge case)
Unit tests for PancakeSwapOracle, UniswapOracle, SentinelOracle
Fork tests for DeviationSentinel on BSC
Deployed and verified on BSC testnet

NOTE: As discussed with @fred-venus Will work on this comment in future versions.

fred-venus · 2025-12-08T14:45:57Z

I am thinking should we put it governance repo like https://github.com/VenusProtocol/governance-contracts/pull/163/files

contracts/PriceDeviationSentinel/PriceDeviationSentinel.sol

contracts/DeviationSentinel/DeviationSentinel.sol

contracts/PriceDeviationSentinel/PriceDeviationSentinel.sol

contracts/DeviationSentinel/DeviationSentinel.sol

contracts/PriceDeviationSentinel/PriceDeviationSentinel.sol

This reverts commit cffb86f.

Debugger022 · 2026-01-19T14:49:57Z

contracts/DeviationSentinel/Oracles/SentinelOracle.sol

+    /// @param asset Address of the asset
+    /// @return price Price in (36 - asset decimals) format, same as ResilientOracle
+    /// @custom:error TokenNotConfigured is thrown when asset has no oracle configured
+    function getPrice(address asset) external view returns (uint256 price) {


Both the PancakeSwapOracle and UniswapOracle contracts share the same core logic, so we could consolidate them into a single common oracle contract instead of maintaining two separate ones.

Additionally, we could further improve security by aggregating and comparing price data from multiple DEXs, rather than relying on a single source.

Yeah, i agree both points. That's for sure something we could do, together with what u mentioned TokenConfig has only one field, i am thinking we can have more fields:

we could support amm pools array for one asset and each pool attached with a weight, and get price can simply return:
price1 * weight1 + price2 * weight2 etc..

wdty ?

yeah it's sounds good

contracts/DeviationSentinel/Oracles/SentinelOracle.sol

- Remove duplicate CollateralFactorUpdated event overload for isolated pools, consolidate into single event with poolId param - Rewrite DeviationSentinel unit tests with comprehensive coverage: zero-address constructor checks, emode pool iteration, CF caching/restore, deviation direction flips, and sentinel direct price scenarios - Expand PancakeSwapOracle and UniswapOracle test suites with additional edge cases - Add thorough fork test suite covering end-to-end deviation handling on BSC mainnet

[VPD-442] Hashdit Findings for Emergency Pause

[VPD-432] Certik Audit Fixes for Emergency Pause

Conditionally set proxy owner to deployer instead of timelock on local hardhat networks for PancakeSwapOracle and UniswapOracle, matching the pattern already used by SentinelOracle and DeviationSentinel. Also remove waitConfirmations and inline verify calls from the deploy script.

- Replace mock-based tests with actual BSC mainnet fork tests - Use deployed contract addresses from bscmainnet_addresses.json - Execute VIP-900 to set up proper permissions during test setup - Extend Chainlink staleness period for BTCB to handle fork test timing - Cache oracle price at setup to avoid staleness issues during tests - Change test asset from vBNB to vBTCB (vBNB wraps native BNB with no ERC20 underlying) - Add mocha timeout configuration (200s) for fork tests in hardhat.config.ts - Fix linter errors for unused variables

GitGuru7

implementation LGTM, left few nitpicks

GitGuru7 · 2026-02-05T11:15:52Z

contracts/DeviationSentinel/DeviationSentinel.sol

+    /// @param isTrusted Whether the keeper should be trusted
+    /// @custom:event Emits TrustedKeeperUpdated event
+    /// @custom:error ZeroAddress is thrown when keeper address is zero
+    function setTrustedKeeper(address keeper, bool isTrusted) external {


we can consider reverting when isTrusted is unchanged to avoid unnecessary storage writes and also emit the previous value for better tracking.

applicable to other similar setters

The current implementation is correct and functional. Your suggestions are valid optimizations:

Reverting on unchanged value saves gas by avoiding redundant SSTORE operations

Emitting previous value improves off-chain tracking and event indexing

However, the current simpler approach has benefits:

Idempotent behavior (no unexpected reverts)

Less code complexity

Callers don't need to check current state before calling

Since the setter will primarily be called by a whitelisted address, we can assume incorrect parameters won’t be sent. Given that the code is already deployed on mainnet, I’d suggest skipping this. @fred-venus WDYT?T?

i am open, can skip

GitGuru7 · 2026-02-05T11:52:18Z

contracts/DeviationSentinel/DeviationSentinel.sol

+                    //   so we skip restoring to avoid overwriting new pool config with zero values.
+                    // - If storedLT is 0, skip restoration to prevent setting LT=0, which could cause immediate liquidation risk.
+                    //   This also protects against uninitialized storage for new pools.
+                    if (storedCF == 0 && currentCF != 0) {


Can we consider avoiding the storedCF == 0 check and rely only on currentCF != 0 when restoring values? If a CF is updated via a VIP during the pause, should the sentinel still reset it, or is the current behavior intentional ?

The check is correct and intentional. It distinguishes between:

storedCF == 0 because never stored (new pool created during pause) → skip if currentCF != 0 to preserve governance's new pool config

storedCF == 0 because original CF was 0 → restore allowed if currentCF is also 0

See line 475-477 comments:

If storedCF is 0 and currentCF != 0, this pool was added after _setCollateralFactorToZero, so we skip restoring to avoid overwriting new pool config with zero values.

GitGuru7 · 2026-02-05T12:00:23Z

contracts/DeviationSentinel/DeviationSentinel.sol

+    event SupplyUnpaused(address indexed market);
+
+    /// @notice Emitted when collateral factor is updated
+    /// @notice Emitted when collateral factor is updated


Suggested change

/// @notice Emitted when collateral factor is updated

As the code is already deployed on mainnet, I would suggest to skip this @fred-venus WDYT?

yeah, can skip

GitGuru7 · 2026-02-05T12:02:42Z

contracts/DeviationSentinel/DeviationSentinel.sol

+                    uint256 result = CORE_POOL_COMPTROLLER.setCollateralFactor(i, address(market), storedCF, storedLT);
+                    if (result != 0) revert ComptrollerError(result);
+
+                    emit CollateralFactorUpdated(address(market), i, 0, storedCF);


On restore, the event assumes currentCF is always 0, but if governance updates the CF during the pause, this may not hold. Should we consider emitting the actual previous value instead (from currentCF) for better accuracy and traceability?

The current behavior is intentional. The event tracks DeviationSentinel's state transitions:

On pause: oldCF = original CF → newCF = 0 (sentinel zeroing)

On restore: oldCF = 0 → newCF = storedCF (sentinel restoring from what it set)

If governance modifies CF during the pause, that's a separate action outside the sentinel's scope. The sentinel's event accurately reflects: "I'm restoring from 0 (what I set) to the stored value.

Does this make sense?

contracts/DeviationSentinel/Oracles/PancakeSwapOracle.sol

github-actions · 2026-02-06T07:15:55Z

Package	Line Rate	Branch Rate	Health
DeviationSentinel	88%	86%	✔
DeviationSentinel.Oracles	93%	90%	✔
Interfaces	100%	100%	✔
LeverageManager	94%	80%	✔
Libraries	29%	33%	❌
PositionSwapper	12%	10%	❌
SwapHelper	100%	100%	✔
Summary	74% (487 / 661)	60% (227 / 376)	➖

fred-venus · 2026-02-09T03:26:27Z

merge as vip has will be proposed soon

fred-venus · 2026-02-09T03:27:45Z

cc @Debugger022 , once the audit report is available, feel free to create a new pr

## 1.2.0-dev.1 (2026-02-09) * Merge branch 'develop' into feat/vpd-349 ([488caf5](488caf5)) * Merge branch 'feat/vpd-349' of github.com:VenusProtocol/venus-periphery into feat/vpd-349 ([d2085c5](d2085c5)) * Merge branch 'feat/vpd-402' into feat/vpd-349 ([6886d43](6886d43)) * Merge pull request #24 from VenusProtocol/feat/vpd-349 ([69ed443](69ed443)), closes [#24](#24) * Merge pull request #42 from VenusProtocol/feat/vpd-432 ([60151fb](60151fb)), closes [#42](#42) * Merge pull request #44 from VenusProtocol/feat/vpd-442 ([fc47f63](fc47f63)), closes [#44](#44) * feat: add bscmainnet deployments ([5fa293e](5fa293e)) * feat: add comment ([f133295](f133295)) * feat: add emergency pause hashdit final audit report ([ee220df](ee220df)) * feat: added core logic ([a6f913d](a6f913d)) * feat: added unit tests ([0544913](0544913)) * feat: refactor DeviationSentinel fork tests to use mainnet contracts ([f60beef](f60beef)) * feat: updating deployment files ([cc6fd27](cc6fd27)) * feat: updating deployment files ([f1e48df](f1e48df)) * feat: updating deployment files ([ae59cd3](ae59cd3)) * fix: added comments to call resetMarketState ([ab18286](ab18286)) * fix: added deployments ([4884493](4884493)) * fix: added deployments ([153e950](153e950)) * fix: added fork tests for deviation logic ([1e4da12](1e4da12)) * fix: added imports ([ea31a65](ea31a65)) * fix: added reset func ([5d5f549](5d5f549)) * fix: added test case for non 18 reference token decimals ([bffb983](bffb983)) * fix: added uniswap test ([6114fc3](6114fc3)) * fix: change deviation logic ([1f96225](1f96225)) * fix: create seperate oracle contracts ([e7b4dd2](e7b4dd2)) * fix: deployed new contracts ([a6642ef](a6642ef)) * fix: early return ([cc2e8cc](cc2e8cc)) * fix: enable or disable markets ([5f6c5fc](5f6c5fc)) * fix: fix LT ([91ebcb7](91ebcb7)) * fix: fixed decimals ([e57ad40](e57ad40)) * fix: fixed e2e tests ([cb19bbc](cb19bbc)) * fix: fixed interfaces ([5588076](5588076)) * fix: fixed lint ([5ac5e18](5ac5e18)) * fix: fixed pancakeswap calculation ([c527c47](c527c47)) * fix: fixed tests ([c8884a7](c8884a7)) * fix: fixed tests ([355acef](355acef)) * fix: fixed var name ([0769189](0769189)) * fix: handle emode groups ([2838512](2838512)) * fix: i02 ([304beb5](304beb5)) * fix: i04 ([cf89978](cf89978)) * fix: m01 ([390856b](390856b)) * fix: merge conflict ([375b2f2](375b2f2)) * fix: optimise code ([526c91e](526c91e)) * fix: optimise code ([5d443f5](5d443f5)) * fix: optimise code ([7deec40](7deec40)) * fix: optimise code ([c347173](c347173)) * fix: optimise code ([b10769b](b10769b)) * fix: optimise code ([8586af0](8586af0)) * fix: optimised code ([02fde57](02fde57)) * fix: pause supply ([54ccf44](54ccf44)) * fix: reanmed contract ([f77d868](f77d868)) * fix: rearrange files ([a129bd7](a129bd7)) * fix: remove duplicate imports ([db59423](db59423)) * fix: remove unwanted error ([1b24e48](1b24e48)) * fix: removed comments ([ddcdab8](ddcdab8)) * fix: removed comments ([931bab9](931bab9)) * fix: removed unused vars ([f63f361](f63f361)) * fix: skip waitConfirmations on local hardhat network ([15ff9e7](15ff9e7)) * fix: unify CollateralFactorUpdated event and expand test coverage ([f1038c5](f1038c5)) * fix: updated package ([7a8bed6](7a8bed6)) * fix: usd price should be 18 decimals ([edf7840](edf7840)) * fix: use deployer as proxy owner on non-live networks in sentinel deploy ([e8a953d](e8a953d)) * fix: use less vars ([9d66b76](9d66b76)) * fix: use pancakeswap v3 ([4178f93](4178f93)) * fix: vld-02 ([48ef434](48ef434)) * fix: vld-03 ([5340dc5](5340dc5)) * fix: vld-03 ([0e0a69d](0e0a69d)) * fix: vld-06 ([157c217](157c217)) * fix: vld-08 ([b2150b7](b2150b7)) * fix: vld-09 ([5588f48](5588f48)) * fix: vld-12 ([583e2d3](583e2d3)) * chore: added events and custom errors ([1fcfcf7](1fcfcf7)) * chore: cleanup unused code and add SPDX identifier ([b209db7](b209db7)) * chore: fixed Deviation testing ([629fc61](629fc61)) * chore: fixed linter error ([aa7ffe4](aa7ffe4)) * chore: retrigger ci ([f88054a](f88054a))

feat: added core logic

a6f913d

web3rover self-assigned this Dec 8, 2025

web3rover added 6 commits December 8, 2025 12:50

fix: usd price should be 18 decimals

edf7840

fix: handle emode groups

2838512

fix: use pancakeswap v3

4178f93

fix: updated package

7a8bed6

fix: fixed pancakeswap calculation

c527c47

fix: optimised code

02fde57

web3rover added 2 commits December 8, 2025 22:55

fix: added uniswap test

6114fc3

fix: added fork tests for deviation logic

1e4da12