This repository has been archived by the owner on Mar 10, 2020. It is now read-only.
feat: support name.resolve of peerid as cid #1145
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a hacky way to support CIDs when API is go-ipfs 0.4.22 or older (convert toBase58btc before sending) License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
lidel
commented
Nov 5, 2019
src/name/resolve.js
Outdated
Comment on lines
18
to
29
| // normalize PeerIDs to Base58btc, so it works with go-ipfs <=0.4.22 | ||
| if (typeof (args) === 'string') { | ||
| try { | ||
| const path = args.split('/') | ||
| if (path.length > 2) { | ||
| path[2] = multihash.toB58String(new CID(path[2]).multihash) | ||
| args = path.join('/') | ||
| } | ||
| } catch (err) { | ||
| // noop | ||
| } | ||
| } |
There was a problem hiding this comment.
is there a better way? :/
There was a problem hiding this comment.
All we need to do is stringify args if it is a CID. We will skip the interface-ipfs-core test here until go-ipfs supports it.
There was a problem hiding this comment.
We don't stringify CIDs anywhere else, removed this hack and added TODO to unskip mentioned test when go-ipfs implements this.
There was a problem hiding this comment.
Ah interesting, I guess the querystring module calls toString on the CID then...
|
Please would you add the updated |
License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
|
@alanshaw done |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates to
interface-ipfs-core": "^0.118.0which includes test for resolving paths with libp2p-key represented as CIDv1 Base32.Open Problem
The problem is that go-ipfs 0.4.22 does not support PeerID as CID, so test added in ipfs-inactive/interface-js-ipfs-core#553 fails:
@alanshaw not sure how to handle this. Wait for go-ipfs v0.5.0? Fixup on the fly in the meantime?A hacky way to support CIDs when API is go-ipfs 0.4.22 or older is to convert toBase58btc before sending. See below.For now we skip cidv1b32 test, added todo to unskip when go-ipfs ships with native support.
closes #1147