-
Notifications
You must be signed in to change notification settings - Fork 8
[VEN-3018]: Initial Risk Steward contract #115
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
base: develop
Are you sure you want to change the base?
Conversation
e17395f to
1fcc60b
Compare
0d56e60 to
c9fa6c4
Compare
b581081 to
16ed613
Compare
a03a723 to
24908dd
Compare
9e7c889 to
b73301f
Compare
…e-contracts into acm-risk-steward
| * @dev Disabling renounceOwnership function. | ||
| */ | ||
| function renounceOwnership() public override { | ||
| revert(" renounceOwnership() is not allowed"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| function _setMarketSupplyCaps(address[] calldata, uint256[] calldata) external; | ||
|
|
||
| function _setMarketBorrowCaps(address[] calldata, uint256[] calldata) external; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this PR would merge before this one, then we can have unified comptroller interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use the legacy interface first, and the new one in the V2/V3 of the Risk Stewards
deploy/011-risk-stewards.ts
Outdated
| proxyContract: "OptimizedTransparentUpgradeableProxy", | ||
| execute: { | ||
| methodName: "initialize", | ||
| args: [accessControlManager.address, 5000], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider replacing the hardcoded value 5000 with a constant or variable — this improves readability and makes future changes easier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| /** | ||
| * @notice Emitted when the max delta bps is updated | ||
| */ | ||
| event MaxDeltaBpsUpdated(uint256 oldMaxDeltaBps, uint256 indexed newMaxDeltaBps); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add indexed for oldMaxDeltaBps as well for consistency.
| * @param newValue The new supply cap value | ||
| * @custom:event Emits SupplyCapUpdated with the market and new supply cap | ||
| */ | ||
| function _updateSupplyCaps(address market, uint256 newValue) internal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can add ensureNonzeroAddress(market) as safety validation .
| * @param newValue The new borrow cap value | ||
| * @custom:event Emits BorrowCapUpdated with the market and new borrow cap | ||
| */ | ||
| function _updateBorrowCaps(address market, uint256 newValue) internal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can add ensureNonzeroAddress(market) as safety validation .
| /** | ||
| * @notice Thrown when an update type that is not supported is operated on | ||
| */ | ||
| error UnsupportedUpdateType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error UnsupportedUpdateType(string updateType) , this will upgrade the revert to revert UnsupportedUpdateType(update.updateType);
helpers/deploy/deploymentUtils.ts
Outdated
| zksyncmainnet: "", | ||
| berachainbartio: "", | ||
| berachainbepolia: "", | ||
| }[networkName]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to fill contract addresses for remaining chains if needed or remove it .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
This is an initial Risk Steward contract that accepts updates for supply and borrow caps to be applied view permissions granted by the ACM.