Audit fix (#17)

* audit fix

* change voting period

* fix kton interest

* fix test

* little change

* fix

* RingDAO init params

* fmt

* access control enumerable upgradeable

* update deploy script

* typo

.github/workflows/ci.yml

@@ -32,6 +32,9 @@ jobs:
         with:
           version: nightly
 
+      - name: Install Dependencies
+        run: yarn --frozen-lockfile
+
       - name: Run Forge build
         run: |
           forge --version

foundry.toml

@@ -2,7 +2,7 @@
 src = "src"
 out = "out"
 libs = ["lib"]
-sender = "0x94655E3Af9AbCe0806254574903f174b91305Aa1"
+sender = "0xb34Fcb159951e6EBBeF063eCB21C1c6aa65729E7"
 force = true
 ffi = true
 ast = true

script/Deploy.s.sol

@@ -13,6 +13,12 @@ import {GovernanceRing} from "../src/governance/GovernanceRing.sol";
 import {RingDAO, IVotes, TimelockControllerUpgradeable} from "../src/governance/RingDAO.sol";
 
 contract DeployScript is Script {
+    address deposit = 0x0634cf1c19Ce993A468Fa7c362208141C854736c;
+    address timelock = 0xDAE15e7DA1C998a650796541DF6fFEB437cC20E4;
+    address gRING = 0x4Ef76E24851f694BEe6a64F6345b873081d4F308;
+    address ringDAO = 0x3aaF69F34AA8527b4CEe546DD691aD24c1fB7AEa;
+    address hub = 0xD497EF1C7A8732e0761d57429Df5edc17fEaD6e6;
+
     function setUp() public {}
 
     function run() public {
@@ -22,50 +28,57 @@ contract DeployScript is Script {
         address multisig = 0x040f331774Ed6BB161412B4cEDb1358B382aF3A5;
         safeconsole.log("Multisig: ", multisig);
 
-        address deposit = Upgrades.deployTransparentProxy(
+        address deposit_PROXY = Upgrades.deployTransparentProxy(
             "Deposit.sol:Deposit", multisig, abi.encodeCall(Deposit.initialize, ("RING Deposit NFT", "RDPS"))
         );
-        safeconsole.log("Depoist: ", deposit);
-        safeconsole.log("Depoist_Logic: ", Upgrades.getImplementationAddress(deposit));
+        safeconsole.log("Depoist: ", deposit_PROXY);
+        safeconsole.log("Depoist_Logic: ", Upgrades.getImplementationAddress(deposit_PROXY));
 
         uint256 minDelay = 3 days;
-        address timelock = Upgrades.deployTransparentProxy(
+
+        address[] memory proposers = new address[](1);
+        proposers[0] = ringDAO;
+        address timelock_PROXY = Upgrades.deployTransparentProxy(
             "RingTimelockController.sol:RingTimelockController",
             multisig,
-            abi.encodeCall(RingTimelockController.initialize, (minDelay, new address[](0), new address[](0), multisig))
+            abi.encodeCall(RingTimelockController.initialize, (minDelay, proposers, new address[](0), multisig))
         );
-        safeconsole.log("Timelock: ", timelock);
-        safeconsole.log("Timelock_Logic: ", Upgrades.getImplementationAddress(timelock));
+        safeconsole.log("Timelock: ", timelock_PROXY);
+        safeconsole.log("Timelock_Logic: ", Upgrades.getImplementationAddress(timelock_PROXY));
 
-        address gRING = Upgrades.deployTransparentProxy(
+        address gRING_PROXY = Upgrades.deployTransparentProxy(
             "GovernanceRing.sol:GovernanceRing",
             timelock,
-            abi.encodeCall(GovernanceRing.initialize, (multisig, deposit, "Governance RING", "gRING"))
+            abi.encodeCall(GovernanceRing.initialize, (multisig, hub, deposit, "Governance RING", "gRING"))
         );
-        safeconsole.log("gRING: ", gRING);
-        safeconsole.log("gRING_Logic: ", Upgrades.getImplementationAddress(gRING));
+        safeconsole.log("gRING: ", gRING_PROXY);
+        safeconsole.log("gRING_Logic: ", Upgrades.getImplementationAddress(gRING_PROXY));
 
-        address ringDAO = Upgrades.deployTransparentProxy(
+        address ringDAO_PROXY = Upgrades.deployTransparentProxy(
             "RingDAO.sol:RingDAO",
             timelock,
             abi.encodeCall(
-                RingDAO.initialize, (IVotes(gRING), TimelockControllerUpgradeable(payable(timelock)), "RingDAO")
+                RingDAO.initialize,
+                (
+                    IVotes(gRING),
+                    TimelockControllerUpgradeable(payable(timelock)),
+                    1 days,
+                    30 days,
+                    1_000_000 * 1e18,
+                    "RingDAO"
+                )
             )
         );
-        safeconsole.log("RingDAO: ", ringDAO);
-        safeconsole.log("RingDAO_Logic: ", Upgrades.getImplementationAddress(ringDAO));
+        safeconsole.log("RingDAO: ", ringDAO_PROXY);
+        safeconsole.log("RingDAO_Logic: ", Upgrades.getImplementationAddress(ringDAO_PROXY));
 
-        address hub = Upgrades.deployTransparentProxy(
+        address hub_PROXY = Upgrades.deployTransparentProxy(
             "CollatorStakingHub.sol:CollatorStakingHub",
             timelock,
             abi.encodeCall(CollatorStakingHub.initialize, (gRING, deposit))
         );
-        safeconsole.log("Hub: ", hub);
-        safeconsole.log("Hub_Logic: ", Upgrades.getImplementationAddress(hub));
-
-        // RingTimelockController(timelock).grantRole(RingTimelockController(timelock).PROPOSER_ROLE(), ringDAO);
-        // RingTimelockController(gRING).grantRole(GovernanceRing(gRING).MINTER_ROLE(), hub);
-        // RingTimelockController(gRING).grantRole(GovernanceRing(gRING).BURNER_ROLE(), hub);
+        safeconsole.log("Hub: ", hub_PROXY);
+        safeconsole.log("Hub_Logic: ", Upgrades.getImplementationAddress(hub_PROXY));
 
         vm.stopBroadcast();
     }

src/collator/CollatorSet.sol

@@ -45,7 +45,7 @@ abstract contract CollatorSet is Initializable, CollatorStakingHubStorage {
         address next = collators[prev];
         // No duplicate collator allowed.
         require(collators[cur] == address(0), "!cur");
-        // Next collaotr must in the list.
+        // Next collator must in the list.
         require(next != address(0), "!prev");
         require(_verifyIndex(prev, votes, next), "!votes");
         collators[cur] = next;
@@ -77,10 +77,10 @@ abstract contract CollatorSet is Initializable, CollatorStakingHubStorage {
     function _updateVotes(address cur, uint256 newVotes, address oldPrev, address newPrev) internal {
         require(_isValid(cur), "!valid");
         require(collators[cur] != address(0), "!cur");
-        require(collators[oldPrev] != address(0), "!oldPrev");
+        require(collators[oldPrev] != address(0), "!oldPrev1");
         require(collators[newPrev] != address(0), "!newPrev");
         if (oldPrev == newPrev) {
-            require(_isPrevCollator(cur, oldPrev), "!oldPrev");
+            require(_isPrevCollator(cur, oldPrev), "!oldPrev2");
             require(_verifyIndex(newPrev, newVotes, collators[cur]), "!votes");
             votesOf[cur] = newVotes;
         } else {

src/collator/CollatorStakingHub.sol

@@ -18,7 +18,9 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
     using EnumerableSet for EnumerableSet.UintSet;
 
     // The lock-up period starts with the stake or inscrease stake.
-    uint256 public constant LOCK_PERIOD = 1 days;
+    uint256 public constant STAKING_LOCK_PERIOD = 1 days;
+    // The lock-up period starts with the collator commsission update;
+    uint256 public constant COMMISSION_LOCK_PERIOD = 7 days;
     // Staking Pallet Account.
     address public constant STAKING_PALLET = 0x6D6F646C64612f7374616B690000000000000000;
     // 0 ~ 100
@@ -60,12 +62,12 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
 
         poolOf[collator] = pool;
         _addCollator(collator, 0, prev);
-        _collect(collator, commission);
+        _collate(collator, commission);
         emit NominationPoolCreated(pool, collator, prev);
     }
 
     function _stake(address collator, address account, uint256 assets) internal {
-        stakingLocks[collator][account] = LOCK_PERIOD + block.timestamp;
+        stakingLocks[collator][account] = STAKING_LOCK_PERIOD + block.timestamp;
         address pool = poolOf[collator];
         require(pool != address(0), "!collator");
         INominationPool(pool).stake(account, assets);
@@ -91,14 +93,14 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
     function stakeRING(address collator, address oldPrev, address newPrev) public payable nonReentrant {
         _stake(collator, msg.sender, msg.value);
         _increaseVotes(collator, _assetsToVotes(commissionOf[collator], msg.value), oldPrev, newPrev);
-        stakedRINGOf[msg.sender] += msg.value;
+        stakedRINGOf[collator][msg.sender] += msg.value;
     }
 
     function unstakeRING(address collator, uint256 assets, address oldPrev, address newPrev) public nonReentrant {
         _unstake(collator, msg.sender, assets);
         payable(msg.sender).sendValue(assets);
         _reduceVotes(collator, _assetsToVotes(commissionOf[collator], assets), oldPrev, newPrev);
-        stakedRINGOf[msg.sender] -= assets;
+        stakedRINGOf[collator][msg.sender] -= assets;
     }
 
     function stakeDeposit(address collator, uint256 depositId, address oldPrev, address newPrev) public nonReentrant {
@@ -124,12 +126,15 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
         require(_stakedDeposits[account].remove(depositId), "!remove");
     }
 
-    function collect(uint256 commission, address oldPrev, address newPrev) public nonReentrant {
+    function collate(uint256 commission, address oldPrev, address newPrev) public nonReentrant {
         address collator = msg.sender;
         require(poolOf[collator] != address(0), "!collator");
+        require(commissionLocks[collator] < block.timestamp, "!locked");
+        require(commissionOf[collator] != commission, "same");
         _removeCollator(collator, oldPrev);
-        _collect(collator, commission);
+        _collate(collator, commission);
         _addCollator(collator, _assetsToVotes(commission, stakedOf(collator)), newPrev);
+        commissionLocks[collator] = COMMISSION_LOCK_PERIOD + block.timestamp;
     }
 
     /// @dev Distribute collator reward from Staking Pallet Account.
@@ -151,8 +156,8 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
         return INominationPool(pool).totalSupply();
     }
 
-    function _collect(address collator, uint256 commission) internal {
-        require(commission <= COMMISSION_BASE);
+    function _collate(address collator, uint256 commission) internal {
+        require(commission <= COMMISSION_BASE, "!commission");
         commissionOf[collator] = commission;
         emit CommissionUpdated(collator, commission);
     }

src/collator/CollatorStakingHubStorage.sol

@@ -21,10 +21,12 @@ contract CollatorStakingHubStorage {
     mapping(address => address) public poolOf;
     // collator => commission
     mapping(address => uint256) public commissionOf;
-    // collator => user => lockTime
+    // collator => user => stakingLockTime
     mapping(address => mapping(address => uint256)) public stakingLocks;
-    // user => staked ring
-    mapping(address => uint256) public stakedRINGOf;
+    // collator => commissonLockTime
+    mapping(address => uint256) public commissionLocks;
+    // user => collator => staked ring
+    mapping(address => mapping(address => uint256)) public stakedRINGOf;
     // user => staked depositIds
     mapping(address => EnumerableSet.UintSet) internal _stakedDeposits;
 

src/collator/NominationPool.sol

@@ -76,14 +76,14 @@ contract NominationPool {
 
     function stake(address account, uint256 amount) external onlyHub updateReward(account) {
         require(amount > 0, "Cannot stake 0");
-        _totalSupply = _totalSupply + amount;
+        _totalSupply += amount;
         _balances[account] += amount;
         emit Staked(account, amount);
     }
 
     function withdraw(address account, uint256 amount) public onlyHub updateReward(account) {
         require(amount > 0, "Cannot withdraw 0");
-        _totalSupply = _totalSupply - amount;
+        _totalSupply -= amount;
         _balances[account] -= amount;
         emit Withdrawn(account, amount);
     }

src/deposit/Deposit.sol

@@ -169,7 +169,7 @@ contract Deposit is
 
     function computeInterest(uint256 value, uint256 months) public view returns (uint256) {
         uint256 interest = INTERESTS[months];
-        return value * interest / 10_000;
+        return value * interest / 10_000 / 10_000;
     }
 
     function isClaimRequirePenalty(uint256 id) public view returns (bool) {

src/governance/GovernanceRing.sol

@@ -40,7 +40,10 @@ contract GovernanceRing is
     event WrapDeposit(address indexed account, address indexed token, uint256 depositId);
     event UnwrapDeposit(address indexed account, address indexed token, uint256 depositId);
 
-    function initialize(address admin, address dps, string memory name, string memory symbol) public initializer {
+    function initialize(address admin, address hub, address dps, string memory name, string memory symbol)
+        public
+        initializer
+    {
         DEPOSIT = IDeposit(dps);
         __ERC20_init(name, symbol);
         __AccessControlEnumerable_init();
@@ -49,6 +52,9 @@ contract GovernanceRing is
         __ReentrancyGuard_init();
 
         _grantRole(DEFAULT_ADMIN_ROLE, admin);
+
+        _grantRole(MINTER_ROLE, hub);
+        _grantRole(BURNER_ROLE, hub);
     }
 
     /// @custom:oz-upgrades-unsafe-allow constructor

src/governance/RingDAO.sol

@@ -21,15 +21,19 @@ contract RingDAO is
         _disableInitializers();
     }
 
-    function initialize(IVotes _token, TimelockControllerUpgradeable _timelock, string memory name)
-        public
-        initializer
-    {
+    function initialize(
+        IVotes token,
+        TimelockControllerUpgradeable timelock,
+        uint48 initialVotingDelay,
+        uint32 initialVotingPeriod,
+        uint256 initialProposalThreshold,
+        string memory name
+    ) public initializer {
         __Governor_init(name);
-        __GovernorSettings_init(1 days, 2 weeks, 1_000_000 * 1e18);
+        __GovernorSettings_init(initialVotingDelay, initialVotingPeriod, initialProposalThreshold);
         __GovernorCountingSimple_init();
-        __GovernorVotes_init(_token);
-        __GovernorTimelockControl_init(_timelock);
+        __GovernorVotes_init(token);
+        __GovernorTimelockControl_init(timelock);
     }
 
     // The following functions are overrides required by Solidity.

src/governance/RingTimelockController.sol

@@ -3,12 +3,40 @@ pragma solidity 0.8.20;
 
 import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
 import "@openzeppelin/contracts-upgradeable/governance/TimelockControllerUpgradeable.sol";
+import "@openzeppelin/contracts-upgradeable/access/extensions/AccessControlEnumerableUpgradeable.sol";
 
-contract RingTimelockController is Initializable, TimelockControllerUpgradeable {
+contract RingTimelockController is Initializable, TimelockControllerUpgradeable, AccessControlEnumerableUpgradeable {
     function initialize(uint256 minDelay, address[] memory proposers, address[] memory executors, address admin)
         public
         initializer
     {
         __TimelockController_init(minDelay, proposers, executors, admin);
+        __AccessControlEnumerable_init();
+    }
+
+    function _grantRole(bytes32 role, address account)
+        internal
+        override(AccessControlUpgradeable, AccessControlEnumerableUpgradeable)
+        returns (bool)
+    {
+        return super._grantRole(role, account);
+    }
+
+    function _revokeRole(bytes32 role, address account)
+        internal
+        override(AccessControlUpgradeable, AccessControlEnumerableUpgradeable)
+        returns (bool)
+    {
+        return super._revokeRole(role, account);
+    }
+
+    function supportsInterface(bytes4 interfaceId)
+        public
+        view
+        virtual
+        override(TimelockControllerUpgradeable, AccessControlEnumerableUpgradeable)
+        returns (bool)
+    {
+        return super.supportsInterface(interfaceId);
     }
 }