Skip to content

support native eth in integration tests #340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 29, 2023
Merged

support native eth in integration tests #340

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
39c5553
wip: support native eth in integration tests
wadealexc Nov 20, 2023
24090ff
test: full support for native ETH in integration test framework
wadealexc Nov 27, 2023
5c1f4d6
test: finish native eth integration
wadealexc Nov 28, 2023
3eba0cc
test: update comments and use const for native eth address
wadealexc Nov 29, 2023
61943fb
test: add support for multiple validators within one pod
wadealexc Nov 29, 2023
fd6986b
test: add HOLDS_ALL flag to create users that hold random amts of eve…
wadealexc Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/contracts/libraries/BeaconChainProofs.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ library BeaconChainProofs {
/**
* @dev Retrieves a validator's pubkey hash
*/
function getPubkeyHash(bytes32[] memory validatorFields) internal pure returns (bytes32) {
function getPubkeyHash(bytes32[] memory validatorFields) internal pure returns (bytes32) {
return
validatorFields[VALIDATOR_PUBKEY_INDEX];
}
Expand All @@ -466,7 +466,7 @@ library BeaconChainProofs {
/**
* @dev Retrieves a validator's effective balance (in gwei)
*/
function getEffectiveBalanceGwei(bytes32[] memory validatorFields) internal pure returns (uint64) {
function getEffectiveBalanceGwei(bytes32[] memory validatorFields) internal pure returns (uint64) {
return
Endian.fromLittleEndianUint64(validatorFields[VALIDATOR_BALANCE_INDEX]);
}
Expand Down
41 changes: 25 additions & 16 deletions src/test/integration/IntegrationBase.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ abstract contract IntegrationBase is IntegrationDeployer {
function _newRandomStaker() internal returns (User, IStrategy[] memory, uint[] memory) {
(User staker, IStrategy[] memory strategies, uint[] memory tokenBalances) = _randUser();

assert_HasUnderlyingTokenBalances(staker, strategies, tokenBalances, "_newStaker: failed to award token balances");
assert_HasUnderlyingTokenBalances(staker, strategies, tokenBalances, "_newRandomStaker: failed to award token balances");

return (staker, strategies, tokenBalances);
}
Expand All @@ -33,9 +33,9 @@ abstract contract IntegrationBase is IntegrationDeployer {
operator.registerAsOperator();
operator.depositIntoEigenlayer(strategies, tokenBalances);

assert_Snap_AddedStakerShares(operator, strategies, tokenBalances, "_newOperator: failed to add delegatable shares");
assert_Snap_AddedOperatorShares(operator, strategies, tokenBalances, "_newOperator: failed to award shares to operator");
assertTrue(delegationManager.isOperator(address(operator)), "_newOperator: operator should be registered");
assert_Snap_AddedStakerShares(operator, strategies, tokenBalances, "_newRandomOperator: failed to add delegatable shares");
assert_Snap_AddedOperatorShares(operator, strategies, tokenBalances, "_newRandomOperator: failed to award shares to operator");
assertTrue(delegationManager.isOperator(address(operator)), "_newRandomOperator: operator should be registered");

return (operator, strategies, tokenBalances);
}
Expand Down Expand Up @@ -88,9 +88,13 @@ abstract contract IntegrationBase is IntegrationDeployer {

uint actualShares;
if (strat == BEACONCHAIN_ETH_STRAT) {
// TODO
// actualShares = eigenPodManager.podOwnerShares(address(user));
revert("unimplemented");
// TODO - is this the right way to handle this?
int shares = eigenPodManager.podOwnerShares(address(user));
if (shares < 0) {
revert("assert_HasExpectedShares: negative shares");
}

actualShares = uint(shares);
} else {
actualShares = strategyManager.stakerStrategyShares(address(user), strat);
}
Expand Down Expand Up @@ -251,9 +255,7 @@ abstract contract IntegrationBase is IntegrationDeployer {

uint tokenBalance = tokenBalances[i];
if (strat == BEACONCHAIN_ETH_STRAT) {
// TODO - need to calculate this
// expectedShares[i] = eigenPodManager.underlyingToShares(tokenBalance);
revert("_calculateExpectedShares: unimplemented for native eth");
expectedShares[i] = tokenBalances[i];
} else {
expectedShares[i] = strat.underlyingToShares(tokenBalance);
}
Expand All @@ -271,9 +273,7 @@ abstract contract IntegrationBase is IntegrationDeployer {
IStrategy strat = strategies[i];

if (strat == BEACONCHAIN_ETH_STRAT) {
// TODO - need to calculate this
// expectedTokens[i] = eigenPodManager.underlyingToShares(tokenBalance);
revert("_calculateExpectedShares: unimplemented for native eth");
expectedTokens[i] = shares[i];
} else {
expectedTokens[i] = strat.sharesToUnderlying(shares[i]);
}
Expand Down Expand Up @@ -323,8 +323,13 @@ abstract contract IntegrationBase is IntegrationDeployer {
IStrategy strat = strategies[i];

if (strat == BEACONCHAIN_ETH_STRAT) {
// curShares[i] = eigenPodManager.podOwnerShares(address(staker));
revert("TODO: unimplemented");
// TODO - is this the right way to handle this?
int shares = eigenPodManager.podOwnerShares(address(staker));
if (shares < 0) {
revert("_getStakerShares: negative shares");
}

curShares[i] = uint(shares);
Copy link
Contributor

@ChaoticWalrus ChaoticWalrus Nov 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to answer the question asked in the TODO here:
I think this is OK as-is.
We should make sure we have some coverage for users having negative shares, but it is a bit of an "edge case", so I feel alright with it requiring a bit more special handling, and non-negative shares being the default.

Copy link
Contributor

@ChaoticWalrus ChaoticWalrus Nov 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other option is probably making every share amount an int256 and then casting back to uint256 as necessary, but that just sounds like wayyyy more of a pain

Copy link
Contributor Author

@wadealexc wadealexc Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thoughts.

Agreed that non-negative is a good default, since we should only see negative shares if we do a balance update after a withdrawal -- and IMO that deserves its own top-level test.

If/when we write that test we can write a balance handler that specifically allows negative balances. In the meantime, if we hit negative here failing is fine because that shouldn't happen.

} else {
curShares[i] = strategyManager.stakerStrategyShares(address(staker), strat);
}
Expand All @@ -349,7 +354,11 @@ abstract contract IntegrationBase is IntegrationDeployer {
uint[] memory balances = new uint[](tokens.length);

for (uint i = 0; i < tokens.length; i++) {
balances[i] = tokens[i].balanceOf(address(staker));
if (address(tokens[i]) == address(0)) {
balances[i] = address(staker).balance;
} else {
balances[i] = tokens[i].balanceOf(address(staker));
}
}

return balances;
Expand Down
39 changes: 29 additions & 10 deletions src/test/integration/IntegrationDeployer.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ import "src/contracts/permissions/PauserRegistry.sol";

import "src/test/mocks/EmptyContract.sol";
import "src/test/mocks/ETHDepositMock.sol";
import "src/test/mocks/BeaconChainOracleMock.sol";
import "src/test/integration/mocks/BeaconChainOracleMock.t.sol";
import "src/test/integration/mocks/BeaconChainMock.t.sol";

import "src/test/integration/User.t.sol";

Expand Down Expand Up @@ -54,6 +55,7 @@ abstract contract IntegrationDeployer is Test, IUserDeployer {
// Mock Contracts to deploy
ETHPOSDepositMock ethPOSDeposit;
BeaconChainOracleMock beaconChainOracle;
BeaconChainMock public beaconChain;

// ProxyAdmin
ProxyAdmin eigenLayerProxyAdmin;
Expand Down Expand Up @@ -253,7 +255,12 @@ abstract contract IntegrationDeployer is Test, IUserDeployer {
ethStrats.push(BEACONCHAIN_ETH_STRAT);
mixedStrats.push(BEACONCHAIN_ETH_STRAT);

// Create time machine and set block timestamp forward so we can create EigenPod proofs in the past
timeMachine = new TimeMachine();
timeMachine.setProofGenStartTime(2 hours);

// Create mock beacon chain / proof gen interface
beaconChain = new BeaconChainMock(timeMachine, beaconChainOracle);
}

/// @dev Deploy a strategy and its underlying token, push to global lists of tokens/strategies, and whitelist
Expand Down Expand Up @@ -332,7 +339,7 @@ abstract contract IntegrationDeployer is Test, IUserDeployer {
// User will use `delegateToBySignature` and `depositIntoStrategyWithSignature`
user = User(new User_SignedMethods());
} else {
revert("_newUser: unimplemented userType");
revert("_randUser: unimplemented userType");
}

// For the specific asset selection we made, get a random assortment of
Expand Down Expand Up @@ -378,14 +385,20 @@ abstract contract IntegrationDeployer is Test, IUserDeployer {
tokenBalances[i] = balance;
strategies[i] = strat;
}

return (strategies, tokenBalances);
} else if (assetType == HOLDS_ETH) {
revert("_getRandAssets: HOLDS_ETH unimplemented");
strategies = new IStrategy[](1);
tokenBalances = new uint[](1);

// Award the user 32 ETH
uint amount = 32 ether;
Copy link
Collaborator

@ypatil12 ypatil12 Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we award the user some value of ETH in the set {<32 ETH, 32 ETH, > 32ETH} ?

Copy link
Contributor Author

@wadealexc wadealexc Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great question, I was wondering too.

There is a slight hitch with just using random ETH values: EigenPod.stake requires that msg.value == 32 ether. Also, the deposit contract itself has a few requirements that I'd want the mock beacon chain to enforce:

  • depositValue >= 1 ether
  • depositValue % 1 gwei == 0
  • depositValue / 1 gwei <= type(u64).max

The easy, immediate changes we could make:

  • Have BeaconChainMock.newValidator enforce the deposit contract conditions I listed above
  • Create users that are awarded some multiple of 32 ETH (1x, 2x, 3x, etc) - and automatically deploy more validators to the same pod.

I think these would be valuable additions, so will work on those now.

Copy link
Contributor Author

@wadealexc wadealexc Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your question about having validators with other nonstandard balances is a good one though. So here's the idea: since you don't actually need to call stake to deposit and verify withdrawal credentials - we have a userType that skips that step in favor of depositing directly into the beacon chain.

Here's my what the changelog could look like, adding onto the changes I described above:

  1. We change the SIGNED_METHODS usertype: it's now called ALT_METHODS. The concept is a user type that always calls the "non-default, but equivalent" methods.
  2. We change User_SignedMethods (now called User_AltMethods) to override depositIntoEigenlayer. The new version does NOT call eigenPodManager.stake. Instead, it creates a pod, then directly deposits and verifies credentials. And this would mean it could handle other values for ETH (those supported by the deposit contract, anyway)
  3. When dealing random assets, if we're handling assetType == HOLDS_ETH and userType == ALT_METHODS, we give them a random multiple of 32 ETH, but we ALSO add or subtract a random amount of gwei, e.g: { -10 gwei, -1 gwei, -0 gwei, +1 gwei, +10 gwei }

WDYT?

Copy link
Collaborator

@ypatil12 ypatil12 Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

User_AltMethods additions make sense to me. Is the reasoning of subtracting a random amount of gwei to test deposit value % 1 Gwei == 0 invariant from the deposit contract?

Copy link
Contributor Author

@wadealexc wadealexc Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's because whether the user is using the deposit contract directly or going through epm.stake, the deposit contract will enforce that value % 1 gwei is true. Plus, the beacon chain only deals in gwei-denominated balances, so we can't deposit 1 wei and have it actually be reflected anywhere.

This is just to keep things true to life. If we had non-gwei-denominated beacon chain balances we'd have to add a lot of bloat handling a case that shouldn't be possible IRL.

Copy link
Contributor Author

@wadealexc wadealexc Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have added the first set of changes in 3eba0cc.

Also did the renaming from SIGNED_METHODS to ALT_METHODS. Have not implemented the direct-stake variant, though.

cheats.deal(address(user), amount);

strategies[0] = BEACONCHAIN_ETH_STRAT;
tokenBalances[0] = amount;
} else if (assetType == HOLDS_MIX) {
revert("_getRandAssets: HOLDS_MIX unimplemented");
revert("_dealRandAssets: HOLDS_MIX unimplemented");
} else {
revert("_getRandAssets: assetType unimplemented");
revert("_dealRandAssets: assetType unimplemented");
}

return (strategies, tokenBalances);
Expand Down Expand Up @@ -467,10 +480,16 @@ abstract contract IntegrationDeployer is Test, IUserDeployer {

for (uint i = 0; i < strategies.length; i++) {
IStrategy strat = strategies[i];
IERC20 underlyingToken = strat.underlyingToken();

emit log_named_string("token name: ", IERC20Metadata(address(underlyingToken)).name());
emit log_named_uint("token balance: ", tokenBalances[i]);
if (strat == BEACONCHAIN_ETH_STRAT) {
emit log_named_string("token name: ", "Native ETH");
emit log_named_uint("token balance: ", tokenBalances[i]);
} else {
IERC20 underlyingToken = strat.underlyingToken();

emit log_named_string("token name: ", IERC20Metadata(address(underlyingToken)).name());
emit log_named_uint("token balance: ", tokenBalances[i]);
}
}
}
}
1 change: 0 additions & 1 deletion src/test/integration/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,4 @@ function testFuzz_deposit_delegate_EXAMPLE(uint24 _random) public {
### What needs to be done?

* Suggest or PR cleanup if you have ideas. Currently, the `IntegrationDeployer` contract is pretty messy.
* Currently, the only supported assetTypes are `NO_ASSETS` and `HOLDS_LST`. There are flags for `HOLDS_ETH` and `HOLDS_MIXED`, but we need to implement `EigenPod` proof generation/usage before they can be used.
* Coordinate in Slack to pick out some user flows to write tests for!
12 changes: 12 additions & 0 deletions src/test/integration/TimeMachine.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ contract TimeMachine is Test {
bool pastExists = false;
uint lastSnapshot;

uint64 public proofGenStartTime;

function createSnapshot() public returns (uint) {
uint snapshot = cheats.snapshot();
lastSnapshot = snapshot;
Expand All @@ -30,4 +32,14 @@ contract TimeMachine is Test {
function warpToPresent(uint curState) public {
cheats.revertTo(curState);
}

/// @dev Sets the timestamp we use for proof gen to now,
/// then sets block timestamp to now + secondsAgo.
///
/// This means we can create mock proofs using an oracle time
/// of `proofGenStartTime`.
function setProofGenStartTime(uint secondsAgo) public {
proofGenStartTime = uint64(block.timestamp);
cheats.warp(block.timestamp + secondsAgo);
}
}
88 changes: 84 additions & 4 deletions src/test/integration/User.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,20 @@ import "forge-std/Test.sol";
import "src/contracts/core/DelegationManager.sol";
import "src/contracts/core/StrategyManager.sol";
import "src/contracts/pods/EigenPodManager.sol";
import "src/contracts/pods/EigenPod.sol";

import "src/contracts/interfaces/IDelegationManager.sol";
import "src/contracts/interfaces/IStrategy.sol";

import "src/test/integration/TimeMachine.t.sol";
import "src/test/integration/mocks/BeaconChainMock.t.sol";

interface IUserDeployer {
function delegationManager() external view returns (DelegationManager);
function strategyManager() external view returns (StrategyManager);
function eigenPodManager() external view returns (EigenPodManager);
function timeMachine() external view returns (TimeMachine);
function beaconChain() external view returns (BeaconChainMock);
}

contract User is Test {
Expand All @@ -29,7 +32,19 @@ contract User is Test {

TimeMachine timeMachine;

/// @dev Native restaker state vars

BeaconChainMock beaconChain;
EigenPod pod;

// TODO - change this to handle multiple validators
uint40 validatorIndex;

IStrategy constant BEACONCHAIN_ETH_STRAT = IStrategy(0xbeaC0eeEeeeeEEeEeEEEEeeEEeEeeeEeeEEBEaC0);
IERC20 constant NATIVE_ETH = IERC20(0xbeaC0eeEeeeeEEeEeEEEEeeEEeEeeeEeeEEBEaC0);
uint constant GWEI_TO_WEI = 1e9;



constructor() {
IUserDeployer deployer = IUserDeployer(msg.sender);
Expand All @@ -38,13 +53,22 @@ contract User is Test {
strategyManager = deployer.strategyManager();
eigenPodManager = deployer.eigenPodManager();
timeMachine = deployer.timeMachine();

beaconChain = deployer.beaconChain();
pod = EigenPod(payable(eigenPodManager.createPod()));
}

modifier createSnapshot() virtual {
timeMachine.createSnapshot();
_;
}

receive() external payable {}

/**
* DelegationManager methods:
*/

function registerAsOperator() public createSnapshot virtual {
IDelegationManager.OperatorDetails memory details = IDelegationManager.OperatorDetails({
earningsReceiver: address(this),
Expand All @@ -63,8 +87,24 @@ contract User is Test {
uint tokenBalance = tokenBalances[i];

if (strat == BEACONCHAIN_ETH_STRAT) {
// TODO handle this flow - need to deposit into EPM + prove credentials
revert("depositIntoEigenlayer: unimplemented");
eigenPodManager.stake{ value: tokenBalance }("", "", bytes32(0));

(uint40 newValidatorIndex, CredentialsProofs memory proofs) =
beaconChain.newValidator({
balanceWei: tokenBalance,
withdrawalCreds: _podWithdrawalCredentials()
});

validatorIndex = newValidatorIndex;

pod.verifyWithdrawalCredentials({
oracleTimestamp: proofs.oracleTimestamp,
stateRootProof: proofs.stateRootProof,
validatorIndices: proofs.validatorIndices,
validatorFieldsProofs: proofs.validatorFieldsProofs,
validatorFields: proofs.validatorFields
});

} else {
IERC20 underlyingToken = strat.underlyingToken();
underlyingToken.approve(address(strategyManager), tokenBalance);
Expand Down Expand Up @@ -130,6 +170,28 @@ contract User is Test {

if (strat == BEACONCHAIN_ETH_STRAT) {
tokens[i] = IERC20(address(0));

// If we're withdrawing as tokens, we need to process a withdrawal proof first
if (receiveAsTokens) {
emit log("exiting validator and processing withdrawals...");
BeaconWithdrawal memory proofs = beaconChain.exitValidator(validatorIndex);

uint64 withdrawableBefore = pod.withdrawableRestakedExecutionLayerGwei();

pod.verifyAndProcessWithdrawals({
oracleTimestamp: proofs.oracleTimestamp,
stateRootProof: proofs.stateRootProof,
withdrawalProofs: proofs.withdrawalProofs,
validatorFieldsProofs: proofs.validatorFieldsProofs,
validatorFields: proofs.validatorFields,
withdrawalFields: proofs.withdrawalFields
});

uint64 withdrawableAfter = pod.withdrawableRestakedExecutionLayerGwei();

emit log_named_uint("pod withdrawable before: ", withdrawableBefore);
emit log_named_uint("pod withdrawable after: ", withdrawableAfter);
}
} else {
tokens[i] = strat.underlyingToken();
}
Expand All @@ -139,6 +201,10 @@ contract User is Test {

return tokens;
}

function _podWithdrawalCredentials() internal view returns (bytes memory) {
return abi.encodePacked(bytes1(uint8(1)), bytes11(0), address(pod));
}
}

/// @notice A user contract that implements 1271 signatures
Expand Down Expand Up @@ -176,8 +242,22 @@ contract User_SignedMethods is User {
uint tokenBalance = tokenBalances[i];

if (strat == BEACONCHAIN_ETH_STRAT) {
// TODO handle this flow - need to deposit into EPM + prove credentials
revert("depositIntoEigenlayer: unimplemented");
eigenPodManager.stake{ value: tokenBalance }("", "", bytes32(0));

(uint40 newValidatorIndex, CredentialsProofs memory proofs) = beaconChain.newValidator({
balanceWei: tokenBalance,
withdrawalCreds: _podWithdrawalCredentials()
});

validatorIndex = newValidatorIndex;

pod.verifyWithdrawalCredentials({
oracleTimestamp: proofs.oracleTimestamp,
stateRootProof: proofs.stateRootProof,
validatorIndices: proofs.validatorIndices,
validatorFieldsProofs: proofs.validatorFieldsProofs,
validatorFields: proofs.validatorFields
});
} else {
// Approve token
IERC20 underlyingToken = strat.underlyingToken();
Expand Down
Loading