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.

Author: Jorropo

blockservice/blockservice.go

@@ -47,11 +47,8 @@ type BlockService interface {
 	io.Closer
 	BlockGetter
 
-	// Blockstore returns a reference to the underlying blockstore
-	Blockstore() blockstore.Blockstore
-
-	// Exchange returns a reference to the underlying exchange (usually bitswap)
-	Exchange() exchange.Interface
+	// Has indicates if a block is present locally.
+	Has(ctx context.Context, c cid.Cid) (bool, error)
 
 	// AddBlock puts a given block to the underlying datastore
 	AddBlock(ctx context.Context, o blocks.Block) error
@@ -132,21 +129,6 @@ func New(bs blockstore.Blockstore, exchange exchange.Interface, opts ...Option)
 func NewWriteThrough(bs blockstore.Blockstore, exchange exchange.Interface) BlockService {
 	return New(bs, exchange, WriteThrough())
 }
-
-// Blockstore returns the blockstore behind this blockservice.
-func (s *blockService) Blockstore() blockstore.Blockstore {
-	return s.blockstore
-}
-
-// Exchange returns the exchange behind this blockservice.
-func (s *blockService) Exchange() exchange.Interface {
-	return s.exchange
-}
-
-func (s *blockService) Allowlist() verifcid.Allowlist {
-	return s.allowlist
-}
-
 func (s *blockService) NewSession(ctx context.Context) BlockGetter {
 	ses := s.grabSessionFromContext(ctx)
 	if ses != nil {
@@ -447,14 +429,13 @@ func (s *session) grabSession() exchange.Fetcher {
 			s.sesctx = nil // early gc
 		}()
 
-		ex := s.bs.Exchange()
-		if ex == nil {
+		if s.bs.exchange == nil {
 			return
 		}
-		s.ses = ex // always fallback to non session fetches
 
-		sesEx, ok := ex.(exchange.SessionExchange)
+		sesEx, ok := s.bs.exchange.(exchange.SessionExchange)
 		if !ok {
+			s.ses = s.bs.exchange // always fallback to non session fetches
 			return
 		}
 		s.ses = sesEx.NewSession(s.sesctx)
@@ -499,3 +480,11 @@ func (s *blockService) grabSessionFromContext(ctx context.Context) *session {
 
 	return sss
 }
+
+func (s *blockService) Has(ctx context.Context, c cid.Cid) (bool, error) {
+	if err := verifcid.ValidateCid(s.allowlist, c); err != nil {
+		return false, err
+	}
+
+	return s.blockstore.Has(ctx, c)
+}

gateway/blocks_backend.go

@@ -11,7 +11,6 @@ import (
 	"time"
 
 	"github.com/ipfs/boxo/blockservice"
-	blockstore "github.com/ipfs/boxo/blockstore"
 	"github.com/ipfs/boxo/fetcher"
 	bsfetcher "github.com/ipfs/boxo/fetcher/impl/blockservice"
 	"github.com/ipfs/boxo/files"
@@ -51,7 +50,6 @@ import (
 
 // BlocksBackend is an [IPFSBackend] implementation based on a [blockservice.BlockService].
 type BlocksBackend struct {
-	blockStore   blockstore.Blockstore
 	blockService blockservice.BlockService
 	dagService   format.DAGService
 	resolver     resolver.Resolver
@@ -143,7 +141,6 @@ func NewBlocksBackend(blockService blockservice.BlockService, opts ...BlocksBack
 	}
 
 	return &BlocksBackend{
-		blockStore:   blockService.Blockstore(),
 		blockService: blockService,
 		dagService:   dagService,
 		resolver:     r,
@@ -680,7 +677,7 @@ func (bb *BlocksBackend) IsCached(ctx context.Context, p path.Path) bool {
 		return false
 	}
 
-	has, _ := bb.blockStore.Has(ctx, rp.RootCid())
+	has, _ := bb.blockService.Has(ctx, rp.RootCid())
 	return has
 }
 

ipld/merkledag/merkledag_test.go

@@ -16,9 +16,13 @@ import (
 	"testing"
 	"time"
 
+	testinstance "github.com/ipfs/boxo/bitswap/testinstance"
+	tn "github.com/ipfs/boxo/bitswap/testnet"
 	. "github.com/ipfs/boxo/ipld/merkledag"
 	mdpb "github.com/ipfs/boxo/ipld/merkledag/pb"
 	dstest "github.com/ipfs/boxo/ipld/merkledag/test"
+	mockrouting "github.com/ipfs/boxo/routing/mock"
+	delay "github.com/ipfs/go-ipfs-delay"
 
 	bserv "github.com/ipfs/boxo/blockservice"
 	bstest "github.com/ipfs/boxo/blockservice/test"
@@ -507,10 +511,12 @@ func TestCantGet(t *testing.T) {
 }
 
 func TestFetchGraph(t *testing.T) {
-	var dservs []ipld.DAGService
-	bsis := bstest.Mocks(2)
-	for _, bsi := range bsis {
-		dservs = append(dservs, NewDAGService(bsi))
+	net := tn.VirtualNetwork(mockrouting.NewServer(), delay.Fixed(0))
+	sg := testinstance.NewTestInstanceGenerator(net, nil, nil)
+	instances := sg.Instances(2)
+	dservs := [2]ipld.DAGService{
+		NewDAGService(bserv.New(instances[0].Blockstore(), instances[0].Exchange)),
+		NewDAGService(bserv.New(instances[1].Blockstore(), instances[1].Exchange)),
 	}
 
 	read := io.LimitReader(u.NewTimeSeededRand(), 1024*32)
@@ -522,7 +528,7 @@ func TestFetchGraph(t *testing.T) {
 	}
 
 	// create an offline dagstore and ensure all blocks were fetched
-	bs := bserv.New(bsis[1].Blockstore(), offline.Exchange(bsis[1].Blockstore()))
+	bs := bserv.New(instances[1].Blockstore(), nil)
 
 	offlineDS := NewDAGService(bs)
 
@@ -547,10 +553,12 @@ func TestFetchGraphWithDepthLimit(t *testing.T) {
 	}
 
 	testF := func(t *testing.T, tc testcase) {
-		var dservs []ipld.DAGService
-		bsis := bstest.Mocks(2)
-		for _, bsi := range bsis {
-			dservs = append(dservs, NewDAGService(bsi))
+		net := tn.VirtualNetwork(mockrouting.NewServer(), delay.Fixed(0))
+		sg := testinstance.NewTestInstanceGenerator(net, nil, nil)
+		instances := sg.Instances(2)
+		dservs := [2]ipld.DAGService{
+			NewDAGService(bserv.New(instances[0].Blockstore(), instances[0].Exchange)),
+			NewDAGService(bserv.New(instances[1].Blockstore(), instances[1].Exchange)),
 		}
 
 		root := makeDepthTestingGraph(t, dservs[0])
@@ -561,7 +569,7 @@ func TestFetchGraphWithDepthLimit(t *testing.T) {
 		}
 
 		// create an offline dagstore and ensure all blocks were fetched
-		bs := bserv.New(bsis[1].Blockstore(), offline.Exchange(bsis[1].Blockstore()))
+		bs := bserv.New(instances[1].Blockstore(), offline.Exchange(instances[1].Blockstore()))
 
 		offlineDS := NewDAGService(bs)