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

Deneb: publish block v1 ssz #12622

Merged
merged 1 commit into from
Jul 17, 2023
Merged

Conversation

james-prysm
Copy link
Contributor

@james-prysm james-prysm commented Jul 14, 2023

What type of PR is this?

Feature

What does this PR do? Why is it needed?
implements the following apis for deneb

  • publishBlindedBlock ssz
  • publishBlock ssz

Which issues(s) does this PR fix?

part of #12248

@james-prysm james-prysm added the Deneb PRs or issues for the Deneb upgrade label Jul 14, 2023
@james-prysm james-prysm changed the base branch from develop to deneb-integration July 14, 2023 00:56
@james-prysm james-prysm force-pushed the deneb-publish-v1-ssz branch 3 times, most recently from 2d07b5b to c939831 Compare July 14, 2023 18:09
@james-prysm james-prysm changed the title Deneb publish block v1 ssz Deneb: publish block v1 ssz Jul 14, 2023
@james-prysm james-prysm marked this pull request as ready for review July 14, 2023 19:19
@james-prysm james-prysm requested review from prestonvanloon and a team as code owners July 14, 2023 19:19
@james-prysm james-prysm requested review from rauljordan, rkapka, kasey and terencechain and removed request for a team and prestonvanloon July 14, 2023 19:19
}
blindedBlock, err := migration.BlindedDenebToV1Alpha1SignedBlock(blkContent.SignedBlindedBlock)
if err != nil {
return nil, status.Errorf(codes.InvalidArgument, "Submitted block is not blinded: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Error message is incorrect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm I think this should be right , instead of checking the is blinded function it's trying to migrate the type but if it's empty or doesn't marshal correctly I think we can say this. what do you think.

@@ -466,6 +467,29 @@ func TestServer_SubmitBlindedBlockSSZ(t *testing.T) {
_, err = server.SubmitBlindedBlockSSZ(sszCtx, blockReq)
assert.NotNil(t, err)
})
t.Run("Deneb", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add a Deneb full test case

@james-prysm james-prysm force-pushed the deneb-publish-v1-ssz branch from c939831 to aae17b3 Compare July 17, 2023 14:47
@james-prysm james-prysm force-pushed the deneb-publish-v1-ssz branch from aae17b3 to f676ee7 Compare July 17, 2023 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deneb PRs or issues for the Deneb upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants