From 4c3dbae3c05598a3358cdb8d50f4713f58f0ef91 Mon Sep 17 00:00:00 2001 From: kasey <489222+kasey@users.noreply.github.com> Date: Sat, 2 Mar 2024 09:11:22 -0600 Subject: [PATCH] tests for origin blob fetching (#13667) * tests for origin blob fetching * Update beacon-chain/sync/initial-sync/service.go Co-authored-by: Preston Van Loon --------- Co-authored-by: Kasey Kirkham Co-authored-by: Preston Van Loon --- beacon-chain/blockchain/process_block_test.go | 2 +- beacon-chain/db/filesystem/ephemeral.go | 2 +- beacon-chain/sync/initial-sync/service.go | 49 ++++++++------ .../sync/initial-sync/service_test.go | 67 +++++++++++++++++++ 4 files changed, 98 insertions(+), 22 deletions(-) diff --git a/beacon-chain/blockchain/process_block_test.go b/beacon-chain/blockchain/process_block_test.go index 2aae1f4f4..0ff6590fd 100644 --- a/beacon-chain/blockchain/process_block_test.go +++ b/beacon-chain/blockchain/process_block_test.go @@ -2115,7 +2115,7 @@ func TestMissingIndices(t *testing.T) { for _, c := range cases { bm, bs := filesystem.NewEphemeralBlobStorageWithMocker(t) t.Run(c.name, func(t *testing.T) { - require.NoError(t, bm.CreateFakeIndices(c.root, c.present)) + require.NoError(t, bm.CreateFakeIndices(c.root, c.present...)) missing, err := missingIndices(bs, c.root, c.expected) if c.err != nil { require.ErrorIs(t, err, c.err) diff --git a/beacon-chain/db/filesystem/ephemeral.go b/beacon-chain/db/filesystem/ephemeral.go index 4c05c76b1..729f815bb 100644 --- a/beacon-chain/db/filesystem/ephemeral.go +++ b/beacon-chain/db/filesystem/ephemeral.go @@ -37,7 +37,7 @@ type BlobMocker struct { // CreateFakeIndices creates empty blob sidecar files at the expected path for the given // root and indices to influence the result of Indices(). -func (bm *BlobMocker) CreateFakeIndices(root [32]byte, indices []uint64) error { +func (bm *BlobMocker) CreateFakeIndices(root [32]byte, indices ...uint64) error { for i := range indices { n := blobNamer{root: root, index: indices[i]} if err := bm.fs.MkdirAll(n.dir(), directoryPermissions); err != nil { diff --git a/beacon-chain/sync/initial-sync/service.go b/beacon-chain/sync/initial-sync/service.go index f7b3f8f8b..bb3f6b967 100644 --- a/beacon-chain/sync/initial-sync/service.go +++ b/beacon-chain/sync/initial-sync/service.go @@ -279,34 +279,22 @@ func (s *Service) markSynced() { close(s.cfg.InitialSyncComplete) } -func (s *Service) fetchOriginBlobs(pids []peer.ID) error { - r, err := s.cfg.DB.OriginCheckpointBlockRoot(s.ctx) - if errors.Is(err, db.ErrNotFoundOriginBlockRoot) { - return nil - } - blk, err := s.cfg.DB.Block(s.ctx, r) - if err != nil { - log.WithField("root", r).Error("Block for checkpoint sync origin root not found in db") - return err - } +func missingBlobRequest(blk blocks.ROBlock, store *filesystem.BlobStorage) (p2ptypes.BlobSidecarsByRootReq, error) { + r := blk.Root() if blk.Version() < version.Deneb { - return nil + return nil, nil } cmts, err := blk.Block().Body().BlobKzgCommitments() if err != nil { log.WithField("root", r).Error("Error reading commitments from checkpoint sync origin block") - return err + return nil, err } if len(cmts) == 0 { - return nil + return nil, nil } - rob, err := blocks.NewROBlockWithRoot(blk, r) + onDisk, err := store.Indices(r) if err != nil { - return err - } - onDisk, err := s.cfg.BlobStorage.Indices(r) - if err != nil { - return errors.Wrapf(err, "error checking existing blobs for checkpoint sync bloc root %#x", r) + return nil, errors.Wrapf(err, "error checking existing blobs for checkpoint sync block root %#x", r) } req := make(p2ptypes.BlobSidecarsByRootReq, 0, len(cmts)) for i := range cmts { @@ -315,8 +303,29 @@ func (s *Service) fetchOriginBlobs(pids []peer.ID) error { } req = append(req, ð.BlobIdentifier{BlockRoot: r[:], Index: uint64(i)}) } + return req, nil +} + +func (s *Service) fetchOriginBlobs(pids []peer.ID) error { + r, err := s.cfg.DB.OriginCheckpointBlockRoot(s.ctx) + if errors.Is(err, db.ErrNotFoundOriginBlockRoot) { + return nil + } + blk, err := s.cfg.DB.Block(s.ctx, r) + if err != nil { + log.WithField("root", fmt.Sprintf("%#x", r)).Error("Block for checkpoint sync origin root not found in db") + return err + } + rob, err := blocks.NewROBlockWithRoot(blk, r) + if err != nil { + return err + } + req, err := missingBlobRequest(rob, s.cfg.BlobStorage) + if err != nil { + return err + } if len(req) == 0 { - log.WithField("nBlobs", len(cmts)).WithField("root", fmt.Sprintf("%#x", r)).Debug("All checkpoint block blobs are present") + log.WithField("root", fmt.Sprintf("%#x", r)).Debug("All blobs for checkpoint block are present") return nil } shufflePeers(pids) diff --git a/beacon-chain/sync/initial-sync/service_test.go b/beacon-chain/sync/initial-sync/service_test.go index efc711cb5..84efcfd03 100644 --- a/beacon-chain/sync/initial-sync/service_test.go +++ b/beacon-chain/sync/initial-sync/service_test.go @@ -16,6 +16,7 @@ import ( "github.com/prysmaticlabs/prysm/v5/beacon-chain/verification" "github.com/prysmaticlabs/prysm/v5/cmd/beacon-chain/flags" "github.com/prysmaticlabs/prysm/v5/config/params" + "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" eth "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v5/testing/assert" @@ -420,3 +421,69 @@ func TestService_Synced(t *testing.T) { s.synced.Set() assert.Equal(t, true, s.Synced()) } + +func TestMissingBlobRequest(t *testing.T) { + cases := []struct { + name string + setup func(t *testing.T) (blocks.ROBlock, *filesystem.BlobStorage) + nReq int + err error + }{ + { + name: "pre-deneb", + setup: func(t *testing.T) (blocks.ROBlock, *filesystem.BlobStorage) { + cb, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlockCapella()) + require.NoError(t, err) + rob, err := blocks.NewROBlockWithRoot(cb, [32]byte{}) + require.NoError(t, err) + return rob, nil + }, + nReq: 0, + }, + { + name: "deneb zero commitments", + setup: func(t *testing.T) (blocks.ROBlock, *filesystem.BlobStorage) { + bk, _ := util.GenerateTestDenebBlockWithSidecar(t, [32]byte{}, 0, 0) + return bk, nil + }, + nReq: 0, + }, + { + name: "2 commitments, all missing", + setup: func(t *testing.T) (blocks.ROBlock, *filesystem.BlobStorage) { + bk, _ := util.GenerateTestDenebBlockWithSidecar(t, [32]byte{}, 0, 2) + fs := filesystem.NewEphemeralBlobStorage(t) + return bk, fs + }, + nReq: 2, + }, + { + name: "2 commitments, 1 missing", + setup: func(t *testing.T) (blocks.ROBlock, *filesystem.BlobStorage) { + bk, _ := util.GenerateTestDenebBlockWithSidecar(t, [32]byte{}, 0, 2) + bm, fs := filesystem.NewEphemeralBlobStorageWithMocker(t) + require.NoError(t, bm.CreateFakeIndices(bk.Root(), 1)) + return bk, fs + }, + nReq: 1, + }, + { + name: "2 commitments, 0 missing", + setup: func(t *testing.T) (blocks.ROBlock, *filesystem.BlobStorage) { + bk, _ := util.GenerateTestDenebBlockWithSidecar(t, [32]byte{}, 0, 2) + bm, fs := filesystem.NewEphemeralBlobStorageWithMocker(t) + require.NoError(t, bm.CreateFakeIndices(bk.Root(), 0, 1)) + return bk, fs + }, + nReq: 0, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + blk, store := c.setup(t) + req, err := missingBlobRequest(blk, store) + require.NoError(t, err) + require.Equal(t, c.nReq, len(req)) + }) + } +}