Skip to content

Commit

Permalink
audit fix
Browse files Browse the repository at this point in the history
  • Loading branch information
@hujw77
hujw77 committed Jun 19, 2024
1 parent 13c516f commit 773e7f2
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 16 deletions.
6 changes: 3 additions & 3 deletions src/collator/CollatorSet.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 10 additions & 5 deletions src/collator/CollatorStakingHub.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -65,7 +67,7 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
}

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);
Expand All @@ -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 {
Expand Down Expand Up @@ -127,9 +129,12 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
function collect(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);
_addCollator(collator, _assetsToVotes(commission, stakedOf(collator)), newPrev);
commissionLocks[collator] = COMMISSION_LOCK_PERIOD + block.timestamp;
}

/// @dev Distribute collator reward from Staking Pallet Account.
Expand All @@ -152,7 +157,7 @@ contract CollatorStakingHub is ReentrancyGuardUpgradeable, CollatorSet {
}

function _collect(address collator, uint256 commission) internal {
require(commission <= COMMISSION_BASE);
require(commission <= COMMISSION_BASE, "!commission");
commissionOf[collator] = commission;
emit CommissionUpdated(collator, commission);
}
Expand Down
6 changes: 4 additions & 2 deletions src/collator/CollatorStakingHubStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ contract CollatorStakingHubStorage {
mapping(address => uint256) public commissionOf;
// collator => user => lockTime
mapping(address => mapping(address => uint256)) public stakingLocks;
// user => staked ring
mapping(address => uint256) public stakedRINGOf;
// collator => lockTime
mapping(address => uint256) public commissionLocks;
// user => collaotr => staked ring
mapping(address => mapping(address => uint256)) public stakedRINGOf;
// user => staked depositIds
mapping(address => EnumerableSet.UintSet) internal _stakedDeposits;

Expand Down
32 changes: 27 additions & 5 deletions test/collator/CollatorStakingHub.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,28 @@ contract CollatorStakingHubTest is Test {
assertEq(hub.stakedOf(charleth), 0);
}

function test_collect() public {
vm.prank(alith);
address a = hub.createNominationPool(HEAD, 1);

vm.prank(alith);
vm.expectRevert(bytes("same"));
hub.collect(1, HEAD, HEAD);

vm.prank(alith);
hub.collect(2, HEAD, HEAD);
assertEq(hub.commissionOf(alith), 2);

vm.prank(alith);
vm.expectRevert("!locked");
hub.collect(3, HEAD, HEAD);

vm.warp(block.timestamp + hub.COMMISSION_LOCK_PERIOD() + 1);
vm.prank(alith);
hub.collect(3, HEAD, HEAD);
assertEq(hub.commissionOf(alith), 3);
}

function test_stakeRING() public {
uint256 stake = 1 ether;
vm.prank(alith);
Expand All @@ -78,7 +100,7 @@ contract CollatorStakingHubTest is Test {
vm.deal(alice, stake);
vm.prank(alice);
hub.stakeRING{value: stake}(alith, HEAD, HEAD);
assertEq(hub.stakedRINGOf(alice), stake);
assertEq(hub.stakedRINGOf(alith, alice), stake);
assertEq(NominationPool(a).balanceOf(alice), stake);
assertEq(hub.votesOf(alith), stake * (100 - commissoin) / 100);
assertEq(hub.stakedOf(alith), stake);
Expand Down Expand Up @@ -117,9 +139,9 @@ contract CollatorStakingHubTest is Test {
vm.deal(alice, stake);
vm.prank(alice);
hub.stakeRING{value: stake}(alith, HEAD, HEAD);
assertEq(hub.stakingLocks(alith, alice), hub.LOCK_PERIOD() + block.timestamp);
assertEq(hub.stakingLocks(alith, alice), hub.STAKING_LOCK_PERIOD() + block.timestamp);

vm.warp(block.timestamp + hub.LOCK_PERIOD() + 1);
vm.warp(block.timestamp + hub.STAKING_LOCK_PERIOD() + 1);

vm.prank(alice);
hub.unstakeRING(alith, stake, HEAD, HEAD);
Expand All @@ -142,7 +164,7 @@ contract CollatorStakingHubTest is Test {
Deposit(deposit).approve(address(hub), id);
vm.prank(alice);
hub.stakeDeposit(alith, id, HEAD, HEAD);
assertEq(hub.stakingLocks(alith, alice), hub.LOCK_PERIOD() + block.timestamp);
assertEq(hub.stakingLocks(alith, alice), hub.STAKING_LOCK_PERIOD() + block.timestamp);
(address account, uint256 assets, address collator) = hub.depositInfos(id);
assertEq(account, alice);
assertEq(assets, stake);
Expand Down Expand Up @@ -174,7 +196,7 @@ contract CollatorStakingHubTest is Test {
vm.prank(alice);
hub.stakeDeposit(alith, id, HEAD, HEAD);

vm.warp(block.timestamp + hub.LOCK_PERIOD() + 1);
vm.warp(block.timestamp + hub.STAKING_LOCK_PERIOD() + 1);

vm.prank(alice);
hub.unstakeDeposit(id, HEAD, HEAD);
Expand Down
4 changes: 3 additions & 1 deletion test/deposit/Deposit.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ contract DepositTest is Test, ERC721Holder {
uint256 id = Deposit(deposit).deposit{value: 1 ether}(1);
assertEq(IERC721(deposit).balanceOf(self), 1);
assertEq(IERC721(deposit).ownerOf(id), self);
assertEq(IERC721Metadata(deposit).tokenURI(id), "");
assertEq(
IERC721Metadata(deposit).tokenURI(id), "ipfs://bafybeih57kauz6npkbafh2x3cv3hyljcakdccuckb4huwnwxxxrtqr5pqe"
);
assertEq(IERC721Enumerable(deposit).tokenOfOwnerByIndex(self, 0), id);
assertEq(IERC721Enumerable(deposit).tokenByIndex(0), id);
assertEq(IERC721Enumerable(deposit).totalSupply(), 1);
Expand Down

0 comments on commit 773e7f2

Please sign in to comment.