Skip to content
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

Embed blob sidecars directly under blob_sidecars property in BlockContents #7145

Merged
merged 6 commits into from
May 16, 2023

Conversation

mehdi-aouadi
Copy link
Contributor

@mehdi-aouadi mehdi-aouadi commented May 15, 2023

PR Description

Remove redundant blob_sidecars property name in BlockContents container. This means removing the BlobSidecars container and its BlobSidecarsSchema (Same for the signed and the signed, blinded ones).
Blob sidecars should be represented as an array under the blob_sidecars property in BlockContents.
In the current implementation there is a redundant blob_sidecars property:
BlockContents:

{
	"beacon_block": {}
	"blob_sidecars": {
		"blob_sidecars": array of BlobSidecar
	}
}

These changes fixes it to become:

{
	"beacon_block": {}
	"blob_sidecars": array of BlobSidecar
}

These changes also fix SignedBlockContents, BlindedBlockContents and SignedBlindedBlockContents

Spec reference here

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@mehdi-aouadi mehdi-aouadi self-assigned this May 15, 2023
@mehdi-aouadi mehdi-aouadi added the Epic Deneb Issues required to implement Deneb upgrade label May 15, 2023
@mehdi-aouadi mehdi-aouadi force-pushed the fix-block-contents-schema branch 2 times, most recently from be06dad to 8b42a49 Compare May 15, 2023 09:26
@mehdi-aouadi mehdi-aouadi force-pushed the fix-block-contents-schema branch from 05b7d5a to 475cb45 Compare May 15, 2023 12:29
@mehdi-aouadi mehdi-aouadi mentioned this pull request May 15, 2023
2 tasks
Copy link
Contributor

@tbenr tbenr left a comment

Choose a reason for hiding this comment

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

looks generally ok. just a couple of renamings

@mehdi-aouadi mehdi-aouadi requested a review from tbenr May 16, 2023 07:38
Copy link
Contributor

@tbenr tbenr left a comment

Choose a reason for hiding this comment

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

LGTM

@mehdi-aouadi mehdi-aouadi merged commit dd9cbbc into Consensys:master May 16, 2023
@mehdi-aouadi mehdi-aouadi deleted the fix-block-contents-schema branch May 16, 2023 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic Deneb Issues required to implement Deneb upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants