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.

Sign up for GitHub

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

Jump to bottom

save light client updates #14618

Merged
merged 28 commits into from
Nov 28, 2024
Merged

save light client updates #14618

merged 28 commits into from
Nov 28, 2024
+1,601 −919

Conversation

Inspector-Butters
Copy link
Contributor

What type of PR is this?
Feature

What does this PR do? Why is it needed?
This PR adds the functionality to save LightClientUpdates in PostProcessBlock.

Which issues(s) does this PR fix?
This PR is needed as part of #12991

rkapka and others added 10 commits November 4, 2024 00:16
@rkapka
Light Client - use the new consensus types (prysmaticlabs#14549)
7a0862e 
* in progress

* completed logic

* var name

* additional logic changes

* fix createDefaultLightClientUpdate

* empty fields

* unused context
@rkapka
Return the correct light client payload proof (prysmaticlabs#14565)
4e5f72b 
* Return the correct payload proof

* changelog <3
@rkapka
Set fields of wrapped proto object in light client setters (prysmatic…
46ae78a 
…labs#14573)

* Set fields of wrapped proto object in light client setters

* changelog <3
@rupam-04 @rkapka
fix TODOs for events (prysmaticlabs#14570)
1a3071f 
* fix TODOs for events

* address review comments

* Update beacon-chain/rpc/eth/events/events.go

Co-authored-by: Radosław Kapka <[email protected]>

* Update beacon-chain/rpc/eth/events/events.go

Co-authored-by: Radosław Kapka <[email protected]>

* nits

---------

Co-authored-by: Radosław Kapka <[email protected]>
@rkapka
align changelog with develop branch
b7e92dc
@rkapka
bzl
59c504d
@Inspector-Butters @rkapka @rupam-04
LC Updates by Range to read from DB (prysmaticlabs#14531)
2242393 
* change updatebyrange

* lcupdateresponse from consensus

* range altair test

* range forks tests

* finish tests

* changelog

* remove unused functions

* Update beacon-chain/rpc/eth/light-client/handlers.go

Co-authored-by: Radosław Kapka <[email protected]>

* Update beacon-chain/rpc/eth/light-client/handlers.go

Co-authored-by: Radosław Kapka <[email protected]>

* use slice instead of array

* refactor code

* refactor tests

* refactor tests

* refactor tests

* add configCleanup in tests

* refactor missing updates testcase

* Light Client - use the new consensus types (prysmaticlabs#14549)

* in progress

* completed logic

* var name

* additional logic changes

* fix createDefaultLightClientUpdate

* empty fields

* unused context

* change updatesByRange to use new structs

* Light Client - use the new consensus types (prysmaticlabs#14549)

* in progress

* completed logic

* var name

* additional logic changes

* fix createDefaultLightClientUpdate

* empty fields

* unused context

* fix rpc/helpers_test

* Return the correct light client payload proof (prysmaticlabs#14565)

* Return the correct payload proof

* changelog <3

* merge

* Set fields of wrapped proto object in light client setters (prysmaticlabs#14573)

* Set fields of wrapped proto object in light client setters

* changelog <3

* fixing tests...

* core tests fixed

* kv tests fixed

* fix TODOs for events (prysmaticlabs#14570)

* fix TODOs for events

* address review comments

* Update beacon-chain/rpc/eth/events/events.go

Co-authored-by: Radosław Kapka <[email protected]>

* Update beacon-chain/rpc/eth/events/events.go

Co-authored-by: Radosław Kapka <[email protected]>

* nits

---------

Co-authored-by: Radosław Kapka <[email protected]>

* tests fixed

* remove unused function

* fix slice capacity

* address issues

* address issues

* fix circular import error

* remove unused func

* fix changelog

---------

Co-authored-by: Radosław Kapka <[email protected]>
Co-authored-by: Radosław Kapka <[email protected]>
Co-authored-by: Rupam Dey <[email protected]>
@rkapka
Light Client: Electra finality branch (prysmaticlabs#14597)
be92c1e 
* extract from lc-p2p branch

* generate code

* trixy's review

* test fixes
@rupam-04 @rkapka
fix issue in CreateDefaultLightClientUpdate function (prysmaticlabs…
d528dbb 
…#14585)

* use state in `CreateDefaultLightClientUpdate`

* lint

* add `stateSlot` to `update.go` structs

* Revert "add `stateSlot` to `update.go` structs"

This reverts commit 84468ae.

* set sync committee based on attestedHeader in updateElectra

* dependencies

* add check to `SetNextSyncCommitteeBranchElectra`

* add detailed error messages to `update.go`

* dependencies

* fix `createDefaultLightClientUpdate`

* deps

* fix errors

* deps

* revert error messages

* deps
save update
79ec529
@Inspector-Butters Inspector-Butters requested a review from a team as a code owner November 4, 2024 16:36
@Inspector-Butters Inspector-Butters requested review from rkapka, nisdas and james-prysm and removed request for a team November 4, 2024 16:36
Inspector-Butters added 2 commits November 25, 2024 16:07
rkapka
rkapka reviewed Nov 26, 2024
View reviewed changes
beacon-chain/blockchain/process_block_helpers.go Show resolved Hide resolved
beacon-chain/blockchain/process_block_helpers.go Outdated
Comment on lines 182 to 197
if oldUpdate == nil {
if err := s.cfg.BeaconDB.SaveLightClientUpdate(cfg.ctx, period, update); err != nil {
log.WithError(err).Error("Could not save light client update")
}
} else {
isNewUpdateBetter, err := lightclient.IsBetterUpdate(update, oldUpdate)
if err != nil {
log.WithError(err).Error("Could not compare light client updates")
return
}
if isNewUpdateBetter {
if err := s.cfg.BeaconDB.SaveLightClientUpdate(cfg.ctx, period, update); err != nil {
log.WithError(err).Error("Could not save light client update")
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

  • I think there is a way to rearrange this code to deduplicate the call to SaveLightClientUpdate.
  • It would be good to have a debug log when the old update is better. That may help in future debugging of why updates are not being saved properly to the DB.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added the logs. but I could not rearrange the code in a nice way to deduplice the call to SaveLightClientUpdate. feel free to suggest changes.

beacon-chain/blockchain/process_block_helpers.go Outdated Show resolved Hide resolved
beacon-chain/blockchain/process_block_test.go Outdated Show resolved Hide resolved
beacon-chain/blockchain/process_block_helpers.go Outdated Show resolved Hide resolved
beacon-chain/rpc/eth/light-client/helpers_test.go Outdated Show resolved Hide resolved
testing/util/lightclient.go Outdated Show resolved Hide resolved
consensus-types/light-client/update.go Outdated Show resolved Hide resolved
beacon-chain/db/iface/interface.go Outdated Show resolved Hide resolved
consensus-types/interfaces/light_client.go Outdated Show resolved Hide resolved
@rkapka rkapka merged commit ae779a7 into prysmaticlabs:epf-light-client Nov 28, 2024
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rkapka rkapka rkapka left review comments

@nisdas nisdas Awaiting requested review from nisdas nisdas is a code owner automatically assigned from prysmaticlabs/core-team

@james-prysm james-prysm Awaiting requested review from james-prysm james-prysm is a code owner automatically assigned from prysmaticlabs/core-team

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Inspector-Butters @rkapka @rupam-04