Skip to content
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.

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

fix: reject credential proofs if activation epoch is not set #668

Merged
merged 2 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions docs/core/EigenPod.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ A withdrawal credential proof uses a validator's [`ValidatorIndex`][custom-types
* `pubkey`: A BLS pubkey hash, used to uniquely identify the validator within the `EigenPod`
* `withdrawal_credentials`: Used to verify that the validator will withdraw its principal to this `EigenPod` if it exits the beacon chain
* `effective_balance`: The balance of the validator, updated once per epoch and capped at 32 ETH. Used to award shares to the Pod Owner
* `activation_epoch`: Initially set to `type(uint64).max`, this value is updated when a validator reaches a balance of at least 32 ETH, designating the validator is ready to become active on the beacon chain. **This method requires that a validator is either already active, or in the process of activating on the beacon chain.**
* `exit_epoch`: Initially set to `type(uint64).max`, this value is updated when a validator initiates exit from the beacon chain. **This method requires that a validator has not initiated an exit from the beacon chain.**
* If a validator has been exited prior to calling `verifyWithdrawalCredentials`, their ETH can be accounted for, awarded shares, and/or withdrawn via the checkpoint system (see [Checkpointing Validators](#checkpointing-validators)).

Expand All @@ -132,6 +133,7 @@ _Note that it is not required to verify your validator's withdrawal credentials_
* `stateRootProof` MUST verify a `beaconStateRoot` against the `beaconBlockRoot` returned from the EIP-4788 oracle
* For each validator:
* The validator MUST NOT have been previously-verified (`VALIDATOR_STATUS` should be `INACTIVE`)
* The validator's `activation_epoch` MUST NOT equal `type(uint64).max` (aka `FAR_FUTURE_EPOCH`)
* The validator's `exit_epoch` MUST equal `type(uint64).max` (aka `FAR_FUTURE_EPOCH`)
* The validator's `withdrawal_credentials` MUST be pointed to the `EigenPod`
* `validatorFieldsProof` MUST be a valid merkle proof of the corresponding `validatorFields` under the `beaconStateRoot` at the given `validatorIndex`
Expand Down
2 changes: 1 addition & 1 deletion pkg/bindings/AVSDirectory/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/BeaconChainProofs/binding.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/bindings/DelegationManager/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/EigenPod/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/EigenPodManager/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/EigenStrategy/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/RewardsCoordinator/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/StrategyBase/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/StrategyBaseTVLLimits/binding.go

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/bindings/StrategyManager/binding.go

Large diffs are not rendered by default.

9 changes: 8 additions & 1 deletion src/contracts/libraries/BeaconChainProofs.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ library BeaconChainProofs {
uint256 internal constant VALIDATOR_WITHDRAWAL_CREDENTIALS_INDEX = 1;
uint256 internal constant VALIDATOR_BALANCE_INDEX = 2;
uint256 internal constant VALIDATOR_SLASHED_INDEX = 3;
uint256 internal constant VALIDATOR_ACTIVATION_EPOCH_INDEX = 5;
uint256 internal constant VALIDATOR_EXIT_EPOCH_INDEX = 6;

/// @notice Slot/Epoch timings
Expand Down Expand Up @@ -269,7 +270,7 @@ library BeaconChainProofs {
/// 1: withdrawal credentials
/// 2: effective balance
/// 3: slashed?
/// 4: activation elligibility epoch
/// 4: activation eligibility epoch
/// 5: activation epoch
/// 6: exit epoch
/// 7: withdrawable epoch
Expand All @@ -294,6 +295,12 @@ library BeaconChainProofs {
Endian.fromLittleEndianUint64(validatorFields[VALIDATOR_BALANCE_INDEX]);
}

/// @dev Retrieves a validator's activation epoch
function getActivationEpoch(bytes32[] memory validatorFields) internal pure returns (uint64) {
return
Endian.fromLittleEndianUint64(validatorFields[VALIDATOR_ACTIVATION_EPOCH_INDEX]);
}

/// @dev Retrieves true IFF a validator is marked slashed
function isValidatorSlashed(bytes32[] memory validatorFields) internal pure returns (bool) {
return validatorFields[VALIDATOR_SLASHED_INDEX] != 0;
Expand Down
18 changes: 18 additions & 0 deletions src/contracts/pods/EigenPod.sol
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,24 @@ contract EigenPod is
"EigenPod._verifyWithdrawalCredentials: validator must be inactive to prove withdrawal credentials"
);

// Validator should be active on the beacon chain, or in the process of activating.
// This implies the validator has reached the minimum effective balance required
// to become active on the beacon chain.
//
// This check is important because the Pectra upgrade will move any validators that
// do NOT have an activation epoch to a "pending deposit queue," temporarily resetting
// their current and effective balances to 0. This balance can be restored if a deposit
// is made to bring the validator's balance above the minimum activation balance.
// (See https://github.com/ethereum/consensus-specs/blob/dev/specs/electra/fork.md#upgrading-the-state)
//
// In the context of EigenLayer slashing, this temporary reset would allow pod shares
// to temporarily decrease, then be restored later. This would effectively prevent these
// shares from being slashable on EigenLayer for a short period of time.
require(
validatorFields.getActivationEpoch() != BeaconChainProofs.FAR_FUTURE_EPOCH,
"EigenPod._verifyWithdrawalCredentials: validator must be in the process of activating"
);

// Validator should not already be in the process of exiting. This is an important property
// this method needs to enforce to ensure a validator cannot be already-exited by the time
// its withdrawal credentials are verified.
Expand Down
6 changes: 5 additions & 1 deletion src/test/integration/mocks/BeaconChainMock.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ contract BeaconChainMock is PrintUtils {
bytes32 pubkeyHash;
bytes withdrawalCreds;
uint64 effectiveBalanceGwei;
uint64 activationEpoch;
uint64 exitEpoch;
}

Expand Down Expand Up @@ -458,6 +459,7 @@ contract BeaconChainMock is PrintUtils {
pubkeyHash: sha256(abi.encodePacked(_dummyPubkey, bytes16(0))),
withdrawalCreds: "",
effectiveBalanceGwei: dummyBalanceGwei,
activationEpoch: BeaconChainProofs.FAR_FUTURE_EPOCH,
exitEpoch: BeaconChainProofs.FAR_FUTURE_EPOCH
}));
_setCurrentBalance(validatorIndex, dummyBalanceGwei);
Expand All @@ -474,6 +476,7 @@ contract BeaconChainMock is PrintUtils {
pubkeyHash: sha256(abi.encodePacked(_pubkey, bytes16(0))),
withdrawalCreds: withdrawalCreds,
effectiveBalanceGwei: balanceGwei,
activationEpoch: currentEpoch(),
exitEpoch: BeaconChainProofs.FAR_FUTURE_EPOCH
}));
_setCurrentBalance(validatorIndex, balanceGwei);
Expand Down Expand Up @@ -817,8 +820,9 @@ contract BeaconChainMock is PrintUtils {
vFields[BeaconChainProofs.VALIDATOR_PUBKEY_INDEX] = v.pubkeyHash;
vFields[BeaconChainProofs.VALIDATOR_WITHDRAWAL_CREDENTIALS_INDEX] = bytes32(v.withdrawalCreds);
vFields[BeaconChainProofs.VALIDATOR_BALANCE_INDEX] = _toLittleEndianUint64(v.effectiveBalanceGwei);
vFields[BeaconChainProofs.VALIDATOR_EXIT_EPOCH_INDEX] = _toLittleEndianUint64(v.exitEpoch);
vFields[BeaconChainProofs.VALIDATOR_SLASHED_INDEX] = bytes32(abi.encode(v.isSlashed));
vFields[BeaconChainProofs.VALIDATOR_ACTIVATION_EPOCH_INDEX] = _toLittleEndianUint64(v.activationEpoch);
vFields[BeaconChainProofs.VALIDATOR_EXIT_EPOCH_INDEX] = _toLittleEndianUint64(v.exitEpoch);

return vFields;
}
Expand Down
40 changes: 40 additions & 0 deletions src/test/unit/EigenPodUnit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,24 @@ contract EigenPodUnitTests is EigenLayerUnitTestSetup, EigenPodPausingConstants,
return (staker, amount);
}

