From aae173f825b95a9e7923a7f53d5e9e13dcb85d58 Mon Sep 17 00:00:00 2001 From: sakulstra Date: Fri, 5 Jul 2024 13:57:11 +0200 Subject: [PATCH] feat: use ownable --- .gitmodules | 3 + lib/openzeppelin-contracts-upgradeable | 1 + remappings.txt | 1 + src/contracts/ERC20.sol | 8 +-- src/contracts/IStakeToken.sol | 1 + src/contracts/RoleManager.sol | 77 -------------------------- src/contracts/StakeToken.sol | 51 +++++++---------- tests/Slashing.t.sol | 14 ++--- 8 files changed, 37 insertions(+), 119 deletions(-) create mode 160000 lib/openzeppelin-contracts-upgradeable delete mode 100644 src/contracts/RoleManager.sol diff --git a/.gitmodules b/.gitmodules index b96e019..08e0cb5 100644 --- a/.gitmodules +++ b/.gitmodules @@ -10,3 +10,6 @@ [submodule "lib/aave-helpers"] path = lib/aave-helpers url = https://github.com/bgd-labs/aave-helpers +[submodule "lib/openzeppelin-contracts-upgradeable"] + path = lib/openzeppelin-contracts-upgradeable + url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable diff --git a/lib/openzeppelin-contracts-upgradeable b/lib/openzeppelin-contracts-upgradeable new file mode 160000 index 0000000..723f8ca --- /dev/null +++ b/lib/openzeppelin-contracts-upgradeable @@ -0,0 +1 @@ +Subproject commit 723f8cab09cdae1aca9ec9cc1cfa040c2d4b06c1 diff --git a/remappings.txt b/remappings.txt index 91db8f5..e26dc0e 100644 --- a/remappings.txt +++ b/remappings.txt @@ -4,6 +4,7 @@ etherscan/stkAAVE:openzeppelin-contracts/=etherscan/stkAAVE/StakedAaveV3/lib/ope ds-test/=lib/forge-std/lib/ds-test/src/ forge-std/=lib/aave-helpers/lib/forge-std/src/ openzeppelin-contracts/=lib/openzeppelin-contracts/ +openzeppelin-contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/ aave-token-v3/=lib/aave-token-v3/src/ aave-helpers/=lib/aave-helpers/src/ aave-address-book/=lib/aave-helpers/lib/aave-address-book/src diff --git a/src/contracts/ERC20.sol b/src/contracts/ERC20.sol index 3b911e0..3dae2d6 100644 --- a/src/contracts/ERC20.sol +++ b/src/contracts/ERC20.sol @@ -8,11 +8,11 @@ pragma solidity ^0.8.20; import {IERC20} from 'openzeppelin-contracts/contracts/token/ERC20/IERC20.sol'; import {IERC20Metadata} from 'openzeppelin-contracts/contracts/token/ERC20/extensions/IERC20Metadata.sol'; -import {Context} from 'openzeppelin-contracts/contracts/utils/Context.sol'; import {IERC20Errors} from 'openzeppelin-contracts/contracts/interfaces/draft-IERC6093.sol'; -import {Initializable} from 'openzeppelin-contracts/contracts/proxy/utils/Initializable.sol'; +import {Initializable} from 'openzeppelin-contracts-upgradeable/contracts/proxy/utils/Initializable.sol'; import {SafeCast} from 'openzeppelin-contracts/contracts/utils/math/SafeCast.sol'; import {DelegationMode} from 'aave-token-v3/DelegationAwareBalance.sol'; +import {OwnableUpgradeable} from 'openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol'; /** * @dev Implementation of the {IERC20} interface. @@ -37,7 +37,7 @@ import {DelegationMode} from 'aave-token-v3/DelegationAwareBalance.sol'; * by listening to said events. Other implementations of the EIP may not emit * these events, as it isn't required by the specification. */ -abstract contract ERC20 is Context, Initializable, IERC20, IERC20Metadata, IERC20Errors { +abstract contract ERC20 is OwnableUpgradeable, IERC20, IERC20Metadata, IERC20Errors { struct DelegationAwareBalance { uint104 balance; // maximum is 10T of 18 decimal asset uint72 delegatedPropositionBalance; @@ -68,7 +68,7 @@ abstract contract ERC20 is Context, Initializable, IERC20, IERC20Metadata, IERC2 function _initializeMetadata( string calldata name_, string calldata symbol_ - ) internal initializer { + ) internal onlyInitializing { _name = name_; _symbol = symbol_; } diff --git a/src/contracts/IStakeToken.sol b/src/contracts/IStakeToken.sol index e27c5b3..1c2644d 100644 --- a/src/contracts/IStakeToken.sol +++ b/src/contracts/IStakeToken.sol @@ -22,6 +22,7 @@ interface IStakeToken is IAaveDistributionManager { event ExchangeRateChanged(uint216 exchangeRate); event FundsReturned(uint256 amount); event SlashingSettled(); + event SlashingAdminChanged(address newAdmin); /** * @dev Allows staking a specified amount of STAKED_TOKEN diff --git a/src/contracts/RoleManager.sol b/src/contracts/RoleManager.sol deleted file mode 100644 index fdd684f..0000000 --- a/src/contracts/RoleManager.sol +++ /dev/null @@ -1,77 +0,0 @@ -pragma solidity ^0.8.0; - -/** - * @title RoleManager - * @notice Generic role manager to manage slashing and cooldown admin in StakedAaveV3. - * It implements a claim admin role pattern to safely migrate between different admin addresses - * @author Aave - **/ -contract RoleManager { - struct InitAdmin { - uint256 role; - address admin; - } - - mapping(uint256 => address) private _admins; - mapping(uint256 => address) private _pendingAdmins; - - event PendingAdminChanged(address indexed newPendingAdmin, uint256 role); - event RoleClaimed(address indexed newAdmin, uint256 role); - - modifier onlyRoleAdmin(uint256 role) { - require(_admins[role] == msg.sender, 'CALLER_NOT_ROLE_ADMIN'); - _; - } - - modifier onlyPendingRoleAdmin(uint256 role) { - require(_pendingAdmins[role] == msg.sender, 'CALLER_NOT_PENDING_ROLE_ADMIN'); - _; - } - - /** - * @dev returns the admin associated with the specific role - * @param role the role associated with the admin being returned - **/ - function getAdmin(uint256 role) public view returns (address) { - return _admins[role]; - } - - /** - * @dev returns the pending admin associated with the specific role - * @param role the role associated with the pending admin being returned - **/ - function getPendingAdmin(uint256 role) public view returns (address) { - return _pendingAdmins[role]; - } - - /** - * @dev sets the pending admin for a specific role - * @param role the role associated with the new pending admin being set - * @param newPendingAdmin the address of the new pending admin - **/ - function setPendingAdmin(uint256 role, address newPendingAdmin) public onlyRoleAdmin(role) { - _pendingAdmins[role] = newPendingAdmin; - emit PendingAdminChanged(newPendingAdmin, role); - } - - /** - * @dev allows the caller to become a specific role admin - * @param role the role associated with the admin claiming the new role - **/ - function claimRoleAdmin(uint256 role) external onlyPendingRoleAdmin(role) { - _admins[role] = msg.sender; - _pendingAdmins[role] = address(0); - emit RoleClaimed(msg.sender, role); - } - - function _initAdmins(InitAdmin[] memory initAdmins) internal { - for (uint256 i = 0; i < initAdmins.length; i++) { - require( - _admins[initAdmins[i].role] == address(0) && initAdmins[i].admin != address(0), - 'ADMIN_CANNOT_BE_INITIALIZED' - ); - _admins[initAdmins[i].role] = initAdmins[i].admin; - emit RoleClaimed(initAdmins[i].admin, initAdmins[i].role); - } - } -} diff --git a/src/contracts/StakeToken.sol b/src/contracts/StakeToken.sol index 30a2217..0675d7a 100644 --- a/src/contracts/StakeToken.sol +++ b/src/contracts/StakeToken.sol @@ -10,7 +10,6 @@ import {IERC20Permit} from 'openzeppelin-contracts/contracts/token/ERC20/extensi import {ERC20Permit} from './ERC20Permit.sol'; import {AaveDistributionManager} from './AaveDistributionManager.sol'; -import {RoleManager} from './RoleManager.sol'; import {IStakeToken} from './IStakeToken.sol'; import {IAaveDistributionManager} from './IAaveDistributionManager.sol'; import {IRewardsController} from './IRewardsController.sol'; @@ -18,15 +17,12 @@ import {IRewardsController} from './IRewardsController.sol'; import {PercentageMath} from './lib/PercentageMath.sol'; import {DistributionTypes} from './lib/DistributionTypes.sol'; -contract StakeToken is ERC20Permit, AaveDistributionManager, RoleManager, IStakeToken { +contract StakeToken is ERC20Permit, AaveDistributionManager, IStakeToken { using SafeERC20 for IERC20; using PercentageMath for uint256; using SafeCast for uint256; using SafeCast for uint104; - uint256 public constant SLASH_ADMIN_ROLE = 0; - uint256 public constant COOLDOWN_ADMIN_ROLE = 1; - uint256 public constant CLAIM_HELPER_ROLE = 2; uint216 public constant INITIAL_EXCHANGE_RATE = 1e18; uint256 public constant EXCHANGE_RATE_UNIT = 1e18; @@ -56,18 +52,10 @@ contract StakeToken is ERC20Permit, AaveDistributionManager, RoleManager, IStake /// @notice Flag determining if there's an ongoing slashing event that needs to be settled bool private DEPRECATED_inPostSlashingPeriod; - modifier onlySlashingAdmin() { - require(msg.sender == getAdmin(SLASH_ADMIN_ROLE), 'CALLER_NOT_SLASHING_ADMIN'); - _; - } + address internal slashingAdmin; - modifier onlyCooldownAdmin() { - require(msg.sender == getAdmin(COOLDOWN_ADMIN_ROLE), 'CALLER_NOT_COOLDOWN_ADMIN'); - _; - } - - modifier onlyClaimHelper() { - require(msg.sender == getAdmin(CLAIM_HELPER_ROLE), 'CALLER_NOT_CLAIM_HELPER'); + modifier onlySlashingAdmin() { + require(msg.sender == slashingAdmin, 'CALLER_NOT_SLASHING_ADMIN'); _; } @@ -99,14 +87,8 @@ contract StakeToken is ERC20Permit, AaveDistributionManager, RoleManager, IStake uint256 cooldownSeconds ) external virtual initializer { _initializeMetadata(name, symbol); - - InitAdmin[] memory initAdmins = new InitAdmin[](3); - initAdmins[0] = InitAdmin(SLASH_ADMIN_ROLE, slashingAdmin); - initAdmins[1] = InitAdmin(COOLDOWN_ADMIN_ROLE, cooldownPauseAdmin); - initAdmins[2] = InitAdmin(CLAIM_HELPER_ROLE, claimHelper); - - _initAdmins(initAdmins); - + _transferOwnership(slashingAdmin); + _setSlashingAdmin(slashingAdmin); _setMaxSlashablePercentage(maxSlashablePercentage); _setCooldownSeconds(cooldownSeconds); _updateExchangeRate(INITIAL_EXCHANGE_RATE); @@ -129,7 +111,7 @@ contract StakeToken is ERC20Permit, AaveDistributionManager, RoleManager, IStake _configureAssets(assetsConfigInput); } - function setDistributionEnd(uint256 newDistributionEnd) external onlyEmissionManager { + function setDistributionEnd(uint256 newDistributionEnd) external onlyOwner { require(newDistributionEnd >= block.timestamp, 'END_MUST_BE_GE_NOW'); AssetData storage assetConfig = assets[address(this)]; _updateAssetStateInternal(address(this), assetConfig, totalSupply()); @@ -137,6 +119,15 @@ contract StakeToken is ERC20Permit, AaveDistributionManager, RoleManager, IStake emit DistributionEndChanged(newDistributionEnd); } + function setSlashingAdmin(address newSlashingAdmin) external onlyOwner { + _setSlashingAdmin(newSlashingAdmin); + } + + function _setSlashingAdmin(address newSlashingAdmin) internal { + slashingAdmin = newSlashingAdmin; + emit SlashingAdminChanged(newSlashingAdmin); + } + /// @inheritdoc IStakeToken function previewStake(uint256 assets) public view returns (uint256) { return (assets * _currentExchangeRate) / EXCHANGE_RATE_UNIT; @@ -179,7 +170,7 @@ contract StakeToken is ERC20Permit, AaveDistributionManager, RoleManager, IStake } /// @inheritdoc IStakeToken - function cooldownOnBehalfOf(address from) external onlyClaimHelper { + function cooldownOnBehalfOf(address from) external onlyOwner { _cooldown(from); } @@ -189,7 +180,7 @@ contract StakeToken is ERC20Permit, AaveDistributionManager, RoleManager, IStake } /// @inheritdoc IStakeToken - function redeemOnBehalf(address from, address to, uint256 amount) external onlyClaimHelper { + function redeemOnBehalf(address from, address to, uint256 amount) external onlyOwner { _redeem(from, to, amount.toUint104()); } @@ -203,7 +194,7 @@ contract StakeToken is ERC20Permit, AaveDistributionManager, RoleManager, IStake address from, address to, uint256 amount - ) external onlyClaimHelper returns (uint256) { + ) external onlyOwner returns (uint256) { return _claimRewards(from, to, amount); } @@ -219,7 +210,7 @@ contract StakeToken is ERC20Permit, AaveDistributionManager, RoleManager, IStake address to, uint256 claimAmount, uint256 redeemAmount - ) external onlyClaimHelper { + ) external onlyOwner { _claimRewards(from, to, claimAmount); _redeem(from, to, redeemAmount.toUint104()); } @@ -271,7 +262,7 @@ contract StakeToken is ERC20Permit, AaveDistributionManager, RoleManager, IStake } /// @inheritdoc IStakeToken - function setCooldownSeconds(uint256 cooldownSeconds) external onlyCooldownAdmin { + function setCooldownSeconds(uint256 cooldownSeconds) external onlyOwner { _setCooldownSeconds(cooldownSeconds); } diff --git a/tests/Slashing.t.sol b/tests/Slashing.t.sol index a19559b..0add697 100644 --- a/tests/Slashing.t.sol +++ b/tests/Slashing.t.sol @@ -7,6 +7,7 @@ import {ERC20} from 'openzeppelin-contracts/contracts/token/ERC20/ERC20.sol'; import {ProxyAdmin} from 'openzeppelin-contracts/contracts/proxy/transparent/ProxyAdmin.sol'; import {TransparentUpgradeableProxy} from 'openzeppelin-contracts/contracts/proxy/transparent/TransparentUpgradeableProxy.sol'; import {StkTestUtils} from './StkTestUtils.t.sol'; +import {OwnableUpgradeable} from 'openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol'; contract Slashing is StkTestUtils { function setUp() public { @@ -167,14 +168,11 @@ contract Slashing is StkTestUtils { function test_changeSlashingAdmin() public { address newUser = vm.addr(1000); - try stakeToken.setPendingAdmin(stakeToken.SLASH_ADMIN_ROLE(), newUser) {} catch Error( - string memory reason - ) { - require(keccak256(bytes(reason)) == keccak256(bytes('CALLER_NOT_ROLE_ADMIN'))); - } + vm.expectRevert( + abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, address(this)) + ); + stakeToken.setSlashingAdmin(newUser); vm.startPrank(admin); - stakeToken.setPendingAdmin(stakeToken.SLASH_ADMIN_ROLE(), newUser); - vm.startPrank(newUser); - stakeToken.claimRoleAdmin(stakeToken.SLASH_ADMIN_ROLE()); + stakeToken.setSlashingAdmin(newUser); } }