WIP: Deneb integration no spectest or proto by kasey · Pull Request #12773 · OffchainLabs/prysm

kasey · 2023-08-22T02:57:33Z

What type of PR is this?
Other

What does this PR do? Why is it needed?

This PR represents deneb-integration rebased onto deneb-integration-spectest, which builds on deneb-integration-proto. As such the diff for this PR should show the diff for deneb-integration minus the proto and spectests changes, with the goal of making the deneb-integration branch easier to review.

builds on #12772

Co-authored-by: Nishant Das <nishdas93@gmail.com> Fix Migration Of State (#12423)

* validator signing feature migrated from eip4844 branch, added unit tests * Update proto/prysm/v1alpha1/beacon_block.proto --------- Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* BlobSidecarsByRoot RPC handler * BlobSidecarsByRange rpc handler (#12499) Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com> --------- Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>

Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>

* test: skip deneb e2e test with an issue * Unskip more stuff

* Add a new blob channel * fix mock * reset the channel * keep a map of channels * gazelle * do not overwrite map * remove pre-declaration

* binary search is the wrong tool for the job, derp * restore protection from array index panics --------- Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>

Co-authored-by: terence tsao <terence@prysmaticlabs.com>

beacon-chain/blockchain/receive_block.go

beacon-chain/blockchain/service.go

nisdas · 2023-08-23T08:12:40Z

beacon-chain/core/altair/attestation.go

 	}
 	matchedSrcTgt := matchedSrc && matchedTgt
-	if matchedSrcTgt && delay <= slotsPerEpoch {
+	// Before Deneb no attestation should pass validation without having delay <= slotsPerEpoch.


why is this changed ?

EIP7045

See: #12565

I understand the eip, but doesn't this break pre-deneb core code ?

No. Because before Deneb, we will fail earlier anyway (See comment). @potuz idea to keep the code clean

nisdas · 2023-08-23T08:14:12Z

beacon-chain/core/blocks/attestation.go

-			params.BeaconConfig().SlotsPerEpoch,
-		)
+
+	if beaconState.Version() < version.Deneb {


don't we have validity checks for attestations after deneb ? what happens if we get a very old attestation here

Do we not need to validate that it is at least the current or previous epoch ?

It's verified above. See if data.Target.Epoch != prevEpoch && data.Target.Epoch != currEpoch {

nisdas · 2023-08-23T08:19:30Z

beacon-chain/core/helpers/attestation.go

+	}
+	attSlotEpoch := slots.ToEpoch(attSlot)
+	if attSlotEpoch != currentEpoch && attSlotEpoch != prevEpoch {
+		return fmt.Errorf("attestation slot %d not within current epoch %d or previous epoch %d", attSlot, currentEpoch, prevEpoch)


can you follow the error pattern as how above ? With attError

ill take this

nisdas · 2023-08-23T08:43:11Z

beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go

-			return nil, errors.Wrap(err, "could not build block in parallel")
-		}
-	} else {
+	var wg sync.WaitGroup


This needs to be changed back to the flag, otherwise we are removing the feature

That's right. Was hoping we do this at the major release

ill take this

nisdas · 2023-08-23T08:44:37Z

beacon-chain/rpc/prysm/v1alpha1/validator/proposer.go

-		}
+// ProposeBeaconBlock is called by a proposer during its assigned slot to create a block in an attempt
+// to get it processed by the beacon node as the canonical head.
+func (vs *Server) ProposeBeaconBlock(ctx context.Context, req *ethpb.GenericSignedBeaconBlock) (*ethpb.ProposeResponse, error) {


Yeah the whole feature gets wiped out here, most likely was due to a bad rebase

This was intended. I would like to fully remove non build block parallel at a major point release. The reason is that this code path is becoming hard to maintain. Now we will have to write get blobs from both builder and local EL for two paths. I'll think about this more. But that's right we can do this anymore because we are merging to develop now.

ill take this

nisdas · 2023-08-23T08:52:49Z

beacon-chain/state/state-native/hasher.go

 	case version.Capella:
 		fieldRoots = make([][]byte, params.BeaconConfig().BeaconStateCapellaFieldCount)
+	case version.Deneb:
+		fieldRoots = make([][]byte, params.BeaconConfig().BeaconStateCapellaFieldCount) // Deneb has the same state field count as Capella.


we should still set it as a seprate variable here

grabbing this

addressed in #12806

nisdas · 2023-08-23T08:56:00Z

beacon-chain/state/state-native/state_trie.go

 	case version.Capella:
 		fieldCount = params.BeaconConfig().BeaconStateCapellaFieldCount
+	case version.Deneb:
+		fieldCount = params.BeaconConfig().BeaconStateCapellaFieldCount


same thing here

addressed in #12806

nisdas · 2023-08-23T09:11:10Z

consensus-types/payload-attribute/getters.go

@@ -61,8 +61,8 @@ func (a *data) PbV2() (*enginev1.PayloadAttributesV2, error) {
 	if a == nil {
 		return nil, errNilPayloadAttribute
 	}
-	if a.version != version.Capella {
-		return nil, consensus_types.ErrNotSupported("PayloadAttributePbV2", a.version)
+	if a.version < version.Capella {


why is the conditional changed

This is indeed a bug. PbV2 should be the same for deneb but not anymore

ill take this

terencechain and others added 30 commits August 21, 2023 21:53

Add deneb protobufs (#12363)

088f9fb

Add deneb block to consensus types (#12368)

c90dd45

Add Deneb state (#12375)

b91e71f

Deneb DB methods (#12379)

17b0cdb

Deneb integration: Beacon API getStateV2 (#12424)

e781815

Co-authored-by: Nishant Das <nishdas93@gmail.com> Fix Migration Of State (#12423)

Add Blob Gossip (#12413)

d7accd5

P2P: broadcast blob (#12419)

fe15bc5

Add Deneb upgrade function (#12433)

2b2f3b0

validator signing deneb (#12449)

a8f6577

* validator signing feature migrated from eip4844 branch, added unit tests * Update proto/prysm/v1alpha1/beacon_block.proto --------- Co-authored-by: Radosław Kapka <rkapka@wp.pl>

Deneb: implements engine API end points (#12384)

64b9afe

Update block's commitment size (#12470)

7187b7b

Proposer RPC: GetBlock for Deneb (#12481)

d858acf

Add data gas used field to execution payload (#12488)

e8d380e

Update ProposeBeaconBlock Prysm RPC for Deneb (Non builder) (#12495)

6002dcf

feat(rpc/validator): stream and use deneb block (#12510)

d4de3cc

feat(blockchain): update payload attribute for deneb (#12509)

aafc029

feat(config): update max blobs per block to 6 (#12512)

198d421

introducing deneb changes and blobs to builder (#12477)

fe64709

BlobSidecarsByRoot (#12420)

24fdc73

* BlobSidecarsByRoot RPC handler * BlobSidecarsByRange rpc handler (#12499) Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com> --------- Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>

Integrate generate genesis (#12523)

39c3396

Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>

ssz detection support for deneb (#12537)

8cf2f91

Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>

feat(prysmctl): request blobs (#12544)

d58ed9a

fix: devnet6 interop issues (#12545)

67f497b

feat(deneb): proposer rpc to handle builder flow (#12554)

01c7b88

migrating Beacon API code from eip4844 to deneb-integration (#12568)

c6ab7a4

Update grpc to 1.53.0 (#12606)

6551f72

migrating get blob from eip4844 and deneb-integration (#12603)

93bb801

adding ssz response for get blobs beacon api endpoint (#12611)

6ffbd2d

test(e2e): deneb basic (#12546)

687d71a

fix: add blob batch flags to main/usage (#12614)

1d96ad6

james-prysm and others added 18 commits August 21, 2023 21:53

implementing publishblockv2 for deneb (#12662)

f73257a

fixing broken test (#12673)

36f7ae7

renaming data to blobs in execution payload (#12664)

f429ab3

Validate blobs feature (#12574)

f451a0c

test:beta.1 deneb tests (#12680)

7a94ee1

feat: implement eip4788 (#12570)

5198f51

feat: log blob commitment count (#12723)

7560913

feat: add validator blob signing (#12730)

789fc27

Fix: payload attribute v3 return nil (#12736)

52b77d4

fixing route handler and small cleanup items (#12726)

e3961a2

feat: save blobs to db for subscriber (#12734)

cd85eef

Update gossip_scoring_params.go (#12732)

8f257b1

fix: validator returning the right # of blobs (#12738)

dd0afb6

Update go.sum

8a22daf

test: skip deneb e2e test with tracking (#12751)

3500f4b

* test: skip deneb e2e test with an issue * Unskip more stuff

Blob channel (#12753)

bc8f3df

* Add a new blob channel * fix mock * reset the channel * keep a map of channels * gazelle * do not overwrite map * remove pre-declaration

binary search is the wrong tool for the job, derp (#12757)

b17a75a

* binary search is the wrong tool for the job, derp * restore protection from array index panics --------- Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>

Save blob last stage init sync (#12764)

dc0ad53

Co-authored-by: terence tsao <terence@prysmaticlabs.com>

kasey requested a review from a team as a code owner August 22, 2023 02:57

kasey requested review from potuz, saolyn and terencechain and removed request for a team August 22, 2023 02:57

kasey mentioned this pull request Aug 22, 2023

WIP: Deneb integration minus protobuf changes #12771

Closed

nisdas reviewed Aug 23, 2023

View reviewed changes

This was referenced Aug 24, 2023

Don't call kzg.Start if deneb fork isn't "scheduled" #12803

Merged

re-use metadata bucket, group db constants #12804

Merged

give deneb it's own field count config value #12806

Merged

terencechain mentioned this pull request Aug 25, 2023

chore: improve att error #12813

Merged

kasey closed this Sep 6, 2023

Conversation

kasey commented Aug 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

kasey commented Aug 22, 2023 •

edited

Loading