fix: remove TraverseLinksOnlyOnce on piece CID

[Jorropo]

This follows lotus's implementation:
https://github.com/filecoin-project/lotus/blob/a843c52e38da13da489cbe6b290ea49b2660b3fb/node/impl/client/client.go#L1405-L1412

I have no idea if that would fix things tho. (need tests)

[alvin-reyes]

Just adding here. Boost doesn't have the TraverseLinksOnlyOnce

https://github.com/filecoin-project/boost/blob/fe1b0139be4c769a1348abd9bb0a6e4e55973435/cmd/boostx/utils_cmd.go#L310

[ribasushi]

This patch is backwards, DO NOT merge. It is lotus gencar that needs fixing

TraverseLinksOnlyOnce is the "canonocal" way

• kubo does it: https://github.com/ipfs/kubo/blob/v0.14.0/core/commands/dag/export.go#L52
• lotus retrieval does it: https://github.com/filecoin-project/lotus/blob/v1.16.1/node/impl/client/client.go#L1036

cc @rvagg to confirm as he worked in this area recently

N.b. really using any type of traversal for filecoin storage is a mistake as it is way too fragile. Instead you need to use a dagstore to save the car-files as is and then transfer them on to an SP.

[Jorropo]

It is lotus gencar that needs fixing

I totally agree, but isn't there a spec about how to generate piece CID from a DAG ?
I would assume lotus & boost follow it and you need to update that first no ?

kubo does it

Kubo doesn't care about filecoin's convention, it just want an efficient format.

DO NOT merge

I don't see why not merging if it's a good first aid thing.
They could use this to fix uploads right now while they work on a better solution.

[ribasushi]

isn't there a spec about how to generate piece CID from a DAG

@Jorropo no. Filecoin stores streams of bytes. This is what PieceCid is computed against. E.g.:

~$ dd if=/dev/urandom bs=10m count=1 | stream-commp 
1+0 records in
1+0 records out
10485760 bytes transferred in 0.544082 secs (19272383 bytes/sec)

CommPCid: baga6ea4seaqlgevbrrqilsnqknbh42kyxyftk4he2nbk2sjkeaup3pebctvmoaa
Payload:            10485760 bytes
Unpadded piece:     16646144 bytes
Padded piece:       16777216 bytes

☝️ I could make an off-network transfer deal with this and an SP will take it and prove it with zero problems.

However, effectively all retrieval subsystems currently deployed on Filecoin expect by "gentleman's agreement" these opaque bytes to be a CarV1, with enumerable IPLD blocks and all that. So you store car files. How these files were constructed is unspecified, and there are multiple variations in the wild already ( not just the mismatch above ). Hence my comment about it being unworkably fragile. See also the end of the commit msg here: ribasushi/spade@1fb8a2f005cafc

[Jorropo]

Filecoin stores streams of bytes.

I know that, but AFAIK estuary's transfer protocol is graphsync which is not a stream of bytes but a stream of IPLD blocks.

Isn't there a cannonical way to serialise an IPLD dag (such as got from a graphsync session) into a stream of bytes ? (and wouldn't that have the bad no TraverseLinksOnlyOnce option)

That is my current understanding of the situation, pls tell me if I'm wrong.

[Jorropo]

I could make an off-network transfer deal with this and an SP will take it and prove it with zero problems.

Yeah I get that you could just make something better, I'm arguing to use this bad option (if that fixes it) while a better solution is worked on.

[ribasushi]

Isn't there a cannonical way to serialise an IPLD dag (such as got from a graphsync session) into a stream of bytes

No, and there can't be a generic one ( think dags larger than 32g )

Kubo doesn't care about filecoin's convention, it just want an efficient format.

Kubo is what produces a large chunk of the stuff in the wild right now.

However I think your actual problem is deeper - anyone switching from go-car-v1 to go-car-v2 will have this problem, since go-car-v1 always had this option behavior on: https://github.com/ipld/go-car/blob/v2.4.1/selectivecar.go#L231-L232

cc @hannahhoward for 🧠⛈️

[Jorropo]

Kubo is what produces a large chunk of the stuff in the wild right now.

Offtopic but, we want to make that multithreaded making them non deterministic btw. (not that matter if you are using a protocol that send opaque raw bytes instead of an IPLD dag).

[rvagg]

First, on TraverseLinksOnlyOnce—we introduced it as a way to optimise the go-ipld-prime for the exhaustive case (so any use of an explore-all or match-all). Because go-ipld-prime can run traversals on selectors that have conditions, there are many cases where you want to make sure it can revisit the same blocks more than once under different conditions. BUT this is not the case for what we're doing here, or for the standard online-deal or "here's a root and my block store, put it on filecoin" case, where we want to run an exhaustive selector and put every visited block into the deal—this is what's coded in all over the place for deals and what we're looking at here. Of course it's not the only way to make deals, more on that below.

