Include chain height when running custom decision logic

filecoin-project · Jul 12, 2020 · a190687 · a190687
1 parent 50ce1e2
commit a190687
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 68 deletions.
diff --git a/storagemarket/impl/provider.go b/storagemarket/impl/provider.go
@@ -31,11 +31,6 @@ import (
 	"github.com/filecoin-project/go-fil-markets/storagemarket/network"
 )
 
-// DefaultDealAcceptanceBuffer is the minimum number of epochs ahead of the current epoch
-// a deal's StartEpoch must be for the deal to be accepted.
-// The StartEpoch must be more than simply greater than the current epoch because we
-// need time to transfer data, publish the deal on chain, and seal the sector with the data
-var DefaultDealAcceptanceBuffer = abi.ChainEpoch(100)
 var _ storagemarket.StorageProvider = &Provider{}
 var _ network.StorageReceiver = &Provider{}
 
@@ -61,7 +56,6 @@ type Provider struct {
 	dataTransfer              datatransfer.Manager
 	universalRetrievalEnabled bool
 	customDealDeciderFunc     DealDeciderFunc
-	dealAcceptanceBuffer      abi.ChainEpoch
 	pubSub                    *pubsub.PubSub
 
 	deals fsm.Group
@@ -78,21 +72,13 @@ func EnableUniversalRetrieval() StorageProviderOption {
 	}
 }
 
-// DealAcceptanceBuffer allows a provider to set a buffer (in epochs) to account for the time
-// required for data transfer, deal verification, publishing, sealing, and committing.
-func DealAcceptanceBuffer(buffer abi.ChainEpoch) StorageProviderOption {
-	return func(p *Provider) {
-		p.dealAcceptanceBuffer = buffer
-	}
-}
-
-// DealDeciderFunc is a function which evaluates an incoming deal to decide if
-// it its accepted
+// DealDeciderFunc is a function which evaluates an incoming deal and the current chain height to decide if
+// the deal is accepted
 // It returns:
 // - boolean = true if deal accepted, false if rejected
 // - string = reason deal was not excepted, if rejected
 // - error = if an error occurred trying to decide
-type DealDeciderFunc func(context.Context, storagemarket.MinerDeal) (bool, string, error)
+type DealDeciderFunc func(context.Context, storagemarket.MinerDeal, abi.ChainEpoch) (bool, string, error)
 
 // CustomDealDecisionLogic allows a provider to call custom decision logic when validating incoming
 // deal proposals
