P2P: broadcast blob by terencechain · Pull Request #12419 · OffchainLabs/prysm

terencechain · 2023-05-17T20:02:39Z

Add BroadcastBlob to enable the broadcast of blob. Similar to Broadcast sync committee and attestation design

james-prysm · 2023-05-17T20:51:21Z

beacon-chain/p2p/broadcaster.go

+func (s *Service) BroadcastBlob(ctx context.Context, subnet uint64, blob *ethpb.SignedBlobSidecar) error {
+	ctx, span := trace.StartSpan(ctx, "p2p.BroadcastBlob")
+	defer span.End()
+	if blob == nil {


is this the only validation we need to do before a blob sidecar is broadcasted? do we need to validate blob with kzg?

This is just broadcast from p2p's perspective. Validations (if there are any) are done before this

nisdas

As a guide you can look at broadcastAttestation broadcastSyncCommittee , we do some additional checks to ensure we have peers in those particular topics. The current PR simply broadcasts the object as it is

rkapka · 2023-05-18T11:51:04Z

beacon-chain/p2p/broadcaster.go

+}
+
+func (s *Service) broadcastBlobBackground(ctx context.Context, subnet uint64, blobSidecar *ethpb.SignedBlobSidecar, forkDigest [4]byte) {
+	_, span := trace.StartSpan(ctx, "p2p.broadcastBlob")


Suggested change

_, span := trace.StartSpan(ctx, "p2p.broadcastBlob")

_, span := trace.StartSpan(ctx, "p2p.broadcastBlobBackground")

renamed the function to broadcastBlob

rkapka · 2023-05-18T11:53:46Z

beacon-chain/p2p/broadcaster_test.go

+	if len(p1.BHost.Network().Peers()) == 0 {
+		t.Fatal("No peers")
+	}


This can be changed to

require.NotEqual(t, 0, len(p1.BHost.Network().Peers()), "No peers")

rkapka · 2023-05-18T11:54:45Z

beacon-chain/p2p/broadcaster_test.go

+		if !proto.Equal(result, blobSidecar) {
+			tt.Errorf("Did not receive expected message, got %+v, wanted %+v", result, blobSidecar)


If I remember correctly, you merged a utility assertion function for comparing proto objects. It can be used here.

rkapka · 2023-05-18T11:55:40Z

beacon-chain/p2p/broadcaster_test.go

+	if util.WaitTimeout(&wg, 1*time.Second) {
+		t.Error("Failed to receive pubsub within 1s")
+	}


This can be replaced with

assert.Equal(t, false, util.WaitTimeout(&wg, 1*time.Second), "Failed to receive pubsub within 1s")

terencechain · 2023-05-18T18:00:17Z

As a guide you can look at broadcastAttestation broadcastSyncCommittee , we do some additional checks to ensure we have peers in those particular topics. The current PR simply broadcasts the object as it is

66d1811

beacon-chain/p2p/subnets.go

nisdas · 2023-05-19T03:53:12Z

beacon-chain/p2p/subnets.go

+// the relevant lock. This is used to differentiate
+// blob subnets from others. This is deliberately
+// chosen as less than 4(blob subnet count).
+const blobSubnetLockerVal = 4


This will conflict with attestation subnets, you should change it to 110 <=

terencechain added Ready For Review labels May 17, 2023

terencechain self-assigned this May 17, 2023

terencechain requested a review from a team as a code owner May 17, 2023 20:02

terencechain requested review from potuz, rauljordan and saolyn and removed request for a team May 17, 2023 20:02

james-prysm reviewed May 17, 2023

View reviewed changes

nisdas reviewed May 18, 2023

View reviewed changes

rkapka reviewed May 18, 2023

View reviewed changes

terencechain force-pushed the deneb-integration branch from 6e9f6fd to ce4a937 Compare May 18, 2023 17:09

terencechain requested a review from prestonvanloon as a code owner May 18, 2023 17:09

terencechain force-pushed the blob-broadcast branch from b7f8ffc to 33dda46 Compare May 18, 2023 17:41

james-prysm approved these changes May 18, 2023

View reviewed changes

rkapka approved these changes May 18, 2023

View reviewed changes

rkapka reviewed May 18, 2023

View reviewed changes

beacon-chain/p2p/subnets.go Outdated Show resolved Hide resolved

nisdas reviewed May 19, 2023

View reviewed changes

terencechain added the blocked label May 19, 2023

kasey force-pushed the deneb-integration branch from ce4a937 to ca11e4a Compare May 19, 2023 17:26

terencechain force-pushed the blob-broadcast branch from 703450c to 4a9b18b Compare May 19, 2023 18:15

kasey force-pushed the deneb-integration branch from ca11e4a to 220beca Compare May 22, 2023 16:43

terencechain and others added 6 commits May 23, 2023 09:03

Broadcast blob

a156674

Add retry attempt

9bdfb8f

Test feedback

e8c12f2

Update beacon-chain/p2p/subnets.go

0423099

Set blob subnet locker val to 110

4660b31

rm new line

9568071

terencechain force-pushed the blob-broadcast branch from dbbd8d2 to 9568071 Compare May 23, 2023 16:03

james-prysm pushed a commit that referenced this pull request May 23, 2023

P2P: broadcast blob (#12419)

e2b8843

terencechain added a commit that referenced this pull request May 26, 2023

P2P: broadcast blob (#12419)

ebbf038

terencechain added a commit that referenced this pull request May 31, 2023

P2P: broadcast blob (#12419)

f08bb6c

terencechain added a commit that referenced this pull request Jun 7, 2023

P2P: broadcast blob (#12419)

cf9ac0b

terencechain added a commit that referenced this pull request Jun 12, 2023

P2P: broadcast blob (#12419)

a127ad9

terencechain added a commit that referenced this pull request Jun 12, 2023

P2P: broadcast blob (#12419)

21ad92e

terencechain added a commit that referenced this pull request Jun 16, 2023

P2P: broadcast blob (#12419)

600da6e

terencechain added a commit that referenced this pull request Jun 27, 2023

P2P: broadcast blob (#12419)

0b67197

terencechain added a commit that referenced this pull request Jul 7, 2023

P2P: broadcast blob (#12419)

2e41bc7

terencechain added a commit that referenced this pull request Jul 9, 2023

P2P: broadcast blob (#12419)

8f8780e

terencechain added a commit that referenced this pull request Jul 10, 2023

P2P: broadcast blob (#12419)

e05cb0a

kasey pushed a commit that referenced this pull request Jul 20, 2023

P2P: broadcast blob (#12419)

347a063

james-prysm pushed a commit that referenced this pull request Aug 4, 2023

P2P: broadcast blob (#12419)

011b958

terencechain added a commit that referenced this pull request Aug 16, 2023

P2P: broadcast blob (#12419)

b509aae

kasey pushed a commit that referenced this pull request Aug 21, 2023

P2P: broadcast blob (#12419)

837774e

kasey pushed a commit that referenced this pull request Aug 22, 2023

P2P: broadcast blob (#12419)

4d10ba1

kasey pushed a commit that referenced this pull request Aug 22, 2023

P2P: broadcast blob (#12419)

1efca3b

kasey pushed a commit that referenced this pull request Aug 22, 2023

P2P: broadcast blob (#12419)

fe15bc5

kasey pushed a commit that referenced this pull request Aug 23, 2023

P2P: broadcast blob (#12419)

ce6e991

kasey pushed a commit that referenced this pull request Aug 23, 2023

P2P: broadcast blob (#12419)

cf07f8c

kasey pushed a commit that referenced this pull request Aug 23, 2023

P2P: broadcast blob (#12419)

cde2d27

kasey pushed a commit that referenced this pull request Aug 24, 2023

P2P: broadcast blob (#12419)

3a774ad

kasey pushed a commit that referenced this pull request Aug 24, 2023

P2P: broadcast blob (#12419)

e146c7b

prestonvanloon pushed a commit that referenced this pull request Aug 24, 2023

P2P: broadcast blob (#12419)

2a4269d

prestonvanloon pushed a commit that referenced this pull request Aug 24, 2023

P2P: broadcast blob (#12419)

9e60adf

prestonvanloon pushed a commit that referenced this pull request Aug 24, 2023

P2P: broadcast blob (#12419)

3d2f8cf

prestonvanloon pushed a commit that referenced this pull request Aug 24, 2023

P2P: broadcast blob (#12419)

aa010a5

prestonvanloon pushed a commit that referenced this pull request Aug 30, 2023

P2P: broadcast blob (#12419)

dec911b

prestonvanloon pushed a commit that referenced this pull request Aug 30, 2023

P2P: broadcast blob (#12419)

a344257

prestonvanloon pushed a commit that referenced this pull request Aug 31, 2023

P2P: broadcast blob (#12419)

c035711

	_, span := trace.StartSpan(ctx, "p2p.broadcastBlob")
	_, span := trace.StartSpan(ctx, "p2p.broadcastBlobBackground")

		if !proto.Equal(result, blobSidecar) {
		tt.Errorf("Did not receive expected message, got %+v, wanted %+v", result, blobSidecar)

Conversation

terencechain commented May 17, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nisdas left a comment

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!

terencechain commented May 18, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants