From 1570ef5f477d18926c9cbc7679db934ec5098a78 Mon Sep 17 00:00:00 2001 From: LexLuthr Date: Wed, 22 Mar 2023 17:44:39 +0400 Subject: [PATCH 1/3] fail deal if start epoch passed --- storagemarket/deal_execution.go | 5 +++++ storagemarket/provider.go | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/storagemarket/deal_execution.go b/storagemarket/deal_execution.go index ba2d145d9..ab460944e 100644 --- a/storagemarket/deal_execution.go +++ b/storagemarket/deal_execution.go @@ -50,6 +50,11 @@ func (p *Provider) runDeal(deal *types.ProviderDealState, dh *dealHandler) { p.saveDealToDB(dh.Publisher, deal) } + // Fail offline deals or retry early if start epoch has passed + if err := p.checkDealProposalStartEpoch(deal); err != nil { + p.failDeal(dh.Publisher, deal, err, false) + } + // Execute the deal err := p.execDeal(deal, dh) if err == nil { diff --git a/storagemarket/provider.go b/storagemarket/provider.go index 79faa2eeb..8d3dd4dae 100644 --- a/storagemarket/provider.go +++ b/storagemarket/provider.go @@ -459,6 +459,11 @@ func (p *Provider) Start() error { continue } + // Fail deals if start epoch has passed + if err := p.checkDealProposalStartEpoch(deal); err != nil { + p.failDeal(dh.Publisher, deal, err, false) + } + // If it's an offline deal, and the deal data hasn't yet been // imported, just wait for the SP operator to import the data if deal.IsOffline && deal.InboundFilePath == "" { From c2b8c1e2d69247febe4a86fb60a9372dda865a4c Mon Sep 17 00:00:00 2001 From: LexLuthr Date: Wed, 22 Mar 2023 18:55:19 +0400 Subject: [PATCH 2/3] add suggestion --- storagemarket/provider.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/storagemarket/provider.go b/storagemarket/provider.go index 8d3dd4dae..7c407fe9a 100644 --- a/storagemarket/provider.go +++ b/storagemarket/provider.go @@ -461,7 +461,8 @@ func (p *Provider) Start() error { // Fail deals if start epoch has passed if err := p.checkDealProposalStartEpoch(deal); err != nil { - p.failDeal(dh.Publisher, deal, err, false) + go p.failDeal(dh.Publisher, deal, err, false) + continue } // If it's an offline deal, and the deal data hasn't yet been From f7f744f7741417dc302b35d19007050cb156a582 Mon Sep 17 00:00:00 2001 From: Dirk McCormick Date: Thu, 13 Apr 2023 11:41:03 +0200 Subject: [PATCH 3/3] test: add deal expiry on startup test --- storagemarket/deal_execution.go | 5 -- storagemarket/provider_test.go | 112 ++++++++++++++++++++++++++++++-- 2 files changed, 106 insertions(+), 11 deletions(-) diff --git a/storagemarket/deal_execution.go b/storagemarket/deal_execution.go index ab460944e..ba2d145d9 100644 --- a/storagemarket/deal_execution.go +++ b/storagemarket/deal_execution.go @@ -50,11 +50,6 @@ func (p *Provider) runDeal(deal *types.ProviderDealState, dh *dealHandler) { p.saveDealToDB(dh.Publisher, deal) } - // Fail offline deals or retry early if start epoch has passed - if err := p.checkDealProposalStartEpoch(deal); err != nil { - p.failDeal(dh.Publisher, deal, err, false) - } - // Execute the deal err := p.execDeal(deal, dh) if err == nil { diff --git a/storagemarket/provider_test.go b/storagemarket/provider_test.go index 624aec6ab..2db36d5be 100644 --- a/storagemarket/provider_test.go +++ b/storagemarket/provider_test.go @@ -38,10 +38,12 @@ import ( "github.com/filecoin-project/go-state-types/abi" "github.com/filecoin-project/go-state-types/big" "github.com/filecoin-project/go-state-types/builtin/v9/market" + "github.com/filecoin-project/go-state-types/crypto" acrypto "github.com/filecoin-project/go-state-types/crypto" lapi "github.com/filecoin-project/lotus/api" lotusmocks "github.com/filecoin-project/lotus/api/mocks" test "github.com/filecoin-project/lotus/chain/events/state/mock" + chaintypes "github.com/filecoin-project/lotus/chain/types" "github.com/filecoin-project/lotus/node/repo" sealing "github.com/filecoin-project/lotus/storage/pipeline" "github.com/filecoin-project/lotus/storage/sealer/storiface" @@ -706,6 +708,74 @@ func TestDealRestartAfterManualRecoverableErrors(t *testing.T) { } } +func TestDealRestartFailExpiredDeal(t *testing.T) { + ctx := context.Background() + + tcs := []struct { + name string + isOffline bool + }{{ + name: "online", + isOffline: false, + }, { + name: "offline", + isOffline: true, + }} + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + // setup the provider test harness + var chainHead *chaintypes.TipSet + harness := NewHarness(t, withChainHeadFunction(func(ctx context.Context) (*chaintypes.TipSet, error) { + return chainHead, nil + })) + + chainHead, err := mockTipset(harness.MinerAddr, 5) + require.NoError(t, err) + + // start the provider test harness + harness.Start(t, ctx) + defer harness.Stop() + + // build the deal proposal + opts := []dealProposalOpt{} + if tc.isOffline { + opts = append(opts, withOfflineDeal()) + } + td := harness.newDealBuilder(t, 1, opts...).withCommpBlocking(true).build() + + // execute deal + err = td.executeAndSubscribe() + require.NoError(t, err) + + err = td.waitForCheckpoint(dealcheckpoints.Accepted) + require.NoError(t, err) + + // shutdown the existing provider and create a new provider + harness.shutdownAndCreateNewProvider(t) + + // update the test deal state with the new provider + _ = td.updateWithRestartedProvider(harness) + + // simulate a chain height that is greater that the epoch by which + // the deal should have been sealed + chainHead, err = mockTipset(harness.MinerAddr, td.params.ClientDealProposal.Proposal.StartEpoch+10) + require.NoError(t, err) + + // start the provider + err = harness.Provider.Start() + require.NoError(t, err) + + // expect the deal to fail on startup because it has expired + require.Eventually(t, func() bool { + dl, err := harness.Provider.Deal(ctx, td.params.DealUUID) + require.NoError(t, err) + return strings.Contains(dl.Err, "expired") + }, 5*time.Second, 100*time.Millisecond) + }) + } +} + // Tests scenario that a contract deal fails fatally when PublishStorageDeal fails. func TestContractDealFatalFailAfterPublishError(t *testing.T) { ctx := context.Background() @@ -1284,6 +1354,8 @@ type ProviderHarness struct { DAGStore *shared_testutil.MockDagStoreWrapper } +type ChainHeadFn func(ctx context.Context) (*chaintypes.TipSet, error) + type providerConfig struct { mockCtrl *gomock.Controller @@ -1304,8 +1376,9 @@ type providerConfig struct { minPieceSize abi.PaddedPieceSize maxPieceSize abi.PaddedPieceSize - localCommp bool - dealFilter dealfilter.StorageDealFilter + localCommp bool + dealFilter dealfilter.StorageDealFilter + chainHeadFn ChainHeadFn } type harnessOpt func(pc *providerConfig) @@ -1383,6 +1456,12 @@ func withDealFilter(filter dealfilter.StorageDealFilter) harnessOpt { } } +func withChainHeadFunction(fn ChainHeadFn) harnessOpt { + return func(pc *providerConfig) { + pc.chainHeadFn = fn + } +} + func NewHarness(t *testing.T, opts ...harnessOpt) *ProviderHarness { ctrl := gomock.NewController(t) pc := &providerConfig{ @@ -1540,10 +1619,17 @@ func NewHarness(t *testing.T, opts ...harnessOpt) *ProviderHarness { require.NoError(t, err) ph.Provider = prov - // Creates chain tipset with height 5 - chainHead, err := test.MockTipset(minerAddr, 1) - require.NoError(t, err) - fn.EXPECT().ChainHead(gomock.Any()).Return(chainHead, nil).AnyTimes() + chainHeadFn := pc.chainHeadFn + if chainHeadFn == nil { + // Creates chain tipset with height 5 + chainHead, err := test.MockTipset(minerAddr, 1) + require.NoError(t, err) + chainHeadFn = func(ctx context.Context) (*chaintypes.TipSet, error) { + return chainHead, nil + } + } + fn.EXPECT().ChainHead(gomock.Any()).DoAndReturn(chainHeadFn).AnyTimes() + fn.EXPECT().StateDealProviderCollateralBounds(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(lapi.DealCollateralBounds{ Min: abi.NewTokenAmount(1), Max: abi.NewTokenAmount(1), @@ -2301,3 +2387,17 @@ func copyFile(source string, dest string) error { return nil } + +func mockTipset(minerAddr address.Address, height abi.ChainEpoch) (*chaintypes.TipSet, error) { + dummyCid, _ := cid.Parse("bafkqaaa") + return chaintypes.NewTipSet([]*chaintypes.BlockHeader{{ + Miner: minerAddr, + Height: height, + ParentStateRoot: dummyCid, + Messages: dummyCid, + ParentMessageReceipts: dummyCid, + BlockSig: &crypto.Signature{Type: crypto.SigTypeBLS}, + BLSAggregate: &crypto.Signature{Type: crypto.SigTypeBLS}, + Timestamp: 1, + }}) +}