It's true that without TraverseLinksOnlyOnce, a complex DAG may have the same block visited multiple times. This is not (generally) the case for UnixFS DAGs which are fairly straightforward but we're seeing people actually use IPLD to make complex DAGs (yay, but also, whoa), so there are going to be cases where a go-ipld-prime traversal will want to revisit blocks, and if you're doing a "run the traversal and watch which blocks are loaded and in which order", which we do for the case in question, then this matters. But, that's why single-visit is also built into SelectiveCar @ https://github.com/ipld/go-car/blob/3264624f19239211abfa12bf91471ceb0cfc9afc/selectivecar.go#L231-L232. This has been there since pre-Filecoin launch and it's been stable ever since, there's not been a version of SelectiveCar that doesn't do this. There is not a v2 form of SelectiveCar, it's all v0 (i.e. it's in the go-car root, not the v2 directory). There is a v2/ *Selective* thing that mirrors the same functionality @ https://github.com/ipld/go-car/blob/master/v2/selective.go and you can see it doesn't have quite the same code. But it defers down through a "writing loader" that does the same thing @ https://github.com/ipld/go-car/blob/3264624f19239211abfa12bf91471ceb0cfc9afc/v2/internal/loader/writing_loader.go#L107-L110 and it also uses the BlockstoreAllowDuplicatePuts option to do the same thing from a API perspective (default = false ... and I think this is probably not be right for non-exhaustive selectors but that's a problem in a different direction, filed that @ https://github.com/ipld/go-car-priv/issues/22). BUT this form of selective CAR (v2) isn't in wide use, doing some GitHub code searches for the various factory functions exported there show some of Will's newer experimental code (w3rc, gateway-prime) and some of Aarsh's Saturn code. Boost, Lotus, Markets, FilClient, all use SelectiveCar from go-car v0 which has this where they want to do an exhaustive DAG walk-and-save, and in these (exhaustive) cases, TraverseLinksOnlyOnce is a very-recommended optimisation that should be applied to make it faster and also avoid the complex-DAG problems that MaxTraversalLinks exists to bail-out on (it should also always be applied to avoid DoSing the traverser, but TraverseLinksOnlyOnce means that's almost impossible, so yay for exhaustive + simple DAGs). So we should have that option here, we should enable it in Lotus, we should even make sure

The algorithm for storing complete DAGs is pretty straightforward and when followed e2e should (will) result in a stable CommP, but we have an explosion of tooling doing this and a lot of variety in what we are storing, and as @ribasushi is alluding to, we have an increasing number of clients wanting to just throw arbitrary CARs (or arbitrary bytes) into Filecoin.

There is a philosophical question about whether Filecoin should be block-device or a DAG-based block-store—that seems to define a large amount of disagreements in this deal-making area; I'm not entirely sure why it generates so much heat except that the "just store my bytes" people are frustrated and impatient by the fact that Lotus @ launch has been complete-DAG focused and we've been very slow to make space for the "I don't want you to re-walk my DAG" and "just store my bytes" people (offline deals + bidbot helped, now Boost HTTP is really completing that too I believe).

When we have people storing non-DAGs, or just payloads with bundles of blocks that they want to be able to get out in that order, then we get instability in CommP because we can't apply our stable traversal rules to it. When doing a classic "I have a DAG, here's the root and access to my blockstore, go store it" type of deal then it's all cool, we can do that with go-merkledag/Walk (old way, exhaustive, pretty efficient but not very compatible with the newer parts of our stack) or go-ipld-prime traversal with TraverseLinksOnlyOnce for efficiency and MaxTraversalLinks because currently our traversals aren't optimised for complex DAGs. But that same algorithm works with SelectiveCar and Graphsync, it's built into Lotus, Boost's non-HTTP deals, Markets. Boost HTTP doesn't so you can just throw it a CAR and a CommP and I believe it'll just CommP the CAR bytes on its end without a re-traversal.

BUT we need to be very clear that this isn't the always-case, there are a lot of deals being done on arbitrary CARs or just arbitrary bytes and there's a good case for this and we should be aware of it and allow for it.

If Estuary sticks with the plain IPFS DAG model ("here's my DAG and here's its root") then it can stick to the SelectiveCar model as long as it's all done internally - allowing the user to provide a CommP for their CAR is a bad idea unless that CAR is going e2e into the deal (it's not). You have to re-traverse the DAG into a properly stable form and then CommP the result. I believe it used to be the case until application-research/estuary#250, but looking through the Esturary code it appears it'll still be doing a "do I have a CommP for this?" lookup in the database before doing a deal so maybe it's still got old user-supplied CommP's hanging around.