From 3d086dfb43218db3c162ad99c05629c3f8b5cde9 Mon Sep 17 00:00:00 2001 From: aarshkshah1992 Date: Mon, 14 Jun 2021 09:40:34 +0530 Subject: [PATCH] changes as per review --- cmd/lotus-storage-miner/allinfo_test.go | 9 ++++- itests/ccupgrade_test.go | 9 ++++- itests/deals_test.go | 36 +++++++++++++++++--- itests/gateway_test.go | 9 ++++- itests/kit/deals.go | 45 ++++++++++--------------- markets/retrievaladapter/provider.go | 18 +++++----- 6 files changed, 82 insertions(+), 44 deletions(-) diff --git a/cmd/lotus-storage-miner/allinfo_test.go b/cmd/lotus-storage-miner/allinfo_test.go index cbe65524e..9f2dc626e 100644 --- a/cmd/lotus-storage-miner/allinfo_test.go +++ b/cmd/lotus-storage-miner/allinfo_test.go @@ -61,7 +61,14 @@ func TestMinerAllInfo(t *testing.T) { t.Run("pre-info-all", run) dh := kit.NewDealHarness(t, client, miner) - dh.MakeFullDeal(context.Background(), 6, false, false, 0) + _, _, _ = dh.MakeFullDeal(kit.MakeFullDealParams{ + Ctx: context.Background(), + Rseed: 6, + CarExport: false, + FastRet: false, + StartEpoch: 0, + DoRetrieval: true, + }) t.Run("post-info-all", run) } diff --git a/itests/ccupgrade_test.go b/itests/ccupgrade_test.go index 28abac171..196cf7106 100644 --- a/itests/ccupgrade_test.go +++ b/itests/ccupgrade_test.go @@ -94,7 +94,14 @@ func runTestCCUpgrade(t *testing.T, b kit.APIBuilder, blocktime time.Duration, u dh := kit.NewDealHarness(t, client, miner) - dh.MakeFullDeal(context.Background(), 6, false, false, 0) + _, _, _ = dh.MakeFullDeal(kit.MakeFullDealParams{ + Ctx: context.Background(), + Rseed: 6, + CarExport: false, + FastRet: false, + StartEpoch: 0, + DoRetrieval: true, + }) // Validate upgrade diff --git a/itests/deals_test.go b/itests/deals_test.go index 8e7df76dc..e5909cb80 100644 --- a/itests/deals_test.go +++ b/itests/deals_test.go @@ -106,10 +106,24 @@ func runQuotePriceForUnsealedRetrieval(t *testing.T, b kit.APIBuilder, blocktime dh := kit.NewDealHarness(t, client, miner) - _, info, fcid := dh.MakeFullDealNoRetrieval(ctx, 6, false, startEpoch) + _, info, fcid := dh.MakeFullDeal(kit.MakeFullDealParams{ + Ctx: ctx, + Rseed: 6, + CarExport: false, + FastRet: false, + StartEpoch: startEpoch, + DoRetrieval: false, + }) // one more storage deal for the same data - _, _, fcid2 := dh.MakeFullDealNoRetrieval(ctx, 6, false, startEpoch) + _, _, fcid2 := dh.MakeFullDeal(kit.MakeFullDealParams{ + Ctx: ctx, + Rseed: 6, + CarExport: false, + FastRet: false, + StartEpoch: startEpoch, + DoRetrieval: false, + }) require.Equal(t, fcid, fcid2) // fetch quote -> zero for unsealed price since unsealed file already exists. @@ -495,7 +509,14 @@ func runFullDealCycles(t *testing.T, n int, b kit.APIBuilder, blocktime time.Dur baseseed := 6 for i := 0; i < n; i++ { - dh.MakeFullDeal(context.Background(), baseseed+i, carExport, fastRet, startEpoch) + _, _, _ = dh.MakeFullDeal(kit.MakeFullDealParams{ + Ctx: context.Background(), + Rseed: baseseed + i, + CarExport: carExport, + FastRet: fastRet, + StartEpoch: startEpoch, + DoRetrieval: true, + }) } } @@ -601,5 +622,12 @@ func runZeroPricePerByteRetrievalDealFlow(t *testing.T, b kit.APIBuilder, blockt err = miner.MarketSetRetrievalAsk(ctx, ask) require.NoError(t, err) - dh.MakeFullDeal(ctx, 6, false, false, startEpoch) + _, _, _ = dh.MakeFullDeal(kit.MakeFullDealParams{ + Ctx: ctx, + Rseed: 6, + CarExport: false, + FastRet: false, + StartEpoch: startEpoch, + DoRetrieval: true, + }) } diff --git a/itests/gateway_test.go b/itests/gateway_test.go index 7f1b70f2d..f5eb4e363 100644 --- a/itests/gateway_test.go +++ b/itests/gateway_test.go @@ -206,7 +206,14 @@ func TestGatewayDealFlow(t *testing.T) { dealStartEpoch := abi.ChainEpoch(2 << 12) dh := kit.NewDealHarness(t, nodes.lite, nodes.miner) - dh.MakeFullDeal(ctx, 6, false, false, dealStartEpoch) + dh.MakeFullDeal(kit.MakeFullDealParams{ + Ctx: ctx, + Rseed: 6, + CarExport: false, + FastRet: false, + StartEpoch: dealStartEpoch, + DoRetrieval: true, + }) } func TestGatewayCLIDealFlow(t *testing.T) { diff --git a/itests/kit/deals.go b/itests/kit/deals.go index 2d36ee5da..f85618901 100644 --- a/itests/kit/deals.go +++ b/itests/kit/deals.go @@ -34,6 +34,15 @@ type DealHarness struct { miner TestMiner } +type MakeFullDealParams struct { + Ctx context.Context + Rseed int + CarExport bool + FastRet bool + StartEpoch abi.ChainEpoch + DoRetrieval bool +} + // NewDealHarness creates a test harness that contains testing utilities for deals. func NewDealHarness(t *testing.T, client api.FullNode, miner TestMiner) *DealHarness { return &DealHarness{ @@ -43,9 +52,9 @@ func NewDealHarness(t *testing.T, client api.FullNode, miner TestMiner) *DealHar } } -func (dh *DealHarness) MakeFullDealNoRetrieval(ctx context.Context, rseed int, fastRet bool, startEpoch abi.ChainEpoch) ([]byte, +func (dh *DealHarness) MakeFullDeal(params MakeFullDealParams) ([]byte, *api.DealInfo, cid.Cid) { - res, _, data, err := CreateImportFile(ctx, dh.client, rseed, 0) + res, _, data, err := CreateImportFile(params.Ctx, dh.client, params.Rseed, 0) if err != nil { dh.t.Fatal(err) } @@ -53,41 +62,23 @@ func (dh *DealHarness) MakeFullDealNoRetrieval(ctx context.Context, rseed int, f fcid := res.Root fmt.Println("FILE CID: ", fcid) - deal := dh.StartDeal(ctx, fcid, fastRet, startEpoch) + deal := dh.StartDeal(params.Ctx, fcid, params.FastRet, params.StartEpoch) // TODO: this sleep is only necessary because deals don't immediately get logged in the dealstore, we should fix this time.Sleep(time.Second) - dh.WaitDealSealed(ctx, deal, false, false, nil) + dh.WaitDealSealed(params.Ctx, deal, false, false, nil) // Retrieval - info, err := dh.client.ClientGetDealInfo(ctx, *deal) + info, err := dh.client.ClientGetDealInfo(params.Ctx, *deal) require.NoError(dh.t, err) + if params.DoRetrieval { + dh.TestRetrieval(params.Ctx, fcid, &info.PieceCID, params.CarExport, data) + } + return data, info, fcid } -func (dh *DealHarness) MakeFullDeal(ctx context.Context, rseed int, carExport, fastRet bool, startEpoch abi.ChainEpoch) { - res, _, data, err := CreateImportFile(ctx, dh.client, rseed, 0) - if err != nil { - dh.t.Fatal(err) - } - - fcid := res.Root - fmt.Println("FILE CID: ", fcid) - - deal := dh.StartDeal(ctx, fcid, fastRet, startEpoch) - - // TODO: this sleep is only necessary because deals don't immediately get logged in the dealstore, we should fix this - time.Sleep(time.Second) - dh.WaitDealSealed(ctx, deal, false, false, nil) - - // Retrieval - info, err := dh.client.ClientGetDealInfo(ctx, *deal) - require.NoError(dh.t, err) - - dh.TestRetrieval(ctx, fcid, &info.PieceCID, carExport, data) -} - func (dh *DealHarness) StartDeal(ctx context.Context, fcid cid.Cid, fastRet bool, startEpoch abi.ChainEpoch) *cid.Cid { maddr, err := dh.miner.ActorAddress(ctx) if err != nil { diff --git a/markets/retrievaladapter/provider.go b/markets/retrievaladapter/provider.go index 748425407..95b7e1b3c 100644 --- a/markets/retrievaladapter/provider.go +++ b/markets/retrievaladapter/provider.go @@ -123,15 +123,9 @@ func (rpn *retrievalProviderNode) IsUnsealed(ctx context.Context, sectorID abi.S return rpn.pp.IsUnsealed(ctx, ref, storiface.UnpaddedByteIndex(offset), length) } -// `storageDeals` param here is the list of storage deals made for the `payloadCID` the retrieval client is looking for. -// -// `pieceCID` is the CID of the specific Piece we want to retrieve the payload from. The client can either mandate that -// we retrieve the payload from a specific piece or we choose a Piece to retrieve the payload from, prioritizing -// a Piece for which an unsealed sector file already exists if possible. -// -// 1. For the `VerifiedDeal` flag in the response `PricingInput`, we are looking to answer the question "does there exist any verified storage deal for this `payloadCID`" ? -// -// 2. We also want to ensure that we return the `PieceSize` for the actual piece we want to retrieve the deal from. +// GetRetrievalPricingInput takes a set of candidate storage deals that can serve a retrieval request, +// and returns an minimally populated PricingInput. This PricingInput should be enhanced +// with more data, and passed to the pricing function to determine the final quoted price. func (rpn *retrievalProviderNode) GetRetrievalPricingInput(ctx context.Context, pieceCID cid.Cid, storageDeals []abi.DealID) (retrievalmarket.PricingInput, error) { resp := retrievalmarket.PricingInput{} @@ -154,13 +148,17 @@ func (rpn *retrievalProviderNode) GetRetrievalPricingInput(ctx context.Context, resp.PieceSize = ds.Proposal.PieceSize.Unpadded() } + // If we've discovered a verified deal with the required PieceCID, we don't need + // to lookup more deals and we're done. if resp.VerifiedDeal && resp.PieceSize != 0 { break } } + // Note: The piece size can never actually be zero. We only use it to here + // to assert that we didn't find a matching piece. if resp.PieceSize == 0 { - return resp, xerrors.New("failed to find matching piece, PieceSize is zero") + return resp, xerrors.New("failed to find matching piece") } return resp, nil