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

feat: pectra compatibility #1053

Open
wants to merge 11 commits into
base: feat/prooftra
Choose a base branch
from
4 changes: 4 additions & 0 deletions src/contracts/interfaces/IEigenPod.sol
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ interface IEigenPod is IEigenPodErrors, IEigenPodEvents {
) external;

/// @notice Called by EigenPodManager when the owner wants to create another ETH validator.
/// @dev This function only supports staking to a 0x01 validator. For compounding validators, please interact directly with the deposit contract.
function stake(bytes calldata pubkey, bytes calldata signature, bytes32 depositDataRoot) external payable;

/**
Expand Down Expand Up @@ -351,4 +352,7 @@ interface IEigenPod is IEigenPodErrors, IEigenPodEvents {
function getParentBlockRoot(
uint64 timestamp
) external view returns (bytes32);

/// @notice Returns the timestamp of the Pectra fork, read from the `EigenPodManager` contract
function getPectraForkTimestamp() external view returns (uint64);
Comment on lines +355 to +357
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this added to the EigenPod interface?

It already exists on the EPM - why duplicate?

}
22 changes: 22 additions & 0 deletions src/contracts/interfaces/IEigenPodManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ interface IEigenPodManagerErrors {
/// @dev Thrown when the pods shares are negative and a beacon chain balance update is attempted.
/// The podOwner should complete legacy withdrawal first.
error LegacyWithdrawalsNotCompleted();
/// @dev Thrown when caller is not the proof timestamp setter
error OnlyProofTimestampSetter();
}

interface IEigenPodManagerEvents {
Expand Down Expand Up @@ -57,6 +59,12 @@ interface IEigenPodManagerEvents {

/// @notice Emitted when an operator is slashed and shares to be burned are increased
event BurnableETHSharesIncreased(uint256 shares);

/// @notice Emitted when the Pectra fork timestamp is updated
event PectraForkTimestampSet(uint64 newPectraForkTimestamp);

/// @notice Emitted when the proof timestamp setter is updated
event ProofTimestampSetterSet(address newProofTimestampSetter);
}

interface IEigenPodManagerTypes {
Expand Down Expand Up @@ -120,6 +128,16 @@ interface IEigenPodManager is
int256 balanceDeltaWei
) external;

/// @notice Sets the address that can set proof timestamps
function setProofTimestampSetter(
address newProofTimestampSetter
) external;
wadealexc marked this conversation as resolved.
Show resolved Hide resolved

/// @notice Sets the Pectra fork timestamp, only callable by `proofTimestampSetter`
function setPectraForkTimestamp(
uint64 timestamp
) external;

/// @notice Returns the address of the `podOwner`'s EigenPod if it has been deployed.
function ownerToPod(
address podOwner
Expand Down Expand Up @@ -169,4 +187,8 @@ interface IEigenPodManager is

/// @notice Returns the accumulated amount of beacon chain ETH Strategy shares
function burnableETHShares() external view returns (uint256);

/// @notice Returns the timestamp of the Pectra hard fork
/// @dev Specifically, this returns the timestamp of the first non-missed slot at or after the Pectra hard fork
function pectraForkTimestamp() external view returns (uint64);
}
32 changes: 26 additions & 6 deletions src/contracts/libraries/BeaconChainProofs.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ library BeaconChainProofs {
/// | HEIGHT: VALIDATOR_TREE_HEIGHT
/// individual validators
uint256 internal constant BEACON_BLOCK_HEADER_TREE_HEIGHT = 3;
uint256 internal constant BEACON_STATE_TREE_HEIGHT = 5;
uint256 internal constant DENEB_BEACON_STATE_TREE_HEIGHT = 5;
uint256 internal constant PECTRA_BEACON_STATE_TREE_HEIGHT = 6;
uint256 internal constant BALANCE_TREE_HEIGHT = 38;
uint256 internal constant VALIDATOR_TREE_HEIGHT = 40;

Expand Down Expand Up @@ -134,17 +135,21 @@ library BeaconChainProofs {
/// @param validatorFieldsProof a merkle proof of inclusion of `validatorFields` under `beaconStateRoot`
/// @param validatorIndex the validator's unique index
function verifyValidatorFields(
uint64 proofTimestamp,
uint64 pectraForkTimestamp,
bytes32 beaconStateRoot,
bytes32[] calldata validatorFields,
bytes calldata validatorFieldsProof,
uint40 validatorIndex
) internal view {
require(validatorFields.length == VALIDATOR_FIELDS_LENGTH, InvalidValidatorFieldsLength());

uint256 beaconStateTreeHeight = getBeaconStateTreeHeight(proofTimestamp, pectraForkTimestamp);

/// Note: the reason we use `VALIDATOR_TREE_HEIGHT + 1` here is because the merklization process for
/// this container includes hashing the root of the validator tree with the length of the validator list
require(
validatorFieldsProof.length == 32 * ((VALIDATOR_TREE_HEIGHT + 1) + BEACON_STATE_TREE_HEIGHT),
validatorFieldsProof.length == 32 * ((VALIDATOR_TREE_HEIGHT + 1) + beaconStateTreeHeight),
InvalidProofLength()
);

Expand Down Expand Up @@ -185,10 +190,16 @@ library BeaconChainProofs {
/// against the same balance container root.
/// @param beaconBlockRoot merkle root of the beacon block
/// @param proof a beacon balance container root and merkle proof of its inclusion under `beaconBlockRoot`
function verifyBalanceContainer(bytes32 beaconBlockRoot, BalanceContainerProof calldata proof) internal view {
function verifyBalanceContainer(
uint64 proofTimestamp,
uint64 pectraForkTimestamp,
bytes32 beaconBlockRoot,
BalanceContainerProof calldata proof
) internal view {
uint256 beaconStateTreeHeight = getBeaconStateTreeHeight(proofTimestamp, pectraForkTimestamp);

require(
proof.proof.length == 32 * (BEACON_BLOCK_HEADER_TREE_HEIGHT + BEACON_STATE_TREE_HEIGHT),
InvalidProofLength()
proof.proof.length == 32 * (BEACON_BLOCK_HEADER_TREE_HEIGHT + beaconStateTreeHeight), InvalidProofLength()
);

/// This proof combines two proofs, so its index accounts for the relative position of leaves in two trees:
Expand All @@ -197,7 +208,7 @@ library BeaconChainProofs {
/// -- beaconStateRoot
/// | HEIGHT: BEACON_STATE_TREE_HEIGHT
/// ---- balancesContainerRoot
uint256 index = (STATE_ROOT_INDEX << (BEACON_STATE_TREE_HEIGHT)) | BALANCE_CONTAINER_INDEX;
uint256 index = (STATE_ROOT_INDEX << (beaconStateTreeHeight)) | BALANCE_CONTAINER_INDEX;

require(
Merkle.verifyInclusionSha256({
Expand Down Expand Up @@ -312,4 +323,13 @@ library BeaconChainProofs {
) internal pure returns (uint64) {
return Endian.fromLittleEndianUint64(validatorFields[VALIDATOR_EXIT_EPOCH_INDEX]);
}

/// @dev We check if the proofTimestamp is <= pectraForkTimestamp because a `proofTimestamp` at the `pectraForkTimestamp`
/// is considered to be Pre-Pectra given the EIP-4788 oracle returns the parent block.
function getBeaconStateTreeHeight(
uint64 proofTimestamp,
uint64 pectraForkTimestamp
) internal pure returns (uint256) {
return proofTimestamp <= pectraForkTimestamp ? DENEB_BEACON_STATE_TREE_HEIGHT : PECTRA_BEACON_STATE_TREE_HEIGHT;
}
Comment on lines +327 to +334
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just making some notes:

Before we set pectraForkTimestamp in the EPM, it returns 0. This will cause all proofs to be interpreted as pectra proofs. However, we expect contracts to be paused if this value is unset, so that's fine 👍

Additionally, if the contracts are unpaused but the value is still unset, this logic means all proofs are interpreted as pectra proofs, even if we're working with a dencun block. This is good fallback behavior, as the worst case means some proofs will revert until we set the timestamp. 👍

It's the reverse behavior we wanted to avoid - where dencun proof sizes are allowed for pectra blocks. That shouldn't be possible here 👍

}
27 changes: 23 additions & 4 deletions src/contracts/pods/EigenPod.sol
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ contract EigenPod is Initializable, ReentrancyGuardUpgradeable, EigenPodPausingC

// Verify `balanceContainerProof` against `beaconBlockRoot`
BeaconChainProofs.verifyBalanceContainer({
proofTimestamp: checkpointTimestamp,
pectraForkTimestamp: getPectraForkTimestamp(),
beaconBlockRoot: checkpoint.beaconBlockRoot,
proof: balanceContainerProof
});
Expand Down Expand Up @@ -254,6 +256,7 @@ contract EigenPod is Initializable, ReentrancyGuardUpgradeable, EigenPodPausingC
for (uint256 i = 0; i < validatorIndices.length; i++) {
// forgefmt: disable-next-item
totalAmountToBeRestakedWei += _verifyWithdrawalCredentials(
beaconTimestamp,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, I wonder about the usage of getParentBlockRoot above.

Can beaconTimestamp belong to the block after the fork, but the proof itself is over the parent block (aka pre-fork)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

For the record I think I'm correct here. 2 scenarios:

  1. I try to verify withdrawal credentials the block after the hard fork. getParentBlockRoot grabs a block from before the hard fork; that's what my proof needs to be against.
  2. I start a checkpoint just after the hard fork. The checkpoint block root used for proofs is from a block just before the hard fork.

Naive solution: have the fork selector logic in the proofs library check proofTimestamp - 12. However, this runs into potential issues with skipped slots -- I might have a proof timestamp 2 blocks after the hard fork, but there was a skipped slot right at the fork, so the proof block is from before the fork.

I think, unfortunately, we need to pause proofs just before the fork, and unpause just after the fork (once we see the first valid block). Although I'm not sure this entirely fixes the issue... will think more.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we need to pause + upgrade just after the fork + unpause? ugh.

Copy link
Collaborator Author

@ypatil12 ypatil12 Feb 6, 2025

Choose a reason for hiding this comment

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

Synced offline, here is what we need to do:

  • Prevent deneb proofs from being submitted to pectra blocks
  • Ensure that the PECTRA_FORK_TIMESTAMP is the first timestamp at or after the pectra hard fork for which there is a non-missed slot. Checkpoint proofs store the proof timestamp. If there are missed slots at the hard fork timestamp, it's possible, like Alex mentions above, that the beaconTimestamp is post pectra but the block header is deneb.

To do this, here is the process:

  1. Pause checkpoint starting & credential proofs
  2. Upgrade after fork is hit
  3. Set pectra fork timestamp to the first timestamp at which there is a pectra block header
  4. Unpause

Copy link
Collaborator

Choose a reason for hiding this comment

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

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

Using the image above, we want to determine what proof type to use, given the proof timestamp tp. Because tp is used to look up a parent block in the EIP-4788 oracle, its proof type corresponds to the last non-skipped block. So, if:

  • tp > t1, use pectra logic
  • tp <= t1, use dencun logic

Given our beacon state tree height getter: https://github.com/layr-labs/eigenlayer-contracts/blob/b4852c74cdbe43fea2f7330ea0dc752fcf10b6e9/src/contracts/libraries/BeaconChainProofs.sol#L327-L334

... pectraForkTimestamp should be set to the first valid, non-skipped block after the Pectra hard fork.

stateRootProof.beaconStateRoot,
validatorIndices[i],
validatorFieldsProofs[i],
Expand Down Expand Up @@ -341,6 +344,8 @@ contract EigenPod is Initializable, ReentrancyGuardUpgradeable, EigenPodPausingC

// Verify Validator container proof against `beaconStateRoot`
BeaconChainProofs.verifyValidatorFields({
proofTimestamp: beaconTimestamp,
pectraForkTimestamp: getPectraForkTimestamp(),
beaconStateRoot: stateRootProof.beaconStateRoot,
validatorFields: proof.validatorFields,
validatorFieldsProof: proof.proof,
Expand Down Expand Up @@ -378,6 +383,7 @@ contract EigenPod is Initializable, ReentrancyGuardUpgradeable, EigenPodPausingC
}

/// @notice Called by EigenPodManager when the owner wants to create another ETH validator.
/// @dev This function only supports staking to a 0x01 validator. For compounding validators, please interact directly with the deposit contract.
function stake(
bytes calldata pubkey,
bytes calldata signature,
Expand Down Expand Up @@ -419,13 +425,13 @@ contract EigenPod is Initializable, ReentrancyGuardUpgradeable, EigenPodPausingC
* @param validatorFields are the fields of the "Validator Container", refer to consensus specs
*/
function _verifyWithdrawalCredentials(
uint64 beaconTimestamp,
bytes32 beaconStateRoot,
uint40 validatorIndex,
bytes calldata validatorFieldsProof,
bytes32[] calldata validatorFields
) internal returns (uint256) {
bytes32 pubkeyHash = validatorFields.getPubkeyHash();
ValidatorInfo memory validatorInfo = _validatorPubkeyHashToInfo[pubkeyHash];
ValidatorInfo memory validatorInfo = _validatorPubkeyHashToInfo[validatorFields.getPubkeyHash()];

// Withdrawal credential proofs should only be processed for "INACTIVE" validators
require(validatorInfo.status == VALIDATOR_STATUS.INACTIVE, CredentialsAlreadyVerified());
Expand Down Expand Up @@ -473,7 +479,8 @@ contract EigenPod is Initializable, ReentrancyGuardUpgradeable, EigenPodPausingC

// Ensure the validator's withdrawal credentials are pointed at this pod
require(
validatorFields.getWithdrawalCredentials() == bytes32(_podWithdrawalCredentials()),
validatorFields.getWithdrawalCredentials() == bytes32(_podWithdrawalCredentials())
|| validatorFields.getWithdrawalCredentials() == bytes32(_podCompoundingWithdrawalCredentials()),
WithdrawalCredentialsNotForEigenPod()
);

Expand All @@ -484,6 +491,8 @@ contract EigenPod is Initializable, ReentrancyGuardUpgradeable, EigenPodPausingC

// Verify passed-in validatorFields against verified beaconStateRoot:
BeaconChainProofs.verifyValidatorFields({
proofTimestamp: beaconTimestamp,
pectraForkTimestamp: getPectraForkTimestamp(),
beaconStateRoot: beaconStateRoot,
validatorFields: validatorFields,
validatorFieldsProof: validatorFieldsProof,
Expand All @@ -499,7 +508,7 @@ contract EigenPod is Initializable, ReentrancyGuardUpgradeable, EigenPodPausingC
currentCheckpointTimestamp == 0 ? lastCheckpointTimestamp : currentCheckpointTimestamp;

// Proofs complete - create the validator in state
_validatorPubkeyHashToInfo[pubkeyHash] = ValidatorInfo({
_validatorPubkeyHashToInfo[validatorFields.getPubkeyHash()] = ValidatorInfo({
validatorIndex: validatorIndex,
restakedBalanceGwei: restakedBalanceGwei,
lastCheckpointedAt: lastCheckpointedAt,
Expand Down Expand Up @@ -665,6 +674,10 @@ contract EigenPod is Initializable, ReentrancyGuardUpgradeable, EigenPodPausingC
return abi.encodePacked(bytes1(uint8(1)), bytes11(0), address(this));
}

function _podCompoundingWithdrawalCredentials() internal view returns (bytes memory) {
return abi.encodePacked(bytes1(uint8(2)), bytes11(0), address(this));
}

///@notice Calculates the pubkey hash of a validator's pubkey as per SSZ spec
function _calculateValidatorPubkeyHash(
bytes memory validatorPubkey
Expand Down Expand Up @@ -731,4 +744,10 @@ contract EigenPod is Initializable, ReentrancyGuardUpgradeable, EigenPodPausingC
require(success && result.length > 0, InvalidEIP4788Response());
return abi.decode(result, (bytes32));
}

/// @notice Returns the timestamp of the Pectra fork, read from the `EigenPodManager` contract
/// @dev Specifically, this returns the timestamp of the first non-missed slot at or after the Pectra hard fork
function getPectraForkTimestamp() public view returns (uint64) {
return eigenPodManager.pectraForkTimestamp();
}
}
21 changes: 21 additions & 0 deletions src/contracts/pods/EigenPodManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ contract EigenPodManager is
_;
}

modifier onlyProofTimestampSetter() {
require(msg.sender == proofTimestampSetter, OnlyProofTimestampSetter());
_;
}

constructor(
IETHPOSDeposit _ethPOS,
IBeacon _eigenPodBeacon,
Expand Down Expand Up @@ -223,6 +228,22 @@ contract EigenPodManager is
emit BurnableETHSharesIncreased(addedSharesToBurn);
}

/// @notice Sets the address that can set proof timestamps
function setProofTimestampSetter(
address newProofTimestampSetter
) external onlyOwner {
proofTimestampSetter = newProofTimestampSetter;
emit ProofTimestampSetterSet(newProofTimestampSetter);
}

/// @notice Sets the pectra fork timestamp
function setPectraForkTimestamp(
uint64 timestamp
) external onlyProofTimestampSetter {
pectraForkTimestamp = timestamp;
emit PectraForkTimestampSet(timestamp);
}

// INTERNAL FUNCTIONS

function _deployPod() internal returns (IEigenPod) {
Expand Down
8 changes: 7 additions & 1 deletion src/contracts/pods/EigenPodManagerStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ abstract contract EigenPodManagerStorage is IEigenPodManager {
/// @notice Returns the amount of `shares` that have been slashed on EigenLayer but not burned yet.
uint256 public burnableETHShares;

/// @notice The address that can set proof timestamps
address public proofTimestampSetter;

/// @notice The timestamp of the Pectra proof
uint64 public pectraForkTimestamp;

constructor(IETHPOSDeposit _ethPOS, IBeacon _eigenPodBeacon, IDelegationManager _delegationManager) {
ethPOS = _ethPOS;
eigenPodBeacon = _eigenPodBeacon;
Expand All @@ -102,5 +108,5 @@ abstract contract EigenPodManagerStorage is IEigenPodManager {
* variables without shifting down storage in the inheritance chain.
* See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
*/
uint256[42] private __gap;
uint256[41] private __gap;
}
2 changes: 2 additions & 0 deletions src/test/harnesses/EigenPodHarness.sol
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,14 @@ contract EigenPodHarness is EigenPod {
}

function verifyWithdrawalCredentials(
uint64 beaconTimestamp,
bytes32 beaconStateRoot,
uint40 validatorIndex,
bytes calldata validatorFieldsProof,
bytes32[] calldata validatorFields
) public returns (uint256) {
return _verifyWithdrawalCredentials(
beaconTimestamp,
beaconStateRoot,
validatorIndex,
validatorFieldsProof,
Expand Down
Loading
Loading