From 6665dd6e2b65c140d39be0ac3f001e01a63cdce1 Mon Sep 17 00:00:00 2001 From: 0xean <0xean.eth@gmail.com> Date: Tue, 30 Apr 2024 12:00:07 -0400 Subject: [PATCH] refactor errors and naming of struct --- foundry/src/FoxStakingV1.sol | 69 ++++++++++--------- foundry/src/StakingInfo.sol | 4 +- ...UnstakingInfo.sol => UnstakingRequest.sol} | 2 +- foundry/test/FOXStakingTestUnstake.t.sol | 48 +++++++------ foundry/test/FOXStakingTestWithdraw.t.sol | 2 +- 5 files changed, 69 insertions(+), 56 deletions(-) rename foundry/src/{UnstakingInfo.sol => UnstakingRequest.sol} (82%) diff --git a/foundry/src/FoxStakingV1.sol b/foundry/src/FoxStakingV1.sol index 69c6ae8..5af05fd 100644 --- a/foundry/src/FoxStakingV1.sol +++ b/foundry/src/FoxStakingV1.sol @@ -8,7 +8,7 @@ import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/Own import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; import {PausableUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol"; import {StakingInfo} from "./StakingInfo.sol"; -import {UnstakingInfo} from "./UnstakingInfo.sol"; +import {UnstakingRequest} from "./UnstakingRequest.sol"; contract FoxStakingV1 is Initializable, @@ -167,14 +167,14 @@ contract FoxStakingV1 is info.stakingBalance -= amount; info.unstakingBalance += amount; - UnstakingInfo memory unstakingInfo = UnstakingInfo({ + UnstakingRequest memory unstakingRequest = UnstakingRequest({ unstakingBalance: amount, cooldownExpiry: block.timestamp + cooldownPeriod }); - info.unstakingInfo.push(unstakingInfo); // append to the end of unstakingInfo array + info.unstakingRequests.push(unstakingRequest); // append to the end of unstakingRequests array - emit Unstake(msg.sender, amount, unstakingInfo.cooldownExpiry); + emit Unstake(msg.sender, amount, unstakingRequest.cooldownExpiry); } /// @notice Allows a user to withdraw a specified claim by index @@ -183,48 +183,55 @@ contract FoxStakingV1 is uint256 index ) public whenNotPaused whenWithdrawalsNotPaused { StakingInfo storage info = stakingInfo[msg.sender]; - require(info.unstakingInfo.length > 0, "No balance to withdraw"); + require( + info.unstakingRequests.length > 0, + "No unstaking requests found" + ); - require(info.unstakingInfo.length > index, "invalid index"); + require(info.unstakingRequests.length > index, "invalid index"); - UnstakingInfo memory unstakingInfo = info.unstakingInfo[index]; + UnstakingRequest memory unstakingRequest = info.unstakingRequests[ + index + ]; require( - block.timestamp >= unstakingInfo.cooldownExpiry, + block.timestamp >= unstakingRequest.cooldownExpiry, "Not cooled down yet" ); - if (info.unstakingInfo.length > 1) { + if (info.unstakingRequests.length > 1) { // we have more elements in the array, so shift the last element to the index being withdrawn // and then shorten the array by 1 - info.unstakingInfo[index] = info.unstakingInfo[ - info.unstakingInfo.length - 1 + info.unstakingRequests[index] = info.unstakingRequests[ + info.unstakingRequests.length - 1 ]; - info.unstakingInfo.pop(); + info.unstakingRequests.pop(); } else { // the array is done, we can delete the whole thing - delete info.unstakingInfo; + delete info.unstakingRequests; } - info.unstakingBalance -= unstakingInfo.unstakingBalance; - foxToken.safeTransfer(msg.sender, unstakingInfo.unstakingBalance); - emit Withdraw(msg.sender, unstakingInfo.unstakingBalance); + info.unstakingBalance -= unstakingRequest.unstakingBalance; + foxToken.safeTransfer(msg.sender, unstakingRequest.unstakingBalance); + emit Withdraw(msg.sender, unstakingRequest.unstakingBalance); } /// @notice processes the most recent unstaking request available to the user, else reverts. function withdraw() external { StakingInfo memory info = stakingInfo[msg.sender]; - uint256 length = info.unstakingInfo.length; - require(length > 0, "No balance to withdraw"); + uint256 length = info.unstakingRequests.length; + require(length > 0, "No unstaking requests found"); uint256 indexToProcess; uint256 earliestCooldownExpiry = type(uint256).max; for (uint256 i; i < length; i++) { - UnstakingInfo memory unstakingInfo = info.unstakingInfo[i]; - if (block.timestamp >= unstakingInfo.cooldownExpiry) { + UnstakingRequest memory unstakingRequest = info.unstakingRequests[ + i + ]; + if (block.timestamp >= unstakingRequest.cooldownExpiry) { // this claim is ready to be processed - if (unstakingInfo.cooldownExpiry < earliestCooldownExpiry) { + if (unstakingRequest.cooldownExpiry < earliestCooldownExpiry) { // we found a more recent claim we can process. - earliestCooldownExpiry = unstakingInfo.cooldownExpiry; + earliestCooldownExpiry = unstakingRequest.cooldownExpiry; indexToProcess = i; } } @@ -261,22 +268,22 @@ contract FoxStakingV1 is } /// @notice helper function to access dynamic array nested in struct from external sources - /// @param account The address we're getting the unstaking info for. - /// @param index The index of the unstaking info array we're getting. - function getUnstakingInfo( + /// @param account The address we're getting the unstaking request for. + /// @param index The index of the unstaking request array we're getting. + function getUnstakingRequest( address account, uint256 index - ) external view returns (UnstakingInfo memory) { - return stakingInfo[account].unstakingInfo[index]; + ) external view returns (UnstakingRequest memory) { + return stakingInfo[account].unstakingRequests[index]; } - /// @notice returns the numbery of ustaking info elements for a given address + /// @notice returns the numbery of ustaking request elements for a given address /// @dev useful for off chain processing /// @param account The address we're getting the unstaking info count for. - /// @return length The number of unstaking info elements. - function getUnstakingInfoCount( + /// @return length The number of unstaking request elements. + function getUnstakingRequestCount( address account ) external view returns (uint256) { - return stakingInfo[account].unstakingInfo.length; + return stakingInfo[account].unstakingRequests.length; } } diff --git a/foundry/src/StakingInfo.sol b/foundry/src/StakingInfo.sol index d5f8291..962db4f 100644 --- a/foundry/src/StakingInfo.sol +++ b/foundry/src/StakingInfo.sol @@ -1,11 +1,11 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.25; -import {UnstakingInfo} from "./UnstakingInfo.sol"; +import {UnstakingRequest} from "./UnstakingRequest.sol"; struct StakingInfo { uint256 stakingBalance; uint256 unstakingBalance; string runeAddress; - UnstakingInfo[] unstakingInfo; + UnstakingRequest[] unstakingRequests; } diff --git a/foundry/src/UnstakingInfo.sol b/foundry/src/UnstakingRequest.sol similarity index 82% rename from foundry/src/UnstakingInfo.sol rename to foundry/src/UnstakingRequest.sol index 96e8384..96e83da 100644 --- a/foundry/src/UnstakingInfo.sol +++ b/foundry/src/UnstakingRequest.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.25; -struct UnstakingInfo { +struct UnstakingRequest { uint256 unstakingBalance; uint256 cooldownExpiry; } diff --git a/foundry/test/FOXStakingTestUnstake.t.sol b/foundry/test/FOXStakingTestUnstake.t.sol index 2d7cd0c..8c309c4 100644 --- a/foundry/test/FOXStakingTestUnstake.t.sol +++ b/foundry/test/FOXStakingTestUnstake.t.sol @@ -247,7 +247,7 @@ contract FOXStakingTestUnstake is Test { ) = foxStaking.stakingInfo(user); uint256 cooldownExpiry_one = foxStaking - .getUnstakingInfo(user, 0) + .getUnstakingRequest(user, 0) .cooldownExpiry; vm.assertEq(cooldownExpiry_one, block.timestamp + 28 days); @@ -282,7 +282,7 @@ contract FOXStakingTestUnstake is Test { ) = foxStaking.stakingInfo(user); uint256 cooldownExpiry_three = foxStaking - .getUnstakingInfo(user, 0) + .getUnstakingRequest(user, 0) .cooldownExpiry; vm.assertGt(cooldownExpiry_three, block.timestamp); @@ -321,7 +321,7 @@ contract FOXStakingTestUnstake is Test { ) = foxStaking.stakingInfo(user); uint256 cooldownExpiry_one = foxStaking - .getUnstakingInfo(user, 0) + .getUnstakingRequest(user, 0) .cooldownExpiry; vm.assertEq(cooldownExpiry_one, block.timestamp + 28 days); @@ -341,7 +341,7 @@ contract FOXStakingTestUnstake is Test { foxStaking.unstake(302); uint256 cooldownExpiry_two = foxStaking - .getUnstakingInfo(user, 1) + .getUnstakingRequest(user, 1) .cooldownExpiry; vm.assertEq(cooldownExpiry_two, block.timestamp + 28 days); @@ -361,7 +361,7 @@ contract FOXStakingTestUnstake is Test { foxStaking.unstake(303); uint256 cooldownExpiry_three = foxStaking - .getUnstakingInfo(user, 2) + .getUnstakingRequest(user, 2) .cooldownExpiry; vm.assertEq(cooldownExpiry_three, block.timestamp + 28 days); @@ -374,7 +374,7 @@ contract FOXStakingTestUnstake is Test { vm.assertEq(unstakingBalance_three, 301 + 302 + 303); vm.assertEq(stakingBalance_three, 1000 - 301 - 302 - 303); - vm.assertEq(foxStaking.getUnstakingInfoCount(user), 3); + vm.assertEq(foxStaking.getUnstakingRequestCount(user), 3); // Time warp to first expiry vm.warp(cooldownExpiry_one); @@ -403,10 +403,10 @@ contract FOXStakingTestUnstake is Test { // check the length of the array is correct // and the sums match the staking info - vm.assertEq(foxStaking.getUnstakingInfoCount(user), 2); + vm.assertEq(foxStaking.getUnstakingRequestCount(user), 2); vm.assertEq( - foxStaking.getUnstakingInfo(user, 0).unstakingBalance + - foxStaking.getUnstakingInfo(user, 1).unstakingBalance, + foxStaking.getUnstakingRequest(user, 0).unstakingBalance + + foxStaking.getUnstakingRequest(user, 1).unstakingBalance, 302 + 303 ); @@ -434,8 +434,11 @@ contract FOXStakingTestUnstake is Test { vm.assertEq(unstakingBalance_five, 303); // check the lenght of the array is correct and the indexes have shifted - vm.assertEq(foxStaking.getUnstakingInfoCount(user), 1); - vm.assertEq(foxStaking.getUnstakingInfo(user, 0).unstakingBalance, 303); + vm.assertEq(foxStaking.getUnstakingRequestCount(user), 1); + vm.assertEq( + foxStaking.getUnstakingRequest(user, 0).unstakingBalance, + 303 + ); // Time warp to third expiry vm.warp(cooldownExpiry_three); @@ -454,7 +457,7 @@ contract FOXStakingTestUnstake is Test { ) = foxStaking.stakingInfo(user); vm.assertEq(stakingBalance_six, 1000 - 301 - 302 - 303); vm.assertEq(unstakingBalance_six, 0); - vm.assertEq(foxStaking.getUnstakingInfoCount(user), 0); + vm.assertEq(foxStaking.getUnstakingRequestCount(user), 0); vm.stopPrank(); } @@ -488,7 +491,7 @@ contract FOXStakingTestUnstake is Test { ) = foxStaking.stakingInfo(user); uint256 cooldownExpiry_one = foxStaking - .getUnstakingInfo(user, 0) + .getUnstakingRequest(user, 0) .cooldownExpiry; vm.assertEq(cooldownExpiry_one, block.timestamp + 28 days); @@ -507,7 +510,7 @@ contract FOXStakingTestUnstake is Test { foxStaking.unstake(302); uint256 cooldownExpiry_two = foxStaking - .getUnstakingInfo(user, 1) + .getUnstakingRequest(user, 1) .cooldownExpiry; vm.assertEq(cooldownExpiry_two, block.timestamp + 2 days); @@ -527,7 +530,7 @@ contract FOXStakingTestUnstake is Test { foxStaking.unstake(303); uint256 cooldownExpiry_three = foxStaking - .getUnstakingInfo(user, 2) + .getUnstakingRequest(user, 2) .cooldownExpiry; vm.assertEq(cooldownExpiry_three, block.timestamp + 2 days); @@ -555,8 +558,11 @@ contract FOXStakingTestUnstake is Test { vm.expectRevert("Not cooled down yet"); foxStaking.withdraw(0); - vm.assertEq(foxStaking.getUnstakingInfo(user, 0).unstakingBalance, 301); - vm.assertEq(foxStaking.getUnstakingInfoCount(user), 1); + vm.assertEq( + foxStaking.getUnstakingRequest(user, 0).unstakingBalance, + 301 + ); + vm.assertEq(foxStaking.getUnstakingRequestCount(user), 1); vm.warp(block.timestamp + 28 days); // calling again should withdraw the 301 @@ -602,10 +608,10 @@ contract FOXStakingTestUnstake is Test { function testWithdrawReverts() public { vm.startPrank(user); - vm.expectRevert("No balance to withdraw"); + vm.expectRevert("No unstaking requests found"); foxStaking.withdraw(); - vm.expectRevert("No balance to withdraw"); + vm.expectRevert("No unstaking requests found"); foxStaking.withdraw(5); foxStaking.unstake(100); @@ -628,10 +634,10 @@ contract FOXStakingTestUnstake is Test { foxStaking.withdraw(); foxStaking.withdraw(); - vm.expectRevert("No balance to withdraw"); + vm.expectRevert("No unstaking requests found"); foxStaking.withdraw(); - vm.expectRevert("No balance to withdraw"); + vm.expectRevert("No unstaking requests found"); foxStaking.withdraw(5); } diff --git a/foundry/test/FOXStakingTestWithdraw.t.sol b/foundry/test/FOXStakingTestWithdraw.t.sol index 8675315..15cd7f0 100644 --- a/foundry/test/FOXStakingTestWithdraw.t.sol +++ b/foundry/test/FOXStakingTestWithdraw.t.sol @@ -158,7 +158,7 @@ contract FOXStakingTestWithdraw is Test { vm.warp(block.timestamp + 28 days); // Try to withdraw 0 - vm.expectRevert("No balance to withdraw"); + vm.expectRevert("No unstaking requests found"); foxStaking.withdraw(); // Check user wallet balance of FOX is still 0