Skip to content

Commit

Permalink
Merge pull request #293 from Layr-Labs/test/strategy-manager-cleanup
Browse files Browse the repository at this point in the history
StrategyManager Unit Test Refactor
  • Loading branch information
8sunyuan authored Nov 3, 2023
2 parents d498377 + d30521e commit 1d926f2
Show file tree
Hide file tree
Showing 10 changed files with 1,154 additions and 819 deletions.
2 changes: 1 addition & 1 deletion src/test/WithdrawalMigration.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity =0.8.12;

import "../test/EigenLayerTestHelper.t.sol";
import "./unit/Utils.sol";
import "src/test/utils/Utils.sol";
import "./mocks/ERC20Mock.sol";

///@notice This set of tests shadow forks the contracts from M1, queues withdrawals, and tests the migration functionality
Expand Down
93 changes: 93 additions & 0 deletions src/test/tree/EigenPodManager.tree
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
├── EigenPodManager Tree (*** denotes that integrationt tests are needed to validate path)
├── when contract is deployed and initialized
│ └── it should properly set storage
├── when initialize called again
│ └── it should revert
├── when createPod called
│ ├── given the user has already created a pod
│ │ └── it should revert
│ ├── given that the max number of pods has been deployed
│ │ └── it should revert
│ └── given the user has not created a pod
│ └── it should deploy a pod
├── when stake is called
│ ├── given the user has not created a pod
│ │ └── it should deploy a pod
│ └── given the user has already created a pod
│ └── it should call stake on the eigenPod
├── when setMaxPods is called
│ ├── given the user is not the pauser
│ │ └── it should revert
│ └── given the user is the pauser
│ └── it should set the max pods
├── when updateBeaconChainOracle is called
│ ├── given the user is not the owner
│ │ └── it should revert
│ └── given the user is the owner
│ └── it should set the beacon chain oracle
├── when addShares is called
│ ├── given that the caller is not the delegationManager
│ │ └── it should revert
│ ├── given that the podOwner address is 0
│ │ └── it should revert
│ ├── given that the shares amount is negative
│ │ └── it should revert
│ ├── given that the shares is not a whole gwei amount
│ │ └── it should revert
│ └── given that all of the above conditions are satisfied
│ └── it should update the podOwnerShares
├── when removeShares is called
│ ├── given that the caller is not the delegationManager
│ │ └── it should revert
│ ├── given that the shares amount is negative
│ │ └── it should revert
│ ├── given that the shares is not a whole gwei amount
│ │ └── it should revert
│ ├── given that removing shares results in the pod owner having negative shares
│ │ └── it should revert
│ └── given that all of the above conditions are satisfied
│ └── it should update the podOwnerShares
├── when withdrawSharesAsTokens is called
│ ├── given that the podOwner is address 0
│ │ └── it should revert
│ ├── given that the destination is address 0
│ │ └── it should revert
│ ├── given that the shares amount is negative
│ │ └── it should revert
│ ├── given that the shares is not a whole gwei amount
│ │ └── it should revert
│ ├── given that the current podOwner shares are negative
│ │ ├── given that the shares to withdraw are larger in magnitude than the shares of the podOwner
│ │ │ └── it should set the podOwnerShares to 0 and decrement shares to withdraw by the share deficit
│ │ └── given that the shares to withdraw are smaller in magnitude than shares of the podOwner
│ │ └── it should increment the podOwner shares by the shares to withdraw
│ └── given that the pod owner shares are positive
│ └── it should withdraw restaked ETH from the eigenPod
├── when shares are adjusted ***
│ ├── given that sharesBefore is negative or 0
│ │ ├── given that sharesAfter is negative or zero
│ │ │ └── the change in delegateable shares should be 0
│ │ └── given that sharesAfter is positive
│ │ └── the change in delegateable shares should be positive
│ └── given that sharesBefore is positive
│ ├── given that sharesAfter is negative or zero
│ │ └── the change in delegateable shares is negative sharesBefore
│ └── given that sharesAfter is positive
│ └── the change in delegateable shares is the difference between sharesAfter and sharesBefore
└── when recordBeaconChainETHBalanceUpdate is called
├── given that the podOwner's eigenPod is not the caller
│ └── it should revert
├── given that the podOwner is a zero address
│ └── it should revert
├── given that sharesDelta is not a whole gwei amount
│ ├── it should revert
│ └── given that the shares delta is valid
│ └── it should update the podOwnerShares
├── given that the change in delegateable shares is positive ***
│ └── it should increase delegated shares on the delegationManager
├── given that the change in delegateable shares is negative ***
│ └── it should decrease delegated shares on the delegationManager
├── given that the change in delegateable shares is 0 ***
│ └── it should only update the podOwnerShares
└── given that the function is reentered ***
└── it should revert
4 changes: 2 additions & 2 deletions src/test/tree/EigenPodManagerUnit.tree
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
├── EigenPodManager Tree (*** denotes that integrationt tests are needed to validate path)
├── EigenPodManager Tree (*** denotes that integration tests are needed to validate path)
├── when contract is deployed and initialized
│ └── it should properly set storage
├── when initialize called again
Expand All @@ -23,7 +23,7 @@
├── when updateBeaconChainOracle is called
│ ├── given the user is not the owner
│ │ └── it should revert
│ └── given the user is the owner
│ └── given the user is the owner
│ └── it should set the beacon chain oracle
├── when addShares is called
│ ├── given that the caller is not the delegationManager
Expand Down
111 changes: 111 additions & 0 deletions src/test/tree/StrategyManangerUnit.tree
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
├── StrategyManagerUnit.t.sol (*** denotes that integration tests are needed to validate path)
├── initialize
| ├── given that initialized is only called once
│ │ └── it should set the storage variables correctly (owner, strategyWhitelister, pauserRegistry)
│ └── given that initialize is called again
│ └── it should revert
├── depositIntoStrategy()
│ ├── given that deposits paused
│ │ └── it should revert
│ ├── given the function is re-entered
│ │ └── it should revert
│ ├── given that the strategy is not whitelisted
│ │ └── it should revert
│ ├── given the token safeTransferFrom() reverts
│ │ └── it should revert
│ └── given that token safeTransferFrom() succeeds
│ ├── given the staker has existing shares in strategy (not first deposit)
│ │ └── it should increase shares, nonce. while stakerStrategyListLength is unchanged
│ ├── given the staker has no existing shares in strategy (first deposit)
│ │ └── stakerStrategyListLength increases by 1 and shares increase
│ ├── given the staker has delegated to a operator ***
│ │ └── it should deposit successfully with shares increase, including delegated shares
│ └── given the staker is not delegated
│ └── it should deposit successfully with shares increase
├── depositIntoStrategyWithSignature()
│ ├── given that deposits paused
│ │ └── it should revert
│ ├── given the function is re-entered
│ │ └── it should revert
│ ├── given the signature expired
│ │ └── it should revert
│ ├── given that deposits paused and strategy not whitelisted
│ │ └── it should revert
│ ├── given the staker is a EOA
│ │ ├── given the signature verification fails
│ │ │ └── it should revert
│ │ └── given the signature verification succeeds
│ │ ├── given the token safeTransferFrom reverts
│ │ │ └── it should revert
│ │ └── given the token safeTransferFrom succeeds
│ │ ├── given that the staker has delegated to a operator ***
│ │ │ └── it should deposit successfully with shares and nonce increase, including delegated shares
│ │ └── given that the staker is not delegated
│ │ └── it should deposit successfully with shares and nonce increase
│ └── given the staker is a contract
│ ├── given the contract isn't EIP1271 compliant
│ │ └── it should revert
│ ├── given the signature verification fails, isValidSignature() return != EIP1271_MAGICVALUE
│ │ └── it should revert
│ └── given the signature verification succeeds, isValidSignature() returns EIP1271_MAGICVALUE
│ ├── given the token safeTransferFrom reverts
│ │ └── it should revert
│ └── given the token safeTransferFrom succeeds
│ ├── given the staker has delegated to a operator ***
│ │ └── it should deposit successfully with shares and nonce increase, including delegated shares
│ └── given the staker is not delegated
│ └── it should deposit successfully with shares and nonce increase
├── removeShares()
│ ├── given not called by DelegationManager
│ │ └── it should revert
│ ├── given the share amount is 0
│ │ └── it should revert
│ ├── given the share amount is too high, higher than deposited amount
│ │ └── it should revert
│ ├── given the share amount is equal to the deposited amount
│ │ └── staker shares should be 0 with decremented stakerStrategyListLength
│ └── given the share amount is less than the deposited amount
│ └── staker shares should now be deposited - shares amount, unchanged stakerStrategyListLength
├── addShares()
│ ├── given not called by DelegationManager
│ │ └── it should revert
│ ├── given the share amount is 0
│ │ └── it should revert
│ ├── given the staker is 0 address
│ │ └── it should revert
│ ├── given adding shares with 0 existing shares
│ │ └── it should increase shares and increment stakerStrategyListLength
│ ├── given adding shares with 0 existing shares and staker has MAX_STAKER_STRATEGY_LIST_LENGTH
│ │ └── it should revert
│ └── given the adding shares with > 0 existing shares
│ └── it should increase shares, unchanged stakerStrategyListLength
├── withdrawSharesAsTokens()
│ ├── given not called by DelegationManager
│ │ └── it should revert
│ └── given that deposited strategy is called
│ │ └── it should withdraw tokens from strategy with token balanceOf() update
├── setStrategyWhitelister()
│ ├── given not called by owner
│ │ └── it should revert
│ └── given called by owner address
│ └── it should update strategyWhitelister address
├── addStrategiesToDepositWhitelist()
│ ├── given not called by strategyWhitelister address
│ │ └── it should revert
│ └── given the strategyWhitelister address is called
│ ├── given adding one single strategy that is already whitelisted
│ │ └── it should not emit StrategyAddedToDepositWhitelist with mapping still true
│ ├── given adding one single strategy
│ │ └── it should whitelist the new strategy with mapping set to true
│ └── given adding multiple strategies to whitelist
│ └── it should whitelist all new strategies with mappings set to true
└── removeStrategiesFromDepositWhitelist()
├── given not called by strategyWhitelister address
│ └── it should revert
└── given called by strategyWhitelister address
├── given removing one single strategy that is not whitelisted
│ └── it shouldn't emit StrategyRemovedFromDepositWhitelist with mapping still false
├── given removing one single strategy
│ └── it should de-whitelist the new strategy with mapping set to false
└── given removing multiple strategies to whitelist
└── it should de-whitelist all specified strategies with mappings set to false
33 changes: 6 additions & 27 deletions src/test/unit/EigenPodManagerUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,10 @@ import "forge-std/Test.sol";

