Audit part2

src/collator/CollatorStakingHub.sol

@@ -28,7 +28,7 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
 
     event Staked(address indexed pool, address collator, address account, uint256 assets);
     event Unstaked(address indexed pool, address collator, address account, uint256 assets);
-    event NominationPoolCreated(address indexed stRING, address collator, address prev);
+    event NominationPoolCreated(address indexed pool, address collator);
     event CommissionUpdated(address indexed collator, uint256 commission);
 
     modifier onlySystem() {
@@ -48,28 +48,73 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
         _disableInitializers();
     }
 
-    function createNominationPool(address prev, uint256 commission) public returns (address pool) {
+    /// 1. Create nomination pool
+    /// 2. Join collator set
+    /// 3. Set commsission
+    function join(address prev, uint256 commission) public returns (address pool) {
+        address collator = msg.sender;
+        pool = createNominationPool();
+        _updateCommission(collator, commission);
+        _addCollator(collator, 0, prev);
+    }
+
+    /// 1. Join collator set
+    /// 2. Set commission
+    function collate(address prev, uint256 commission) public {
+        address collator = msg.sender;
+        require(poolOf[collator] != address(0), "!pool");
+        _updateCommission(collator, commission);
+        _addCollator(collator, _assetsToVotes(commission, stakedOf(collator)), prev);
+    }
+
+    /// 1. Exit collator set
+    /// 2. Clear commission
+    function chill(address prev) public {
+        address collator = msg.sender;
+        require(poolOf[collator] != address(0), "!pool");
+        _setCommisson(collator, 0);
+        _removeCollator(collator, prev);
+    }
+
+    function createNominationPool() public returns (address pool) {
         address collator = msg.sender;
         require(poolOf[collator] == address(0), "created");
 
-        uint256 index = count;
         bytes memory bytecode = type(NominationPool).creationCode;
-        bytes memory initCode = bytes.concat(bytecode, abi.encode(collator, index));
+        bytes memory initCode = bytes.concat(bytecode, abi.encode(collator));
         assembly {
             pool := create2(0, add(initCode, 32), mload(initCode), 0)
         }
         require(pool != address(0), "!create2");
-
         poolOf[collator] = pool;
-        _addCollator(collator, 0, prev);
-        _collate(collator, commission);
-        emit NominationPoolCreated(pool, collator, prev);
+        emit NominationPoolCreated(pool, collator);
+    }
+
+    function updateCommission(uint256 commission, address oldPrev, address newPrev) public nonReentrant {
+        address collator = msg.sender;
+        require(poolOf[collator] != address(0), "!pool");
+        require(commissionOf[collator] != commission, "same");
+        _removeCollator(collator, oldPrev);
+        _updateCommission(collator, commission);
+        _addCollator(collator, _assetsToVotes(commission, stakedOf(collator)), newPrev);
+    }
+
+    function _updateCommission(address collator, uint256 commission) internal {
+        require(commissionLocks[collator] < block.timestamp, "!locked");
+        _setCommisson(collator, commission);
+        commissionLocks[collator] = COMMISSION_LOCK_PERIOD + block.timestamp;
+    }
+
+    function _setCommisson(address collator, uint256 commission) internal {
+        require(commission <= COMMISSION_BASE, "!commission");
+        commissionOf[collator] = commission;
+        emit CommissionUpdated(collator, commission);
     }
 
     function _stake(address collator, address account, uint256 assets) internal {
         stakingLocks[collator][account] = STAKING_LOCK_PERIOD + block.timestamp;
         address pool = poolOf[collator];
-        require(pool != address(0), "!collator");
+        require(pool != address(0), "!pool");
         INominationPool(pool).stake(account, assets);
         IGRING(gRING).mint(account, assets);
         emit Staked(pool, collator, account, assets);
@@ -78,7 +123,7 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
     function _unstake(address collator, address account, uint256 assets) internal {
         require(stakingLocks[collator][account] < block.timestamp, "!locked");
         address pool = poolOf[collator];
-        require(pool != address(0), "!collator");
+        require(pool != address(0), "!pool");
         IGRING(gRING).burn(account, assets);
         INominationPool(pool).withdraw(account, assets);
         emit Unstaked(pool, collator, account, assets);
@@ -126,24 +171,13 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
         require(_stakedDeposits[account].remove(depositId), "!remove");
     }
 
-    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);
-        _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.
     ///      The amount of the reward must be passed in via msg.value.
     /// @notice Only Staking Pallet Account could call this function.
     /// @param collator The collator address to distribute reward.
     function distributeReward(address collator) public payable onlySystem nonReentrant {
         address pool = poolOf[collator];
-        require(pool != address(0), "!collator");
+        require(pool != address(0), "!pool");
         uint256 rewards = msg.value;
         uint256 commission_ = rewards * commissionOf[collator] / COMMISSION_BASE;
         payable(collator).sendValue(commission_);
@@ -152,18 +186,12 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
 
     function stakedOf(address collator) public view returns (uint256) {
         address pool = poolOf[collator];
-        require(pool != address(0), "!collator");
+        require(pool != address(0), "!pool");
         return INominationPool(pool).totalSupply();
     }
 
-    function _collate(address collator, uint256 commission) internal {
-        require(commission <= COMMISSION_BASE, "!commission");
-        commissionOf[collator] = commission;
-        emit CommissionUpdated(collator, commission);
-    }
-
     function _assetsToVotes(uint256 commission, uint256 assets) internal pure returns (uint256) {
-        return assets * (100 - commission) / 100;
+        return assets * (COMMISSION_BASE - commission) / COMMISSION_BASE;
     }
 
     function stakedDepositsOf(address account) public view returns (uint256[] memory) {

src/collator/CollatorStakingHubStorage.sol

@@ -5,17 +5,17 @@ import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
 
 contract CollatorStakingHubStorage {
     // ---------------------- CollatorSetStorage ----------------------------
-    // collator count;
+    // collator count
     uint256 public count;
-    // ordered collators.
+    // ordered collators
     mapping(address => address) public collators;
     // collator => votes = staked_ring * (1 - commission)
     mapping(address => uint256) public votesOf;
 
     // ---------------------- CollatorStakingHubStorage ---------------------
     // Governance RING
     address public gRING;
-    // Deposit NFT.
+    // Deposit NFT
     address public DEPOSIT;
     // collator => nomination pool
     mapping(address => address) public poolOf;
@@ -25,7 +25,7 @@ contract CollatorStakingHubStorage {
     mapping(address => mapping(address => uint256)) public stakingLocks;
     // collator => commissonLockTime
     mapping(address => uint256) public commissionLocks;
-    // user => collator => staked ring
+    // collator => user => staked ring
     mapping(address => mapping(address => uint256)) public stakedRINGOf;
     // user => staked depositIds
     mapping(address => EnumerableSet.UintSet) internal _stakedDeposits;

src/collator/NominationPool.sol

@@ -9,7 +9,6 @@ contract NominationPool {
 
     /* ========== STATE VARIABLES ========== */
 
-    uint256 public id;
     address public hub;
     address public collator;
     uint256 public periodFinish = 0;
@@ -31,10 +30,9 @@ contract NominationPool {
 
     /* ========== CONSTRUCTOR ========== */
 
-    constructor(address collator_, uint256 index) {
+    constructor(address collator_) {
         hub = msg.sender;
         collator = collator_;
-        id = index;
     }
 
     /* ========== VIEWS ========== */
@@ -56,16 +54,10 @@ contract NominationPool {
             return rewardPerTokenStored;
         }
         return rewardPerTokenStored + (lastTimeRewardApplicable() - lastUpdateTime) * rewardRate * 1e18 / _totalSupply;
-        // return rewardPerTokenStored.add(
-        //     lastTimeRewardApplicable().sub(lastUpdateTime).mul(rewardRate).mul(1e18).div(_totalSupply)
-        // );
     }
 
     function earned(address account) public view returns (uint256) {
         return _balances[account] * (rewardPerToken() - userRewardPerTokenPaid[account]) / 1e18 + rewards[account];
-        // return _balances[account].mul(rewardPerToken().sub(userRewardPerTokenPaid[account])).div(1e18).add(
-        //     rewards[account]
-        // );
     }
 
     function getRewardForDuration() external view returns (uint256) {

test/collator/CollatorStakingHub.t.sol

@@ -47,18 +47,15 @@ contract CollatorStakingHubTest is Test {
 
     function test_createNominationPool() public {
         vm.prank(alith);
-        address a = hub.createNominationPool(HEAD, 1);
-        assertEq(NominationPool(a).id(), 0);
+        address a = hub.join(HEAD, 1);
         assertEq(NominationPool(a).hub(), address(hub));
         assertEq(NominationPool(a).collator(), alith);
         vm.prank(baltathar);
-        address b = hub.createNominationPool(HEAD, 2);
-        assertEq(NominationPool(b).id(), 1);
+        address b = hub.join(HEAD, 2);
         assertEq(NominationPool(b).hub(), address(hub));
         assertEq(NominationPool(b).collator(), baltathar);
         vm.prank(charleth);
-        address c = hub.createNominationPool(baltathar, 3);
-        assertEq(NominationPool(c).id(), 2);
+        address c = hub.join(baltathar, 3);
         assertEq(NominationPool(c).hub(), address(hub));
         assertEq(NominationPool(c).collator(), charleth);
         assertEq(hub.collators(HEAD), baltathar);
@@ -70,33 +67,35 @@ contract CollatorStakingHubTest is Test {
         assertEq(hub.stakedOf(charleth), 0);
     }
 
-    function test_collect() public {
+    function test_updateCommission() public {
         vm.prank(alith);
-        address a = hub.createNominationPool(HEAD, 1);
+        address a = hub.join(HEAD, 1);
+
+        vm.warp(block.timestamp + hub.COMMISSION_LOCK_PERIOD() + 1);
 
         vm.prank(alith);
         vm.expectRevert(bytes("same"));
-        hub.collate(1, HEAD, HEAD);
+        hub.updateCommission(1, HEAD, HEAD);
 
         vm.prank(alith);
-        hub.collate(2, HEAD, HEAD);
+        hub.updateCommission(2, HEAD, HEAD);
         assertEq(hub.commissionOf(alith), 2);
 
         vm.prank(alith);
         vm.expectRevert("!locked");
-        hub.collate(3, HEAD, HEAD);
+        hub.updateCommission(3, HEAD, HEAD);
 
         vm.warp(block.timestamp + hub.COMMISSION_LOCK_PERIOD() + 1);
         vm.prank(alith);
-        hub.collate(3, HEAD, HEAD);
+        hub.updateCommission(3, HEAD, HEAD);
         assertEq(hub.commissionOf(alith), 3);
     }
 
     function test_stakeRING() public {
         uint256 stake = 1 ether;
         vm.prank(alith);
         uint256 commissoin = 1;
-        address a = hub.createNominationPool(HEAD, commissoin);
+        address a = hub.join(HEAD, commissoin);
         vm.deal(alice, stake);
         vm.prank(alice);
         hub.stakeRING{value: stake}(alith, HEAD, HEAD);
@@ -111,7 +110,7 @@ contract CollatorStakingHubTest is Test {
         uint256 stake = 1 ether;
         uint256 commissoin = 1;
         vm.prank(alith);
-        address a = hub.createNominationPool(HEAD, commissoin);
+        address a = hub.join(HEAD, commissoin);
 
         vm.deal(alice, stake);
         vm.prank(alice);
@@ -134,7 +133,7 @@ contract CollatorStakingHubTest is Test {
         uint256 stake = 1 ether;
         uint256 commissoin = 1;
         vm.prank(alith);
-        address a = hub.createNominationPool(HEAD, commissoin);
+        address a = hub.join(HEAD, commissoin);
 
         vm.deal(alice, stake);
         vm.prank(alice);
@@ -158,7 +157,7 @@ contract CollatorStakingHubTest is Test {
 
         uint256 commissoin = 1;
         vm.prank(alith);
-        address a = hub.createNominationPool(HEAD, commissoin);
+        address a = hub.join(HEAD, commissoin);
 
         vm.prank(alice);
         Deposit(deposit).approve(address(hub), id);
@@ -189,7 +188,7 @@ contract CollatorStakingHubTest is Test {
 
         uint256 commissoin = 1;
         vm.prank(alith);
-        address a = hub.createNominationPool(HEAD, commissoin);
+        address a = hub.join(HEAD, commissoin);
 
         vm.prank(alice);
         Deposit(deposit).approve(address(hub), id);

test/collator/NominationPool.t.sol

@@ -16,7 +16,7 @@ contract NominationPoolTest is Test {
     address bob = address(new Guy());
 
     function setUp() public {
-        pool = new NominationPool(self, 0);
+        pool = new NominationPool(self);
         assertEq(pool.rewardsDuration(), REWARDS_DURATION);
     }
 
@@ -33,7 +33,6 @@ contract NominationPoolTest is Test {
 
     function test_constructor() public view {
         assertEq(pool.totalSupply(), 0);
-        assertEq(pool.id(), 0);
         assertEq(pool.hub(), self);
         assertEq(pool.collator(), self);
     }