Skip to content
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.

Sign up for GitHub

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

add governor role for access to setters for term strategy parameters #75

Merged
merged 2 commits into from
Oct 29, 2024
+84 −57
Merged

add governor role for access to setters for term strategy parameters #75

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b16ede
add governor role for access to setters for term strategy parameters
aazhou1 Oct 26, 2024
6ad4cee
Merge branch 'master' into governor-role
aazhou1 Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/deploy-sepolia-strategy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,5 @@ jobs:
REPOTOKEN_CONCENTRATION_LIMIT: ${{ github.event.inputs.repoTokenConcentrationLimit }}
ADMIN_ADDRESS: ${{ vars.ADMIN_ADDRESS }}
DEVOPS_ADDRESS: ${{ vars.DEVOPS_ADDRESS }}
GOVERNOR_ROLE_ADDRESS: ${{ vars.GOVERNOR_ROLE_ADDRESS }}

4 changes: 3 additions & 1 deletion script/Strategy.s.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ contract DeployStrategy is Script {
address discountRateAdapterAddress = vm.envAddress("DISCOUNT_RATE_ADAPTER_ADDRESS");
address admin = vm.envAddress("ADMIN_ADDRESS");
address devops = vm.envAddress("DEVOPS_ADDRESS");
address governorRoleAddress = vm.envAddress("GOVERNOR_ROLE_ADDRESS");
address termController = vm.envOr("TERM_CONTROLLER_ADDRESS", address(0));
uint256 discountRateMarkup = vm.envOr("DISCOUNT_RATE_MARKUP", uint256(0));
address collateralTokenAddr = vm.envOr("COLLATERAL_TOKEN_ADDR", address(0));
Expand All @@ -36,7 +37,8 @@ contract DeployStrategy is Script {
name,
yearnVaultAddress,
discountRateAdapterAddress,
address(eventEmitter)
address(eventEmitter),
governorRoleAddress
);

console.log("deployed strateghy contract to");
Expand Down
35 changes: 21 additions & 14 deletions src/Strategy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {ITermAuction} from "./interfaces/term/ITermAuction.sol";
import {RepoTokenList, RepoTokenListData} from "./RepoTokenList.sol";
import {TermAuctionList, TermAuctionListData, PendingOffer} from "./TermAuctionList.sol";
import {RepoTokenUtils} from "./RepoTokenUtils.sol";
import {AccessControl} from "@openzeppelin/contracts/access/AccessControl.sol";

// Import interfaces for many popular DeFi projects, or add your own!
//import "../interfaces/<protocol>/<Interface>.sol";
Expand All @@ -33,7 +34,7 @@ import {RepoTokenUtils} from "./RepoTokenUtils.sol";

// NOTE: To implement permissioned functions you can use the onlyManagement, onlyEmergencyAuthorized and onlyKeepers modifiers

contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
contract Strategy is BaseStrategy, Pausable, AccessControl, ReentrancyGuard {
using SafeERC20 for IERC20;
using RepoTokenList for RepoTokenListData;
using TermAuctionList for TermAuctionListData;
Expand All @@ -47,6 +48,8 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
error RepoTokenBlacklisted(address repoToken);
error DepositPaused();

bytes32 public constant GOVERNOR_ROLE = keccak256("GOVERNOR_ROLE");

// Immutable state variables
ITermVaultEvents public immutable TERM_VAULT_EVENT_EMITTER;
uint256 public immutable PURCHASE_TOKEN_PRECISION;
Expand Down Expand Up @@ -81,23 +84,23 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
/**
* @notice Pause the contract
*/
function pauseDeposit() external onlyManagement {
function pauseDeposit() external onlyRole(GOVERNOR_ROLE) {
depositLock = true;
TERM_VAULT_EVENT_EMITTER.emitDepositPaused();
}

/**
* @notice Unpause the contract
*/
function unpauseDeposit() external onlyManagement {
function unpauseDeposit() external onlyRole(GOVERNOR_ROLE) {
depositLock = false;
TERM_VAULT_EVENT_EMITTER.emitDepositUnpaused();
}

/**
* @notice Pause the contract
*/
function pauseStrategy() external onlyManagement {
function pauseStrategy() external onlyRole(GOVERNOR_ROLE) {
_pause();
depositLock = true;
TERM_VAULT_EVENT_EMITTER.emitStrategyPaused();
Expand All @@ -106,7 +109,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
/**
* @notice Unpause the contract
*/
function unpauseStrategy() external onlyManagement {
function unpauseStrategy() external onlyRole(GOVERNOR_ROLE) {
_unpause();
depositLock = false;
TERM_VAULT_EVENT_EMITTER.emitStrategyUnpaused();
Expand All @@ -118,7 +121,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
*/
function setTermController(
address newTermController
) external onlyManagement {
) external onlyRole(GOVERNOR_ROLE) {
require(newTermController != address(0));
require(ITermController(newTermController).getProtocolReserveAddress() != address(0));
address current = address(currTermController);
Expand All @@ -136,7 +139,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
*/
function setDiscountRateAdapter(
address newAdapter
) external onlyManagement {
) external onlyRole(GOVERNOR_ROLE) {
ITermDiscountRateAdapter newDiscountRateAdapter = ITermDiscountRateAdapter(newAdapter);
require(address(newDiscountRateAdapter.TERM_CONTROLLER()) != address(0));
TERM_VAULT_EVENT_EMITTER.emitDiscountRateAdapterUpdated(
Expand All @@ -152,7 +155,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
*/
function setTimeToMaturityThreshold(
uint256 newTimeToMaturityThreshold
) external onlyManagement {
) external onlyRole(GOVERNOR_ROLE) {
TERM_VAULT_EVENT_EMITTER.emitTimeToMaturityThresholdUpdated(
timeToMaturityThreshold,
newTimeToMaturityThreshold
Expand All @@ -167,7 +170,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
*/
function setRequiredReserveRatio(
uint256 newRequiredReserveRatio
) external onlyManagement {
) external onlyRole(GOVERNOR_ROLE) {
TERM_VAULT_EVENT_EMITTER.emitRequiredReserveRatioUpdated(
requiredReserveRatio,
newRequiredReserveRatio
Expand All @@ -181,7 +184,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
*/
function setRepoTokenConcentrationLimit(
uint256 newRepoTokenConcentrationLimit
) external onlyManagement {
) external onlyRole(GOVERNOR_ROLE) {
TERM_VAULT_EVENT_EMITTER.emitRepoTokenConcentrationLimitUpdated(
repoTokenConcentrationLimit,
newRepoTokenConcentrationLimit
Expand All @@ -195,7 +198,7 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
*/
function setDiscountRateMarkup(
uint256 newDiscountRateMarkup
) external onlyManagement {
) external onlyRole(GOVERNOR_ROLE) {
TERM_VAULT_EVENT_EMITTER.emitDiscountRateMarkupUpdated(
discountRateMarkup,
newDiscountRateMarkup
Expand All @@ -210,15 +213,15 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
function setCollateralTokenParams(
address tokenAddr,
uint256 minCollateralRatio
) external onlyManagement {
) external onlyRole(GOVERNOR_ROLE) {
TERM_VAULT_EVENT_EMITTER.emitMinCollateralRatioUpdated(
tokenAddr,
minCollateralRatio
);
repoTokenListData.collateralTokenParams[tokenAddr] = minCollateralRatio;
}

function setRepoTokenBlacklist(address repoToken, bool blacklisted) external onlyManagement {
function setRepoTokenBlacklist(address repoToken, bool blacklisted) external onlyRole(GOVERNOR_ROLE) {
TERM_VAULT_EVENT_EMITTER.emitRepoTokenBlacklistUpdated(repoToken, blacklisted);
repoTokenBlacklist[repoToken] = blacklisted;
}
Expand Down Expand Up @@ -1091,13 +1094,15 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
* @param _yearnVault The address of the Yearn vault
* @param _discountRateAdapter The address of the discount rate adapter
* @param _eventEmitter The address of the event emitter
* @param _governorAddress The address of the governor
*/
constructor(
address _asset,
string memory _name,
address _yearnVault,
address _discountRateAdapter,
address _eventEmitter
address _eventEmitter,
address _governorAddress
) BaseStrategy(_asset, _name) {
YEARN_VAULT = IERC4626(_yearnVault);
TERM_VAULT_EVENT_EMITTER = ITermVaultEvents(_eventEmitter);
Expand All @@ -1111,6 +1116,8 @@ contract Strategy is BaseStrategy, Pausable, ReentrancyGuard {
requiredReserveRatio = 0.2e18;
discountRateMarkup = 0.005e18;
repoTokenConcentrationLimit = 0.1e18;

_grantRole(GOVERNOR_ROLE, _governorAddress);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing assignment of DEFAULT_ADMIN_ROLE

Consider assigning the DEFAULT_ADMIN_ROLE to the governor address to enable future role management. Without an account holding DEFAULT_ADMIN_ROLE, it won't be possible to grant or revoke roles after deployment.

Apply this diff to the constructor to set the DEFAULT_ADMIN_ROLE:

 constructor(
     address _asset,
     string memory _name,
     address _yearnVault,
     address _discountRateAdapter,
     address _eventEmitter,
     address _governorAddress
 ) BaseStrategy(_asset, _name) {
     YEARN_VAULT = IERC4626(_yearnVault);
     TERM_VAULT_EVENT_EMITTER = ITermVaultEvents(_eventEmitter);
     PURCHASE_TOKEN_PRECISION = 10 ** ERC20(asset).decimals();
     discountRateAdapter = ITermDiscountRateAdapter(_discountRateAdapter);
     IERC20(_asset).safeApprove(_yearnVault, type(uint256).max);
     timeToMaturityThreshold = 45 days;
     requiredReserveRatio = 0.2e18;
     discountRateMarkup = 0.005e18;
     repoTokenConcentrationLimit = 0.1e18;

+    _grantRole(DEFAULT_ADMIN_ROLE, _governorAddress);
     _grantRole(GOVERNOR_ROLE, _governorAddress);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_grantRole(GOVERNOR_ROLE, _governorAddress);
constructor(
address _asset,
string memory _name,
address _yearnVault,
address _discountRateAdapter,
address _eventEmitter,
address _governorAddress
) BaseStrategy(_asset, _name) {
YEARN_VAULT = IERC4626(_yearnVault);
TERM_VAULT_EVENT_EMITTER = ITermVaultEvents(_eventEmitter);
PURCHASE_TOKEN_PRECISION = 10 ** ERC20(asset).decimals();
discountRateAdapter = ITermDiscountRateAdapter(_discountRateAdapter);
IERC20(_asset).safeApprove(_yearnVault, type(uint256).max);
timeToMaturityThreshold = 45 days;
requiredReserveRatio = 0.2e18;
discountRateMarkup = 0.005e18;
repoTokenConcentrationLimit = 0.1e18;
_grantRole(DEFAULT_ADMIN_ROLE, _governorAddress);
_grantRole(GOVERNOR_ROLE, _governorAddress);
}

}

/*//////////////////////////////////////////////////////////////
Expand Down
22 changes: 11 additions & 11 deletions src/test/TestUSDCIntegration.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ contract TestUSDCIntegration is Setup {
repoToken1MonthAuction = new MockTermAuction(repoToken1Month);
repoToken1YearAuction = new MockTermAuction(repoToken1Year);

vm.startPrank(management);
vm.startPrank(governor);
termStrategy.setCollateralTokenParams(address(mockCollateral), 0.5e18);
termStrategy.setTimeToMaturityThreshold(3 weeks);
termStrategy.setRepoTokenConcentrationLimit(1e18);
Expand Down Expand Up @@ -248,7 +248,7 @@ contract TestUSDCIntegration is Setup {

termController.setOracleRate(repoToken1Month.termRepoId(), 0.05e6);

vm.startPrank(management);
vm.startPrank(governor);
termStrategy.setCollateralTokenParams(address(mockCollateral), 0.5e18);
termStrategy.setTimeToMaturityThreshold(3 weeks);
vm.stopPrank();
Expand Down Expand Up @@ -303,11 +303,11 @@ contract TestUSDCIntegration is Setup {
function testRepoTokenBlacklist() public {
address testUser = vm.addr(0x11111);
vm.prank(testUser);
vm.expectRevert("!management");
vm.expectRevert();
termStrategy.setRepoTokenBlacklist(address(repoToken1Week), true);
vm.stopPrank();

vm.prank(management);
vm.prank(governor);
termStrategy.setRepoTokenBlacklist(address(repoToken1Week), true);
vm.stopPrank();

Expand All @@ -320,13 +320,13 @@ contract TestUSDCIntegration is Setup {
address testUser = vm.addr(0x11111);
mockUSDC.mint(testUser, 1e18);
vm.prank(testUser);
vm.expectRevert("!management");
vm.expectRevert();
termStrategy.pauseDeposit();
vm.expectRevert("!management");
vm.expectRevert();
termStrategy.unpauseDeposit();
vm.stopPrank();

vm.prank(management);
vm.prank(governor);
termStrategy.pauseDeposit();
vm.stopPrank();

Expand All @@ -338,7 +338,7 @@ contract TestUSDCIntegration is Setup {
IERC4626(address(termStrategy)).deposit(1e6, testUser);
vm.stopPrank();

vm.prank(management);
vm.prank(governor);
termStrategy.unpauseDeposit();
vm.stopPrank();

Expand All @@ -354,14 +354,14 @@ contract TestUSDCIntegration is Setup {
TermDiscountRateAdapter valid = new TermDiscountRateAdapter(address(termController), adminWallet);

vm.prank(testUser);
vm.expectRevert("!management");
vm.expectRevert();
termStrategy.setDiscountRateAdapter(address(valid));

vm.prank(management);
vm.prank(governor);
vm.expectRevert();
termStrategy.setDiscountRateAdapter(address(invalid));

vm.prank(management);
vm.prank(governor);
termStrategy.setDiscountRateAdapter(address(valid));
vm.stopPrank();

Expand Down
10 changes: 7 additions & 3 deletions src/test/TestUSDCOffers.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ contract TestUSDCSubmitOffer is Setup {
repoToken1WeekAuction = new MockTermAuction(repoToken1Week);
repoToken100WeekAuction = new MockTermAuction(repoToken100Week);

vm.startPrank(management);
vm.startPrank(governor);
termStrategy.setCollateralTokenParams(address(mockCollateral), 0.5e18);
termStrategy.setTimeToMaturityThreshold(3 weeks);
termStrategy.setRepoTokenConcentrationLimit(1e18);
Expand Down Expand Up @@ -88,9 +88,11 @@ contract TestUSDCSubmitOffer is Setup {
}

function testSubmitOfferBelowLiquidReserveRatio() public {
vm.startPrank(management);
vm.startPrank(governor);
termStrategy.setRequiredReserveRatio(0.5e18);
vm.stopPrank();

vm.startPrank(management);
vm.expectRevert(abi.encodeWithSelector(Strategy.BalanceBelowRequiredReserveRatio.selector));
termStrategy.submitAuctionOffer(
repoToken1WeekAuction, address(repoToken1Week), bytes32("offer id hash 1"), bytes32("test price"), 51e6
Expand Down Expand Up @@ -132,9 +134,11 @@ contract TestUSDCSubmitOffer is Setup {
}

function testSubmitOfferFailsIfMinCollatRatioisZero() public {
vm.startPrank(management);
vm.startPrank(governor);
termStrategy.setCollateralTokenParams(address(mockCollateral), 0);
vm.stopPrank();

vm.startPrank(management);
vm.expectRevert(abi.encodeWithSelector(RepoTokenList.InvalidRepoToken.selector, address(repoToken1Week)));
termStrategy.submitAuctionOffer(
repoToken1WeekAuction, address(repoToken1Week), bytes32("offer id hash 1"), bytes32("test price"), 1e6
Expand Down
Loading