Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failure to retrieve when root payload CID is an identity CID #715

Merged
merged 4 commits into from
Jan 26, 2023
Merged

Failure to retrieve when root payload CID is an identity CID #715

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e58621
test(itests): demonstrate failure to retrieve
hannahhoward Aug 19, 2022
0deee0b
fix: cleanup test & handle identity roots
rvagg Aug 23, 2022
9b370fa
chore: update go-fil-markets with identity CID fix
rvagg Sep 1, 2022
0412500
doc: make it clear InlineBuilder isn't recommended
rvagg Sep 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions car/car_offset_writer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ func DAGImport(dserv format.DAGService, fi io.Reader) (format.Node, error) {
Maxlinks: 1024,
RawLeaves: true,

// NOTE: InlineBuilder not recommended, we are using this to test identity CIDs
CidBuilder: cidutil.InlineBuilder{
Builder: prefix,
Limit: 32,
Expand Down
3 changes: 2 additions & 1 deletion itests/dummydeal_offline_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ func TestDummydealOffline(t *testing.T) {

// make an offline deal
offlineDealUuid := uuid.New()
res, err := f.MakeDummyDeal(offlineDealUuid, carFilepath, rootCid, "", true)
dealRes, err := f.MakeDummyDeal(offlineDealUuid, carFilepath, rootCid, "", true)
res := dealRes.Result
require.NoError(t, err)
require.True(t, res.Accepted)
res, err = f.Boost.BoostOfflineDealWithData(context.Background(), offlineDealUuid, carFilepath)
Expand Down
37 changes: 19 additions & 18 deletions itests/dummydeal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ func TestDummydealOnline(t *testing.T) {
failingFilepath, err := testutil.CreateRandomFile(tempdir, 5, 2000000)
require.NoError(t, err)

// NOTE: these calls to CreateDenseCARv2 have the identity CID builder enabled so will
// produce a root identity CID for this case. So we're testing deal-making and retrieval
// where a DAG has an identity CID root
rootCid, carFilepath, err := testutil.CreateDenseCARv2(tempdir, randomFilepath)
require.NoError(t, err)

Expand All @@ -56,7 +59,7 @@ func TestDummydealOnline(t *testing.T) {
// Make a deal
res, err := f.MakeDummyDeal(dealUuid, carFilepath, rootCid, server.URL+"/"+filepath.Base(carFilepath), false)
require.NoError(t, err)
require.True(t, res.Accepted)
require.True(t, res.Result.Accepted)
log.Debugw("got response from MarketDummyDeal", "res", spew.Sdump(res))

time.Sleep(2 * time.Second)
Expand All @@ -66,28 +69,26 @@ func TestDummydealOnline(t *testing.T) {
failingDealUuid := uuid.New()
res2, err2 := f.MakeDummyDeal(failingDealUuid, failingCarFilepath, failingRootCid, server.URL+"/"+filepath.Base(failingCarFilepath), false)
require.NoError(t, err2)
require.Contains(t, res2.Reason, "no space left", res2.Reason)
require.Contains(t, res2.Result.Reason, "no space left", res2.Result.Reason)
log.Debugw("got response from MarketDummyDeal for failing deal", "res2", spew.Sdump(res2))

// Wait for the first deal to be added to a sector and cleaned up so space is made
err = f.WaitForDealAddedToSector(dealUuid)
require.NoError(t, err)
time.Sleep(100 * time.Millisecond)

// *********************************************************************
// Disable this part of the test for now because there is a bug in lotus
// introduced in this PR that causes the test to fail:
// https://github.com/filecoin-project/lotus/pull/9174/files#top
// *********************************************************************
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nonsense This was previously commented out in a separate update. I added it back in to see if things are passing now which seems to be the case.


//// Make a third deal - it should succeed because the first deal has been cleaned up
//passingDealUuid := uuid.New()
//res2, err2 = f.MakeDummyDeal(passingDealUuid, failingCarFilepath, failingRootCid, server.URL+"/"+filepath.Base(failingCarFilepath), false)
//require.NoError(t, err2)
//require.True(t, res2.Accepted)
//log.Debugw("got response from MarketDummyDeal", "res2", spew.Sdump(res2))
//
//// Wait for the deal to be added to a sector
//err = f.WaitForDealAddedToSector(passingDealUuid)
//require.NoError(t, err)
// Make a third deal - it should succeed because the first deal has been cleaned up
passingDealUuid := uuid.New()
res2, err2 = f.MakeDummyDeal(passingDealUuid, failingCarFilepath, failingRootCid, server.URL+"/"+filepath.Base(failingCarFilepath), false)
require.NoError(t, err2)
require.True(t, res2.Result.Accepted)
log.Debugw("got response from MarketDummyDeal", "res2", spew.Sdump(res2))

// Wait for the deal to be added to a sector
err = f.WaitForDealAddedToSector(passingDealUuid)
require.NoError(t, err)

// rootCid is an identity CID
outFile := f.RetrieveDirect(ctx, t, rootCid, &res.DealParams.ClientDealProposal.Proposal.PieceCID, true)
kit.AssertFilesEqual(t, randomFilepath, outFile)
}
82 changes: 56 additions & 26 deletions itests/framework/framework.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ import (
"github.com/filecoin-project/lotus/storage/paths"
"github.com/filecoin-project/lotus/storage/pipeline/sealiface"
"github.com/google/uuid"
blocks "github.com/ipfs/go-block-format"
"github.com/ipfs/go-cid"
"github.com/ipfs/go-datastore"
files "github.com/ipfs/go-ipfs-files"
Expand All @@ -59,6 +60,7 @@ import (
"github.com/ipld/go-car"
"github.com/libp2p/go-libp2p"
"github.com/libp2p/go-libp2p/core/host"
"github.com/multiformats/go-multihash"
"github.com/stretchr/testify/require"
"golang.org/x/sync/errgroup"
)
Expand Down Expand Up @@ -477,7 +479,12 @@ func (f *TestFramework) WaitForDealAddedToSector(dealUuid uuid.UUID) error {
}
}

func (f *TestFramework) MakeDummyDeal(dealUuid uuid.UUID, carFilepath string, rootCid cid.Cid, url string, isOffline bool) (*api.ProviderDealRejectionInfo, error) {
type DealResult struct {
DealParams types.DealParams
Result *api.ProviderDealRejectionInfo
}

func (f *TestFramework) MakeDummyDeal(dealUuid uuid.UUID, carFilepath string, rootCid cid.Cid, url string, isOffline bool) (*DealResult, error) {
cidAndSize, err := storagemarket.GenerateCommP(carFilepath)
if err != nil {
return nil, err
Expand Down Expand Up @@ -543,7 +550,11 @@ func (f *TestFramework) MakeDummyDeal(dealUuid uuid.UUID, carFilepath string, ro
},
}

return f.Client.StorageDeal(f.ctx, dealParams, peerID)
res, err := f.Client.StorageDeal(f.ctx, dealParams, peerID)
return &DealResult{
DealParams: dealParams,
Result: res,
}, err
}

func (f *TestFramework) signProposal(addr address.Address, proposal *market.DealProposal) (*market.ClientDealProposal, error) {
Expand Down Expand Up @@ -667,6 +678,47 @@ func (f *TestFramework) Retrieve(ctx context.Context, t *testing.T, deal *cid.Ci
require.NoError(t, err)
require.NotEmpty(t, offers, "no offers")

return f.retrieve(ctx, t, offers[0], carExport)
}

func (f *TestFramework) ExtractFileFromCAR(ctx context.Context, t *testing.T, file *os.File) string {
bserv := dstest.Bserv()
ch, err := car.LoadCar(ctx, bserv.Blockstore(), file)
require.NoError(t, err)

var b blocks.Block
if ch.Roots[0].Prefix().MhType == multihash.IDENTITY {
mh, err := multihash.Decode(ch.Roots[0].Hash())
require.NoError(t, err)
b, err = blocks.NewBlockWithCid(mh.Digest, ch.Roots[0])
require.NoError(t, err)
} else {
b, err = bserv.GetBlock(ctx, ch.Roots[0])
require.NoError(t, err)
}

nd, err := ipld.Decode(b)
require.NoError(t, err)

dserv := dag.NewDAGService(bserv)
fil, err := unixfile.NewUnixfsFile(ctx, dserv, nd)
require.NoError(t, err)

tmpFile := path.Join(t.TempDir(), fmt.Sprintf("file-in-car-%d", rand.Uint32()))
err = files.WriteTo(fil, tmpFile)
require.NoError(t, err)

return tmpFile
}

func (f *TestFramework) RetrieveDirect(ctx context.Context, t *testing.T, root cid.Cid, pieceCid *cid.Cid, carExport bool) string {
offer, err := f.FullNode.ClientMinerQueryOffer(ctx, f.MinerAddr, root, pieceCid)
require.NoError(t, err)

return f.retrieve(ctx, t, offer, carExport)
}

func (f *TestFramework) retrieve(ctx context.Context, t *testing.T, offer lapi.QueryOffer, carExport bool) string {
p := path.Join(t.TempDir(), "ret-car-"+t.Name())
carFile, err := os.Create(p)
require.NoError(t, err)
Expand All @@ -680,7 +732,7 @@ func (f *TestFramework) Retrieve(ctx context.Context, t *testing.T, deal *cid.Ci
updates, err := f.FullNode.ClientGetRetrievalUpdates(updatesCtx)
require.NoError(t, err)

retrievalRes, err := f.FullNode.ClientRetrieve(ctx, offers[0].Order(caddr))
retrievalRes, err := f.FullNode.ClientRetrieve(ctx, offer.Order(caddr))
require.NoError(t, err)
consumeEvents:
for {
Expand Down Expand Up @@ -708,7 +760,7 @@ consumeEvents:

require.NoError(t, f.FullNode.ClientExport(ctx,
lapi.ExportRef{
Root: root,
Root: offer.Root,
DealID: retrievalRes.DealID,
},
lapi.FileRef{
Expand All @@ -723,25 +775,3 @@ consumeEvents:

return ret
}

func (f *TestFramework) ExtractFileFromCAR(ctx context.Context, t *testing.T, file *os.File) string {
bserv := dstest.Bserv()
ch, err := car.LoadCar(ctx, bserv.Blockstore(), file)
require.NoError(t, err)

b, err := bserv.GetBlock(ctx, ch.Roots[0])
require.NoError(t, err)

nd, err := ipld.Decode(b)
require.NoError(t, err)

dserv := dag.NewDAGService(bserv)
fil, err := unixfile.NewUnixfsFile(ctx, dserv, nd)
require.NoError(t, err)

tmpFile := path.Join(t.TempDir(), fmt.Sprintf("file-in-car-%d", rand.Uint32()))
err = files.WriteTo(fil, tmpFile)
require.NoError(t, err)

return tmpFile
}
1 change: 1 addition & 0 deletions testutil/car.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ func WriteUnixfsDAGTo(path string, into ipldformat.DAGService) (cid.Cid, error)
params := ihelper.DagBuilderParams{
Maxlinks: unixfsLinksPerLevel,
RawLeaves: true,
// NOTE: InlineBuilder not recommended, we are using this to test identity CIDs
CidBuilder: cidutil.InlineBuilder{
Builder: prefix,
Limit: 126,
Expand Down
1 change: 1 addition & 0 deletions transport/httptransport/http_transport_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,7 @@ func dagImport(dserv format.DAGService, fi io.Reader) (format.Node, error) {
Maxlinks: 1024,
RawLeaves: true,

// NOTE: InlineBuilder not recommended, we are using this to test identity CIDs
CidBuilder: cidutil.InlineBuilder{
Builder: prefix,
Limit: 32,
Expand Down