feat: Issue Credentials V2: W3C/ jsonld credentials

[NB-MikeRichardson]

Merge of jsonld-credentials -> 0.3.0-pre

[TimoGlastra]

Hmm it looks like github somehow added all my comments twice, please ignore the duplication! :)

[codecov-commenter]

Codecov Report

Merging #795 (ffa5403) into main (ab403c9) will increase coverage by 0.07%. The diff coverage is 84.87%.

@@            Coverage Diff             @@
##             main     #795      +/-   ##
==========================================
+ Coverage   88.34%   88.42%   +0.07%     
==========================================
  Files         701      706       +5     
  Lines       16110    16341     +231     
  Branches     2597     2639      +42     
==========================================
+ Hits        14232    14449     +217     
- Misses       1871     1885      +14     
  Partials        7        7              

Impacted Files	Coverage Δ
...ges/core/src/modules/credentials/CredentialsApi.ts	86.91% <ø> (ø)
...les/credentials/formats/CredentialFormatService.ts	100.00% <ø> (ø)
...ntials/formats/indy/IndyCredentialFormatService.ts	87.95% <0.00%> (+0.52%)	:arrow_up:
...s/protocol/v2/handlers/V2IssueCredentialHandler.ts	96.00% <ø> (ø)
...s/protocol/v2/handlers/V2OfferCredentialHandler.ts	67.74% <ø> (ø)
...protocol/v2/handlers/V2RequestCredentialHandler.ts	96.42% <0.00%> (ø)
...ckages/core/src/modules/dids/domain/DidDocument.ts	96.47% <ø> (ø)
...e/src/modules/proofs/formats/ProofFormatService.ts	100.00% <ø> (ø)
...credentials/formats/jsonld/JsonLdOptionsRFC0593.ts	57.14% <57.14%> (ø)
...ls/formats/jsonld/JsonLdCredentialFormatService.ts	86.30% <86.30%> (ø)
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[JamesKEbert]

Hey folks! I've just tried to run this within a React Native environment--has that been recently tested? I've ran into a number of issues. The most problematic though is that the documentLoaderNode require's https, which doesn't work in a RN context. I noticed Karim's addition that provided a conditional that would load the appropriate documentLoader, however the imports are already occurring so therefore the dependency still fails at bundle time.

Is this likely to work on RN right now, or am I troubleshooting at a problematic point in development and it's on the list to fix RN support?

I appreciate all the effort happening here :) Thanks!

[TimoGlastra]

@NB-MikeRichardson what is the status of this PR? It seems it is undoing a lot of other changes from previous PRs

[genaris]

Hey @NB-MikeRichardson, I see there are some changes that seem not to be related with your PR (e.g. docs/setup-*, src/crypto/BbsService.ts) that I think were actually updated by some other PRs merged after your branch was first created. :thinking: Could you check if everything is in place?

[NB-MikeRichardson]

Hey @NB-MikeRichardson, I see there are some changes that seem not to be related with your PR (e.g. docs/setup-*, src/crypto/BbsService.ts) that I think were actually updated by some other PRs merged after your branch was first created. 🤔 Could you check if everything is in place?

I have taken out the files mentioned above; for the life of me I have no idea how they ended up in this PR but good spot anyway!

[TimoGlastra]

There's a lot of checks failing @NB-MikeRichardson, could you address those?

[NB-MikeRichardson]

There's a lot of checks failing @NB-MikeRichardson, could you address those?

I can't see any which are a cause of this PR. Which ones do you think I need to fix?

What on earth is this all about:-

Error: src/ReactNativeFileSystem.ts(1,33): error TS2307: Cannot find module '@aries-framework/core' or its corresponding type declarations. Error: src/ReactNativeFileSystem.ts(3,36): error TS2307: Cannot find module '@aries-framework/core' or its corresponding type declarations. Error: src/ReactNativeFileSystem.ts(20,38): error TS2583: Cannot find name 'Promise'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2015' or later. Error: src/ReactNativeFileSystem.ts(24,51): error TS2583: Cannot find name 'Promise'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2015' or later. Error: src/ReactNativeFileSystem.ts(31,36): error TS2583: Cannot find name 'Promise'. Do you need to change your target library? Try changing the 'lib' compiler option to 'es2015' or later. Error: src/index.ts(4,40): error TS2307: Cannot find module '@aries-framework/core' or its corresponding type declarations. Error: src/index.ts(14,15): error TS2304: Cannot find name 'global'. Error: src/index.ts(15,19): error TS2304: Cannot find name 'global'.

[TimoGlastra]

CI is only failing for Node 17/18 for the BBS tests, all other tests should pass. The 0.3.0-pre does not have these same errors so I think it is something to do with your branch specifically

[NB-MikeRichardson]

The only test failing is the 18.x BBS test as far as I can see.

If there are no further comments can we get this merged?

[TimoGlastra]

Can you fix the DCO?

[NB-MikeRichardson]

I can do even though none of the commits are actually mine. I’ll do it anyway.

From: Timo Glastra @.> Sent: 04 October 2022 14:09 To: hyperledger/aries-framework-javascript @.> Cc: Mike Richardson @.>; Mention @.> Subject: Re: [hyperledger/aries-framework-javascript] feat: Issue Credentials V2: W3C/ jsonld credentials (PR #795)

Can you fix the DCO?

— Reply to this email directly, view it on GitHubhttps://github.com/hyperledger/aries-framework-javascript/pull/795#issuecomment-1266783728, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AWM6GLMV4XGJQEH5AIIOVHTWBQF6BANCNFSM5W43K5RQ. You are receiving this because you were mentioned.Message ID: @.***>

[TimoGlastra]

I can do even though none of the commits are actually mine. I’ll do it anyway.

I can't merge otherwise 🤷

[TimoGlastra]

If there are no further comments can we get this merged?

There's quite some comments that haven't been resolved yet, some could lead to security issues

[genaris]

I can do even though none of the commits are actually mine. I’ll do it anyway.

I can't merge otherwise 🤷

Regarding this particular issue, that happens regularly even if commits were actually signed off (but the DCO script detects an inconsistency), I noticed that there is a new "Set DCO to pass" button (only available to maintainers). Could this be a solution to these typical problems?

[morrieinmaas]

This is urgent (to be merged) as it costs @NB-MikeRichardson a lot of overhead.

[TimoGlastra]

Last remarks, then we can merge

[TimoGlastra]

This is urgent (to be merged) as it costs @NB-MikeRichardson a lot of overhead.

Yes this has been outstanding for way too long. Sorry on this from my side for reviewing it slowly. I think as discussed before: smaller PRs that are easier to review. As a tip to you in the future Mike, please read the RFC carefully and handle all edge cases before making a PR. This will save me a lot of time in reviewing, and save you a lot of time in doing multiple round trips.

[NB-MikeRichardson]

Advice on reading RFCs more carefully noted.

I will get these done asap. Please can you raise an issue with all these small issues that need to be done 'post merge.'

I am happy to work on them.

I have raised the issue: Issue #1089