/// @dev Opposite of Endian.fromLittleEndianUint64
function _toLittleEndianUint64(uint64 num) internal pure returns (bytes32) {
uint256 lenum;

// Rearrange the bytes from big-endian to little-endian format
lenum |= uint256((num & 0xFF) << 56);
lenum |= uint256((num & 0xFF00) << 40);
lenum |= uint256((num & 0xFF0000) << 24);
lenum |= uint256((num & 0xFF000000) << 8);
lenum |= uint256((num & 0xFF00000000) >> 8);
lenum |= uint256((num & 0xFF0000000000) >> 24);
lenum |= uint256((num & 0xFF000000000000) >> 40);
lenum |= uint256((num & 0xFF00000000000000) >> 56);

// Shift the little-endian bytes to the end of the bytes32 value
return bytes32(lenum << 192);
}

/*******************************************************************************
verifyWithdrawalCredentials Assertions
*******************************************************************************/
Expand Down Expand Up @@ -840,6 +858,28 @@ contract EigenPodUnitTests_verifyWithdrawalCredentials is EigenPodUnitTests, Pro
cheats.stopPrank();
}

/// @notice modify validator activation epoch to cause a revert
function testFuzz_revert_activationEpochNotSet(uint256 rand) public {
(EigenPodUser staker,) = _newEigenPodStaker({ rand: rand });
(uint40[] memory validators, ) = staker.startValidators();
EigenPod pod = staker.pod();
CredentialProofs memory proofs = beaconChain.getCredentialProofs(validators);

proofs.validatorFields[0][BeaconChainProofs.VALIDATOR_ACTIVATION_EPOCH_INDEX]
= _toLittleEndianUint64(BeaconChainProofs.FAR_FUTURE_EPOCH);

cheats.startPrank(address(staker));
cheats.expectRevert("EigenPod._verifyWithdrawalCredentials: validator must be in the process of activating");
pod.verifyWithdrawalCredentials({
beaconTimestamp: proofs.beaconTimestamp,
stateRootProof: proofs.stateRootProof,
validatorIndices: validators,
validatorFieldsProofs: proofs.validatorFieldsProofs,
validatorFields: proofs.validatorFields
});
cheats.stopPrank();
}

/// @notice modify validator proof length to cause a revert
function testFuzz_revert_invalidValidatorProofLength(uint256 rand) public {
(EigenPodUser staker,) = _newEigenPodStaker({ rand: rand });
Expand Down
Loading