Skip to content

wrap kzgs/proofs/blobs fields as BlobsBundle#5562

Merged
etan-status merged 2 commits intounstablefrom
dev/etan/df-blobsbundle
Nov 4, 2023
Merged

wrap kzgs/proofs/blobs fields as BlobsBundle#5562
etan-status merged 2 commits intounstablefrom
dev/etan/df-blobsbundle

Conversation

@etan-status
Copy link
Copy Markdown
Contributor

Less type conversion / copying by keeping the BlobsBundle together.

Less type conversion / copying by keeping the `BlobsBundle` together.
forkyBlck is capella.BeaconBlock:
Opt.none(seq[BlobSidecar])
else:
static: doAssert "Unknown BeaconBlock type"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Switching static: doAssert to Opt.none means that it won't trigger automatically anymore when the E-fork is added, but will incorrectly and silently use the pre-Deneb/sidecar version.

Better to find out about missing functionality at compile-time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It will take the Deneb version, not the none version (consensusFork >= ConsensusFork.Deneb), and will compile-time fail if no longer compatible

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The other one will compile-time break on Electra because DenebBlockContents requires a Deneb block, while the generated code will try to put an Electra block into it.

This instance here will be fine with Electra as well (using the blob logic from deneb)

@etan-status etan-status enabled auto-merge (squash) November 4, 2023 11:21
@etan-status etan-status merged commit a05278e into unstable Nov 4, 2023
@etan-status etan-status deleted the dev/etan/df-blobsbundle branch November 4, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants