Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Beacon API: get blob fix retention cases #13585

Merged
merged 6 commits into from
Feb 6, 2024
Merged

Beacon API: get blob fix retention cases #13585

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e6d472a
fixing the handling for certain cases
james-prysm Feb 5, 2024
d4e0109
fixing tests
james-prysm Feb 5, 2024
051b50f
Merge branch 'develop' into blob-api-handle-retention
james-prysm Feb 6, 2024
41765bb
Merge branch 'develop' into blob-api-handle-retention
james-prysm Feb 6, 2024
4a6113f
Update beacon-chain/rpc/eth/blob/handlers_test.go
james-prysm Feb 6, 2024
3b2070d
update comment based on review
james-prysm Feb 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions beacon-chain/rpc/eth/blob/BUILD.bazel
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this isn't needed if i just use p.BeaconDB.Block()

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
Open in desktop
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
Open in desktop
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
Loading