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

init ipfs in default document loader #513

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

F-Node-Karlsruhe
Copy link

Simple IPFS support in the default document loader with origin #512

Could be improved with the use of multiple public gateways and Promise.any() as shown in the issue above.

@F-Node-Karlsruhe
Copy link
Author

Example with W3C credential context:

ipfs://QmcpYBo2zpfSKdBAqY8HPSoBhEoVAb1WjPP2muehvYcCgs -> https://ipfs.io/ipfs/QmcpYBo2zpfSKdBAqY8HPSoBhEoVAb1WjPP2muehvYcCgs

Copy link
Member

@dlongley dlongley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm -- so at first glance at the PR title my thought was "We want to support IPFS but not in core." But looking at this PR, there's really not a whole lot here to make it happen.

@davidlehn what are your thoughts?

lib/documentLoaders/node.js Outdated Show resolved Hide resolved
@dlongley
Copy link
Member

@davidlehn, also, if we do want to accept this PR, I recommend we should reconsider adding the URL itself (or at least the beginning of it if it's huge) to the error message string itself (I know it's already in the details) -- as that has been a common complaint since those details are often not surfaced cleanly in various tools.

F-Node-Karlsruhe and others added 3 commits March 21, 2023 19:22
Co-authored-by: Dave Longley <dlongley@digitalbazaar.com>
Co-authored-by: Dave Longley <dlongley@digitalbazaar.com>
Co-authored-by: Dave Longley <dlongley@digitalbazaar.com>
@nklomp
Copy link

nklomp commented Mar 21, 2023

I think it would be nice if you could optionally provide your own IPFS host, with the current value as default.

@F-Node-Karlsruhe
Copy link
Author

F-Node-Karlsruhe commented Mar 21, 2023

I think it would be nice if you could optionally provide your own IPFS host, with the current value as default.

That would be great. In my opinion an semi-custom array along with Promise.any() like so would be desirable, but may be too much for the core/default document loader.

The proposed implementation might be just enough. A custom gateway is not necessarily needed due to the discoverability of content within the IPFS network; allowing to fetch content from your custom node through an arbitrary gateway. This might rather be integrated in one's own custom documentLoader.

F-Node-Karlsruhe added a commit to F-Node-Karlsruhe/ipfs-docs that referenced this pull request Mar 31, 2023
@F-Node-Karlsruhe
Copy link
Author

Should i further develop this PR to make it more sophisticated in order to merge or will you implement the feature from scratch in the next iteration anyway?

We are using IPFS for context quite a lot already and it would be really convenient to have the resolution embedded in the standard library :)

@dlongley
Copy link
Member

dlongley commented Jun 7, 2023

@davidlehn can we get your thoughts here? I think we would like this functionality, it's just a question of how it's done.

@nklomp
Copy link

nklomp commented Jun 7, 2023

Imo for VCI it should be pretty clear if you look at the spec and want something like this. Start with the issuer url in the DID doc, and follow normal OID4VCI discovery from there.

You are making sure that the issuer and well-knowns are served from the same origin this way. Our lib will not work if that is not the case, and we certainly wouldn't be adding something where well-knowns potentially might be served from different origins.

@ntn-x2
Copy link

ntn-x2 commented Jun 15, 2023

Hello! Has there been any progress lately on this PR?
Checking in because of OwnYourData/didlint#3, which results in our DID method (did:kilt), not being properly verified by the DIDLint tool. We saw that replacing IPFS links with any HTTP-based gateway (e.g., ipfs.io), correctly verifies the document structure.

@F-Node-Karlsruhe
Copy link
Author

Example with W3C credential context:

ipfs://QmcpYBo2zpfSKdBAqY8HPSoBhEoVAb1WjPP2muehvYcCgs -> https://ipfs.io/ipfs/QmcpYBo2zpfSKdBAqY8HPSoBhEoVAb1WjPP2muehvYcCgs

Based upon the IPFS native url scheme resolving towards an HTTP IPFS gateway

@F-Node-Karlsruhe
Copy link
Author

I think it would be nice if you could optionally provide your own IPFS host, with the current value as default.

True that. The ipfs.io gateway has not been very reliable recently. One should carefully choose the default/s

https://cloudflare-ipfs.com seems to be a good choice as well

Btw. curl has released support for IPFS already https://curl.se/changes.html#8_4_0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants