diff --git a/beacon-chain/das/cache.go b/beacon-chain/das/cache.go index 3e8d781bead9..9702743f944e 100644 --- a/beacon-chain/das/cache.go +++ b/beacon-chain/das/cache.go @@ -92,7 +92,7 @@ func (e *cacheEntry) filter(root [32]byte, kc safeCommitmentArray) ([]blocks.ROB if e.diskSummary.AllAvailable(kc.count()) { return nil, nil } - scs := make([]blocks.ROBlob, kc.count()) + scs := make([]blocks.ROBlob, 0, kc.count()) for i := uint64(0); i < fieldparams.MaxBlobsPerBlock; i++ { // We already have this blob, we don't need to write it or validate it. if e.diskSummary.HasIndex(i) { @@ -111,7 +111,7 @@ func (e *cacheEntry) filter(root [32]byte, kc safeCommitmentArray) ([]blocks.ROB if !bytes.Equal(kc[i], e.scs[i].KzgCommitment) { return nil, errors.Wrapf(errCommitmentMismatch, "root=%#x, index=%#x, commitment=%#x, block commitment=%#x", root, i, e.scs[i].KzgCommitment, kc[i]) } - scs[i] = *e.scs[i] + scs = append(scs, *e.scs[i]) } return scs, nil diff --git a/beacon-chain/das/cache_test.go b/beacon-chain/das/cache_test.go index 923503b9895c..cd271fc516ed 100644 --- a/beacon-chain/das/cache_test.go +++ b/beacon-chain/das/cache_test.go @@ -3,9 +3,14 @@ package das import ( "testing" + "github.com/prysmaticlabs/prysm/v5/beacon-chain/db/filesystem" + "github.com/prysmaticlabs/prysm/v5/config/params" + "github.com/prysmaticlabs/prysm/v5/consensus-types/blocks" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" "github.com/prysmaticlabs/prysm/v5/testing/require" + "github.com/prysmaticlabs/prysm/v5/testing/util" + "github.com/prysmaticlabs/prysm/v5/time/slots" ) func TestCacheEnsureDelete(t *testing.T) { @@ -23,3 +28,145 @@ func TestCacheEnsureDelete(t *testing.T) { var nilEntry *cacheEntry require.Equal(t, nilEntry, c.entries[k]) } + +type filterTestCaseSetupFunc func(t *testing.T) (*cacheEntry, safeCommitmentArray, []blocks.ROBlob) + +func filterTestCaseSetup(slot primitives.Slot, nBlobs int, onDisk []int, numExpected int) filterTestCaseSetupFunc { + return func(t *testing.T) (*cacheEntry, safeCommitmentArray, []blocks.ROBlob) { + blk, blobs := util.GenerateTestDenebBlockWithSidecar(t, [32]byte{}, slot, nBlobs) + commits, err := commitmentsToCheck(blk, blk.Block().Slot()) + require.NoError(t, err) + entry := &cacheEntry{} + if len(onDisk) > 0 { + od := map[[32]byte][]int{blk.Root(): onDisk} + sumz := filesystem.NewMockBlobStorageSummarizer(t, od) + sum := sumz.Summary(blk.Root()) + entry.setDiskSummary(sum) + } + expected := make([]blocks.ROBlob, 0, nBlobs) + for i := 0; i < commits.count(); i++ { + if entry.diskSummary.HasIndex(uint64(i)) { + continue + } + // If we aren't telling the cache a blob is on disk, add it to the expected list and stash. + expected = append(expected, blobs[i]) + require.NoError(t, entry.stash(&blobs[i])) + } + require.Equal(t, numExpected, len(expected)) + return entry, commits, expected + } +} + +func TestFilterDiskSummary(t *testing.T) { + denebSlot, err := slots.EpochStart(params.BeaconConfig().DenebForkEpoch) + require.NoError(t, err) + cases := []struct { + name string + setup filterTestCaseSetupFunc + }{ + { + name: "full blobs, all on disk", + setup: filterTestCaseSetup(denebSlot, 6, []int{0, 1, 2, 3, 4, 5}, 0), + }, + { + name: "full blobs, first on disk", + setup: filterTestCaseSetup(denebSlot, 6, []int{0}, 5), + }, + { + name: "full blobs, middle on disk", + setup: filterTestCaseSetup(denebSlot, 6, []int{2}, 5), + }, + { + name: "full blobs, last on disk", + setup: filterTestCaseSetup(denebSlot, 6, []int{5}, 5), + }, + { + name: "full blobs, none on disk", + setup: filterTestCaseSetup(denebSlot, 6, []int{}, 6), + }, + { + name: "one commitment, on disk", + setup: filterTestCaseSetup(denebSlot, 1, []int{0}, 0), + }, + { + name: "one commitment, not on disk", + setup: filterTestCaseSetup(denebSlot, 1, []int{}, 1), + }, + { + name: "two commitments, first on disk", + setup: filterTestCaseSetup(denebSlot, 2, []int{0}, 1), + }, + { + name: "two commitments, last on disk", + setup: filterTestCaseSetup(denebSlot, 2, []int{1}, 1), + }, + { + name: "two commitments, none on disk", + setup: filterTestCaseSetup(denebSlot, 2, []int{}, 2), + }, + { + name: "two commitments, all on disk", + setup: filterTestCaseSetup(denebSlot, 2, []int{0, 1}, 0), + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + entry, commits, expected := c.setup(t) + // first (root) argument doesn't matter, it is just for logs + got, err := entry.filter([32]byte{}, commits) + require.NoError(t, err) + require.Equal(t, len(expected), len(got)) + }) + } +} + +func TestFilter(t *testing.T) { + denebSlot, err := slots.EpochStart(params.BeaconConfig().DenebForkEpoch) + require.NoError(t, err) + cases := []struct { + name string + setup func(t *testing.T) (*cacheEntry, safeCommitmentArray, []blocks.ROBlob) + err error + }{ + { + name: "commitments mismatch - extra sidecar", + setup: func(t *testing.T) (*cacheEntry, safeCommitmentArray, []blocks.ROBlob) { + entry, commits, expected := filterTestCaseSetup(denebSlot, 6, []int{0, 1}, 4)(t) + commits[5] = nil + return entry, commits, expected + }, + err: errCommitmentMismatch, + }, + { + name: "sidecar missing", + setup: func(t *testing.T) (*cacheEntry, safeCommitmentArray, []blocks.ROBlob) { + entry, commits, expected := filterTestCaseSetup(denebSlot, 6, []int{0, 1}, 4)(t) + entry.scs[5] = nil + return entry, commits, expected + }, + err: errMissingSidecar, + }, + { + name: "commitments mismatch - different bytes", + setup: func(t *testing.T) (*cacheEntry, safeCommitmentArray, []blocks.ROBlob) { + entry, commits, expected := filterTestCaseSetup(denebSlot, 6, []int{0, 1}, 4)(t) + entry.scs[5].KzgCommitment = []byte("nope") + return entry, commits, expected + }, + err: errCommitmentMismatch, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + entry, commits, expected := c.setup(t) + // first (root) argument doesn't matter, it is just for logs + got, err := entry.filter([32]byte{}, commits) + if c.err != nil { + require.ErrorIs(t, err, c.err) + return + } + require.NoError(t, err) + require.Equal(t, len(expected), len(got)) + }) + } +} diff --git a/beacon-chain/sync/rpc_send_request_test.go b/beacon-chain/sync/rpc_send_request_test.go index c8cabe66a645..4fb2321e60e6 100644 --- a/beacon-chain/sync/rpc_send_request_test.go +++ b/beacon-chain/sync/rpc_send_request_test.go @@ -482,7 +482,10 @@ func TestSendRequest_SendBeaconBlocksByRootRequest(t *testing.T) { func TestBlobValidatorFromRootReq(t *testing.T) { rootA := bytesutil.PadTo([]byte("valid"), 32) rootB := bytesutil.PadTo([]byte("invalid"), 32) - header := ðpb.SignedBeaconBlockHeader{} + header := ðpb.SignedBeaconBlockHeader{ + Header: ðpb.BeaconBlockHeader{Slot: 0}, + Signature: make([]byte, fieldparams.BLSSignatureLength), + } blobSidecarA0 := util.GenerateTestDenebBlobSidecar(t, bytesutil.ToBytes32(rootA), header, 0, []byte{}, make([][]byte, 0)) blobSidecarA1 := util.GenerateTestDenebBlobSidecar(t, bytesutil.ToBytes32(rootA), header, 1, []byte{}, make([][]byte, 0)) blobSidecarB0 := util.GenerateTestDenebBlobSidecar(t, bytesutil.ToBytes32(rootB), header, 0, []byte{}, make([][]byte, 0)) @@ -590,7 +593,8 @@ func TestBlobValidatorFromRangeReq(t *testing.T) { t.Run(c.name, func(t *testing.T) { vf := blobValidatorFromRangeReq(c.req) header := ðpb.SignedBeaconBlockHeader{ - Header: ðpb.BeaconBlockHeader{Slot: c.responseSlot}, + Header: ðpb.BeaconBlockHeader{Slot: c.responseSlot}, + Signature: make([]byte, fieldparams.BLSSignatureLength), } sc := util.GenerateTestDenebBlobSidecar(t, [32]byte{}, header, 0, []byte{}, make([][]byte, 0)) err := vf(sc) diff --git a/beacon-chain/verification/batch.go b/beacon-chain/verification/batch.go index c84923769958..080a74044b6e 100644 --- a/beacon-chain/verification/batch.go +++ b/beacon-chain/verification/batch.go @@ -46,10 +46,12 @@ func (batch *BlobBatchVerifier) VerifiedROBlobs(ctx context.Context, blk blocks. if len(scs) == 0 { return nil, nil } + blkSig := blk.Signature() // We assume the proposer is validated wrt the block in batch block processing before performing the DA check. // So at this stage we just need to make sure the value being signed and signature bytes match the block. for i := range scs { - if blk.Signature() != bytesutil.ToBytes96(scs[i].SignedBlockHeader.Signature) { + blobSig := bytesutil.ToBytes96(scs[i].SignedBlockHeader.Signature) + if blkSig != blobSig { return nil, ErrBatchSignatureMismatch } // Extra defensive check to make sure the roots match. This should be unnecessary in practice since the root from diff --git a/consensus-types/blocks/kzg_test.go b/consensus-types/blocks/kzg_test.go index 9febaafef908..8bc6c5498315 100644 --- a/consensus-types/blocks/kzg_test.go +++ b/consensus-types/blocks/kzg_test.go @@ -244,7 +244,8 @@ func Test_VerifyKZGInclusionProof(t *testing.T) { StateRoot: make([]byte, 32), } signedHeader := ðpb.SignedBeaconBlockHeader{ - Header: header, + Header: header, + Signature: make([]byte, fieldparams.BLSSignatureLength), } sidecar := ðpb.BlobSidecar{ Index: uint64(index), diff --git a/consensus-types/blocks/roblob.go b/consensus-types/blocks/roblob.go index bf3fbef68b90..a55a9b03bc19 100644 --- a/consensus-types/blocks/roblob.go +++ b/consensus-types/blocks/roblob.go @@ -1,35 +1,42 @@ package blocks import ( - "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" ) -var errNilBlockHeader = errors.New("received nil beacon block header") - // ROBlob represents a read-only blob sidecar with its block root. type ROBlob struct { *ethpb.BlobSidecar root [32]byte } +func roblobNilCheck(b *ethpb.BlobSidecar) error { + if b == nil { + return errNilBlob + } + if b.SignedBlockHeader == nil || b.SignedBlockHeader.Header == nil { + return errNilBlockHeader + } + if len(b.SignedBlockHeader.Signature) == 0 { + return errMissingBlockSignature + } + return nil +} + // NewROBlobWithRoot creates a new ROBlob with a given root. func NewROBlobWithRoot(b *ethpb.BlobSidecar, root [32]byte) (ROBlob, error) { - if b == nil { - return ROBlob{}, errNilBlock + if err := roblobNilCheck(b); err != nil { + return ROBlob{}, err } return ROBlob{BlobSidecar: b, root: root}, nil } // NewROBlob creates a new ROBlob by computing the HashTreeRoot of the header. func NewROBlob(b *ethpb.BlobSidecar) (ROBlob, error) { - if b == nil { - return ROBlob{}, errNilBlock - } - if b.SignedBlockHeader == nil || b.SignedBlockHeader.Header == nil { - return ROBlob{}, errNilBlockHeader + if err := roblobNilCheck(b); err != nil { + return ROBlob{}, err } root, err := b.SignedBlockHeader.Header.HashTreeRoot() if err != nil { diff --git a/consensus-types/blocks/roblob_test.go b/consensus-types/blocks/roblob_test.go index eb2490360372..12b7bfd13ff4 100644 --- a/consensus-types/blocks/roblob_test.go +++ b/consensus-types/blocks/roblob_test.go @@ -5,50 +5,94 @@ import ( fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" + "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v5/testing/assert" + "github.com/prysmaticlabs/prysm/v5/testing/require" ) -func TestNewROBlobWithRoot(t *testing.T) { - sidecar := ðpb.BlobSidecar{} - root := [32]byte{} - - blob, err := NewROBlobWithRoot(sidecar, root) - assert.NoError(t, err) - assert.Equal(t, root, blob.BlockRoot()) - - blob, err = NewROBlobWithRoot(nil, root) - assert.Equal(t, errNilBlock, err) -} - -// TestNewROBlob tests the NewROBlob function. -func TestNewROBlob(t *testing.T) { - h := ðpb.SignedBeaconBlockHeader{ - Header: ðpb.BeaconBlockHeader{ - ParentRoot: make([]byte, fieldparams.RootLength), - StateRoot: make([]byte, fieldparams.RootLength), - BodyRoot: make([]byte, fieldparams.RootLength), +func TestROBlobNilChecks(t *testing.T) { + cases := []struct { + name string + bfunc func(t *testing.T) *ethpb.BlobSidecar + err error + root []byte + }{ + { + name: "nil signed blob", + bfunc: func(t *testing.T) *ethpb.BlobSidecar { + return nil + }, + err: errNilBlob, + root: bytesutil.PadTo([]byte("sup"), 32), + }, + { + name: "nil signed block header", + bfunc: func(t *testing.T) *ethpb.BlobSidecar { + return ðpb.BlobSidecar{ + SignedBlockHeader: nil, + } + }, + err: errNilBlockHeader, + root: bytesutil.PadTo([]byte("sup"), 32), + }, + { + name: "nil inner header", + bfunc: func(t *testing.T) *ethpb.BlobSidecar { + return ðpb.BlobSidecar{ + SignedBlockHeader: ðpb.SignedBeaconBlockHeader{ + Header: nil, + }, + } + }, + err: errNilBlockHeader, + root: bytesutil.PadTo([]byte("sup"), 32), + }, + { + name: "nil signature", + bfunc: func(t *testing.T) *ethpb.BlobSidecar { + return ðpb.BlobSidecar{ + SignedBlockHeader: ðpb.SignedBeaconBlockHeader{ + Header: ðpb.BeaconBlockHeader{ + ParentRoot: make([]byte, fieldparams.RootLength), + StateRoot: make([]byte, fieldparams.RootLength), + BodyRoot: make([]byte, fieldparams.RootLength), + }, + Signature: nil, + }, + } + }, + err: errMissingBlockSignature, + root: bytesutil.PadTo([]byte("sup"), 32), }, - Signature: make([]byte, fieldparams.BLSSignatureLength), } - sidecar := ðpb.BlobSidecar{ - SignedBlockHeader: h, + for _, c := range cases { + t.Run(c.name+" NewROBlob", func(t *testing.T) { + b := c.bfunc(t) + bl, err := NewROBlob(b) + if c.err != nil { + require.ErrorIs(t, err, c.err) + } else { + require.NoError(t, err) + hr, err := b.SignedBlockHeader.HashTreeRoot() + require.NoError(t, err) + require.Equal(t, hr, bl.BlockRoot()) + } + }) + if len(c.root) == 0 { + continue + } + t.Run(c.name+" NewROBlobWithRoot", func(t *testing.T) { + b := c.bfunc(t) + // We want the same validation when specifying a root. + bl, err := NewROBlobWithRoot(b, bytesutil.ToBytes32(c.root)) + if c.err != nil { + require.ErrorIs(t, err, c.err) + } else { + assert.Equal(t, bytesutil.ToBytes32(c.root), bl.BlockRoot()) + } + }) } - - blob, err := NewROBlob(sidecar) - assert.NoError(t, err) - assert.NotNil(t, blob) - - _, err = NewROBlob(nil) - assert.Equal(t, errNilBlock, err) - - sidecar.SignedBlockHeader = nil - _, err = NewROBlob(sidecar) - assert.Equal(t, errNilBlockHeader, err) - - sidecar.SignedBlockHeader = ðpb.SignedBeaconBlockHeader{} - _, err = NewROBlob(sidecar) - assert.Equal(t, errNilBlockHeader, err) } func TestBlockRoot(t *testing.T) { diff --git a/consensus-types/blocks/roblock_test.go b/consensus-types/blocks/roblock_test.go index 841571cb38d9..45abcf994e8e 100644 --- a/consensus-types/blocks/roblock_test.go +++ b/consensus-types/blocks/roblock_test.go @@ -4,9 +4,11 @@ import ( "sort" "testing" + "github.com/prysmaticlabs/prysm/v5/consensus-types/interfaces" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v5/encoding/bytesutil" eth "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" + "github.com/prysmaticlabs/prysm/v5/runtime/version" "github.com/prysmaticlabs/prysm/v5/testing/require" ) @@ -88,3 +90,103 @@ func testROBlock(t *testing.T, slot primitives.Slot, root [32]byte) ROBlock { root: root, } } + +func TestROBlockNilChecks(t *testing.T) { + cases := []struct { + name string + bfunc func(t *testing.T) interfaces.SignedBeaconBlock + err error + root []byte + }{ + { + name: "happy path", + bfunc: func(t *testing.T) interfaces.SignedBeaconBlock { + b, err := NewSignedBeaconBlock(hydrateSignedBeaconBlock()) + require.NoError(t, err) + return b + }, + }, + { + name: "happy path - with root", + bfunc: func(t *testing.T) interfaces.SignedBeaconBlock { + b, err := NewSignedBeaconBlock(hydrateSignedBeaconBlock()) + require.NoError(t, err) + return b + }, + root: bytesutil.PadTo([]byte("sup"), 32), + }, + { + name: "nil signed block", + bfunc: func(t *testing.T) interfaces.SignedBeaconBlock { + return nil + }, + err: ErrNilSignedBeaconBlock, + }, + { + name: "nil signed block - with root", + bfunc: func(t *testing.T) interfaces.SignedBeaconBlock { + return nil + }, + err: ErrNilSignedBeaconBlock, + root: bytesutil.PadTo([]byte("sup"), 32), + }, + { + name: "nil inner block", + bfunc: func(t *testing.T) interfaces.SignedBeaconBlock { + return &SignedBeaconBlock{ + version: version.Deneb, + block: nil, + signature: bytesutil.ToBytes96(nil), + } + }, + err: ErrNilSignedBeaconBlock, + }, + { + name: "nil inner block", + bfunc: func(t *testing.T) interfaces.SignedBeaconBlock { + return &SignedBeaconBlock{ + version: version.Deneb, + block: nil, + signature: bytesutil.ToBytes96(nil), + } + }, + err: ErrNilSignedBeaconBlock, + }, + { + name: "nil block body", + bfunc: func(t *testing.T) interfaces.SignedBeaconBlock { + bb := &BeaconBlock{ + version: version.Deneb, + slot: 0, + proposerIndex: 0, + parentRoot: bytesutil.ToBytes32(nil), + stateRoot: bytesutil.ToBytes32(nil), + body: nil, + } + return &SignedBeaconBlock{ + version: version.Deneb, + block: bb, + signature: bytesutil.ToBytes96(nil), + } + }, + err: ErrNilSignedBeaconBlock, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + b := c.bfunc(t) + var err error + if len(c.root) == 0 { + _, err = NewROBlock(b) + } else { + _, err = NewROBlockWithRoot(b, bytesutil.ToBytes32(c.root)) + } + if c.err != nil { + require.ErrorIs(t, err, c.err) + return + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/consensus-types/blocks/types.go b/consensus-types/blocks/types.go index c714e297268e..0ad61da033e0 100644 --- a/consensus-types/blocks/types.go +++ b/consensus-types/blocks/types.go @@ -27,10 +27,13 @@ const ( var ( // ErrUnsupportedVersion for beacon block methods. ErrUnsupportedVersion = errors.New("unsupported beacon block version") + errNilBlob = errors.New("received nil blob sidecar") errNilBlock = errors.New("received nil beacon block") errNilBlockBody = errors.New("received nil beacon block body") errIncorrectBlockVersion = errors.New(incorrectBlockVersion) errIncorrectBodyVersion = errors.New(incorrectBodyVersion) + errNilBlockHeader = errors.New("received nil beacon block header") + errMissingBlockSignature = errors.New("received nil beacon block signature") ) // BeaconBlockBody is the main beacon block body structure. It can represent any block type.