Native Blocks Ep. 1 - New types and functions #10837
Conversation
| // BeaconBlockIsNil checks if any composite field of input signed beacon block is nil. | ||
| // Access to these nil fields will result in run time panic, | ||
| // it is recommended to run these checks as first line of defense. | ||
| func BeaconBlockIsNil(b interfaces.SignedBeaconBlock) error { |
There was a problem hiding this comment.
This function does not have tests at the moment. This is because it takes an interface parameter and new types don't implement interfaces properly just yet. This will come in the next PR.
consensus-types/blocks/getters.go
Outdated
| func (b *SignedBeaconBlock) SizeSSZ() int { | ||
| pb, err := b.Proto() | ||
| if err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
All panics will be converted to errors once we update fastssz to generate SizeSSZ (int, error)
There was a problem hiding this comment.
b.Proto() can return an error if the inner block type is malformed for any reason. When do you expect the fast-ssz fix to be merged in ?
| // BuildSignedBeaconBlock assembles a block.SignedBeaconBlock interface compatible struct from a | ||
| // given beacon block and the appropriate signature. This method may be used to easily create a | ||
| // signed beacon block. | ||
| func BuildSignedBeaconBlock(blk interfaces.BeaconBlock, signature []byte) (*SignedBeaconBlock, error) { |
There was a problem hiding this comment.
Same comment as #10837 (comment)
|
|
||
| // Header converts the underlying protobuf object from blinded block to header format. | ||
| func (b *SignedBeaconBlock) Header() (*eth.SignedBeaconBlockHeader, error) { | ||
| root, err := b.block.body.HashTreeRoot() |
There was a problem hiding this comment.
should do a nil check here before calling HTR
consensus-types/blocks/getters.go
Outdated
| StateRoot: b.block.stateRoot, | ||
| BodyRoot: root[:], | ||
| }, | ||
| Signature: b.Signature(), |
There was a problem hiding this comment.
no need to call the getter method if you already have access to the block struct
consensus-types/blocks/getters.go
Outdated
| func (b *SignedBeaconBlock) SizeSSZ() int { | ||
| pb, err := b.Proto() | ||
| if err != nil { | ||
| panic(err) |
There was a problem hiding this comment.
b.Proto() can return an error if the inner block type is malformed for any reason. When do you expect the fast-ssz fix to be merged in ?
consensus-types/blocks/getters.go
Outdated
|
|
||
| // ExecutionPayload returns the execution payload of the block body. | ||
| func (b *BeaconBlockBody) ExecutionPayload() (*enginev1.ExecutionPayload, error) { | ||
| if b.version == version.Phase0 || b.version == version.Altair || b.version == version.BellatrixBlind { |
There was a problem hiding this comment.
might be simpler to check for a negative ( version != Bellatrix)
consensus-types/blocks/getters.go
Outdated
|
|
||
| // ExecutionPayloadHeader returns the execution payload header of the block body. | ||
| func (b *BeaconBlockBody) ExecutionPayloadHeader() (*eth.ExecutionPayloadHeader, error) { | ||
| if b.version == version.Phase0 || b.version == version.Altair || b.version == version.Bellatrix { |
consensus-types/blocks/proto.go
Outdated
| return nil, errNilBlock | ||
| } | ||
|
|
||
| pb, ok := proto.Clone(pb).(*eth.SignedBeaconBlock) |
There was a problem hiding this comment.
what is the reason for doing a clone here, this is going to make unmarshalling blocks expensive on memory and cpu
consensus-types/blocks/proto.go
Outdated
| return nil, errNilBlock | ||
| } | ||
|
|
||
| pb, ok := proto.Clone(pb).(*eth.SignedBeaconBlockBellatrix) |
consensus-types/blocks/proto.go
Outdated
| return nil, errNilBlock | ||
| } | ||
|
|
||
| pb, ok := proto.Clone(pb).(*eth.BeaconBlock) |
There was a problem hiding this comment.
same principle here on cloning blocks
consensus-types/blocks/getters.go
Outdated
|
|
||
| // Header converts the underlying protobuf object from blinded block to header format. | ||
| func (b *SignedBeaconBlock) Header() (*eth.SignedBeaconBlockHeader, error) { | ||
| if b.IsNil() || b.block.IsNil() { |
There was a problem hiding this comment.
lets add one more check for the body, since we are doing a htr of it below.
There was a problem hiding this comment.
I don't know why I changed the implementation of SignedBeaconBlock.IsNil. I reverted it back to what's in the current block code, which basically checks everything for nilness: signed block, block and block body.
What type of PR is this?
Feature
What does this PR do? Why is it needed?
This PR introduces all types, methods and functions necessary to migrate towards native blocks. It accomplishes the following tasks from https://www.notion.so/prysmaticlabs/Native-Beacon-Block-c862730f431545b6a57333eaa50025d2:
/consensus-types/blockspackageSignedBeaconBlock,BeaconBlock,BeaconBlockBody. Fields of these types will be existing protobuf objects e.g.eth1Data: *eth.Eth1Data. Having protobuf fields greatly reduces the amount of refactoring needed and does not harm our main goal of making it easier to work with blocks. Each type will contain fields for all versions of the object. An additionalversionfield will be responsible for determining the actual version.init[1]FromProto[2]functions where[1]is the name of the object that we want to create, e.g.SignedBeaconBlockand[2]is the version e.g.Altair, and use these initialization functions when creating an interface object from a protobuf object.Other notes for review
Code extracted from #10799, tests added in this PR.