From 4086dda6e284d63f895ae9740546bc458568678e Mon Sep 17 00:00:00 2001 From: r4bbit <445106+0x-r4bbit@users.noreply.github.com> Date: Thu, 20 Feb 2025 11:49:58 +0100 Subject: [PATCH] refactor(IStakeManager): move error codes to interface and remove unused ones There are a bunch of error codes that are either similar to other ones or not used at all. This commit moves them to the interface and removes the ones that aren't used anymore. Part of the reason we have so many unused errors becuase they had been "reintroduced" in `StakeMath`, which we'll revisit as well as described in #130 --- .gas-report | 4 ++-- .gas-snapshot | 2 +- src/RewardsStreamerMP.sol | 17 +---------------- src/interfaces/IStakeManager.sol | 15 ++++++++++----- test/RewardsStreamerMP.t.sol | 13 +++++++------ 5 files changed, 21 insertions(+), 30 deletions(-) diff --git a/.gas-report b/.gas-report index 64cec11..3c34454 100644 --- a/.gas-report +++ b/.gas-report @@ -78,7 +78,7 @@ |------------------------------------------------------+-----------------+--------+--------+--------+---------| | leave | 86840 | 86840 | 86840 | 86840 | 1 | |------------------------------------------------------+-----------------+--------+--------+--------+---------| -| lock | 14216 | 43993 | 43883 | 87862 | 260 | +| lock | 14216 | 43996 | 43883 | 87862 | 260 | |------------------------------------------------------+-----------------+--------+--------+--------+---------| | migrateToVault | 13541 | 62634 | 15747 | 158614 | 3 | |------------------------------------------------------+-----------------+--------+--------+--------+---------| @@ -150,7 +150,7 @@ |----------------------------------------+-----------------+--------+--------+--------+---------| | leave | 12167 | 124302 | 65554 | 353935 | 4 | |----------------------------------------+-----------------+--------+--------+--------+---------| -| lock | 12097 | 59261 | 59332 | 103310 | 261 | +| lock | 12097 | 59263 | 59332 | 103310 | 261 | |----------------------------------------+-----------------+--------+--------+--------+---------| | migrateToVault | 29012 | 89332 | 31218 | 207768 | 3 | |----------------------------------------+-----------------+--------+--------+--------+---------| diff --git a/.gas-snapshot b/.gas-snapshot index 52ff120..834815e 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -10,7 +10,7 @@ IntegrationTest:testStakeFoo() (gas: 1334385) LeaveTest:test_LeaveShouldProperlyUpdateAccounting() (gas: 6762395) LeaveTest:test_RevertWhenStakeManagerIsTrusted() (gas: 347066) LeaveTest:test_TrustNewStakeManager() (gas: 6826007) -LockTest:test_LockFailsWithInvalidPeriod(uint256) (runs: 1000, μ: 395434, ~: 395460) +LockTest:test_LockFailsWithInvalidPeriod(uint256) (runs: 1000, μ: 395435, ~: 395460) LockTest:test_LockFailsWithNoStake() (gas: 106803) LockTest:test_LockFailsWithZero() (gas: 364368) LockTest:test_LockWithoutPriorLock() (gas: 452687) diff --git a/src/RewardsStreamerMP.sol b/src/RewardsStreamerMP.sol index e7c853d..0874a15 100644 --- a/src/RewardsStreamerMP.sol +++ b/src/RewardsStreamerMP.sol @@ -22,21 +22,6 @@ contract RewardsStreamerMP is IRewardProvider, StakeMath { - error StakingManager__InvalidVault(); - error StakingManager__VaultNotRegistered(); - error StakingManager__VaultAlreadyRegistered(); - error StakingManager__AmountCannotBeZero(); - error StakingManager__TransferFailed(); - error StakingManager__InsufficientBalance(); - error StakingManager__LockingPeriodCannotBeZero(); - error StakingManager__CannotRestakeWithLockedFunds(); - error StakingManager__TokensAreLocked(); - error StakingManager__AlreadyLocked(); - error StakingManager__EmergencyModeEnabled(); - error StakingManager__DurationCannotBeZero(); - error StakingManager__Unauthorized(); - error StakingManager__MigrationTargetHasFunds(); - event VaultMigrated(address indexed from, address indexed to); IERC20 public STAKING_TOKEN; @@ -241,7 +226,7 @@ contract RewardsStreamerMP is } if (lockPeriod == 0) { - revert StakingManager__LockingPeriodCannotBeZero(); + revert StakingManager__DurationCannotBeZero(); } _updateGlobalState(); diff --git a/src/interfaces/IStakeManager.sol b/src/interfaces/IStakeManager.sol index 7a5c6e5..d563045 100644 --- a/src/interfaces/IStakeManager.sol +++ b/src/interfaces/IStakeManager.sol @@ -6,11 +6,16 @@ import { ITrustedCodehashAccess } from "./ITrustedCodehashAccess.sol"; import { IStakeConstants } from "./IStakeConstants.sol"; interface IStakeManager is ITrustedCodehashAccess, IStakeConstants { - error StakingManager__FundsLocked(); - error StakingManager__InvalidLockTime(); - error StakingManager__InsufficientFunds(); - error StakingManager__StakeIsTooLow(); - error StakingManager__NotAllowedToLeave(); + error StakingManager__VaultNotRegistered(); + error StakingManager__VaultAlreadyRegistered(); + error StakingManager__InvalidVault(); + error StakingManager__AmountCannotBeZero(); + error StakingManager__CannotRestakeWithLockedFunds(); + error StakingManager__AlreadyLocked(); + error StakingManager__EmergencyModeEnabled(); + error StakingManager__MigrationTargetHasFunds(); + error StakingManager__Unauthorized(); + error StakingManager__DurationCannotBeZero(); function registerVault() external; function stake(uint256 _amount, uint256 _seconds) external; diff --git a/test/RewardsStreamerMP.t.sol b/test/RewardsStreamerMP.t.sol index 1fe3fa3..e2857c4 100644 --- a/test/RewardsStreamerMP.t.sol +++ b/test/RewardsStreamerMP.t.sol @@ -10,6 +10,7 @@ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; import { Clones } from "@openzeppelin/contracts/proxy/Clones.sol"; +import { IStakeManager } from "../src/interfaces/IStakeManager.sol"; import { IStakeManagerProxy } from "../src/interfaces/IStakeManagerProxy.sol"; import { ITrustedCodehashAccess } from "../src/interfaces/ITrustedCodehashAccess.sol"; import { RewardsStreamerMP } from "../src/RewardsStreamerMP.sol"; @@ -1616,7 +1617,7 @@ contract LockTest is RewardsStreamerMPTest { _stake(alice, 10e18, 0); // Test with period = 0 - vm.expectRevert(RewardsStreamerMP.StakingManager__LockingPeriodCannotBeZero.selector); + vm.expectRevert(IStakeManager.StakingManager__DurationCannotBeZero.selector); _lock(alice, 0); } @@ -1658,7 +1659,7 @@ contract EmergencyExitTest is RewardsStreamerMPTest { vm.prank(admin); streamer.enableEmergencyMode(); - vm.expectRevert(RewardsStreamerMP.StakingManager__EmergencyModeEnabled.selector); + vm.expectRevert(IStakeManager.StakingManager__EmergencyModeEnabled.selector); vm.prank(admin); streamer.enableEmergencyMode(); } @@ -2052,13 +2053,13 @@ contract RewardsStreamerMP_RewardsTest is RewardsStreamerMPTest { function testSetRewards_RevertsBadDuration() public { vm.prank(admin); - vm.expectRevert(RewardsStreamerMP.StakingManager__DurationCannotBeZero.selector); + vm.expectRevert(IStakeManager.StakingManager__DurationCannotBeZero.selector); streamer.setReward(1000, 0); } function testSetRewards_RevertsBadAmount() public { vm.prank(admin); - vm.expectRevert(RewardsStreamerMP.StakingManager__AmountCannotBeZero.selector); + vm.expectRevert(IStakeManager.StakingManager__AmountCannotBeZero.selector); streamer.setReward(0, 10); } @@ -2240,7 +2241,7 @@ contract StakeVaultMigrationTest is RewardsStreamerMPTest { function test_RevertWhenNotOwnerOfMigrationVault() public { // alice tries to migrate to a vault she doesn't own vm.prank(alice); - vm.expectRevert(RewardsStreamerMP.StakingManager__Unauthorized.selector); + vm.expectRevert(IStakeManager.StakingManager__Unauthorized.selector); StakeVault(vaults[alice]).migrateToVault(vaults[bob]); } @@ -2254,7 +2255,7 @@ contract StakeVaultMigrationTest is RewardsStreamerMPTest { newVault.stake(10e18, 0); // alice tries to migrate to a vault that is not empty - vm.expectRevert(RewardsStreamerMP.StakingManager__MigrationTargetHasFunds.selector); + vm.expectRevert(IStakeManager.StakingManager__MigrationTargetHasFunds.selector); StakeVault(vaults[alice]).migrateToVault(address(newVault)); }