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

Minimum DataSize value when validating CARv2 header #310

Open
masih opened this issue Jun 30, 2022 · 4 comments
Open

Minimum DataSize value when validating CARv2 header #310

masih opened this issue Jun 30, 2022 · 4 comments
Assignees

Comments

@masih
Copy link
Member

masih commented Jun 30, 2022

Question: What should the minimum accepted DataSize value be in a CARv2 header?

  • Could it be zero?
  • or should it be the minimum valid size for a CARv1 which is header plus at least one root CID that comes to 26 bytes.

See discussion:

@rvagg
Copy link
Member

rvagg commented Jun 30, 2022

or should it be the minimum valid size for a CARv1 which is header plus at least one root CID that comes to 26 bytes.

Oh yeah, it needs an inner header..

Doesn't need a CID though. I think 0xa265726f6f7473806776657273696f6e01 0x11a265726f6f7473806776657273696f6e01 might be the shortest possible CARv1, it's just {version:1,roots:[]} with a varint length prefix. So that would be 17 18 bytes.

But we need to test what even works with both v0 and v2 CARv1 readers, maybe they don't even like that.

@rvagg rvagg self-assigned this Jun 30, 2022
@rvagg rvagg moved this to 🗄️ Backlog in IPLD team's weekly tracker Jun 30, 2022
@masih
Copy link
Member Author

masih commented Jun 30, 2022

If no roots are needed do you recall the rationale for this by any chance?

@rvagg
Copy link
Member

rvagg commented Jun 30, 2022

Why yes I do! (btw I thought we'd removed that for the v2 reader, but sadly not

if len(ch.Roots) == 0 {
return nil, fmt.Errorf("empty car, no roots")
}
, so I guess we need more than 18 bytes, but just enough for a valid CID). See "Unresolved issues" in the spec: https://ipld.io/specs/transport/car/carv1/#number-of-roots.

go-car was originally designed for Filecoin deals, a single root, which we widened a little for general use but never removed the >0 restriction because we didn't want to upset the Lotus apple-cart (or is that a Lotus cart?). js-car allows for zero roots, which can be useful for "this is just a bundle of blocks" and IIRC web3.storage and some of their backend stuff are using CARs in this way.

@masih
Copy link
Member Author

masih commented Jun 30, 2022

Interesting thank you

@rvagg rvagg moved this from 🗄️ Backlog to 🥞 Todo in IPLD team's weekly tracker Aug 2, 2022
@rvagg rvagg moved this from 🥞 Todo to 🗄️ Backlog in IPLD team's weekly tracker Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🗄️ Backlog
Development

No branches or pull requests

2 participants