From 3eb2fa92849f276ddf6bdfaa8da1a6e6c555735c Mon Sep 17 00:00:00 2001 From: Aayush Rajasekaran Date: Sun, 12 Jul 2020 14:43:48 -0400 Subject: [PATCH] Include chain height when running custom decision logic --- storagemarket/impl/provider.go | 56 ++++++------------- storagemarket/impl/provider_test.go | 4 -- .../impl/providerstates/provider_states.go | 15 +++-- .../providerstates/provider_states_test.go | 26 +-------- 4 files changed, 24 insertions(+), 77 deletions(-) diff --git a/storagemarket/impl/provider.go b/storagemarket/impl/provider.go index c836b6e95..32e171299 100644 --- 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/provider_test.go b/storagemarket/impl/provider_test.go index 4a5ca9301..316825773 100644 --- a/storagemarket/impl/provider_test.go +++ b/storagemarket/impl/provider_test.go @@ -3,7 +3,6 @@ package storageimpl_test import ( "testing" - "github.com/filecoin-project/specs-actors/actors/abi" "github.com/stretchr/testify/assert" storageimpl "github.com/filecoin-project/go-fil-markets/storagemarket/impl" @@ -13,13 +12,10 @@ func TestConfigure(t *testing.T) { p := &storageimpl.Provider{} assert.False(t, p.UniversalRetrievalEnabled()) - assert.Equal(t, abi.ChainEpoch(0), p.DealAcceptanceBuffer()) p.Configure( storageimpl.EnableUniversalRetrieval(), - storageimpl.DealAcceptanceBuffer(abi.ChainEpoch(123)), ) assert.True(t, p.UniversalRetrievalEnabled()) - assert.Equal(t, abi.ChainEpoch(123), p.DealAcceptanceBuffer()) } diff --git a/storagemarket/impl/providerstates/provider_states.go b/storagemarket/impl/providerstates/provider_states.go index 8fb38f993..de989ce5a 100644 --- a/storagemarket/impl/providerstates/provider_states.go +++ b/storagemarket/impl/providerstates/provider_states.go @@ -38,8 +38,7 @@ 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 @@ -47,7 +46,7 @@ type ProviderStateEntryFunc func(ctx fsm.Context, environment ProviderDealEnviro // 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 index 0e9e9f8f1..f5eaa7b65 100644 --- 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, } @@ -1097,7 +1078,6 @@ type fakeEnvironment struct { decisionError error fs filestore.FileStore pieceStore piecestore.PieceStore - dealAcceptanceBuffer abi.ChainEpoch expectedTags map[string]struct{} receivedTags map[string]struct{} } @@ -1139,10 +1119,6 @@ func (fe *fakeEnvironment) PieceStore() piecestore.PieceStore { return fe.pieceStore } -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 }