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

Feature: Include unique id with withdrawal verification -- urgently needed for projects that process many withdrawals #471

Open
3 tasks
youfoundron opened this issue Mar 9, 2024 · 3 comments

Comments

@youfoundron
Copy link

User story
As an EigenPod contract owner I want the events PartialWithdrawalRedeemed & FullWithdrawalRedeemed to include a uint256 withdrawalIndex (this is a unique id that marks the position of a withdrawal within beacon chain history), so that I may parse through event logs and know which withdrawals have already been redeemed for a pod.

The WithdrawalIndex has been in-spec since Capella

I propose modifying the event interfaces on IEigenPod.sol like so:

    /// @notice Emitted when an ETH validator is prove to have withdrawn from the beacon chain
    event FullWithdrawalRedeemed(
        uint40 validatorIndex,
        uint64 withdrawalTimestamp,
        address indexed recipient,
        uint64 withdrawalAmountGwei,
        uint256 indexed withdrawalIndex // new log
    );

    /// @notice Emitted when a partial withdrawal claim is successfully redeemed
    event PartialWithdrawalRedeemed(
        uint40 validatorIndex,
        uint64 withdrawalTimestamp,
        address indexed recipient,
        uint64 partialWithdrawalAmountGwei,
        uint256 indexed withdrawalIndex // new log
    );

Actions

  • Modify IEigenPod.sol event interfaces PartialWithdrawalRedeemed & FullWithdrawalRedeemed to include withdrawalIndex
  • Update BeaconChainProofs.sol with the capability to parse the withdrawalIndex value from submitted withdrawal proofs
  • Update EigenPod.sol to pass the withdrawalIndex when emitting the relevant events
@youfoundron youfoundron changed the title Feature: Include unique id with withdrawal verification -- urgently needed for projects that handle lots of withdrawal processing Feature: Include unique id with withdrawal verification -- urgently needed for projects that process many withdrawals Mar 9, 2024
@youfoundron
Copy link
Author

youfoundron commented Mar 11, 2024

Without this, withdrawal tracking / redemption is a brittle process for everyone.

FWIW, It seems this information should be accessible from within the submitted proof.

Please see:

uint64 withdrawalIndex;
bytes32 blockRoot;
bytes32 slotRoot;

Granted, there is a bit of a naming collision here as what EL calls the withdrawalIndex is actually referring to a withdrawal's index within a given slot (0 - 15).

However, if the event emitted this value along with the slotRoot it would be enough information to uniquely identify a withdrawal and solve the broader problem of not being able to uniquely identify withdrawal redemptions from on-chain data.

Here is what I mean:

    event XWithdrawalRedeemed(
        uint40 validatorIndex,
        uint64 withdrawalTimestamp,
        address indexed recipient,
        uint64 withdrawalAmountGwei,
        bytes32 indexed slotRoot,  // withdrawalProof.slotRoot
        uint64 indexed withdrawalSlotIndex  // withdrawalProof.withdrawalIndex
    );

While this approach is not as clean as the one offered in my original post, it would technically solve the problem without requiring any changes to proof generation logic.

@gpsanant
Copy link
Contributor

Hey there. unfortunately since we're so close to our mainnet launch, we're not comfortable making this change.

Could you explain the impact of the change? From the timestamp (which gives you the slot) and the validator index, you should be able to get the withdrawal index?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants
@youfoundron @gpsanant and others