diff --git a/contracts/PointTokenHub.sol b/contracts/PointTokenHub.sol index a72648f..8d7dd09 100644 --- a/contracts/PointTokenHub.sol +++ b/contracts/PointTokenHub.sol @@ -2,14 +2,16 @@ pragma solidity ^0.8.13; import {UUPSUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/proxy/utils/UUPSUpgradeable.sol"; -import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; +import {AccessControlUpgradeable} from + "openzeppelin-contracts-upgradeable/contracts/access/AccessControlUpgradeable.sol"; import {LibString} from "solady/utils/LibString.sol"; import {ERC20} from "solmate/tokens/ERC20.sol"; import {PToken} from "./PToken.sol"; -contract PointTokenHub is UUPSUpgradeable, OwnableUpgradeable { - mapping(address => bool) isTrusted; // user => isTrusted +contract PointTokenHub is UUPSUpgradeable, AccessControlUpgradeable { + bytes32 public constant MINT_BURN_ROLE = keccak256("MINT_BURN_ROLE"); + mapping(bytes32 => PToken) public pointTokens; // pointsId => pointTokens mapping(bytes32 => RedemptionParams) public redemptionParams; // pointsId => redemptionParams @@ -19,26 +21,21 @@ contract PointTokenHub is UUPSUpgradeable, OwnableUpgradeable { bool isMerkleBased; } - event Trusted(address indexed user, bool isTrusted); event RewardRedemptionSet(bytes32 indexed pointsId, ERC20 rewardToken, uint256 exchangeRate, bool isMerkleBased); - error OnlyTrusted(); - - modifier onlyTrusted() { - if (!isTrusted[msg.sender]) revert OnlyTrusted(); - _; - } - constructor() { _disableInitializers(); } function initialize() public initializer { - __Ownable_init(msg.sender); __UUPSUpgradeable_init(); + __AccessControl_init_unchained(); + _grantRole(DEFAULT_ADMIN_ROLE, msg.sender); } - function mint(address _account, bytes32 _pointsId, uint256 _amount) external onlyTrusted { + // Mint and burn --- + + function mint(address _account, bytes32 _pointsId, uint256 _amount) external onlyRole(MINT_BURN_ROLE) { if (address(pointTokens[_pointsId]) == address(0)) { (string memory name, string memory symbol) = LibString.unpackTwo(_pointsId); // Assume the points id was created using LibString.packTwo. pointTokens[_pointsId] = new PToken{salt: _pointsId}(name, symbol, 18); @@ -47,7 +44,7 @@ contract PointTokenHub is UUPSUpgradeable, OwnableUpgradeable { pointTokens[_pointsId].mint(_account, _amount); } - function burn(address _account, bytes32 _pointsId, uint256 _amount) external onlyTrusted { + function burn(address _account, bytes32 _pointsId, uint256 _amount) external onlyRole(MINT_BURN_ROLE) { pointTokens[_pointsId].burn(_account, _amount); } @@ -57,20 +54,11 @@ contract PointTokenHub is UUPSUpgradeable, OwnableUpgradeable { // Should only be used after rewards have been claimed. function setRedemption(bytes32 _pointsId, ERC20 _rewardToken, uint256 _exchangeRate, bool _isMerkleBased) external - onlyOwner + onlyRole(DEFAULT_ADMIN_ROLE) { redemptionParams[_pointsId] = RedemptionParams(_rewardToken, _exchangeRate, _isMerkleBased); emit RewardRedemptionSet(_pointsId, _rewardToken, _exchangeRate, _isMerkleBased); } - function grantTokenOwnership(address _newOwner, bytes32 _pointsId) external onlyOwner { - pointTokens[_pointsId].transferOwnership(_newOwner); - } - - function setTrusted(address _user, bool _isTrusted) external onlyOwner { - isTrusted[_user] = _isTrusted; - emit Trusted(_user, _isTrusted); - } - - function _authorizeUpgrade(address _newImplementation) internal override onlyOwner {} + function _authorizeUpgrade(address _newImplementation) internal override onlyRole(DEFAULT_ADMIN_ROLE) {} } diff --git a/contracts/PointTokenVault.sol b/contracts/PointTokenVault.sol index e8ae7f3..f9ca506 100644 --- a/contracts/PointTokenVault.sol +++ b/contracts/PointTokenVault.sol @@ -3,17 +3,19 @@ pragma solidity ^0.8.13; import {MerkleProof} from "@openzeppelin/contracts/utils/cryptography/MerkleProof.sol"; import {UUPSUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/proxy/utils/UUPSUpgradeable.sol"; -import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; +import {AccessControlUpgradeable} from + "openzeppelin-contracts-upgradeable/contracts/access/AccessControlUpgradeable.sol"; import {ERC20} from "solmate/tokens/ERC20.sol"; import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol"; import {PointTokenHub} from "./PointTokenHub.sol"; -contract PointTokenVault is UUPSUpgradeable, OwnableUpgradeable { +contract PointTokenVault is UUPSUpgradeable, AccessControlUpgradeable { using SafeTransferLib for ERC20; using MerkleProof for bytes32[]; - bytes32 public constant REDEMPTION_RIGHTS_PREFIX = keccak256(abi.encodePacked("REDEMPTION_RIGHTS")); + bytes32 public constant MERKLE_UPDATER_ROLE = keccak256("MERKLE_UPDATER_ROLE"); + bytes32 public constant REDEMPTION_RIGHTS_PREFIX = keccak256("REDEMPTION_RIGHTS"); PointTokenHub public pointTokenHub; @@ -42,15 +44,15 @@ contract PointTokenVault is UUPSUpgradeable, OwnableUpgradeable { error ProofInvalidOrExpired(); error ClaimTooLarge(); error RewardsNotReleased(); - error InvalidPointsId(); constructor() { _disableInitializers(); } function initialize(PointTokenHub _pointTokenHub) public initializer { - __Ownable_init(msg.sender); __UUPSUpgradeable_init(); + __AccessControl_init_unchained(); + _grantRole(DEFAULT_ADMIN_ROLE, msg.sender); pointTokenHub = _pointTokenHub; } @@ -65,9 +67,9 @@ contract PointTokenVault is UUPSUpgradeable, OwnableUpgradeable { function withdraw(ERC20 _token, uint256 _amount, address _receiver) public { balances[msg.sender][_token] -= _amount; - emit Withdraw(_receiver, address(_token), _amount); - _token.safeTransfer(_receiver, _amount); + + emit Withdraw(_receiver, address(_token), _amount); } function claimPointTokens(Claim[] calldata _claims, address _account) external { @@ -141,7 +143,7 @@ contract PointTokenVault is UUPSUpgradeable, OwnableUpgradeable { // Admin --- - function updateRoot(bytes32 _newRoot) external onlyOwner { + function updateRoot(bytes32 _newRoot) external onlyRole(MERKLE_UPDATER_ROLE) { prevRoot = currRoot; currRoot = _newRoot; emit RootUpdated(prevRoot, currRoot); @@ -149,11 +151,15 @@ contract PointTokenVault is UUPSUpgradeable, OwnableUpgradeable { // To handle arbitrary reward claiming logic. // TODO: kinda scary, can we restrict what the admin can do here? - function execute(address _to, bytes memory _data, uint256 _txGas) external onlyOwner returns (bool success) { + function execute(address _to, bytes memory _data, uint256 _txGas) + external + onlyRole(DEFAULT_ADMIN_ROLE) + returns (bool success) + { assembly { success := delegatecall(_txGas, _to, add(_data, 0x20), mload(_data), 0, 0) } } - function _authorizeUpgrade(address _newImplementation) internal override onlyOwner {} + function _authorizeUpgrade(address _newImplementation) internal override onlyRole(DEFAULT_ADMIN_ROLE) {} } diff --git a/contracts/test/PointTokenVault.t.sol b/contracts/test/PointTokenVault.t.sol index 4b9c5c2..16f67e2 100644 --- a/contracts/test/PointTokenVault.t.sol +++ b/contracts/test/PointTokenVault.t.sol @@ -7,6 +7,7 @@ import {PointTokenHub} from "../PointTokenHub.sol"; import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import {ERC1967Utils} from "openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Utils.sol"; +import {IAccessControl} from "@openzeppelin/contracts/access/IAccessControl.sol"; import {MockERC20} from "solmate/test/utils/mocks/MockERC20.sol"; @@ -28,6 +29,7 @@ contract PointTokenVaultTest is Test { address toly = makeAddr("toly"); address illia = makeAddr("illia"); address admin = makeAddr("admin"); + address merkleUpdater = makeAddr("merkleUpdater"); function setUp() public { pointTokenHub = PointTokenHub( @@ -39,10 +41,11 @@ contract PointTokenVaultTest is Test { ) ); - pointTokenHub.setTrusted(address(pointTokenVault), true); + pointTokenHub.grantRole(pointTokenHub.DEFAULT_ADMIN_ROLE(), address(admin)); + pointTokenHub.grantRole(pointTokenHub.MINT_BURN_ROLE(), address(pointTokenVault)); - pointTokenHub.transferOwnership(address(admin)); - pointTokenVault.transferOwnership(address(admin)); + pointTokenVault.grantRole(pointTokenVault.DEFAULT_ADMIN_ROLE(), address(admin)); + pointTokenVault.grantRole(pointTokenVault.MERKLE_UPDATER_ROLE(), address(merkleUpdater)); // Deploy a mock token pointEarningToken = new MockERC20("Test Token", "TST", 18); @@ -103,15 +106,23 @@ contract PointTokenVaultTest is Test { PointTokenVault newPointTokenVault = new PointTokenVault(); // Only admin can upgrade - vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, vitalik)); + vm.expectRevert( + abi.encodeWithSelector( + IAccessControl.AccessControlUnauthorizedAccount.selector, vitalik, pointTokenHub.DEFAULT_ADMIN_ROLE() + ) + ); vm.prank(vitalik); pointTokenHub.upgradeToAndCall(address(newPointTokenHub), bytes("")); vm.prank(admin); pointTokenHub.upgradeToAndCall(address(newPointTokenHub), bytes("")); - // Only admin can upgrade - vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, vitalik)); + // Only admin role can upgrade + vm.expectRevert( + abi.encodeWithSelector( + IAccessControl.AccessControlUnauthorizedAccount.selector, vitalik, pointTokenVault.DEFAULT_ADMIN_ROLE() + ) + ); vm.prank(vitalik); pointTokenVault.upgradeToAndCall(address(newPointTokenVault), bytes("")); @@ -138,20 +149,28 @@ contract PointTokenVaultTest is Test { function test_UpdateRoot() public { bytes32 root = 0x5842148bc6ebeb52af882a317c765fccd3ae80589b21a9b8cbf21abb630e46a7; - // Only admin can update root - vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, vitalik)); + // Only merkle root updater role can update root + vm.expectRevert( + abi.encodeWithSelector( + IAccessControl.AccessControlUnauthorizedAccount.selector, vitalik, pointTokenVault.MERKLE_UPDATER_ROLE() + ) + ); vm.prank(vitalik); pointTokenVault.updateRoot(root); // Update the root - vm.prank(admin); + vm.prank(merkleUpdater); pointTokenVault.updateRoot(root); } function test_ExecuteAuth(address lad) public { vm.assume(lad != admin); // Only admin can exec - vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, lad)); + vm.expectRevert( + abi.encodeWithSelector( + IAccessControl.AccessControlUnauthorizedAccount.selector, lad, pointTokenVault.DEFAULT_ADMIN_ROLE() + ) + ); vm.prank(lad); pointTokenVault.execute(vitalik, bytes(""), 0); } @@ -187,7 +206,7 @@ contract PointTokenVaultTest is Test { bytes32 pointsId = LibString.packTwo("Eigen Layer Point", "pEL"); - vm.prank(admin); + vm.prank(merkleUpdater); pointTokenVault.updateRoot(root); // Can't claim with the wrong proof @@ -226,7 +245,7 @@ contract PointTokenVaultTest is Test { // Merkle tree created from leaves [keccack(vitalik, pointsId, 1e18), keccack(toly, pointsId, 0.5e18)]. bytes32 root = 0x4e40a10ce33f33a4786960a8bb843fe0e170b651acd83da27abc97176c4bed3c; - vm.prank(admin); + vm.prank(merkleUpdater); pointTokenVault.updateRoot(root); bytes32[] memory vitalikProof = new bytes32[](1); @@ -262,7 +281,7 @@ contract PointTokenVaultTest is Test { bytes32[] memory proof = new bytes32[](1); proof[0] = 0x6d0fcb8de12b1f57f81e49fa18b641487b932cdba4f064409fde3b05d3824ca2; - vm.prank(admin); + vm.prank(merkleUpdater); pointTokenVault.updateRoot(root); PointTokenVault.Claim[] memory claims = new PointTokenVault.Claim[](1); @@ -294,10 +313,10 @@ contract PointTokenVaultTest is Test { validProofVitalikPToken[1] = 0xae126f1299213c869259b52ab24f7270f3cce1de54c187271c52373d8947c2fe; // Set up the Merkle root and redemption parameters - vm.startPrank(admin); + vm.prank(merkleUpdater); pointTokenVault.updateRoot(root); + vm.prank(admin); pointTokenHub.setRedemption(pointsId, rewardToken, 2e18, true); // Set isMerkleBased true - vm.stopPrank(); // Mint tokens and distribute vm.prank(admin); @@ -337,7 +356,7 @@ contract PointTokenVaultTest is Test { bytes32 pointsId = LibString.packTwo("Eigen Layer Point", "pEL"); - vm.prank(admin); + vm.prank(merkleUpdater); pointTokenVault.updateRoot(root); // Can do a partial claim @@ -366,6 +385,7 @@ contract PointTokenVaultTest is Test { // decimals and dust checks // implementation is locked down // fuzz deposit/withdraw/claim + // not just anyone can mint or burn // redemption rights // only msg.sender can use redemption rights // must have point token to use redemption rights