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

Not a valid CID; blockstore-datastore-adapter #3852

Closed
tabcat opened this issue Sep 6, 2021 · 5 comments · Fixed by #3877
Closed

Not a valid CID; blockstore-datastore-adapter #3852

tabcat opened this issue Sep 6, 2021 · 5 comments · Fixed by #3877
Assignees
Labels
need/triage Needs initial labeling and prioritization

Comments

@tabcat
Copy link
Contributor

tabcat commented Sep 6, 2021

  • Version:
    {
    "version": "0.10.4",
    "repo": "10",
    "commit": "",
    "interface-ipfs-core": "^0.150.1"
    }

  • Platform:
    Ubuntu x64 - Chromium Brave

  • Subsystem:
    blockstore-datastore-adapter

Severity:

Low - An optional functionality does not work.

Description:

We have browser tests in orbit-db that bundle orbit-db and use the provided ipfs bundle. the issue is that they use different instances of the CID class due to being bundled separately. This breaks some ipfs methods that run https://github.com/ipfs/js-blockstore-datastore-adapter/blob/b3592d6ba5709e885eeff4a5ecd8ddc09df03d41/src/index.js#L23 .
If we need to change how we've been bundling the tests that's fine. There isn't a non-hacky way i know of to get the same CID class the ipfs instance is using without having access to the IPFS factory.

Steps to reproduce the error:

git clone https://github.com/orbitdb/orbit-db.git &&
cd orbit-db &&
git checkout upgrade-ipfs &&
npm install &&
npm run test:browser-multiple-tabs

you can open test/browser/index.html to run the code in the browser

related:
orbitdb/orbitdb#913 (comment)

@tabcat tabcat added the need/triage Needs initial labeling and prioritization label Sep 6, 2021
@achingbrain
Copy link
Member

Recent versions of the IPFS stack have moved from the cids module to using the CID class exported by the multiformats module - it's smaller and faster!

Please update your code to use multiformats.

Where you had:

const CID = require('cids')

const fromString = new CID('QmFoo')

do:

const { CID } = require('multiformats/cid')

const fromString = CID.parse('QmFoo')

Where you had:

const CID = require('cids')

const fromUint8Array = new CID(Uint8Array.from([0, 1, 2]))

do:

const { CID } = require('multiformats/cid')

const fromString = CID.decode(Uint8Array.from([0, 1, 2]))

@tabcat
Copy link
Contributor Author

tabcat commented Sep 7, 2021

That is how we should be using them, I'll make sure there weren't any misses.

@tabcat
Copy link
Contributor Author

tabcat commented Sep 7, 2021

this is the resulting error and stack trace when running the browser test.

[Error: Error: Not a valid cid
    at g0 (file:///home/anders/Documents/dev/orbitdb/orbit-db/test/browser/ipfs.js:76:85952)
    at Fae.has (file:///home/anders/Documents/dev/orbitdb/orbit-db/test/browser/ipfs.js:76:87790)
    at Object.has (file:///home/anders/Documents/dev/orbitdb/orbit-db/test/browser/ipfs.js:40:130814)
    at Object.has (file:///home/anders/Documents/dev/orbitdb/orbit-db/test/browser/ipfs.js:40:122358)
    at V6e.has (file:///home/anders/Documents/dev/orbitdb/orbit-db/test/browser/ipfs.js:83:218018)
    at V6e.get (file:///home/anders/Documents/dev/orbitdb/orbit-db/test/browser/ipfs.js:83:217565)
    at r (file:///home/anders/Documents/dev/orbitdb/orbit-db/test/browser/ipfs.js:64:32401)
    at aY.get (file:///home/anders/Documents/dev/orbitdb/orbit-db/test/browser/ipfs.js:40:77543)
    at Object.read (file:///home/anders/Documents/dev/orbitdb/orbit-db/test/browser/orbitdb.js:55734:34)
    at OrbitDB.open (file:///home/anders/Documents/dev/orbitdb/orbit-db/test/browser/orbitdb.js:84884:31)]

we are using multiformats/cid in orbit-db-io which is the Object.read part of the error and the last point any orbit-db code touches the cid.

the read function
https://github.com/orbitdb/orbit-db-io/blob/ba7d324505b9fc20928d28d326a23ab328d00bfa/index.js#L53-L75
uses some cid handlers to parse cids using the multiformats/cid class
https://github.com/orbitdb/orbit-db-io/blob/ba7d324505b9fc20928d28d326a23ab328d00bfa/index.js#L12-L39
so the cid being given to ipfs.block.get should be an instance of the multiformats/cid class.

maybe i am missing something or should have been more clear to mention we are using the multiformats/cid class. to me the issue still seems to be that they are using the same cid class but exist as different objects in memory due to being bundled separately which doesnt pass the instanceof check.

achingbrain added a commit that referenced this issue Sep 17, 2021
Using `instanceOf` is not reliable since instances of classes can
cross the CLS/ESM boundary and/or come from different browser bundles.

Instead see if we can treat what we've been handed as a `Key` by
using the `Key.asKey` method.

Fixes #3852
achingbrain added a commit that referenced this issue Sep 17, 2021
Using `instanceOf` is not reliable since instances of classes can cross the CLS/ESM boundary and/or come from different browser bundles.

Instead see if we can treat what we've been handed as a `Key` by using the `Key.asKey` method.

Fixes #3852
@achingbrain
Copy link
Member

Could you try this again please? ipfs@0.58.6 should resolve the issue.

@tabcat
Copy link
Contributor Author

tabcat commented Sep 20, 2021

Working great now 👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
need/triage Needs initial labeling and prioritization
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants