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 a new consolidation test & two new pending deposit tests #4109

Merged
merged 5 commits into from
Jan 30, 2025

Conversation

jtraglia
Copy link
Member

@jtraglia jtraglia commented Jan 29, 2025

This PR will:

  • Fix a few small nits (variable names, function names, spacing, punctuation).
  • Add test_basic_consolidation_source_has_less_than_max_effective_balance -- a test where the source validator (with 0x01 creds) has an effective balance with less than 32 ETH. This shouldn't prevent the consolidation.
  • Add test_apply_pending_deposit_over_min_activation_next_increment -- a test where the deposit amount is 33 ETH. There's an existing test with a deposit amount of 32 ETH + 1 Gwei, but that only ensures the client mod's with the effective balance increment. It wouldn't catch a client that forgets to compare with the max effective balance.
  • Add test_apply_pending_deposit_compounding_withdrawal_credentials_over_max_next_increment -- a test like the previous one, but it's a compounding validator and the values are bigger: 2049 ETH.

Copy link
Contributor

@mkalinin mkalinin left a comment

Choose a reason for hiding this comment

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

LGTM!

@jtraglia jtraglia merged commit 099919f into ethereum:dev Jan 30, 2025
23 checks passed
@jtraglia jtraglia deleted the more-tests branch January 30, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants