Skip to content

Fix electra light client types #6361

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 114 commits into from
Oct 25, 2024
Merged

Conversation

eserilev
Copy link
Member

@eserilev eserilev commented Sep 5, 2024

Issue Addressed

Resolves #6002

Resolves #4022

Proposed Changes

Update light client types to be compatible with electra

Related spec PR:

ethereum/consensus-specs#3811

Blocked by #5915

Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Looks good. I just pushed a small change to reduce new fork burden: 7d67d0d

One small naming nit, and then we can merge

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Sep 23, 2024
@eserilev eserilev added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Sep 23, 2024
Copy link
Collaborator

@dapplion dapplion left a comment

Choose a reason for hiding this comment

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

LGTM

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. ready-for-review The code is ready for review and removed ready-for-review The code is ready for review ready-for-merge This PR is ready to merge. labels Sep 30, 2024
@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Oct 25, 2024
@michaelsproul
Copy link
Member

@mergify queue

Copy link

mergify bot commented Oct 25, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 9d069a9

mergify bot added a commit that referenced this pull request Oct 25, 2024
@mergify mergify bot merged commit 9d069a9 into sigp:unstable Oct 25, 2024
29 checks passed
chong-he pushed a commit to chong-he/lighthouse that referenced this pull request Nov 26, 2024
* persist light client updates

* update beacon chain to serve light client updates

* resolve todos

* cache best update

* extend cache parts

* is better light client update

* resolve merge conflict

* initial api changes

* add lc update db column

* fmt

* added tests

* add sim

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into persist-light-client-updates

* fix some weird issues with the simulator

* tests

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into persist-light-client-updates

* test changes

* merge conflict

* testing

* started work on ef tests and some code clean up

* update tests

* linting

* noop pre altair, were still failing on electra though

* allow for zeroed light client header

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into persist-light-client-updates

* merge unstable

* remove unwraps

* remove unwraps

* fetch bootstrap without always querying for state

* storing bootstrap parts in db

* mroe code cleanup

* test

* prune sync committee branches from dropped chains

* Update light_client_update.rs

* merge unstable

* move functionality to helper methods

* refactor is best update fn

* refactor is best update fn

* improve organization of light client server cache logic

* fork diget calc, and only spawn as many blcoks as we need for the lc update test

* resovle merge conflict

* add electra bootstrap logic, add logic to cache current sync committee

* add latest sync committe branch cache

* fetch lc update from the cache if it exists

* fmt

* Fix beacon_chain tests

* Add debug code to update ranking_order ef test

* Fix compare code

* merge conflicts

* merge conflict

* add better error messaging

* resolve merge conflicts

* remove lc update from basicsim

* rename sync comittte variable and fix persist condition

* refactor get_light_client_update logic

* add better comments, return helpful error messages over http and rpc

* pruning canonical non checkpoint slots

* fix test

* rerun test

* update pruning logic, add tests

* fix tests

* fix imports

* fmt

* refactor db code

* Refactor db method

* Refactor db method

* lc electra changes

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into light-client-electra

* add additional comments

* testing lc merkle changes

* lc electra

* update struct defs

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into light-client-electra

* fix merge

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into persist-light-client-bootstrap

* fix merge

* linting

* merge conflict

* prevent overflow

* enable lc server for http api tests

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into light-client-electra

* get tests working:

* remove related TODOs

* fix test lint

* Merge branch 'persist-light-client-bootstrap' of https://github.com/eserilev/lighthouse into light-client-electra

* fix tests

* fix conflicts

* remove prints

* Merge branch 'persist-light-client-bootstrap' of https://github.com/eserilev/lighthouse into light-client-electra

* remove warning

* resolve conflicts

* merge conflicts

* linting

* remove comments

* cleanup

* linting

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into light-client-electra

* pre/post electra light client cached data

* add proof type alias

* move is_empty_branch method out of impl

* add ssz tests for all forks

* refactor beacon state proof codepaths

* rename method

* fmt

* clean up proof logic

* refactor merkle proof api

* fmt

* Merge branch 'unstable' into light-client-electra

* Use superstruct mapping macros

* Merge branch 'unstable' of https://github.com/sigp/lighthouse into light-client-electra

* rename proof to merkleproof

* fmt

* Resolve merge conflicts

* merge conflicts
mergify bot pushed a commit that referenced this pull request Feb 18, 2025
Fix a regression introduced in this PR:

- #6361

We were indexing into the `MerkleTree` with raw generalized indices, which was incorrect and triggering `debug_assert` failures, as described here:

- #7005


  - Convert `generalized_index` to the correct leaf index prior to proof generation.
- Add sanity checks on indices used in `BeaconState::generate_proof`.
- Remove debug asserts from `MerkleTree::generate_proof` in favour of actual errors. This would have caught the bug earlier.
- Refactor the EF tests so that the merkle validity tests are actually run. They were misconfigured in a way that resulted in them running silently with 0 test cases, and the `check_all_files_accessed.py` script still had an ignore that covered the test files, so this omission wasn't detected.
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 light-client ready-for-merge This PR is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants