-
Notifications
You must be signed in to change notification settings - Fork 999
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
LC Updates by Range to read from DB #14531
base: epf-light-client
Are you sure you want to change the base?
LC Updates by Range to read from DB #14531
Conversation
Co-authored-by: Radosław Kapka <[email protected]>
Co-authored-by: Radosław Kapka <[email protected]>
I am not sure you missed them or are they intentional but I think they were there because you removed the fn call which returned err . |
* in progress * completed logic * var name * additional logic changes * fix createDefaultLightClientUpdate * empty fields * unused context
0e7deff
to
497c50a
Compare
* Return the correct payload proof * changelog <3
…labs#14573) * Set fields of wrapped proto object in light client setters * changelog <3
* 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]>
@@ -243,7 +243,7 @@ func NewLightClientUpdateFromBeaconState( | |||
return result, nil | |||
} | |||
|
|||
func createDefaultLightClientUpdate(currentSlot primitives.Slot) (interfaces.LightClientUpdate, error) { | |||
func CreateDefaultLightClientUpdate(currentSlot primitives.Slot) (interfaces.LightClientUpdate, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made this function public because it's useful in other places as well. specially in tests.
@@ -278,24 +278,40 @@ func createDefaultLightClientUpdate(currentSlot primitives.Slot) (interfaces.Lig | |||
var m proto.Message | |||
if currentEpoch < params.BeaconConfig().CapellaForkEpoch { | |||
m = &pb.LightClientUpdateAltair{ | |||
AttestedHeader: &pb.LightClientHeaderAltair{}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added an empty header here. because the return light_client.NewWrappedHeader(m)
would throw an error otherwise.
Beacon: &pb.BeaconBlockHeader{}, | ||
Execution: &enginev1.ExecutionPayloadHeaderCapella{}, | ||
ExecutionBranch: executionBranch, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above.
Beacon: &pb.BeaconBlockHeader{}, | ||
Execution: &enginev1.ExecutionPayloadHeaderDeneb{}, | ||
ExecutionBranch: executionBranch, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
Beacon: &pb.BeaconBlockHeader{}, | ||
Execution: &enginev1.ExecutionPayloadHeaderDeneb{}, | ||
ExecutionBranch: executionBranch, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
if blockEpoch < params.BeaconConfig().CapellaForkEpoch { | ||
var ok bool | ||
|
||
p, err := execution.EmptyExecutionPayload(version.Capella) | ||
if err != nil { | ||
return nil, errors.Wrap(err, "could not get payload header") | ||
} | ||
payloadHeader, ok = p.(*enginev1.ExecutionPayloadHeaderCapella) | ||
if !ok { | ||
return nil, errors.Wrapf(err, "payload header type %T is not %T", payloadHeader, &enginev1.ExecutionPayloadHeaderCapella{}) | ||
payloadHeader = &enginev1.ExecutionPayloadHeaderCapella{ | ||
ParentHash: make([]byte, fieldparams.RootLength), | ||
FeeRecipient: make([]byte, fieldparams.FeeRecipientLength), | ||
StateRoot: make([]byte, fieldparams.RootLength), | ||
ReceiptsRoot: make([]byte, fieldparams.RootLength), | ||
LogsBloom: make([]byte, fieldparams.LogsBloomLength), | ||
PrevRandao: make([]byte, fieldparams.RootLength), | ||
ExtraData: make([]byte, 0), | ||
BaseFeePerGas: make([]byte, fieldparams.RootLength), | ||
BlockHash: make([]byte, fieldparams.RootLength), | ||
TransactionsRoot: make([]byte, fieldparams.RootLength), | ||
WithdrawalsRoot: make([]byte, fieldparams.RootLength), | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The execution.EmptyExecutionPayload(version.Capella)
returns something that is different than enginev1.ExecutionPayloadHeaderCapella
and it would throw an error when casting. so I just created one here. and it's only for this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
execution.EmptyExecutionPayload(version.Capella)
returns something that is different thanenginev1.ExecutionPayloadHeaderCapella
and it would throw an error when casting. so I just created one here. and it's only for this case.
The execution.EmptyExecutionPayload()
function returns an empty payload, not the payload header. There's no function for the latter, but we can implement it. After that, a typecasting works fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest we implement the function if we need the EmptyExecutionPayloadHeader
somewhere else as well. for now it's only here and this code should do the trick.
fullEnc := make([]byte, len(key)+len(enc)) | ||
copy(fullEnc, key) | ||
copy(fullEnc[len(key):], enc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was forgotten
3db34bd
to
fb48b4a
Compare
What type of PR is this?
Feature
What does this PR do? Why is it needed?
This PR changes the
GetLightClientUpdatesByRange
API function to read the updates from the DB instead of computing them which is a very heavy process.Which issues(s) does this PR fix?
Part of #12991