-
Notifications
You must be signed in to change notification settings - Fork 9
[VPD-29]: Risk Steward V2 #163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
GitGuru7
wants to merge
78
commits into
acm-risk-steward
Choose a base branch
from
feat/vpd-29-risk-steward-v2
base: acm-risk-steward
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+144,983
−56,201
Open
Changes from all commits
Commits
Show all changes
78 commits
Select commit
Hold shift + click to select a range
4e1cf5c
refactor: initial refactor
GitGuru7 71a82c3
feat: add RiskOracle
GitGuru7 101ad3c
feat: add timelock support for risk-steward updates
GitGuru7 66df110
refactor: receiver contract
GitGuru7 2b03837
refactor: remove unnecessary complexity and streamline implementation
GitGuru7 9b6f3e7
feat: add multichain updates support
GitGuru7 0efe2f5
feat: allow immediate execution for safe-delta updates and refactor
GitGuru7 857da07
feat: add CollateralFactorsRiskSteward
GitGuru7 d931e80
feat: add IRMRiskSteward and refactor
GitGuru7 b12aa63
refactor: design improvements
GitGuru7 f0bbad7
refactor: DestinationStewardReceiver
GitGuru7 5e9cefa
refactor: natspecs and minor improvements
GitGuru7 f02e723
fix: enforce whitelisted executors for rejection and drop LI update s…
GitGuru7 ae1fd50
test: add initial setup and cover main execution paths
GitGuru7 b2785e6
test: extend coverage to all update type
GitGuru7 37b89a7
test: cover failure scenarios
GitGuru7 fb1f0ef
test: add risk-oracle unit tests
GitGuru7 07145c7
feat: add fork test setup and required package upgrades
GitGuru7 66b0659
test: add fork test to cover main execution paths
GitGuru7 ae83d48
fix: improve function names and events
GitGuru7 c123c49
feat: add pause support
GitGuru7 c0f9558
fix: move auth check to external functions
GitGuru7 94d7f21
feat: add deployment scripts
GitGuru7 d264915
fix: streamline process-update
GitGuru7 bd71012
fix: extract modifier and update lz options
GitGuru7 78344c9
fix: lint error
GitGuru7 f5124ff
ci: fix hardhat compiler download failures
GitGuru7 244c349
fix: ci faliure
GitGuru7 25c4d6c
ci: update node version in test job
GitGuru7 98d9b43
fix: suppress oz upgrade-safety constructor error in tests
GitGuru7 bb363a6
Merge branch 'acm-risk-steward' into feat/vpd-29-risk-steward-v2
GitGuru7 13141c3
fix: yarn-lock
GitGuru7 515eec0
fix: lint
GitGuru7 9f3b290
fix: resolve pr comments
GitGuru7 0bd5888
feat: expose functions to fetch update types and their count
GitGuru7 a074696
refactor: execute-update to prevent reentrancy risk
GitGuru7 3d3461e
refactor: improvements in destination-steward-receiver
GitGuru7 2d81a84
refactor: allow resend-remote-update to pass optional fee
GitGuru7 274034f
fix: resolve comments
GitGuru7 675e568
refactor: remove Executable from UpdateStatus
GitGuru7 b6ba91b
fix: [M01] Timelock and expiration time mismatch may cause update exe…
GitGuru7 8349b9b
fix: [I02] [I03] Code Optimization
GitGuru7 687ecac
fix: [M01]
GitGuru7 d2ac393
fix: [VEN-1] Multiple Debounce for Cross-Chain Calls
GitGuru7 aedaeb0
fix: [S1] Inconfigurable REMOTE_DELAY
GitGuru7 9967888
fix: [S2] Missing Input Validation
GitGuru7 631a04f
fix: [S3] Missing renounceOwnership() Override
GitGuru7 1865fac
fix: [S4] [S7] RiskOracle Mappings Can Be Improved
GitGuru7 a2612b2
fix: [S8] Inconsistent Error Handling
GitGuru7 95b8efc
fix: [S9] Misaligned Comments
GitGuru7 fe4ea95
fix: [S6] Code Clones
GitGuru7 5b47b4f
fix: [VRR-10] Missing Checks
GitGuru7 9a46116
fix: [VRR-15] CollateralFactorsRiskSteward May Allow Unsupported Upda…
GitGuru7 0a55a89
fix: [VRR-16] delete Keyword Can Be Used
GitGuru7 f4d3fcb
fix [VRR-17] [VRR-18] Missing Redundancy Check
GitGuru7 9a03fcd
fix: [VRR-20] Inconsistent Expiration Logic For Remote Updates
GitGuru7 8a8f3e5
fix: [VRR-21] Not All Contracts Disable Renouncing Ownership
GitGuru7 d6d20bb
fix: [VRR-23] Incomplete/Missing Comments
GitGuru7 a93643d
fix: [VRR-27] Misleading Name
GitGuru7 9ae58f7
fix: [VRR-04] Discussion On Use Of OAppUpgradeable
GitGuru7 ec4ff8a
fix: [VRR-01] Array Length Is Not Cached
GitGuru7 c9b02ac
fix: [S4]
GitGuru7 d10cdfe
fix: add missing NatSpec and update custom error
GitGuru7 7f20b3f
fix: update natspec
GitGuru7 b5fade5
Merge pull request #164 from VenusProtocol/fix/vpd-29-hashdit-audit
GitGuru7 4a3c0b0
Merge pull request #165 from VenusProtocol/fix/vpd-29-quantstamp-audit
GitGuru7 0e6202d
Merge pull request #166 from VenusProtocol/fix/vpd-29-certik
fred-venus 5cbda76
Merge pull request #167 from VenusProtocol/fix/vpd-29-quantstamp-audit
fred-venus d8792a2
chore: remove unused Risk Steward v1 deployments
GitGuru7 63102ae
feat: add risk steward deployemnts on bsctestnet and sepolia
GitGuru7 3ef69fe
feat: updating deployment files
GitGuru7 0931021
fix: redeploy CF and IRM steward on sepolia
GitGuru7 17d7db8
feat: updating deployment files
GitGuru7 0fb8a5a
feat: add risk steward deployments for bscmainnet
GitGuru7 097df33
feat: updating deployment files
GitGuru7 597cea2
revert: rollback base Sepolia deployments
GitGuru7 4539a76
feat: add risk steward testnet deployments on all chains
GitGuru7 2c0da57
feat: updating deployment files
GitGuru7 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| // SPDX-License-Identifier: BSD-3-Clause | ||
| pragma solidity 0.8.25; | ||
|
|
||
| import { AccessControlledV8 } from "../Governance/AccessControlledV8.sol"; | ||
| import { IRiskSteward } from "./Interfaces/IRiskSteward.sol"; | ||
|
|
||
| /** | ||
| * @title BaseRiskSteward | ||
| * @author Venus | ||
| * @notice Abstract base contract for Risk Steward contracts providing common functionality | ||
| * @custom:security-contact https://github.com/VenusProtocol/governance-contracts#discussion | ||
| */ | ||
| abstract contract BaseRiskSteward is IRiskSteward, AccessControlledV8 { | ||
| /// @dev Max basis points i.e., 100% | ||
| uint256 internal constant MAX_BPS = 10000; | ||
|
|
||
| /** | ||
| * @notice The safe delta threshold in basis points. | ||
| * @notice Updates within this delta are considered safe and require no timelock. Updates exceeding this delta require timelock. | ||
| * @dev This is only used by contracts that implement safe delta checks (e.g., MarketCapsRiskSteward, CollateralFactorsRiskSteward) | ||
| */ | ||
| uint256 public safeDeltaBps; | ||
|
|
||
| /** | ||
| * @notice Thrown when trying to renounce ownership | ||
| */ | ||
| error RenounceOwnershipNotAllowed(); | ||
|
|
||
| /** | ||
| * @notice Disables renounceOwnership function | ||
| * @custom:error Throws RenounceOwnershipNotAllowed | ||
| */ | ||
| function renounceOwnership() public pure override { | ||
| revert RenounceOwnershipNotAllowed(); | ||
| } | ||
|
|
||
| /** | ||
| * @notice Checks if the difference between new and current values is within the safe delta threshold. | ||
| * @param newValue The new value to check | ||
| * @param currentValue The current value to compare against | ||
| * @return True if the difference is within the safe delta, false otherwise | ||
| */ | ||
| function _isWithinSafeDelta(uint256 newValue, uint256 currentValue) internal view returns (bool) { | ||
| uint256 diff = newValue > currentValue ? newValue - currentValue : currentValue - newValue; | ||
| uint256 maxDiff = (safeDeltaBps * currentValue) / MAX_BPS; | ||
| return diff <= maxDiff; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,274 @@ | ||
| // SPDX-License-Identifier: BSD-3-Clause | ||
| pragma solidity 0.8.25; | ||
|
|
||
| import { RiskParameterUpdate } from "./Interfaces/IRiskOracle.sol"; | ||
| import { ICorePoolVToken } from "../interfaces/ICorePoolVToken.sol"; | ||
| import { ICorePoolComptroller } from "../interfaces/ICorePoolComptroller.sol"; | ||
| import { IIsolatedPoolsComptroller } from "../interfaces/IIsolatedPoolsComptroller.sol"; | ||
| import { IRiskStewardReceiver } from "./Interfaces/IRiskStewardReceiver.sol"; | ||
| import { BaseRiskSteward } from "./BaseRiskSteward.sol"; | ||
| import { ensureNonzeroAddress } from "@venusprotocol/solidity-utilities/contracts/validators.sol"; | ||
|
|
||
| /** | ||
| * @title CollateralFactorsRiskSteward | ||
| * @author Venus | ||
| * @notice Contract that can update collateral factors and liquidation thresholds received from `RiskStewardReceiver`. | ||
| * @custom:security-contact https://github.com/VenusProtocol/governance-contracts#discussion | ||
| */ | ||
| contract CollateralFactorsRiskSteward is BaseRiskSteward { | ||
| /** | ||
| * @notice The update type for collateral factor and liquidation threshold. | ||
| */ | ||
| string public constant COLLATERAL_FACTORS = "collateralFactors"; | ||
|
|
||
| /** | ||
| * @notice The update type key for collateral factors (keccak256 hash of COLLATERAL_FACTORS) | ||
| */ | ||
| bytes32 public constant COLLATERAL_FACTORS_KEY = keccak256(bytes(COLLATERAL_FACTORS)); | ||
|
|
||
| /** | ||
| * @notice Address of the BNB Core Pool Comptroller. | ||
| * @dev This comptroller is specific to the BNB Core Pool, which uses a different ABI | ||
| * than isolated pools. It is used solely to detect and handle BNB Core Pool | ||
| * markets, and would not be used for remote-chain (isolated pool) deployments. | ||
| */ | ||
| ICorePoolComptroller public immutable CORE_POOL_COMPTROLLER; | ||
|
|
||
| /** | ||
| * @notice Address of the `RiskStewardReceiver` used to validate and dispatch incoming updates. | ||
| */ | ||
| IRiskStewardReceiver public immutable RISK_STEWARD_RECEIVER; | ||
|
|
||
| /** | ||
| * @dev Storage gap for upgradeability. | ||
| */ | ||
| uint256[49] private __gap; | ||
|
|
||
| /** | ||
| * @notice Emitted when collateral factors are updated. | ||
| */ | ||
| event CollateralFactorsUpdated( | ||
| uint256 indexed updateId, | ||
| address indexed market, | ||
| uint256 newCollateralFactor, | ||
| uint256 newLiquidationThreshold | ||
| ); | ||
|
|
||
| /** | ||
| * @notice Emitted when the safe delta bps is updated. | ||
| */ | ||
| event SafeDeltaBpsUpdated(uint256 oldSafeDeltaBps, uint256 newSafeDeltaBps); | ||
|
|
||
| /** | ||
| * @notice Thrown when a `safeDeltaBps` value is greater than `MAX_BPS`. | ||
| */ | ||
| error InvalidSafeDeltaBps(); | ||
|
|
||
| /** | ||
| * @notice Thrown when Core Pool Comptroller.setCollateralFactor fails. | ||
| */ | ||
| error SetCollateralFactorFailed(uint256 errorCode); | ||
|
|
||
| /** | ||
| * @notice Thrown when an invalid pool configuration is used (non-core comptroller with non-zero poolId). | ||
| */ | ||
| error InvalidPool(); | ||
|
|
||
| /** | ||
| * @notice Thrown when an update type that is not supported is operated on. | ||
| */ | ||
| error UnsupportedUpdateType(); | ||
|
|
||
| /** | ||
| * @notice Thrown when the update is not coming from the `RiskStewardReceiver`. | ||
| */ | ||
| error OnlyRiskStewardReceiver(); | ||
|
|
||
| /** | ||
| * @notice Thrown when the two uint256 data length is invalid | ||
| */ | ||
| error InvalidTwoUintLength(); | ||
|
|
||
| /** | ||
| * @notice Thrown when attempting to apply a redundant value (no-op change). | ||
| */ | ||
| error RedundantValue(); | ||
|
|
||
| /** | ||
| * @notice Sets the immutable `CORE_POOL_COMPTROLLER` and `RISK_STEWARD_RECEIVER` addresses and disables initializers. | ||
| * @param corePoolComptroller_ The address of the Core Pool Comptroller | ||
| * @param riskStewardReceiver_ The address of the `RiskStewardReceiver` | ||
| * @custom:error Throws ZeroAddressNotAllowed if any of the addresses are zero | ||
| * @custom:oz-upgrades-unsafe-allow constructor | ||
| */ | ||
| constructor(address corePoolComptroller_, address riskStewardReceiver_) { | ||
| ensureNonzeroAddress(riskStewardReceiver_); | ||
| CORE_POOL_COMPTROLLER = ICorePoolComptroller(corePoolComptroller_); | ||
| RISK_STEWARD_RECEIVER = IRiskStewardReceiver(riskStewardReceiver_); | ||
| _disableInitializers(); | ||
| } | ||
|
|
||
| /** | ||
| * @notice Initializes the contract as ownable and access controlled. | ||
| * @param accessControlManager_ The address of the access control manager | ||
| */ | ||
| function initialize(address accessControlManager_) external initializer { | ||
| __AccessControlled_init(accessControlManager_); | ||
| } | ||
|
|
||
| /** | ||
| * @notice Sets the safe delta bps. | ||
| * @param safeDeltaBps_ The new safe delta bps | ||
| * @custom:access Controlled by AccessControlManager | ||
| * @custom:event Emits SafeDeltaBpsUpdated with the old and new safe delta bps | ||
| * @custom:error Throws InvalidSafeDeltaBps if the safe delta bps is greater than MAX_BPS | ||
| * @custom:error Throws RedundantValue if the new safe delta bps is equal to the current value | ||
| */ | ||
| function setSafeDeltaBps(uint256 safeDeltaBps_) external { | ||
| _checkAccessAllowed("setSafeDeltaBps(uint256)"); | ||
| if (safeDeltaBps_ > MAX_BPS) { | ||
| revert InvalidSafeDeltaBps(); | ||
| } | ||
| uint256 oldSafeDeltaBps = safeDeltaBps; | ||
|
|
||
| if (safeDeltaBps_ == oldSafeDeltaBps) { | ||
| revert RedundantValue(); | ||
| } | ||
| safeDeltaBps = safeDeltaBps_; | ||
| emit SafeDeltaBpsUpdated(oldSafeDeltaBps, safeDeltaBps_); | ||
| } | ||
|
|
||
| /** | ||
| * @notice Checks if an update is safe for direct execution (no timelock required). | ||
| * @param update The update to check. | ||
| * @return True if update is safe for direct execution, false if timelock is required | ||
| * @custom:error Throws UnsupportedUpdateType if the update type is not supported | ||
| * @custom:error Throws RedundantValue if the new collateral factor and liquidation threshold are unchanged | ||
| */ | ||
| function isSafeForDirectExecution(RiskParameterUpdate calldata update) external view returns (bool) { | ||
| if (update.updateTypeKey == COLLATERAL_FACTORS_KEY) { | ||
| // eMode-style updates always require timelock (not safe for direct execution) | ||
| if (update.poolId != 0) return false; | ||
|
|
||
| address comptroller = ICorePoolVToken(update.market).comptroller(); | ||
|
|
||
| (uint256 newCF, uint256 newLT) = _decodeAbiEncodedTwoUint256(update.newValue); | ||
| (uint256 currCF, uint256 currLT) = _getCurrentCollateralFactors(comptroller, update.market); | ||
|
|
||
| // Revert on redundant updates only when both CF and LT are unchanged. | ||
| if (newCF == currCF && newLT == currLT) { | ||
| revert RedundantValue(); | ||
| } | ||
|
|
||
| // If current values are zero, update always requires timelock | ||
| if (currCF == 0 || currLT == 0) return false; | ||
|
|
||
| return _isWithinSafeDelta(newCF, currCF) && _isWithinSafeDelta(newLT, currLT); | ||
| } | ||
|
|
||
| revert UnsupportedUpdateType(); | ||
| } | ||
|
|
||
| /** | ||
| * @notice Applies a collateral parameter update from the `RiskStewardReceiver`. | ||
| * Delta validation and timelock checks are already performed by `RiskStewardReceiver` before execution. | ||
| * @param update RiskParameterUpdate update to apply | ||
| * @custom:access Only callable by the `RiskStewardReceiver` | ||
| * @custom:event Emits CollateralFactorsUpdated with updateId | ||
| * @custom:error Throws OnlyRiskStewardReceiver if the sender is not the `RiskStewardReceiver` | ||
| * @custom:error Throws UnsupportedUpdateType if the update type is not supported | ||
| */ | ||
| function applyUpdate(RiskParameterUpdate calldata update) external { | ||
| if (msg.sender != address(RISK_STEWARD_RECEIVER)) { | ||
| revert OnlyRiskStewardReceiver(); | ||
| } | ||
|
|
||
| address comptroller = ICorePoolVToken(update.market).comptroller(); | ||
Debugger022 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| uint96 poolId = update.poolId; | ||
|
|
||
| if (update.updateTypeKey == COLLATERAL_FACTORS_KEY) { | ||
| _updateCollateralFactors(update.updateId, comptroller, update.market, poolId, update.newValue); | ||
| } else { | ||
fred-venus marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| revert UnsupportedUpdateType(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @notice Updates the collateral factors for the given market. | ||
| * @dev Updates both collateral factor and liquidation threshold together (same setter). | ||
| * @param updateId The update ID from the Risk Oracle | ||
| * @param comptroller The comptroller address | ||
| * @param market The market to update the collateral factors for | ||
| * @param poolId The pool identifier for eMode updates (0 for regular market updates) | ||
| * @param newValue Encoded new collateral factors: `abi.encode(uint256 newCollateralFactor, uint256 newLiquidationThreshold)` | ||
| * @custom:error Throws SetCollateralFactorFailed if the core pool comptroller call to setCollateralFactor returns a non‑zero error code | ||
| * @custom:error Throws InvalidPool if a non‑core comptroller is used together with a non‑zero poolId | ||
| * @custom:event Emits CollateralFactorsUpdated with updateId | ||
| */ | ||
| function _updateCollateralFactors( | ||
| uint256 updateId, | ||
| address comptroller, | ||
| address market, | ||
| uint96 poolId, | ||
| bytes memory newValue | ||
| ) internal { | ||
| (uint256 newCollateralFactor, uint256 newLiquidationThreshold) = _decodeAbiEncodedTwoUint256(newValue); | ||
|
|
||
| if (comptroller == address(CORE_POOL_COMPTROLLER)) { | ||
GitGuru7 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| uint256 errorCode = ICorePoolComptroller(comptroller).setCollateralFactor( | ||
| poolId, | ||
| market, | ||
| newCollateralFactor, | ||
| newLiquidationThreshold | ||
| ); | ||
| if (errorCode != 0) revert SetCollateralFactorFailed(errorCode); | ||
| } else { | ||
| if (poolId != 0) revert InvalidPool(); | ||
|
|
||
| IIsolatedPoolsComptroller(comptroller).setCollateralFactor( | ||
| market, | ||
| newCollateralFactor, | ||
| newLiquidationThreshold | ||
| ); | ||
| } | ||
|
|
||
| emit CollateralFactorsUpdated(updateId, market, newCollateralFactor, newLiquidationThreshold); | ||
| } | ||
|
|
||
| /** | ||
| * @notice Returns the current collateral factors for a market on a given comptroller. | ||
| * @dev Returns both collateral factor and liquidation threshold (updated together via the same setter). | ||
| * @param comptroller The comptroller address | ||
| * @param market The market whose collateral factors are being queried | ||
| * @return currentCollateralFactor The current collateral factor | ||
| * @return currentLiquidationThreshold The current liquidation threshold | ||
| */ | ||
| function _getCurrentCollateralFactors( | ||
| address comptroller, | ||
| address market | ||
| ) internal view returns (uint256 currentCollateralFactor, uint256 currentLiquidationThreshold) { | ||
| if (comptroller == address(CORE_POOL_COMPTROLLER)) { | ||
| (, currentCollateralFactor, , currentLiquidationThreshold, , , ) = ICorePoolComptroller(comptroller) | ||
| .markets(market); | ||
| } else { | ||
| (, currentCollateralFactor, currentLiquidationThreshold) = IIsolatedPoolsComptroller(comptroller).markets( | ||
| market | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * @notice Decodes ABI-encoded bytes into two uint256 values. | ||
| * @dev Expects exactly 64 bytes as produced by abi.encode(uint256,uint256). | ||
| * @param data ABI-encoded (uint256, uint256) payload | ||
| * @return a First uint256 | ||
| * @return b Second uint256 | ||
| * @custom:error Throws InvalidTwoUintLength if data length is not 64 bytes | ||
| */ | ||
| function _decodeAbiEncodedTwoUint256(bytes memory data) internal pure returns (uint256 a, uint256 b) { | ||
| if (data.length != 64) { | ||
| revert InvalidTwoUintLength(); | ||
| } | ||
| (a, b) = abi.decode(data, (uint256, uint256)); | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.