@@ -119,18 +105,17 @@ func NewProvider(net network.StorageMarketNetwork,
 	pio := pieceio.NewPieceIOWithStore(carIO, fs, bs)
 
 	h := &Provider{
-		net:                  net,
-		proofType:            rt,
-		spn:                  spn,
-		fs:                   fs,
-		pio:                  pio,
-		pieceStore:           pieceStore,
-		conns:                connmanager.NewConnManager(),
-		storedAsk:            storedAsk,
-		actor:                minerAddress,
-		dataTransfer:         dataTransfer,
-		dealAcceptanceBuffer: DefaultDealAcceptanceBuffer,
-		pubSub:               pubsub.New(providerDispatcher),
+		net:          net,
+		proofType:    rt,
+		spn:          spn,
+		fs:           fs,
+		pio:          pio,
+		pieceStore:   pieceStore,
+		conns:        connmanager.NewConnManager(),
+		storedAsk:    storedAsk,
+		actor:        minerAddress,
+		dataTransfer: dataTransfer,
+		pubSub:       pubsub.New(providerDispatcher),
 	}
 
 	deals, err := newProviderStateMachine(
@@ -485,11 +470,6 @@ func (p *Provider) Configure(options ...StorageProviderOption) {
 	}
 }
 
-// DealAcceptanceBuffer returns the current deal acceptance buffer
-func (p *Provider) DealAcceptanceBuffer() abi.ChainEpoch {
-	return p.dealAcceptanceBuffer
-}
-
 // UniversalRetrievalEnabled returns whether or not universal retrieval
 // (retrieval by any CID, not just the root payload CID) is enabled
 // for this provider
@@ -648,15 +628,11 @@ func (p *providerDealEnvironment) Disconnect(proposalCid cid.Cid) error {
 	return p.p.conns.Disconnect(proposalCid)
 }
 
-func (p *providerDealEnvironment) DealAcceptanceBuffer() abi.ChainEpoch {
-	return p.p.dealAcceptanceBuffer
-}
-
-func (p *providerDealEnvironment) RunCustomDecisionLogic(ctx context.Context, deal storagemarket.MinerDeal) (bool, string, error) {
+func (p *providerDealEnvironment) RunCustomDecisionLogic(ctx context.Context, deal storagemarket.MinerDeal, ht abi.ChainEpoch) (bool, string, error) {
 	if p.p.customDealDeciderFunc == nil {
 		return true, "", nil
 	}
-	return p.p.customDealDeciderFunc(ctx, deal)
+	return p.p.customDealDeciderFunc(ctx, deal, ht)
 }
 
 var _ providerstates.ProviderDealEnvironment = &providerDealEnvironment{}

diff --git a/storagemarket/impl/providerstates/provider_states.go b/storagemarket/impl/providerstates/provider_states.go
@@ -38,16 +38,15 @@ type ProviderDealEnvironment interface {
 	Disconnect(proposalCid cid.Cid) error
 	FileStore() filestore.FileStore
 	PieceStore() piecestore.PieceStore
-	DealAcceptanceBuffer() abi.ChainEpoch
-	RunCustomDecisionLogic(context.Context, storagemarket.MinerDeal) (bool, string, error)
+	RunCustomDecisionLogic(context.Context, storagemarket.MinerDeal, abi.ChainEpoch) (bool, string, error)
 }
 
 // ProviderStateEntryFunc is the signature for a StateEntryFunc in the provider FSM
 type ProviderStateEntryFunc func(ctx fsm.Context, environment ProviderDealEnvironment, deal storagemarket.MinerDeal) error
 
 // ValidateDealProposal validates a proposed deal against the provider criteria
 func ValidateDealProposal(ctx fsm.Context, environment ProviderDealEnvironment, deal storagemarket.MinerDeal) error {
-	tok, height, err := environment.Node().GetChainHead(ctx.Context())
+	tok, _, err := environment.Node().GetChainHead(ctx.Context())
 	if err != nil {
 		return ctx.Trigger(storagemarket.ProviderEventDealRejected, xerrors.Errorf("node error getting most recent state id: %w", err))
 	}
@@ -62,10 +61,6 @@ func ValidateDealProposal(ctx fsm.Context, environment ProviderDealEnvironment,
 		return ctx.Trigger(storagemarket.ProviderEventDealRejected, xerrors.Errorf("incorrect provider for deal"))
 	}
 
-	if height > proposal.StartEpoch-environment.DealAcceptanceBuffer() {
-		return ctx.Trigger(storagemarket.ProviderEventDealRejected, xerrors.Errorf("deal start epoch is too soon or deal already expired"))
-	}
-
 	// TODO: check StorageCollateral
 
 	minPrice := big.Div(big.Mul(environment.Ask().Price, abi.NewTokenAmount(int64(proposal.PieceSize))), abi.NewTokenAmount(1<<30))
@@ -117,7 +112,11 @@ func ValidateDealProposal(ctx fsm.Context, environment ProviderDealEnvironment,
 // DecideOnProposal allows custom decision logic to run before accepting a deal, such as allowing a manual
 // operator to decide whether or not to accept the deal
 func DecideOnProposal(ctx fsm.Context, environment ProviderDealEnvironment, deal storagemarket.MinerDeal) error {
-	accept, reason, err := environment.RunCustomDecisionLogic(ctx.Context(), deal)
+	_, ht, err := environment.Node().GetChainHead(ctx.Context())
+	if err != nil {
+		return ctx.Trigger(storagemarket.ProviderEventDealRejected, xerrors.Errorf("custom deal decision logic failed to get chain head: %w", err))
+	}
+	accept, reason, err := environment.RunCustomDecisionLogic(ctx.Context(), deal, ht)
 	if err != nil {
 		return ctx.Trigger(storagemarket.ProviderEventDealRejected, xerrors.Errorf("custom deal decision logic failed: %w", err))
 	}

diff --git a/storagemarket/impl/providerstates/provider_states_test.go b/storagemarket/impl/providerstates/provider_states_test.go
@@ -81,23 +81,6 @@ func TestValidateDealProposal(t *testing.T) {
 				require.Equal(t, "deal rejected: node error getting most recent state id: couldn't get id", deal.Message)
 			},
 		},
-		"CurrentHeight <= StartEpoch - DealAcceptanceBuffer() succeeds": {
-			environmentParams: environmentParams{DealAcceptanceBuffer: 10},
-			dealParams:        dealParams{StartEpoch: 200},
-			nodeParams:        nodeParams{Height: 190},
-			dealInspector: func(t *testing.T, deal storagemarket.MinerDeal, env *fakeEnvironment) {
-				tut.AssertDealState(t, storagemarket.StorageDealAcceptWait, deal.State)
-			},
-		},
-		"CurrentHeight > StartEpoch - DealAcceptanceBuffer() fails": {
-			environmentParams: environmentParams{DealAcceptanceBuffer: 10},
-			dealParams:        dealParams{StartEpoch: 200},
-			nodeParams:        nodeParams{Height: 191},
-			dealInspector: func(t *testing.T, deal storagemarket.MinerDeal, env *fakeEnvironment) {
-				tut.AssertDealState(t, storagemarket.StorageDealRejecting, deal.State)
-				require.Equal(t, "deal rejected: deal start epoch is too soon or deal already expired", deal.Message)
-			},
-		},
 		"PricePerEpoch too low": {
 			dealParams: dealParams{
 				StoragePricePerEpoch: abi.NewTokenAmount(5000),
@@ -899,7 +882,6 @@ type environmentParams struct {
 	GenerateCommPError      error
 	SendSignedResponseError error
 	DisconnectError         error
-	DealAcceptanceBuffer    int64
 	TagsProposal            bool
 	RejectDeal              bool
 	RejectReason            string
@@ -1048,7 +1030,6 @@ func makeExecutor(ctx context.Context,
 			rejectDeal:              params.RejectDeal,
 			rejectReason:            params.RejectReason,
 			decisionError:           params.DecisionError,
-			dealAcceptanceBuffer:    abi.ChainEpoch(params.DealAcceptanceBuffer),
 			fs:                      fs,
 			pieceStore:              pieceStore,
 		}
@@ -1143,6 +1124,6 @@ func (fe *fakeEnvironment) DealAcceptanceBuffer() abi.ChainEpoch {
 	return fe.dealAcceptanceBuffer
 }
 
-func (fe *fakeEnvironment) RunCustomDecisionLogic(context.Context, storagemarket.MinerDeal) (bool, string, error) {
+func (fe *fakeEnvironment) RunCustomDecisionLogic(context.Context, storagemarket.MinerDeal, abi.ChainEpoch) (bool, string, error) {
 	return !fe.rejectDeal, fe.rejectReason, fe.decisionError
 }