Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Support identity CIDs #3289

Closed
oed opened this issue Sep 18, 2020 · 6 comments · Fixed by ipfs/js-ipfs-repo#297 or #3616
Closed

Support identity CIDs #3289

oed opened this issue Sep 18, 2020 · 6 comments · Fixed by ipfs/js-ipfs-repo#297 or #3616
Labels
kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked

Comments

@oed
Copy link
Contributor

oed commented Sep 18, 2020

  • Version: 0.50.2

  • Platform: macos

  • Subsystem: dag api

Severity:

Low

Description:

I'm wondering how to work with "identity" CIDs. Tried some apis that I thought would intuitively work with them, but they seem to hang (more info below).

An identity CID is created by using the identity multihash which has codec 0x00 and the value of the hash is just the data itself. An example can be found here: https://cid.ipfs.io/#bafyqacnbmrqxgzdgdeaui

Is there some other way that is recomended to work with identity CIDs than the normal dag api?

Steps to reproduce the error:

Both of these method calls just hang:

await ipfs.dag.get(new CID('bafyqacnbmrqxgzdgdeaui'))
await ipfs.block.get(new CID('bafyqacnbmrqxgzdgdeaui'))
@oed oed added the need/triage Needs initial labeling and prioritization label Sep 18, 2020
@lidel lidel added the kind/bug A bug in existing code (including security flaws) label Sep 21, 2020
@lidel
Copy link
Member

lidel commented Sep 21, 2020

Definitely looks like a bug.
Resolution of identity CIDs should skip content routing and return inlined data immediately.

Ref. bafkqaaa is an empty one

@jacobheun jacobheun added the status/ready Ready to be worked label Sep 24, 2020
@lidel
Copy link
Member

lidel commented Apr 2, 2021

I confirm this is still broken in the latest js-ipfs:

await ipfs.dag.get(new CID('bafkqaaa'))

Identity CIDs are super useful for passing tiny amounts of data, confirmation that node works fine in webui and other places like tests (to speed them up by skippimng content routing where not in scope), so I'm bumping this in priority and adding to our maintenance project board so we can discuss this next week.

I did not dig into js-ipfs internals, but: /dag/get.js delegates things to ipld.get and ipld.resolve – @warpfork/@vmx @achingbrain any thoughts where identity is really handled? (js-ipfs or ipld?)

@lidel lidel added P1 High: Likely tackled by core team if no one steps up and removed need/triage Needs initial labeling and prioritization labels Apr 2, 2021
@lidel lidel changed the title Working with identity CIDs Support identity CIDs Apr 2, 2021
@lidel lidel added this to Backlog in Maintenance Priorities - Go Apr 5, 2021
@lidel lidel moved this from Backlog to Weekly Candidates in Maintenance Priorities - Go Apr 5, 2021
@hannahhoward
Copy link
Contributor

@lidel Heads up on strategy for addressing this:
I did some digging, and it looks like we simply don’t have the equavalent in JS to golang code that adds support for Identity hashes.

My intent was to mirror this strategy in JS -- the way golang handles identity hashes is it adds an extension to the blockstore that when checks if request CIDs are identity hashes, and if so, just returns a block made from the decoded hash. You can find the implementation here: https://github.com/ipfs/go-ipfs-blockstore/blob/master/idstore.go

My goal was just to PR against js-ipfs-repo, with a similar implementation to augment the existing blockstore, and then to bubble that up through js-ipfs.

@achingbrain
Copy link
Member

ipfs-repo exposes instances of interface-datastore as the .blocks property so you'd end up PRing each datastore implementation to make the same change since there's no convenient central routing point there.

You could change the repo to expose a facade that delegates to a datastore instance and do the identity CID check there but I think we need to look at the repo structure in the scope of protocol/web3-dev-team#47

An easier way would be to PR the block service - it's used by the block api and by IPLD, and delegates to either bitswap or the repo so you could do the check there.

@hannahhoward
Copy link
Contributor

@achingbrain .blocks seems not to be a blockstore wrapper on top of interface-datastore by my read --
https://github.com/ipfs/js-ipfs-repo/blob/master/src/blockstore.js
https://github.com/ipfs/js-ipfs-repo/blob/master/src/index.js#L69

My intent was was to put the code in there, and I believe you shouldn't need to patch the interface datastore implementations, because this sits on top. The rationale was just to preserve the symmetry between go-ipfs & js-ipfs strategies, but I'm definitely open to doing it in the block service if you think it's important.

@achingbrain
Copy link
Member

.blocks seems not to be a blockstore wrapper on top of interface-datastore by my read

Quite right, that'll teach me to go by memory - sorry for the confusion.

but I'm definitely open to doing it in the block service if you think it's important.

No, put it in the repo as proposed, I think @vmx will eventually refactor the block service away anyway.

hannahhoward added a commit to hannahhoward/js-ipfs-repo that referenced this issue Apr 7, 2021
creates a layer on top of blockstore which makes the blockstore compatible with identity hashes.
Based off of https://github.com/ipfs/go-ipfs-blockstore/blob/master/idstore.go

fix ipfs/js-ipfs#3289
@lidel lidel moved this from Weekly Candidates to In Progress in Maintenance Priorities - Go Apr 12, 2021
Maintenance Priorities - Go automation moved this from In Progress to Done Apr 14, 2021
achingbrain pushed a commit to ipfs/js-ipfs-repo that referenced this issue Apr 14, 2021
- Separate out typing for the blockstore interface
- Implement `idstore` based on https://github.com/ipfs/go-ipfs-blockstore/blob/master/idstore.go - this leaves the original blockstore implementation unchanged but adds a composable layer applied on top to add compatibility with identity hashes (served as the same blockstore interface)
- add `idstore` in construction of `repo.blocks`

fix ipfs/js-ipfs#3289
achingbrain added a commit that referenced this issue Apr 28, 2021
- Adds support for identity hashes in ipfs-repo blockstore
- Add core interface tests for block.get and dag.get when fetching identity hashes

Refs: ipfs/js-ipfs-repo#297
Closes:  #3289

Co-authored-by: Alex Potsides <alex@achingbrain.net>
@BigLep BigLep removed this from Done in Maintenance Priorities - Go May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up status/ready Ready to be worked
Projects
No open projects
5 participants