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

Add tests for EIP 7251. #6726

Open
wants to merge 1 commit into
base: unstable
Choose a base branch
from
Open

Conversation

bshastry
Copy link

Issue Addressed

This PR addresses the testing coverage for EIP-7251's partial withdrawal functionality in Lighthouse, specifically the is_partially_withdrawable_validator_electra implementation. No existing issue number, but this adds test coverage for the Electra fork implementation.

Proposed Changes

  1. Adds test test_partial_withdrawals_electra in consensus/types/src/validator.rs that covers four scenarios:

    • Compounding validator (0x02 prefix) with 2048 ETH effective balance and excess balance
    • Non-compounding validator (0x01 prefix) with 2048 ETH effective balance
    • Compounding validator with 32 ETH effective balance
    • Non-compounding validator with 32 ETH effective balance
  2. Test verifies that automatic partial withdrawals are only allowed when:

    • Validator has the correct effective balance for both Eth1 and compounding modes
    • Validator has excess balance above effective balance

Additional Info

  1. Test Coverage: This test specifically covers the automatic sweep mechanism for excess balances, which is distinct from user-initiated partial withdrawals.

  2. Future Considerations:

    • May want to add additional test cases for edge cases around the effective balance values
    • Could extend to test interaction with different fork transitions
    • Consider adding integration tests with actual withdrawal processing
  3. Related Specs:

  4. Review Notes:

    • Test uses both 0x01 and 0x02 withdrawal credential prefixes
    • Balance values are chosen to test excess balance scenarios
    • All assertions include descriptive messages for failure reporting

@chong-he chong-he added the electra Required for the Electra/Prague fork label Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electra Required for the Electra/Prague fork
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants