Skip to content

Commit 783f4ca

Browse files
committed
blockservice: remove inner fields access methods
We shouldn't expose inner implementation details of the blockservice to consumers. For example by using the blockstore for the .Has call, the gateway skipped verifcid and blocker checks. Probably fine but not we want to do.
1 parent a76d47d commit 783f4ca

File tree

3 files changed

+32
-37
lines changed

3 files changed

+32
-37
lines changed

blockservice/blockservice.go

+13-23
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,8 @@ type BlockService interface {
4747
io.Closer
4848
BlockGetter
4949

50-
// Blockstore returns a reference to the underlying blockstore
51-
Blockstore() blockstore.Blockstore
52-
53-
// Exchange returns a reference to the underlying exchange (usually bitswap)
54-
Exchange() exchange.Interface
50+
// Has indicates if a block is present locally.
51+
Has(ctx context.Context, c cid.Cid) (bool, error)
5552

5653
// AddBlock puts a given block to the underlying datastore
5754
AddBlock(ctx context.Context, o blocks.Block) error
@@ -125,20 +122,6 @@ func New(bs blockstore.Blockstore, exchange exchange.Interface, opts ...Option)
125122
return service
126123
}
127124

128-
// Blockstore returns the blockstore behind this blockservice.
129-
func (s *blockService) Blockstore() blockstore.Blockstore {
130-
return s.blockstore
131-
}
132-
133-
// Exchange returns the exchange behind this blockservice.
134-
func (s *blockService) Exchange() exchange.Interface {
135-
return s.exchange
136-
}
137-
138-
func (s *blockService) Allowlist() verifcid.Allowlist {
139-
return s.allowlist
140-
}
141-
142125
func (s *blockService) NewSession(ctx context.Context) BlockGetter {
143126
ses := s.grabSessionFromContext(ctx)
144127
if ses != nil {
@@ -439,14 +422,13 @@ func (s *session) grabSession() exchange.Fetcher {
439422
s.sesctx = nil // early gc
440423
}()
441424

442-
ex := s.bs.Exchange()
443-
if ex == nil {
425+
if s.bs.exchange == nil {
444426
return
445427
}
446-
s.ses = ex // always fallback to non session fetches
447428

448-
sesEx, ok := ex.(exchange.SessionExchange)
429+
sesEx, ok := s.bs.exchange.(exchange.SessionExchange)
449430
if !ok {
431+
s.ses = s.bs.exchange // always fallback to non session fetches
450432
return
451433
}
452434
s.ses = sesEx.NewSession(s.sesctx)
@@ -491,3 +473,11 @@ func (s *blockService) grabSessionFromContext(ctx context.Context) *session {
491473

492474
return sss
493475
}
476+
477+
func (s *blockService) Has(ctx context.Context, c cid.Cid) (bool, error) {
478+
if err := verifcid.ValidateCid(s.allowlist, c); err != nil {
479+
return false, err
480+
}
481+
482+
return s.blockstore.Has(ctx, c)
483+
}

gateway/blocks_backend.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"time"
1212

1313
"github.com/ipfs/boxo/blockservice"
14-
blockstore "github.com/ipfs/boxo/blockstore"
1514
"github.com/ipfs/boxo/fetcher"
1615
bsfetcher "github.com/ipfs/boxo/fetcher/impl/blockservice"
1716
"github.com/ipfs/boxo/files"
@@ -51,7 +50,6 @@ import (
5150

5251
// BlocksBackend is an [IPFSBackend] implementation based on a [blockservice.BlockService].
5352
type BlocksBackend struct {
54-
blockStore blockstore.Blockstore
5553
blockService blockservice.BlockService
5654
dagService format.DAGService
5755
resolver resolver.Resolver
@@ -143,7 +141,6 @@ func NewBlocksBackend(blockService blockservice.BlockService, opts ...BlocksBack
143141
}
144142

145143
return &BlocksBackend{
146-
blockStore: blockService.Blockstore(),
147144
blockService: blockService,
148145
dagService: dagService,
149146
resolver: r,
@@ -680,7 +677,7 @@ func (bb *BlocksBackend) IsCached(ctx context.Context, p path.Path) bool {
680677
return false
681678
}
682679

683-
has, _ := bb.blockStore.Has(ctx, rp.RootCid())
680+
has, _ := bb.blockService.Has(ctx, rp.RootCid())
684681
return has
685682
}
686683

ipld/merkledag/merkledag_test.go

+18-10
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,13 @@ import (
1616
"testing"
1717
"time"
1818

19+
testinstance "github.com/ipfs/boxo/bitswap/testinstance"
20+
tn "github.com/ipfs/boxo/bitswap/testnet"
1921
. "github.com/ipfs/boxo/ipld/merkledag"
2022
mdpb "github.com/ipfs/boxo/ipld/merkledag/pb"
2123
dstest "github.com/ipfs/boxo/ipld/merkledag/test"
24+
mockrouting "github.com/ipfs/boxo/routing/mock"
25+
delay "github.com/ipfs/go-ipfs-delay"
2226

2327
bserv "github.com/ipfs/boxo/blockservice"
2428
bstest "github.com/ipfs/boxo/blockservice/test"
@@ -507,10 +511,12 @@ func TestCantGet(t *testing.T) {
507511
}
508512

509513
func TestFetchGraph(t *testing.T) {
510-
var dservs []ipld.DAGService
511-
bsis := bstest.Mocks(2)
512-
for _, bsi := range bsis {
513-
dservs = append(dservs, NewDAGService(bsi))
514+
net := tn.VirtualNetwork(mockrouting.NewServer(), delay.Fixed(0))
515+
sg := testinstance.NewTestInstanceGenerator(net, nil, nil)
516+
instances := sg.Instances(2)
517+
dservs := [2]ipld.DAGService{
518+
NewDAGService(bserv.New(instances[0].Blockstore(), instances[0].Exchange)),
519+
NewDAGService(bserv.New(instances[1].Blockstore(), instances[1].Exchange)),
514520
}
515521

516522
read := io.LimitReader(u.NewTimeSeededRand(), 1024*32)
@@ -522,7 +528,7 @@ func TestFetchGraph(t *testing.T) {
522528
}
523529

524530
// create an offline dagstore and ensure all blocks were fetched
525-
bs := bserv.New(bsis[1].Blockstore(), offline.Exchange(bsis[1].Blockstore()))
531+
bs := bserv.New(instances[1].Blockstore(), nil)
526532

527533
offlineDS := NewDAGService(bs)
528534

@@ -547,10 +553,12 @@ func TestFetchGraphWithDepthLimit(t *testing.T) {
547553
}
548554

549555
testF := func(t *testing.T, tc testcase) {
550-
var dservs []ipld.DAGService
551-
bsis := bstest.Mocks(2)
552-
for _, bsi := range bsis {
553-
dservs = append(dservs, NewDAGService(bsi))
556+
net := tn.VirtualNetwork(mockrouting.NewServer(), delay.Fixed(0))
557+
sg := testinstance.NewTestInstanceGenerator(net, nil, nil)
558+
instances := sg.Instances(2)
559+
dservs := [2]ipld.DAGService{
560+
NewDAGService(bserv.New(instances[0].Blockstore(), instances[0].Exchange)),
561+
NewDAGService(bserv.New(instances[1].Blockstore(), instances[1].Exchange)),
554562
}
555563

556564
root := makeDepthTestingGraph(t, dservs[0])
@@ -561,7 +569,7 @@ func TestFetchGraphWithDepthLimit(t *testing.T) {
561569
}
562570

563571
// create an offline dagstore and ensure all blocks were fetched
564-
bs := bserv.New(bsis[1].Blockstore(), offline.Exchange(bsis[1].Blockstore()))
572+
bs := bserv.New(instances[1].Blockstore(), offline.Exchange(instances[1].Blockstore()))
565573

566574
offlineDS := NewDAGService(bs)
567575

0 commit comments

Comments
 (0)