Light Client - use the new consensus types #14549

rkapka · 2024-10-17T10:53:28Z

What type of PR is this?

Other

What does this PR do? Why is it needed?

The point of this PR is to switch from using protobufs to using light client types defined in consensus-types.

Which issues(s) does this PR fix?

Part of #12991

Acknowledgements

I have read CONTRIBUTING.md.
I have made an appropriate entry to CHANGELOG.md. Not adding an entry because we already have one about adding consensus types for light clients, and this PR is just a continuation of this
I have added a description to this PR with sufficient context for reviewers to understand this PR.

rkapka · 2024-10-17T11:12:52Z

consensus-types/light-client/update.go

@@ -23,6 +23,8 @@ func NewWrappedUpdate(m proto.Message) (interfaces.LightClientUpdate, error) {
 		return NewWrappedUpdateCapella(t)
 	case *pb.LightClientUpdateDeneb:
 		return NewWrappedUpdateDeneb(t)
+	case *pb.LightClientUpdateElectra:


This was missing

rkapka · 2024-10-17T11:16:14Z

beacon-chain/db/kv/lightclient.go

 )

-func (s *Store) SaveLightClientUpdate(ctx context.Context, period uint64, update *ethpbv2.LightClientUpdateWithVersion) error {
-	ctx, span := trace.StartSpan(ctx, "BeaconDB.saveLightClientUpdate")
+func (s *Store) SaveLightClientUpdate(ctx context.Context, period uint64, update interfaces.LightClientUpdate) error {


We can save light client updates similarly how we save blocks - by prepending the version to encoded bytes. That way we don't need the separate XXXWithVersion type

rkapka · 2024-10-17T11:17:16Z

beacon-chain/core/light-client/lightclient.go

@@ -299,46 +329,36 @@ func ComputeWithdrawalsRoot(payload interfaces.ExecutionData) ([]byte, error) {
 	return withdrawalsRoot, nil
 }

-func BlockToLightClientHeader(block interfaces.ReadOnlySignedBeaconBlock) (*ethpbv2.LightClientHeaderContainer, error) {
-	switch block.Version() {
+func BlockToLightClientHeader(ver int, block interfaces.ReadOnlySignedBeaconBlock) (interfaces.LightClientHeader, error) {


As discussed, I added a version parameter, mostly so that we can pass the attested block's version for the finalized block in NewLightClientUpdateFromBeaconState

Could this potentially cause a problem? for example when version is Capella but block is Altair. will calling the block.execution getters on an Altair block result in an error?

rkapka · 2024-10-17T11:19:50Z

beacon-chain/blockchain/process_block_helpers.go

 	if err != nil {
 		return 0, errors.Wrap(err, "could not create light client update")
 	}

 	// Return the result
 	result := &ethpbv2.LightClientFinalityUpdateWithVersion{
 		Version: ethpbv2.Version(signed.Version()),
-		Data:    update,
+		// TODO: Get back to this when revisiting events


I commented out everything event-related. We need to revisit events anyway, so dealing with this now is not that important.

rkapka · 2024-10-17T11:21:51Z

beacon-chain/rpc/eth/light-client/helpers.go

@@ -68,14 +71,13 @@ func createLightClientBootstrapAltair(ctx context.Context, state state.BeaconSta
 		return nil, fmt.Errorf("latest block header root %#x not equal to block root %#x", latestBlockHeaderRoot, beaconBlockRoot)
 	}

-	lightClientHeaderContainer, err := lightclient.BlockToLightClientHeader(block)
+	lightClientHeader, err := lightclient.BlockToLightClientHeader(block.Version(), block)


Is using the block's version fine?

yes it should be.

rupam-04

lgtm

Inspector-Butters · 2024-10-18T15:31:57Z

beacon-chain/core/light-client/lightclient.go

@@ -194,7 +203,7 @@ func NewLightClientUpdateFromBeaconState(
 		// if finalized_block.message.slot != GENESIS_SLOT
 		if finalizedBlock.Block().Slot() != 0 {
 			// update.finalized_header = block_to_light_client_header(finalized_block)
-			finalizedLightClientHeader, err := BlockToLightClientHeader(attestedBlock.Version(), finalizedBlock)
+			finalizedLightClientHeader, err := BlockToLightClientHeader(ctx, currentSlot, finalizedBlock)


I am still curious to see if anything bad happens when you call this function with a Capella slot on an Altair/Bellatrix block.

* in progress * completed logic * var name * additional logic changes * fix createDefaultLightClientUpdate * empty fields * unused context

* 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 (#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 (#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 (#14565) * Return the correct payload proof * changelog <3 * merge * Set fields of wrapped proto object in light client setters (#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 (#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]>