search

attestantio / go-eth2-client

Apache License 2.0
112 stars 65 forks source link

Incorrect `ssz-max` limits for BlockContents #110

Closed jtraglia closed 7 months ago

jtraglia commented 7 months ago

Hi there, I was reviewing the Deneb structures and noticed that *BlockContents limits the number of KZG proofs and blobs to 6 (MAX_BLOBS_PER_BLOCK) rather than 4096 (MAX_BLOB_COMMITMENTS_PER_BLOCK) like I think it should.

https://github.com/attestantio/go-eth2-client/blob/bdc5ab45a2ff2607c80e2481c21441dd0d92349b/api/v1/deneb/blockcontents.go#L24-L28

https://github.com/attestantio/go-eth2-client/blob/bdc5ab45a2ff2607c80e2481c21441dd0d92349b/api/v1/deneb/signedblockcontents.go#L24-L28

For reference, see the definition of BlockContents in the Beacon-APIs repo:

https://github.com/ethereum/beacon-APIs/blob/3b6928b1dedc1cb1850975cc86a6cd58dc4b4861/types/deneb/block_contents.yaml#L2-L14

jtraglia commented 7 months ago

Another reference, this is Prysm's definition for BeaconBlockContentsDeneb:

https://github.com/prysmaticlabs/prysm/blob/6d3c6a63310c44a0145f774a522e4ec7fba10733/proto/prysm/v1alpha1/beacon_block.proto#L459-L463

avalonche commented 7 months ago

yes I think we had some issue publishing ssz encoded blocks to lighthouse, this might be the issue

mcdee commented 7 months ago

Thanks; I'll address this.

@avalonche this is unlikely to be the cause of publishing SSZ blocks as it only changes bounds checks. I have successfully published SSZ-encoded deneb blocks to lighthouse 5.0.0.