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.

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

CAR file headers (may) need to be specified #6

Open
matheus23 opened this issue Aug 21, 2023 · 1 comment
Open

CAR file headers (may) need to be specified #6

matheus23 opened this issue Aug 21, 2023 · 1 comment

Comments

@matheus23
Copy link
Member

The block sending side in CAR mirror sends CAR files. However, some implementations require a header to have at least one root CID (iroh-car being one of them). It's somewhat unclear whether CAR files need to contain all their roots.

From the spec:

I see two possibilities:

  • Put the whole DAG root CID into the CAR file, even if the CAR file doesn't contain the root block, e.g. if it's a message later in the protocol
  • Put the first block's CID that's transferred in the CAR file into the header. That's currently the case in the WIP rs-car-mirror implementation.

Should we specify this at all? We could also just... not specify this, and implementations will need to decide themselves what to put in there, but be lenient in what they allow. (unfortunately iroh-car currently errors out if the header roots are empty)

@expede
Copy link
Member

expede commented Aug 21, 2023

Should we specify this at all? We could also just... not specify this

I'm in favour of leaving it to the implementations, but noting the situation in the spec. It's at a different layer from CAR Mirror. Any app that tries to use an arbitrary CAR library will run into this problem.

Put the first block's CID that's transferred in the CAR file into the header. That's currently the case in the WIP rs-car-mirror implementation.

I personally like this version. As you note, it's a bit of a hack.

This Issue is about the space of all possible CAR libraries, but do we know if there's a strong rationale to making this a hard requirement in iroh-car? Could we PR a change to remove this requirement?

some implementations require a header to have at least one root CID (iroh-car being one of them

Yeah, having some entrypoint is a good best practice IMO. That said, it's certainly not posisble to enforce that this be the rootmost entry in the CAR. If we go with the option that reifies a common root between the blocks, you don't gain anything more than the CID of the CAR itself.

It's somewhat unclear whether CAR files need to contain all their roots.

My guess about why this happened is that requiring all of the roots in the CAR file is exremely brittle. It is desirable so that you can jump around the structure easily, but you'll rediscover this structure as you process the CAR.

Obviously you won't be able to add all of the roots in a lot of streaming CAR cases, too.

@matheus23 matheus23 changed the title CAR file headers need to be specified CAR file headers (may) need to be specified Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants