refactor(storagemarket): remove storedask from provider (#263)

Move stored ask out of provider so its setup and configured seperately. Also allow customizing the DS key
filecoin-project · May 20, 2020 · 2f01d9e · 2f01d9e
1 parent 0f56af3
commit 2f01d9e
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 22 deletions.
diff --git a/retrievalmarket/storage_retrieval_integration_test.go b/retrievalmarket/storage_retrieval_integration_test.go
@@ -11,12 +11,13 @@ import (
 
 	"github.com/filecoin-project/go-address"
 	datatransfer "github.com/filecoin-project/go-data-transfer"
-	"github.com/filecoin-project/go-data-transfer/impl/graphsync"
+	graphsyncimpl "github.com/filecoin-project/go-data-transfer/impl/graphsync"
 	"github.com/filecoin-project/specs-actors/actors/abi"
 	"github.com/filecoin-project/specs-actors/actors/abi/big"
 	"github.com/filecoin-project/specs-actors/actors/builtin/market"
 	"github.com/filecoin-project/specs-actors/actors/builtin/paych"
 	"github.com/ipfs/go-cid"
+	"github.com/ipfs/go-datastore"
 	"github.com/ipld/go-ipld-prime"
 	cidlink "github.com/ipld/go-ipld-prime/linking/cid"
 	"github.com/libp2p/go-libp2p-core/peer"
@@ -37,6 +38,7 @@ import (
 	"github.com/filecoin-project/go-fil-markets/storagemarket"
 	stormkt "github.com/filecoin-project/go-fil-markets/storagemarket/impl"
 	"github.com/filecoin-project/go-fil-markets/storagemarket/impl/requestvalidation"
+	"github.com/filecoin-project/go-fil-markets/storagemarket/impl/storedask"
 	stornet "github.com/filecoin-project/go-fil-markets/storagemarket/network"
 	"github.com/filecoin-project/go-fil-markets/storagemarket/testnodes"
 )
@@ -220,6 +222,8 @@ func newStorageHarness(ctx context.Context, t *testing.T) *storageHarness {
 	)
 	require.NoError(t, err)
 	dt2 := graphsyncimpl.NewGraphSyncDataTransfer(td.Host2, td.GraphSync2, td.DTStoredCounter2)
+	storedAsk, err := storedask.NewStoredAsk(td.Ds2, datastore.NewKey("latest-ask"), providerNode, providerAddr)
+	require.NoError(t, err)
 	provider, err := stormkt.NewProvider(
 		stornet.NewFromLibp2pHost(td.Host2),
 		td.Ds2,
@@ -230,6 +234,7 @@ func newStorageHarness(ctx context.Context, t *testing.T) *storageHarness {
 		providerNode,
 		providerAddr,
 		abi.RegisteredProof_StackedDRG2KiBPoSt,
+		storedAsk,
 	)
 	require.NoError(t, err)
 

diff --git a/storagemarket/impl/provider.go b/storagemarket/impl/provider.go
@@ -13,7 +13,6 @@ import (
 	"github.com/hannahhoward/go-pubsub"
 	"github.com/ipfs/go-cid"
 	"github.com/ipfs/go-datastore"
-	"github.com/ipfs/go-datastore/namespace"
 	blockstore "github.com/ipfs/go-ipfs-blockstore"
 	"github.com/ipld/go-ipld-prime"
 	"github.com/libp2p/go-libp2p-core/peer"
@@ -28,14 +27,17 @@ import (
 	"github.com/filecoin-project/go-fil-markets/storagemarket/impl/connmanager"
 	"github.com/filecoin-project/go-fil-markets/storagemarket/impl/providerstates"
 	"github.com/filecoin-project/go-fil-markets/storagemarket/impl/providerutils"
-	"github.com/filecoin-project/go-fil-markets/storagemarket/impl/storedask"
 	"github.com/filecoin-project/go-fil-markets/storagemarket/network"
 )
 
-var ProviderDsPrefix = "/provider"
 var DefaultDealAcceptanceBuffer = abi.ChainEpoch(100)
 var _ storagemarket.StorageProvider = &Provider{}
 
+type StoredAsk interface {
+	GetAsk(address.Address) *storagemarket.SignedStorageAsk
+	AddAsk(price abi.TokenAmount, duration abi.ChainEpoch, options ...storagemarket.StorageAskOption) error
+}
+
 // Provider is a storage provider implementation
 type Provider struct {
 	net network.StorageMarketNetwork
@@ -47,7 +49,7 @@ type Provider struct {
 	pio                       pieceio.PieceIOWithStore
 	pieceStore                piecestore.PieceStore
 	conns                     *connmanager.ConnManager
-	storedAsk                 *storedask.StoredAsk
+	storedAsk                 StoredAsk
 	actor                     address.Address
 	dataTransfer              datatransfer.Manager
 	universalRetrievalEnabled bool
@@ -94,15 +96,10 @@ func CustomDealDecisionLogic(decider DealDeciderFunc) StorageProviderOption {
 }
 
 // NewProvider returns a new storage provider
-func NewProvider(net network.StorageMarketNetwork, ds datastore.Batching, bs blockstore.Blockstore, fs filestore.FileStore, pieceStore piecestore.PieceStore, dataTransfer datatransfer.Manager, spn storagemarket.StorageProviderNode, minerAddress address.Address, rt abi.RegisteredProof, options ...StorageProviderOption) (storagemarket.StorageProvider, error) {
+func NewProvider(net network.StorageMarketNetwork, ds datastore.Batching, bs blockstore.Blockstore, fs filestore.FileStore, pieceStore piecestore.PieceStore, dataTransfer datatransfer.Manager, spn storagemarket.StorageProviderNode, minerAddress address.Address, rt abi.RegisteredProof, storedAsk StoredAsk, options ...StorageProviderOption) (storagemarket.StorageProvider, error) {
 	carIO := cario.NewCarIO()
 	pio := pieceio.NewPieceIOWithStore(carIO, fs, bs)
 
-	storedAsk, err := storedask.NewStoredAsk(ds, spn, minerAddress)
-	if err != nil {
-		return nil, err
-	}
-
 	h := &Provider{
 		net:                  net,
 		proofType:            rt,
@@ -118,7 +115,7 @@ func NewProvider(net network.StorageMarketNetwork, ds datastore.Batching, bs blo
 		pubSub:               pubsub.New(providerDispatcher),
 	}
 
-	deals, err := fsm.New(namespace.Wrap(ds, datastore.NewKey(ProviderDsPrefix)), fsm.Parameters{
+	deals, err := fsm.New(ds, fsm.Parameters{
 		Environment:     &providerDealEnvironment{h},
 		StateType:       storagemarket.MinerDeal{},
 		StateKeyField:   "State",

diff --git a/storagemarket/impl/storedask/storedask.go b/storagemarket/impl/storedask/storedask.go
@@ -29,11 +29,12 @@ type StoredAsk struct {
 	askLk sync.RWMutex
 	ask   *storagemarket.SignedStorageAsk
 	ds    datastore.Batching
+	dsKey datastore.Key
 	spn   storagemarket.StorageProviderNode
 	actor address.Address
 }
 
-func NewStoredAsk(ds datastore.Batching, spn storagemarket.StorageProviderNode, actor address.Address) (*StoredAsk, error) {
+func NewStoredAsk(ds datastore.Batching, dsKey datastore.Key, spn storagemarket.StorageProviderNode, actor address.Address) (*StoredAsk, error) {
 
 	s := &StoredAsk{
 		ds:    ds,
@@ -113,8 +114,6 @@ func (s *StoredAsk) GetAsk(addr address.Address) *storagemarket.SignedStorageAsk
 	return &ask
 }
 
-var bestAskKey = datastore.NewKey("latest-ask")
-
 func (s *StoredAsk) tryLoadAsk() error {
 	s.askLk.Lock()
 	defer s.askLk.Unlock()
@@ -132,7 +131,7 @@ func (s *StoredAsk) tryLoadAsk() error {
 }
 
 func (s *StoredAsk) loadAsk() error {
-	askb, err := s.ds.Get(bestAskKey)
+	askb, err := s.ds.Get(s.dsKey)
 	if err != nil {
 		return xerrors.Errorf("failed to load most recent ask from disk: %w", err)
 	}
@@ -152,7 +151,7 @@ func (s *StoredAsk) saveAsk(a *storagemarket.SignedStorageAsk) error {
 		return err
 	}
 
-	if err := s.ds.Put(bestAskKey, b); err != nil {
+	if err := s.ds.Put(s.dsKey, b); err != nil {
 		return err
 	}
 

diff --git a/storagemarket/impl/storedask/storedask_test.go b/storagemarket/impl/storedask/storedask_test.go
@@ -23,7 +23,7 @@ func TestStoredAsk(t *testing.T) {
 		},
 	}
 	actor := address.TestAddress2
-	storedAsk, err := storedask.NewStoredAsk(ds, spn, actor)
+	storedAsk, err := storedask.NewStoredAsk(ds, datastore.NewKey("latest-ask"), spn, actor)
 	require.NoError(t, err)
 
 	testPrice := abi.NewTokenAmount(1000000000)
@@ -48,7 +48,7 @@ func TestStoredAsk(t *testing.T) {
 		require.Nil(t, ask)
 	})
 	t.Run("reloading stored ask from disk", func(t *testing.T) {
-		storedAsk2, err := storedask.NewStoredAsk(ds, spn, actor)
+		storedAsk2, err := storedask.NewStoredAsk(ds, datastore.NewKey("latest-ask"), spn, actor)
 		require.NoError(t, err)
 		ask := storedAsk2.GetAsk(actor)
 		require.Equal(t, ask.Ask.Price, testPrice)
@@ -62,7 +62,7 @@ func TestStoredAsk(t *testing.T) {
 			},
 		}
 		// should load cause ask is is still in data store
-		storedAskError, err := storedask.NewStoredAsk(ds, spnStateIDErr, actor)
+		storedAskError, err := storedask.NewStoredAsk(ds, datastore.NewKey("latest-ask"), spnStateIDErr, actor)
 		require.NoError(t, err)
 		err = storedAskError.AddAsk(testPrice, testDuration)
 		require.Error(t, err)
@@ -74,7 +74,7 @@ func TestStoredAsk(t *testing.T) {
 			MinerWorkerError: errors.New("something went wrong"),
 		}
 		// should load cause ask is is still in data store
-		storedAskError, err = storedask.NewStoredAsk(ds, spnMinerWorkerErr, actor)
+		storedAskError, err = storedask.NewStoredAsk(ds, datastore.NewKey("latest-ask"), spnMinerWorkerErr, actor)
 		require.NoError(t, err)
 		err = storedAskError.AddAsk(testPrice, testDuration)
 		require.Error(t, err)
@@ -86,7 +86,7 @@ func TestStoredAsk(t *testing.T) {
 			SignBytesError: errors.New("something went wrong"),
 		}
 		// should load cause ask is is still in data store
-		storedAskError, err = storedask.NewStoredAsk(ds, spnSignBytesErr, actor)
+		storedAskError, err = storedask.NewStoredAsk(ds, datastore.NewKey("latest-ask"), spnSignBytesErr, actor)
 		require.NoError(t, err)
 		err = storedAskError.AddAsk(testPrice, testDuration)
 		require.Error(t, err)

diff --git a/storagemarket/integration_test.go b/storagemarket/integration_test.go
@@ -17,6 +17,7 @@ import (
 	"github.com/filecoin-project/specs-actors/actors/abi/big"
 	"github.com/filecoin-project/specs-actors/actors/builtin/market"
 	"github.com/ipfs/go-cid"
+	"github.com/ipfs/go-datastore"
 	"github.com/ipld/go-ipld-prime"
 	cidlink "github.com/ipld/go-ipld-prime/linking/cid"
 	"github.com/libp2p/go-libp2p-core/peer"
@@ -33,6 +34,7 @@ import (
 	"github.com/filecoin-project/go-fil-markets/storagemarket"
 	storageimpl "github.com/filecoin-project/go-fil-markets/storagemarket/impl"
 	"github.com/filecoin-project/go-fil-markets/storagemarket/impl/requestvalidation"
+	"github.com/filecoin-project/go-fil-markets/storagemarket/impl/storedask"
 	"github.com/filecoin-project/go-fil-markets/storagemarket/network"
 	"github.com/filecoin-project/go-fil-markets/storagemarket/testnodes"
 )
@@ -284,7 +286,10 @@ func newHarness(t *testing.T, ctx context.Context) *harness {
 		&clientNode,
 	)
 	require.NoError(t, err)
+
 	dt2 := graphsync.NewGraphSyncDataTransfer(td.Host2, td.GraphSync2, td.DTStoredCounter2)
+	storedAsk, err := storedask.NewStoredAsk(td.Ds2, datastore.NewKey("latest-ask"), providerNode, providerAddr)
+	assert.NoError(t, err)
 	provider, err := storageimpl.NewProvider(
 		network.NewFromLibp2pHost(td.Host2),
 		td.Ds2,
@@ -295,6 +300,7 @@ func newHarness(t *testing.T, ctx context.Context) *harness {
 		providerNode,
 		providerAddr,
 		abi.RegisteredProof_StackedDRG2KiBPoSt,
+		storedAsk,
 	)
 	assert.NoError(t, err)