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

feat(anoncreds): add legacy indy credential format #1220

Conversation

TimoGlastra
Copy link
Contributor

Adds a LegacyIndyCredentialFormat class to the AnonCreds package that can handle the current indy format as described in RFC 0592. This doesn't support the new ledger agnostic anoncreds yet, but it does implement the old format with the new APIs we have.

I've written some initial tests using this service and the new anoncreds apis implemented using the indy-sdk. I'll add more in follow up prs when the api and proof format are added.

Partially fixes #1130, but the proof format is still todo. This will actually be a lot more work as we still need to make the proof interfaces dynamic and allow for dynamic registration of proof protocols

@TimoGlastra TimoGlastra requested a review from a team as a code owner January 18, 2023 15:11
@TimoGlastra
Copy link
Contributor Author

Question: should we call it LegacyIndyCredentialFormat or just IndyCredentialFormat? Maybe it's too soon to update the naming to legacy?

Copy link
Contributor

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

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

Nothing major, just some small points.

…gistryService.test.ts

Co-authored-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jan 19, 2023

Codecov Report

Merging #1220 (e2b2378) into main (b6ae948) will decrease coverage by 0.62%.
The diff coverage is 62.10%.

@@            Coverage Diff             @@
##             main    #1220      +/-   ##
==========================================
- Coverage   87.64%   87.03%   -0.62%     
==========================================
  Files         739      757      +18     
  Lines       17619    18068     +449     
  Branches     2992     3067      +75     
==========================================
+ Hits        15443    15726     +283     
- Misses       2169     2335     +166     
  Partials        7        7              
Impacted Files Coverage Δ
packages/indy-vdr/src/pool/IndyVdrPoolService.ts 30.12% <30.12%> (ø)
packages/indy-vdr/src/utils/promises.ts 33.33% <33.33%> (ø)
packages/indy-vdr/src/utils/did.ts 37.50% <37.50%> (ø)
packages/indy-vdr/src/error/IndyVdrError.ts 50.00% <50.00%> (ø)
...es/indy-vdr/src/error/IndyVdrNotConfiguredError.ts 50.00% <50.00%> (ø)
packages/indy-vdr/src/error/IndyVdrNotFound.ts 50.00% <50.00%> (ø)
...s/src/formats/LegacyIndyCredentialFormatService.ts 64.73% <64.73%> (ø)
packages/indy-vdr/src/index.ts 66.66% <66.66%> (ø)
packages/anoncreds/src/utils/credential.ts 76.81% <76.81%> (ø)
packages/indy-vdr/src/pool/IndyVdrPool.ts 85.96% <85.96%> (ø)
... and 12 more

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

TimoGlastra and others added 2 commits January 19, 2023 14:13
…ce.ts

Co-authored-by: Berend Sliedrecht <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
@TimoGlastra
Copy link
Contributor Author

Resolved your feedback @blu3beri

@berendsliedrecht
Copy link
Contributor

Question: should we call it LegacyIndyCredentialFormat or just IndyCredentialFormat? Maybe it's too soon to update the naming to legacy?

I think legacy is fine. We are going to name it legacy anyways in the near future and it shows the intention we have with the format.

Copy link
Contributor

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

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

Just one comment about creating an issue on anoncreds-rs. Otherwise it looks good to me.

@TimoGlastra
Copy link
Contributor Author

Just one comment about creating an issue on anoncreds-rs. Otherwise it looks good to me.

can't find the comment 🙃

@berendsliedrecht
Copy link
Contributor

Just one comment about creating an issue on anoncreds-rs. Otherwise it looks good to me.

can't find the comment 🙃

#1220 (comment)

@TimoGlastra TimoGlastra enabled auto-merge (squash) January 24, 2023 14:23
@TimoGlastra TimoGlastra merged commit 13f3740 into openwallet-foundation:main Jan 29, 2023
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.

Update the IndyCredentialFormatService and IndyProofFormatServices to use the AnonCreds interfaces
4 participants