Skip to content

Commit

Permalink
fixes TRST-M-1 - adds additional padding for precision loss (#51)
Browse files Browse the repository at this point in the history 
* fixes TRST-M-1 - adds additional padding for precision loss

* fix: emit events on pause changes TRST-R-1 (#52)

* fix: emit events on pause changes TRST-R-1

* Fix/coding best practices trst r 3 (#53)

* fix: add reentrancy init

* rename internal function

* fix: checks for state prior to changing state

* Fix/cei trst r 5 (#54)

* fix TRST-R-5 to follow CEI pattern

* lint

* use modifiers where we can
  • Loading branch information
@0xean
0xean authored May 30, 2024
1 parent 83b361d commit bd94f64
Show file tree
Hide file tree
Showing 6 changed files with 90 additions and 13 deletions.
29 changes: 21 additions & 8 deletions foundry/src/StakingV1.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ contract StakingV1 is
uint256 public totalStaked;
uint256 public totalCoolingDown;

uint256 public constant REWARD_RATE = 1_000_000_000;
uint256 public constant REWARD_RATE = 1e27;
uint256 public constant WAD = 1e18;
uint256 public lastUpdateTimestamp;
uint256 public rewardPerTokenStored;
Expand All @@ -51,6 +51,9 @@ contract StakingV1 is
string indexed oldRuneAddress,
string indexed newRuneAddress
);
event StakingPausedChanged(bool isPaused);
event WithdrawalsPausedChanged(bool isPaused);
event UnstakingPausedChanged(bool isPaused);

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() {
Expand All @@ -61,6 +64,7 @@ contract StakingV1 is
__Ownable_init(msg.sender);
__UUPSUpgradeable_init();
__Pausable_init();
__ReentrancyGuard_init();
stakingToken = IERC20(stakingTokenAddress);
cooldownPeriod = 28 days;
lastUpdateTimestamp = block.timestamp;
Expand All @@ -75,33 +79,42 @@ contract StakingV1 is
}

/// @notice Pauses deposits
function pauseStaking() external onlyOwner {
function pauseStaking() external onlyOwner whenStakingNotPaused {
stakingPaused = true;
emit StakingPausedChanged(true);
}

/// @notice Unpauses deposits
function unpauseStaking() external onlyOwner {
require(stakingPaused, "Staking is not paused");
stakingPaused = false;
emit StakingPausedChanged(false);
}

/// @notice Pauses withdrawals
function pauseWithdrawals() external onlyOwner {
function pauseWithdrawals() external onlyOwner whenWithdrawalsNotPaused {
withdrawalsPaused = true;
emit WithdrawalsPausedChanged(true);
}

/// @notice Unpauses withdrawals
function unpauseWithdrawals() external onlyOwner {
require(withdrawalsPaused, "Withdrawals are not paused");
withdrawalsPaused = false;
emit WithdrawalsPausedChanged(false);
}

/// @notice Pauses unstaking
function pauseUnstaking() external onlyOwner {
function pauseUnstaking() external onlyOwner whenUnstakingNotPaused {
unstakingPaused = true;
emit UnstakingPausedChanged(true);
}

/// @notice Unpauses unstaking
function unpauseUnstaking() external onlyOwner {
require(unstakingPaused, "Unstaking is not paused");
unstakingPaused = false;
emit UnstakingPausedChanged(false);
}

/// @notice Sets contract-level paused state
Expand Down Expand Up @@ -171,14 +184,14 @@ contract StakingV1 is
"Rune address must be 43 characters"
);
require(amount > 0, "amount to stake must be greater than 0");
updateReward(msg.sender);
stakingToken.safeTransferFrom(msg.sender, address(this), amount);
_updateReward(msg.sender);

StakingInfo storage info = stakingInfo[msg.sender];
info.stakingBalance += amount;
info.runeAddress = runeAddress;
totalStaked += amount;

stakingToken.safeTransferFrom(msg.sender, address(this), amount);
emit Stake(msg.sender, amount, runeAddress);
}

Expand All @@ -196,7 +209,7 @@ contract StakingV1 is
amount <= info.stakingBalance,
"Unstake amount exceeds staked balance"
);
updateReward(msg.sender);
_updateReward(msg.sender);

// Set staking / unstaking amounts
info.stakingBalance -= amount;
Expand Down Expand Up @@ -330,7 +343,7 @@ contract StakingV1 is

/// @notice Updates all variables when changes to staking amounts are made.
/// @param account The address of the account to update.
function updateReward(address account) internal {
function _updateReward(address account) internal {
rewardPerTokenStored = rewardPerToken();
lastUpdateTimestamp = block.timestamp;
StakingInfo storage info = stakingInfo[account];
Expand Down
8 changes: 4 additions & 4 deletions foundry/test/StakingTestAcocunting.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ contract FOXStakingTestStaking is Test {
// we should confirm that they still recieve expected rewards when.

// create mega whale
uint256 megaWhaleAmount = 900_000_000 * 1e18; // 900 million FOX tokens with 18 decimals
uint256 megaWhaleAmount = 9_000_000_000 * 1e18; // 9 billion FOX tokens with 18 decimals
foxToken.makeItRain(userOne, megaWhaleAmount);

vm.prank(userOne);
Expand All @@ -246,16 +246,16 @@ contract FOXStakingTestStaking is Test {

function testRewardAmountsForOverflow() public {
// create mega whale
uint256 megaWhaleAmount = 900_000_000 * 1e18; // 900 million FOX tokens with 18 decimals
uint256 megaWhaleAmount = 9_000_000_000 * 1e18; // 9 billion FOX tokens with 18 decimals
foxToken.makeItRain(userOne, megaWhaleAmount);

vm.prank(userOne);
foxToken.approve(address(foxStaking), megaWhaleAmount);
vm.prank(userOne);
foxStaking.stake(megaWhaleAmount, runeAddressOne);

// advance time by 15 years
vm.warp(block.timestamp + 365 * 15 days);
// advance time by 200 years
vm.warp(block.timestamp + 365 days * 200);

// ensure this does not overflow
foxStaking.earned(userOne);
Expand Down
22 changes: 22 additions & 0 deletions foundry/test/StakingTestStaking.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ contract FOXStakingTestStaking is Test {
}

function testCannotStakeWhenStakingPaused() public {
// confirm we emit the correct event
vm.expectEmit();
emit StakingV1.StakingPausedChanged(true);
foxStaking.pauseStaking();

address user = address(0xBABE);
Expand Down Expand Up @@ -60,6 +63,8 @@ contract FOXStakingTestStaking is Test {
foxStaking.stake(amount, runeAddress);
vm.stopPrank();

vm.expectEmit();
emit StakingV1.StakingPausedChanged(false);
foxStaking.unpauseStaking();

vm.startPrank(user);
Expand Down Expand Up @@ -243,4 +248,21 @@ contract FOXStakingTestStaking is Test {
assertEq(runeAddress, runeAddresses[i]);
}
}

function testStaking_cannotPauseAlreadyPaused() public {
vm.expectEmit();
emit StakingV1.StakingPausedChanged(true);
foxStaking.pauseStaking();

vm.expectRevert("Staking is paused");
foxStaking.pauseStaking();

// unpause and try to unpause again
vm.expectEmit();
emit StakingV1.StakingPausedChanged(false);
foxStaking.unpauseStaking();

vm.expectRevert("Staking is not paused");
foxStaking.unpauseStaking();
}
}
21 changes: 21 additions & 0 deletions foundry/test/StakingTestUnstake.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ contract FOXStakingTestUnstake is Test {
}

function testCannotUnstakeWhenUnstakingPaused() public {
vm.expectEmit();
emit StakingV1.UnstakingPausedChanged(true);
foxStaking.pauseUnstaking();

vm.startPrank(user);
Expand Down Expand Up @@ -76,6 +78,8 @@ contract FOXStakingTestUnstake is Test {
foxStaking.unstake(amount);
vm.stopPrank();

vm.expectEmit();
emit StakingV1.UnstakingPausedChanged(false);
foxStaking.unpauseUnstaking();

vm.startPrank(user);
Expand Down Expand Up @@ -873,4 +877,21 @@ contract FOXStakingTestUnstake is Test {
balAfter = foxToken.balanceOf(user2);
vm.assertEq(balAfter - balBefore, 51);
}

function testUnstake_cannotPauseAlreadyPaused() public {
vm.expectEmit();
emit StakingV1.UnstakingPausedChanged(true);
foxStaking.pauseUnstaking();

vm.expectRevert("Unstaking is paused");
foxStaking.pauseUnstaking();

// unpause and try to unpause again
vm.expectEmit();
emit StakingV1.UnstakingPausedChanged(false);
foxStaking.unpauseUnstaking();

vm.expectRevert("Unstaking is not paused");
foxStaking.unpauseUnstaking();
}
}
21 changes: 21 additions & 0 deletions foundry/test/StakingTestWithdraw.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ contract FOXStakingTestWithdraw is Test {
}

function testCannotWithdrawWhenWithdrawalsPaused() public {
vm.expectEmit();
emit StakingV1.WithdrawalsPausedChanged(true);
foxStaking.pauseWithdrawals();

vm.startPrank(user);
Expand Down Expand Up @@ -86,6 +88,8 @@ contract FOXStakingTestWithdraw is Test {
foxStaking.withdraw();
vm.stopPrank();

vm.expectEmit();
emit StakingV1.WithdrawalsPausedChanged(false);
foxStaking.unpauseWithdrawals();

vm.startPrank(user);
Expand Down Expand Up @@ -170,4 +174,21 @@ contract FOXStakingTestWithdraw is Test {

vm.stopPrank();
}

function testWithdraw_cannotPauseAlreadyPaused() public {
vm.expectEmit();
emit StakingV1.WithdrawalsPausedChanged(true);
foxStaking.pauseWithdrawals();

vm.expectRevert("Withdrawals are paused");
foxStaking.pauseWithdrawals();

// unpause and try to unpause again
vm.expectEmit();
emit StakingV1.WithdrawalsPausedChanged(false);
foxStaking.unpauseWithdrawals();

vm.expectRevert("Withdrawals are not paused");
foxStaking.unpauseWithdrawals();
}
}
2 changes: 1 addition & 1 deletion foundry/test/utils/MockFOXToken.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract MockFOXToken is ERC20 {
constructor() ERC20("Mock FOX Token", "FOX") {
_mint(address(this), 1e27); // 1 billion FOX with 18 decimals
_mint(address(this), 10e27); // 10 billion FOX with 18 decimals - more than mainnet, for testing.
}

function makeItRain(address to, uint256 amount) public {
Expand Down

0 comments on commit bd94f64

Please sign in to comment.