Skip to content

feat: implement electra deposits #1424

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

Merged
merged 60 commits into from
Apr 21, 2025
Merged

feat: implement electra deposits #1424

merged 60 commits into from
Apr 21, 2025

Conversation

LeanSerra
Copy link
Contributor

@LeanSerra LeanSerra commented Apr 9, 2025

Motivation

Implement changes in Electra to deposits + fix all spec-tests related to it.

Description

  • New is_valid_deposit_signature link
  • Modified get_validator_from_deposit link
  • Modified add_validator_to_registry link
    • In our code the function is called Mutators.apply_initial_deposit
    • The code from Deneb to Electra does not actually change, the link in the spec just adds a comment referencing the changes made to the function get_validator_from_deposit
  • Modified apply_deposit link
  • New apply_pending_deposit link
  • New process_pending_deposits link
  • Modified process_operations link
  • Modified process_deposit link
    • The code from Deneb to Electra does not actually change, the link in the spec just adds a comment referencing the changes made to the function apply_deposit
  • New process_deposit_request link

Fixed Spec Tests

test/generated/mainnet/electra/epoch_processing.exs:
previous: 141 tests, 58 failures
now: 141 tests, 13 failures

test/generated/mainnet/electra/operations.exs
previous: 333 tests, 67 failures
now: 333 tests, 23 failures

test/generated/mainnet/electra/random.exs
previous: 16 tests, 16 failures
now: 16 tests, 3 failures

test/generated/mainnet/electra/sanity.exs
previous: 97 tests, 29 failures, 1 skipped
now: 97 tests, 10 failures, 1 skipped

test/generated/minimal/electra/epoch_processing.exs
previous: 156 tests, 59 failures
now: 156 tests, 13 failures

test/generated/minimal/electra/operations.exs
previous: 366 tests, 104 failures
now: 366 tests, 45 failures

test/generated/minimal/electra/random.exs
previous: 16 tests, 16 failures
now: 16 tests, 2 failures

test/generated/minimal/electra/sanity.exs
previous: 104 tests, 31 failures, 1 skipped
now: 104 tests, 12 failures, 1 skipped

tests fixed: 259

Spec test progress

11370 tests, 165 failures, 784 skipped

LeanSerra added 30 commits April 3, 2025 11:21
Base automatically changed from electra_validator_exit to electra-support April 11, 2025 17:50
@LeanSerra LeanSerra marked this pull request as ready for review April 11, 2025 18:37
@LeanSerra LeanSerra requested a review from a team as a code owner April 11, 2025 18:37
Copy link
Collaborator

@rodrigo-o rodrigo-o left a comment

Choose a reason for hiding this comment

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

The PR looks good so far, but it's taking me some time to review and try to make some of the functions more clear, so I will leave a batch of comments here and continue with the rest

Copy link
Collaborator

@rodrigo-o rodrigo-o left a comment

Choose a reason for hiding this comment

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

LGTM apart from the readability comments. Regarding process_pending_deposits we should probably take some time in the future to refactor it and make it more readable but it requires more effort than the suggested alternatives for the other functions.

BTW we went from 424 (in my latest run in electra-support) to 165 failed tests here, so probably a small summary (num of test x suite) that now pass would be useful in the description of the PR.

@LeanSerra LeanSerra merged commit c4bec8f into electra-support Apr 21, 2025
12 of 16 checks passed
@LeanSerra LeanSerra deleted the electra_deposits branch April 21, 2025 12:06
LeanSerra added a commit that referenced this pull request Apr 21, 2025
rodrigo-o pushed a commit that referenced this pull request Apr 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electra elixir Pull requests that update Elixir code
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants