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

create light-client-updates bucket #14266

Merged
merged 31 commits into from
Aug 22, 2024

Conversation

Inspector-Butters
Copy link
Contributor

What type of PR is this?
Feature

What does this PR do? Why is it needed?
This PR intends to address the needs for a light client updates database per #12991 .
This is needed to support light clients.

Which issues(s) does this PR fix?
Partially addresses #12991

Verified

This commit was signed with the committer’s verified signature.
rochdev Roch Devost
@Inspector-Butters Inspector-Butters requested a review from a team as a code owner July 28, 2024 11:58
@CLAassistant
Copy link

CLAassistant commented Jul 28, 2024

CLA assistant check
All committers have signed the CLA.

@Inspector-Butters Inspector-Butters marked this pull request as draft July 28, 2024 11:59
rkapka
rkapka previously approved these changes Jul 29, 2024
Inspector-Butters and others added 5 commits August 6, 2024 16:26
)

* Electra committe validation for aggregate and proof

* review

* update comments
* Refactor get local payload

* Fix go lint: new line
@Inspector-Butters
Copy link
Contributor Author

@rkapka

  • added a LightClientUpdateWithVersion struct to the protos to save in the db.
  • added SaveLightClientUpdate, LightClientUpdate, and LightClientUpdates.
  • stuck at writing the tests, because I can't import functions defined in blockchain/lightclient.go. (import cycle error). any help is appreciated.
  • added beaconDB to the lightclient server in rpc/eth/lightclient/server.go so we can read the updates in the handlers later on.

@Inspector-Butters Inspector-Butters marked this pull request as ready for review August 13, 2024 17:43
@Inspector-Butters
Copy link
Contributor Author

Inspector-Butters commented Aug 14, 2024

@rkapka do we want to have safety guards in the kv interface functions that check that an update is somewhat valid before saving it? for example, the important fields are not nil.

Inspector-Butters and others added 3 commits August 14, 2024 22:11
Copy link
Contributor

@rkapka rkapka left a comment

Choose a reason for hiding this comment

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

We should have a few test cases for LighClientUpdates:

  • startPeriod > endPeriod - error returned
  • startPeriod == endPeriod - one update returned
  • startPeriod > endPeriod:
    • all updates between startPeriod and endPeriod are in the DB - all returned
    • some periods between startPeriod and endPeriod don't have updates - existing updates returned
    • startPeriod is earlier than the first update - return starting from first existing update
    • endPeriod is later than the first update - return up to the last update

Inspector-Butters and others added 6 commits August 18, 2024 16:24
@Inspector-Butters
Copy link
Contributor Author

Inspector-Butters commented Aug 19, 2024

Added tests to cover all the mentioned cases. with the modification of:
some periods between startPeriod and endPeriod don't have updates --> Error returned.

@Inspector-Butters
Copy link
Contributor Author

Inspector-Butters commented Aug 20, 2024

@rkapka The kv.LightClientUpdates() now returns all the available updates in the requested range in a map.

Inspector-Butters and others added 6 commits August 22, 2024 14:46
This reverts commit 33c707f5bd164386449dc14ff27d95ad5f195161.
@rkapka rkapka enabled auto-merge August 22, 2024 15:23
@rkapka rkapka added this pull request to the merge queue Aug 22, 2024
Merged via the queue into prysmaticlabs:develop with commit 022a53f Aug 22, 2024
17 checks passed
james-prysm pushed a commit that referenced this pull request Sep 6, 2024
* create light-client-updates bucket

* Electra committe validation for aggregate and proof (#14317)

* Electra committe validation for aggregate and proof

* review

* update comments

* Refactor get local payload (#14327)

* Refactor get local payload

* Fix go lint: new line

* add lightclient db kv functions

* lightclient db tests

* move blockchain/lightclient.go to core/light-client package

* add comparison check for start and end period

* create testing/utils/lightcilent.go

* lightclient db tests

* fix imports and usages

* fix imports and usages in process_block_helpers

* fix bazel dependencies

* remove unnecessary nil check

* add more tests for lightclient kv functions

* refactor tests

* refactor kv.LightClientUpdates

* fix db to return every update that is available in the requested range

* run gazzele fix

* return empty map in case of empty db

* fix goimports errors

* goimports

* Revert "Auxiliary commit to revert individual files from aa7ce6f"

This reverts commit 33c707f5bd164386449dc14ff27d95ad5f195161.

---------

Co-authored-by: Radosław Kapka <rkapka@wp.pl>
Co-authored-by: terence <terence@prysmaticlabs.com>
Co-authored-by: rkapka <radoslaw.kapka@gmail.com>
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

Successfully merging this pull request may close these issues.

None yet

4 participants