fix: partial revert of 1be7c1a; make traverser process identity CIDs

It turns out that there are obscure cases where this matters, so we can't
as easily just ignore identity CIDs. Specifically, classic online Filecoin
deals that rely on Graphsync _and_ require identity CIDs be stored in the
CARv1 that is used to calculate CommP must see the identity CID pass through
the LinkSystem.

Unfortunately, the easiest way to deal with this is to send them over the wire
as if they are a normal block; which happens to be the "safe" backward
compatible way too. Less easy way would be to simulate it on both ends and
not send them, but we'll take the easy path for now.

Extension of tests in here to make sure that the full DAG is transferred even
in this case. Blockstore _must_ have identity CIDs in them, or be able to
respond properly to them.

Author: rvagg

impl/graphsync_test.go

@@ -352,8 +352,8 @@ func TestGraphsyncIdentityCIDRoundTrip(t *testing.T) {
 	})
 	progressChan, errChan := requestor.Request(ctx, td.host2.ID(), identityDag.RootLink, selectorparse.CommonSelector_ExploreAllRecursively)
 
-	identityDag.VerifyWholeDAG(ctx, progressChan)
 	testutil.VerifyEmptyErrors(ctx, t, errChan)
+	identityDag.VerifyWholeDAG(ctx, progressChan)
 	require.Len(t, td.blockStore1, len(identityDag.AllLinks), "did not store all blocks")
 
 	// verify listener

ipldutil/traverser.go

@@ -1,20 +1,17 @@
 package ipldutil
 
 import (
-	"bytes"
 	"context"
 	"errors"
 	"io"
 	"sync"
 
-	"github.com/ipfs/go-cid"
 	dagpb "github.com/ipld/go-codec-dagpb"
 	"github.com/ipld/go-ipld-prime"
 	cidlink "github.com/ipld/go-ipld-prime/linking/cid"
 	"github.com/ipld/go-ipld-prime/node/basicnode"
 	"github.com/ipld/go-ipld-prime/traversal"
 	"github.com/ipld/go-ipld-prime/traversal/selector"
-	"github.com/multiformats/go-multihash"
 
 	"github.com/filecoin-project/boost-graphsync/panics"
 )
@@ -168,23 +165,7 @@ func (t *traverser) NBlocksTraversed() int {
 	return t.blocksCount
 }
 
-func asIdentity(c cid.Cid) (digest []byte, ok bool, err error) {
-	dmh, err := multihash.Decode(c.Hash())
-	if err != nil {
-		return nil, false, err
-	}
-	ok = dmh.Code == multihash.IDENTITY
-	digest = dmh.Digest
-	return digest, ok, nil
-}
-
 func (t *traverser) loader(lnkCtx ipld.LinkContext, lnk ipld.Link) (io.Reader, error) {
-	if digest, ok, err := asIdentity(lnk.(cidlink.Link).Cid); ok {
-		return io.NopCloser(bytes.NewReader(digest)), nil
-	} else if err != nil {
-		return nil, err
-	}
-
 	// A StorageReadOpener call came in; update the state and release the lock.
 	// We can't simply unlock the mutex inside the <-t.responses case,
 	// as then we'd deadlock with the other side trying to send.

testutil/identity.go

@@ -28,15 +28,17 @@ type TestIdentityDAG struct {
 	t      testing.TB
 	loader ipld.BlockReadOpener
 
-	RootLink ipld.Link
-	AllLinks []ipld.Link
+	RootLink        ipld.Link
+	AllLinks        []ipld.Link
+	AllLinksNoIdent []ipld.Link
 }
 
 /* ugly, but it makes a DAG with paths that look like this but doesn't involved dag-pb or unixfs */
-var identityDagPaths = []string{
+var identityDagLinkPaths = []string{
 	"",
 	"a/!foo",
 	"a/b/!bar",
+	"a/b/c/!baz", // identity
 	"a/b/c/!baz/identity jump",
 	"a/b/c/!baz/identity jump/these are my children/blip",
 	"a/b/c/!baz/identity jump/these are my children/bloop",
@@ -49,13 +51,15 @@ var identityDagPaths = []string{
 func SetupIdentityDAG(
 	ctx context.Context,
 	t testing.TB,
-	lsys ipld.LinkSystem) *TestIdentityDAG {
-
+	lsys ipld.LinkSystem,
+) *TestIdentityDAG {
 	allLinks := make([]ipld.Link, 0)
+	allLinksNoIdent := make([]ipld.Link, 0)
 	store := func(lp datamodel.LinkPrototype, n datamodel.Node) datamodel.Link {
 		l, err := lsys.Store(linking.LinkContext{}, lp, n)
 		require.NoError(t, err)
 		allLinks = append(allLinks, l)
+		allLinksNoIdent = append(allLinksNoIdent, l)
 		return l
 	}
 
@@ -98,6 +102,11 @@ func SetupIdentityDAG(
 		identBytes := must(ipld.Encode(ident, dagjson.Encode))(t)
 		mh := must(multihash.Sum(identBytes, multihash.IDENTITY, len(identBytes)))(t)
 		bazIdent = cidlink.Link{Cid: cid.NewCidV1(cid.DagJSON, mh)}
+		w, wc, err := lsys.StorageWriteOpener(linking.LinkContext{})
+		require.NoError(t, err)
+		w.Write(identBytes)
+		require.NoError(t, wc(bazIdent))
+		allLinks = append(allLinks, bazIdent)
 	}
 	bar := store(djlp, basicnode.NewInt(2020202020202020))
 	foo := store(djlp, basicnode.NewInt(1010101010101010))
@@ -117,10 +126,11 @@ func SetupIdentityDAG(
 	}))(t))
 
 	return &TestIdentityDAG{
-		t:        t,
-		loader:   lsys.StorageReadOpener,
-		RootLink: root,
-		AllLinks: reverse(allLinks), // TODO: slices.Reverse post 1.21
+		t:               t,
+		loader:          lsys.StorageReadOpener,
+		RootLink:        root,
+		AllLinks:        reverse(allLinks), // TODO: slices.Reverse post 1.21
+		AllLinksNoIdent: reverse(allLinksNoIdent),
 	}
 }
 
@@ -150,15 +160,15 @@ func (tid *TestIdentityDAG) checkResponses(responses []graphsync.ResponseProgres
 	for ii, response := range responses {
 		// only check the paths that have links, assume the rest are just describing
 		// the non-link nodes of the DAG
-		if response.Path.String() == identityDagPaths[pathIndex] {
+		if response.Path.String() == identityDagLinkPaths[pathIndex] {
 			if response.LastBlock.Link != nil {
 				expectedLink := tid.AllLinks[pathIndex]
 				require.Equal(tid.t, expectedLink.String(), response.LastBlock.Link.String(), fmt.Sprintf("response %d has correct link (%d)", ii, pathIndex))
 			}
 			pathIndex++
 		}
 	}
-	require.Equal(tid.t, len(identityDagPaths), pathIndex, "traverses all nodes")
+	require.Equal(tid.t, len(identityDagLinkPaths), pathIndex, "traverses all nodes")
 }
 
 func must[T any](v T, err error) func(t testing.TB) T {