import "../../contracts/pods/EigenPodManager.sol";
import "../../contracts/pods/EigenPodPausingConstants.sol";
import "../../contracts/permissions/PauserRegistry.sol";

import "../events/IEigenPodManagerEvents.sol";
import "../utils/EigenLayerUnitTestSetup.sol";
import "../harnesses/EigenPodManagerWrapper.sol";
import "../mocks/DelegationManagerMock.sol";
import "../mocks/SlasherMock.sol";
import "../mocks/StrategyManagerMock.sol";
import "../mocks/EigenPodMock.sol";
import "../mocks/ETHDepositMock.sol";

Expand All @@ -24,14 +20,7 @@ contract EigenPodManagerUnitTests is EigenLayerUnitTestSetup {

using stdStorage for StdStorage;

// Proxy Admin & Pauser Registry
ProxyAdmin public proxyAdmin;
PauserRegistry public pauserRegistry;

// Mocks
StrategyManagerMock public strategyManagerMock;
DelegationManagerMock public delegationManagerMock;
SlasherMock public slasherMock;
IETHPOSDeposit public ethPOSMock;
IEigenPod public eigenPodMockImplementation;
IBeacon public eigenPodBeacon; // Proxy for eigenPodMockImplementation
Expand All @@ -43,19 +32,10 @@ contract EigenPodManagerUnitTests is EigenLayerUnitTestSetup {
IEigenPod public defaultPod;
address public initialOwner = address(this);

function setUp() virtual public {
// Deploy ProxyAdmin
proxyAdmin = new ProxyAdmin();

// Initialize PauserRegistry
address[] memory pausers = new address[](1);
pausers[0] = pauser;
pauserRegistry = new PauserRegistry(pausers, unpauser);
function setUp() virtual override public {
EigenLayerUnitTestSetup.setUp();

// Deploy Mocks
slasherMock = new SlasherMock();
delegationManagerMock = new DelegationManagerMock();
strategyManagerMock = new StrategyManagerMock();
ethPOSMock = new ETHPOSDepositMock();
eigenPodMockImplementation = new EigenPodMock();
eigenPodBeacon = new UpgradeableBeacon(address(eigenPodMockImplementation));
Expand All @@ -72,7 +52,7 @@ contract EigenPodManagerUnitTests is EigenLayerUnitTestSetup {
address(
new TransparentUpgradeableProxy(
address(eigenPodManagerImplementation),
address(proxyAdmin),
address(eigenLayerProxyAdmin),
abi.encodeWithSelector(
EigenPodManager.initialize.selector,
type(uint256).max /*maxPods*/,
Expand All @@ -91,9 +71,8 @@ contract EigenPodManagerUnitTests is EigenLayerUnitTestSetup {
// Set defaultPod
defaultPod = eigenPodManager.getPod(defaultStaker);

// Exclude the zero address, the proxyAdmin and the eigenPodManager itself from fuzzed inputs
// Exclude the zero address, and the eigenPodManager itself from fuzzed inputs
addressIsExcludedFromFuzzedInputs[address(0)] = true;
addressIsExcludedFromFuzzedInputs[address(proxyAdmin)] = true;
addressIsExcludedFromFuzzedInputs[address(eigenPodManager)] = true;
}

Expand Down Expand Up @@ -541,7 +520,7 @@ contract EigenPodManagerUnitTests_ShareAdjustmentCalculationTests is EigenPodMan
slasherMock,
delegationManagerMock
);
proxyAdmin.upgrade(TransparentUpgradeableProxy(payable(address(eigenPodManager))), address(eigenPodManagerWrapper));
eigenLayerProxyAdmin.upgrade(TransparentUpgradeableProxy(payable(address(eigenPodManager))), address(eigenPodManagerWrapper));
}

function testFuzz_shareAdjustment_negativeToNegative(int256 sharesBefore, int256 sharesAfter) public {
Expand Down Expand Up @@ -575,4 +554,4 @@ contract EigenPodManagerUnitTests_ShareAdjustmentCalculationTests is EigenPodMan
int256 sharesDelta = eigenPodManagerWrapper.calculateChangeInDelegatableShares(sharesBefore, sharesAfter);
assertEq(sharesDelta, sharesAfter - sharesBefore, "Shares delta must be equal to the difference between sharesAfter and sharesBefore");
}
}
}
2 changes: 1 addition & 1 deletion src/test/unit/SlasherUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import "../mocks/Reverter.sol";

import "../mocks/ERC20Mock.sol";

import "./Utils.sol";
import "src/test/utils/Utils.sol";

contract SlasherUnitTests is Test, Utils {

Expand Down
Loading

0 comments on commit 1d926f2

Please sign in to comment.