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

Support arbitrary bytes for IPNS Record Value #273

Closed
Winterhuman opened this issue Mar 26, 2022 · 6 comments
Closed

Support arbitrary bytes for IPNS Record Value #273

Winterhuman opened this issue Mar 26, 2022 · 6 comments
Labels
need/triage Needs initial labeling and prioritization

Comments

@Winterhuman
Copy link

Currently, the IPNS spec states the following:

Value (bytes)
It can be any path, such as a path to another IPNS record, a dnslink path (eg. /ipns/example.com) or an IPFS path (eg. /ipfs/Qm...)

However, the implementation of go-ipns differs and won't care if Value is set to a non-path value (https://github.com/ipfs/go-ipns/blob/55c21a4ec0154fde42f56527a0ec1e0eb20195f8/ipns.go#L131), but, I argue supporting arbitrary bytes for Value should be supported.

My main argument for supporting this is to avoid inline CIDs, they're currently in a grey area where there's no consensus on how to limit the length of inline CIDs (or even if they should be limited: multiformats/multihash#130), so being able to set Value to whatever needs to be inlined into the IPNS record directly would be nice to have as an officially supported part of the spec.

(I could also argue it more closely follows the IPLD data model, but, IPNS records aren't IPLD objects anyways)

@Winterhuman Winterhuman added the need/triage Needs initial labeling and prioritization label Mar 26, 2022
@Jorropo
Copy link
Contributor

Jorropo commented Mar 26, 2022

My main argument for supporting this is to avoid inline CIDs, they're currently in a grey area where there's no consensus on how to limit the length of inline CIDs

I don't belive there is any consensus on the maximum size of IPNS's value. You could make the exact same argument about size issues in IPNS record sizes.

@Winterhuman
Copy link
Author

Winterhuman commented Mar 26, 2022

Good point, the IPNS record's maximum size also hasn't been decided. However, the decisions around how and when to limit inline CIDs is much more complicated then limiting IPNS records, inline CIDs have a much wider set of use-cases.

@lidel
Copy link
Member

lidel commented Aug 31, 2022

What is your use case @Winterhuman?
What type of data should be embedded inside IPNS record,
but at the same time should not be represented as an inlined CID?
Is clarifying max size in the spec the only reason here?

I think if we want to update the spec, then we should pick some sensible max size of the entire IPNS record (it is the whole record is what we send over the wire).

Matching the max block size we currently have in IPFS (~2MiB?) sounds like a good starting point.

@Winterhuman
Copy link
Author

Winterhuman commented Aug 31, 2022

@lidel ~2MiB seems like a good idea, however, I think the main (and maybe easier) issue to solve is just the difference between the implementation and the spec (as outlined as one of the goals in #314):

https://github.com/ipfs/go-ipns/blob/55c21a4ec0154fde42f56527a0ec1e0eb20195f8/ipns.go#L131 doesn't care what the Value is, but, the spec says it can be any path without any recommendation for or against arbitrary bytes for Value. Clarifying the spec one way or the other seems like a start, stating whether the Value must be a path or if it can be anything, from there we can think about max size

@aschmahmann
Copy link
Contributor

aschmahmann commented Aug 31, 2022

2MiB seems very large for IPNS records. IMO a limit here should be closer to the size of a CID than a block. While there's no CID size limit realistically systems have limits and some are smaller than others (there are some other issues discussing CID limit sizes). My guess is it's better to stick to less than 10kiB (very rough guesstimate), although maybe more is justified.

Consider for systems like the IPFS public DHT who is storing all those records and how much it'll bloat network storage. Similarly consider IPNS over PubSub and how large messages lead to lots of duplicate data being broadcast (also IIRC gossipsub has something like a 1MiB limit and there are issues discussing why it's a bad idea to send large messages over PubSub rather than sending something like a CID and then pulling the data).

@Winterhuman
Copy link
Author

Winterhuman commented Sep 5, 2022

It looks like #319 states that custom fields can be added to IPNS records, and that Kubo and such have actually been using a newer IPNS spec that's been undocumented up to this point, so that pretty much takes care of putting arbitrary data in IPNS records. I'm happy to close this issue and continue the max-size discussion on there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
Development

No branches or pull requests

4 participants