Skip to content

Commit

Permalink
Beacon API: get blob fix retention cases (#13585)
Browse files Browse the repository at this point in the history 
* fixing the handling for certain cases

* fixing tests

* Update beacon-chain/rpc/eth/blob/handlers_test.go

Co-authored-by: Radosław Kapka <[email protected]>

* update comment based on review

---------

Co-authored-by: Radosław Kapka <[email protected]>
  • Loading branch information
@james-prysm @rkapka
james-prysm and rkapka authored Feb 6, 2024
1 parent 01116f7 commit 1383546
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 31 deletions.
1 change: 1 addition & 0 deletions beacon-chain/rpc/eth/blob/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ go_test(
"//beacon-chain/db/filesystem:go_default_library",
"//beacon-chain/db/testing:go_default_library",
"//beacon-chain/rpc/lookup:go_default_library",
"//beacon-chain/rpc/testutil:go_default_library",
"//beacon-chain/verification:go_default_library",
"//config/params:go_default_library",
"//network/httputil:go_default_library",
Expand Down
107 changes: 93 additions & 14 deletions beacon-chain/rpc/eth/blob/handlers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import (
"net/url"
"strings"
"testing"
"time"

"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/prysmaticlabs/prysm/v4/api/server/structs"
mockChain "github.com/prysmaticlabs/prysm/v4/beacon-chain/blockchain/testing"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/db/filesystem"
testDB "github.com/prysmaticlabs/prysm/v4/beacon-chain/db/testing"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/rpc/lookup"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/rpc/testutil"
"github.com/prysmaticlabs/prysm/v4/beacon-chain/verification"
"github.com/prysmaticlabs/prysm/v4/config/params"
"github.com/prysmaticlabs/prysm/v4/network/httputil"
Expand Down Expand Up @@ -71,8 +73,11 @@ func TestBlobs(t *testing.T) {
writer.Body = &bytes.Buffer{}
blocker := &lookup.BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{Root: blockRoot[:]},
BeaconDB: db,
BlobStorage: bs,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
s := &Server{
Blocker: blocker,
Expand Down Expand Up @@ -116,8 +121,11 @@ func TestBlobs(t *testing.T) {
writer.Body = &bytes.Buffer{}
blocker := &lookup.BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{FinalizedCheckPoint: &eth.Checkpoint{Root: blockRoot[:]}},
BeaconDB: db,
BlobStorage: bs,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
s := &Server{
Blocker: blocker,
Expand All @@ -137,8 +145,11 @@ func TestBlobs(t *testing.T) {
writer.Body = &bytes.Buffer{}
blocker := &lookup.BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{CurrentJustifiedCheckPoint: &eth.Checkpoint{Root: blockRoot[:]}},
BeaconDB: db,
BlobStorage: bs,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
s := &Server{
Blocker: blocker,
Expand All @@ -157,7 +168,10 @@ func TestBlobs(t *testing.T) {
writer := httptest.NewRecorder()
writer.Body = &bytes.Buffer{}
blocker := &lookup.BeaconDbBlocker{
BeaconDB: db,
BeaconDB: db,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BlobStorage: bs,
}
s := &Server{
Expand All @@ -177,7 +191,10 @@ func TestBlobs(t *testing.T) {
writer := httptest.NewRecorder()
writer.Body = &bytes.Buffer{}
blocker := &lookup.BeaconDbBlocker{
BeaconDB: db,
BeaconDB: db,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BlobStorage: bs,
}
s := &Server{
Expand All @@ -198,8 +215,11 @@ func TestBlobs(t *testing.T) {
writer.Body = &bytes.Buffer{}
blocker := &lookup.BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{FinalizedCheckPoint: &eth.Checkpoint{Root: blockRoot[:]}},
BeaconDB: db,
BlobStorage: bs,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
s := &Server{
Blocker: blocker,
Expand All @@ -225,8 +245,11 @@ func TestBlobs(t *testing.T) {
writer.Body = &bytes.Buffer{}
blocker := &lookup.BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{FinalizedCheckPoint: &eth.Checkpoint{Root: blockRoot[:]}},
BeaconDB: db,
BlobStorage: filesystem.NewEphemeralBlobStorage(t), // new ephemeral storage
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: filesystem.NewEphemeralBlobStorage(t), // new ephemeral storage
}
s := &Server{
Blocker: blocker,
Expand All @@ -238,6 +261,59 @@ func TestBlobs(t *testing.T) {
require.NoError(t, json.Unmarshal(writer.Body.Bytes(), resp))
require.Equal(t, len(resp.Data), 0)
})
t.Run("outside retention period returns 200 w/ empty list ", func(t *testing.T) {
u := "http://foo.example/123"
request := httptest.NewRequest("GET", u, nil)
writer := httptest.NewRecorder()
writer.Body = &bytes.Buffer{}
moc := &mockChain.ChainService{FinalizedCheckPoint: &eth.Checkpoint{Root: blockRoot[:]}}
blocker := &lookup.BeaconDbBlocker{
ChainInfoFetcher: moc,
GenesisTimeFetcher: moc, // genesis time is set to 0 here, so it results in current epoch being extremely large
BeaconDB: db,
BlobStorage: bs,
}
s := &Server{
Blocker: blocker,
}

s.Blobs(writer, request)

assert.Equal(t, http.StatusOK, writer.Code)
resp := &structs.SidecarsResponse{}
require.NoError(t, json.Unmarshal(writer.Body.Bytes(), resp))
require.Equal(t, 0, len(resp.Data))
})
t.Run("block without commitments returns 200 w/empty list ", func(t *testing.T) {
denebBlock, _ := util.GenerateTestDenebBlockWithSidecar(t, [32]byte{}, 333, 0)
commitments, err := denebBlock.Block().Body().BlobKzgCommitments()
require.NoError(t, err)
require.Equal(t, len(commitments), 0)
require.NoError(t, db.SaveBlock(context.Background(), denebBlock))

u := "http://foo.example/333"
request := httptest.NewRequest("GET", u, nil)
writer := httptest.NewRecorder()
writer.Body = &bytes.Buffer{}
blocker := &lookup.BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{FinalizedCheckPoint: &eth.Checkpoint{Root: blockRoot[:]}},
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
s := &Server{
Blocker: blocker,
}

s.Blobs(writer, request)

assert.Equal(t, http.StatusOK, writer.Code)
resp := &structs.SidecarsResponse{}
require.NoError(t, json.Unmarshal(writer.Body.Bytes(), resp))
require.Equal(t, 0, len(resp.Data))
})
t.Run("slot before Deneb fork", func(t *testing.T) {
u := "http://foo.example/31"
request := httptest.NewRequest("GET", u, nil)
Expand Down Expand Up @@ -282,8 +358,11 @@ func TestBlobs(t *testing.T) {
writer.Body = &bytes.Buffer{}
blocker := &lookup.BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{FinalizedCheckPoint: &eth.Checkpoint{Root: blockRoot[:]}},
BeaconDB: db,
BlobStorage: bs,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
s := &Server{
Blocker: blocker,
Expand Down
34 changes: 30 additions & 4 deletions beacon-chain/rpc/lookup/blocker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,10 @@ type Blocker interface {

// BeaconDbBlocker is an implementation of Blocker. It retrieves blocks from the beacon chain database.
type BeaconDbBlocker struct {
BeaconDB db.ReadOnlyDatabase
ChainInfoFetcher blockchain.ChainInfoFetcher
BlobStorage *filesystem.BlobStorage
BeaconDB db.ReadOnlyDatabase
ChainInfoFetcher blockchain.ChainInfoFetcher
GenesisTimeFetcher blockchain.TimeFetcher
BlobStorage *filesystem.BlobStorage
}

// Block returns the beacon block for a given identifier. The identifier can be one of:
Expand Down Expand Up @@ -139,6 +140,13 @@ func (p *BeaconDbBlocker) Block(ctx context.Context, id []byte) (interfaces.Read
// - <slot>
// - <hex encoded block root with '0x' prefix>
// - <block root>
//
// cases:
// - no block, 404
// - block exists, no commitment, 200 w/ empty list
// - block exists, has commitments, inside retention period (greater of protocol- or user-specified) serve then w/ 200 unless we hit an error reading them.
// we are technically not supposed to import a block to forkchoice unless we have the blobs, so the nuance here is if we can't find the file and we are inside the protocol-defined retention period, then it's actually a 500.
// - block exists, has commitments, outside retention period (greater of protocol- or user-specified) - ie just like block exists, no commitment
func (p *BeaconDbBlocker) Blobs(ctx context.Context, id string, indices []uint64) ([]*blocks.VerifiedROBlob, *core.RpcError) {
var root []byte
switch id {
Expand Down Expand Up @@ -207,7 +215,25 @@ func (p *BeaconDbBlocker) Blobs(ctx context.Context, id string, indices []uint64
}
}
}

if !p.BeaconDB.HasBlock(ctx, bytesutil.ToBytes32(root)) {
return nil, &core.RpcError{Err: errors.New("block not found"), Reason: core.NotFound}
}
b, err := p.BeaconDB.Block(ctx, bytesutil.ToBytes32(root))
if err != nil {
return nil, &core.RpcError{Err: errors.Wrap(err, "failed to retrieve block from db"), Reason: core.Internal}
}
// if block is not in the retention window return 200 w/ empty list
if !params.WithinDAPeriod(slots.ToEpoch(b.Block().Slot()), slots.ToEpoch(p.GenesisTimeFetcher.CurrentSlot())) {
return make([]*blocks.VerifiedROBlob, 0), nil
}
commitments, err := b.Block().Body().BlobKzgCommitments()
if err != nil {
return nil, &core.RpcError{Err: errors.Wrap(err, "failed to retrieve kzg commitments from block"), Reason: core.Internal}
}
// if there are no commitments return 200 w/ empty list
if len(commitments) == 0 {
return make([]*blocks.VerifiedROBlob, 0), nil
}
if len(indices) == 0 {
m, err := p.BlobStorage.Indices(bytesutil.ToBytes32(root))
if err != nil {
Expand Down
42 changes: 32 additions & 10 deletions beacon-chain/rpc/lookup/blocker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"net/http"
"reflect"
"testing"
"time"

"github.com/ethereum/go-ethereum/common/hexutil"
mockChain "github.com/prysmaticlabs/prysm/v4/beacon-chain/blockchain/testing"
Expand Down Expand Up @@ -180,8 +181,11 @@ func TestGetBlob(t *testing.T) {
t.Run("head", func(t *testing.T) {
blocker := &BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{Root: blockRoot[:]},
BeaconDB: db,
BlobStorage: bs,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
verifiedBlobs, rpcErr := blocker.Blobs(ctx, "head", nil)
assert.Equal(t, rpcErr == nil, true)
Expand Down Expand Up @@ -214,8 +218,11 @@ func TestGetBlob(t *testing.T) {
t.Run("finalized", func(t *testing.T) {
blocker := &BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blockRoot[:]}},
BeaconDB: db,
BlobStorage: bs,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}

verifiedBlobs, rpcErr := blocker.Blobs(ctx, "finalized", nil)
Expand All @@ -225,8 +232,11 @@ func TestGetBlob(t *testing.T) {
t.Run("justified", func(t *testing.T) {
blocker := &BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{CurrentJustifiedCheckPoint: &ethpbalpha.Checkpoint{Root: blockRoot[:]}},
BeaconDB: db,
BlobStorage: bs,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}

verifiedBlobs, rpcErr := blocker.Blobs(ctx, "justified", nil)
Expand All @@ -235,6 +245,9 @@ func TestGetBlob(t *testing.T) {
})
t.Run("root", func(t *testing.T) {
blocker := &BeaconDbBlocker{
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
Expand All @@ -244,6 +257,9 @@ func TestGetBlob(t *testing.T) {
})
t.Run("slot", func(t *testing.T) {
blocker := &BeaconDbBlocker{
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
Expand All @@ -254,8 +270,11 @@ func TestGetBlob(t *testing.T) {
t.Run("one blob only", func(t *testing.T) {
blocker := &BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blockRoot[:]}},
BeaconDB: db,
BlobStorage: bs,
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
verifiedBlobs, rpcErr := blocker.Blobs(ctx, "123", []uint64{2})
assert.Equal(t, rpcErr == nil, true)
Expand All @@ -270,8 +289,11 @@ func TestGetBlob(t *testing.T) {
t.Run("no blobs returns an empty array", func(t *testing.T) {
blocker := &BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{FinalizedCheckPoint: &ethpbalpha.Checkpoint{Root: blockRoot[:]}},
BeaconDB: db,
BlobStorage: filesystem.NewEphemeralBlobStorage(t),
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: filesystem.NewEphemeralBlobStorage(t),
}
verifiedBlobs, rpcErr := blocker.Blobs(ctx, "123", nil)
assert.Equal(t, rpcErr == nil, true)
Expand Down
7 changes: 4 additions & 3 deletions beacon-chain/rpc/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,10 @@ func (s *Service) Start() {
ReplayerBuilder: ch,
}
blocker := &lookup.BeaconDbBlocker{
BeaconDB: s.cfg.BeaconDB,
ChainInfoFetcher: s.cfg.ChainInfoFetcher,
BlobStorage: s.cfg.BlobStorage,
BeaconDB: s.cfg.BeaconDB,
ChainInfoFetcher: s.cfg.ChainInfoFetcher,
GenesisTimeFetcher: s.cfg.GenesisTimeFetcher,
BlobStorage: s.cfg.BlobStorage,
}
rewardFetcher := &rewards.BlockRewardService{Replayer: ch}

Expand Down

0 comments on commit 1383546

Please sign in to comment.