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(indy-vdr): did:sov resolver #1247

Merged
merged 28 commits into from
Jan 30, 2023

Conversation

genaris
Copy link
Contributor

@genaris genaris commented Jan 27, 2023

Following the strategy of 'small PRs', this one adds to indy-vdr package a DID Resolver for did:sov method. It offers the same features as the one based on indy-sdk.

The one for did:indy will be added in a further PR.

vickysomtee and others added 23 commits November 24, 2022 15:24
Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Work Funded by the Government of Ontario

Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: Timo Glastra <[email protected]>
Work Funded by the Government of Ontario

Signed-off-by: vickysomtee <[email protected]>
Work Funded by the Government of Ontario

Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: Karim Stekelenburg <[email protected]>
Signed-off-by: Karim Stekelenburg <[email protected]>
Work funded by the Ontario Government

Signed-off-by: vickysomtee <[email protected]>
Work funded by the Ontario Government

Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Signed-off-by: Ariel Gentile <[email protected]>
Signed-off-by: Ariel Gentile <[email protected]>
Signed-off-by: Ariel Gentile <[email protected]>
Signed-off-by: Ariel Gentile <[email protected]>
@genaris genaris requested a review from a team as a code owner January 27, 2023 01:27
@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2023

Codecov Report

Merging #1247 (b970c90) into main (b6ae948) will decrease coverage by 0.30%.
The diff coverage is 77.07%.

@@            Coverage Diff             @@
##             main    #1247      +/-   ##
==========================================
- Coverage   87.64%   87.35%   -0.30%     
==========================================
  Files         739      760      +21     
  Lines       17619    18149     +530     
  Branches     2992     3081      +89     
==========================================
+ Hits        15443    15854     +411     
- Misses       2169     2288     +119     
  Partials        7        7              
Impacted Files Coverage Δ
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/pool/IndyVdrPoolService.ts 74.69% <74.69%> (ø)
packages/indy-vdr/src/index.ts 75.00% <75.00%> (ø)
packages/anoncreds/src/utils/credential.ts 76.81% <76.81%> (ø)
packages/indy-vdr/src/pool/IndyVdrPool.ts 85.96% <85.96%> (ø)
packages/indy-vdr/src/utils/promises.ts 88.88% <88.88%> (ø)
...ackages/indy-vdr/src/dids/IndyVdrSovDidResolver.ts 92.10% <92.10%> (ø)
... and 15 more

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

Copy link
Contributor

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

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

Some comments, but overall looks good. Nice!

packages/indy-vdr/src/dids/IndyVdrSovDidResolver.ts Outdated Show resolved Hide resolved
Comment on lines 1 to 3
export * from './dids'
export * from './error'
export * from './pool'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to export all these services / methods? I want to be more careful going forward in what we see as public api. I think seeing the specific things built on top of the pool service (so anoncreds registry, did resolver, did registrar) as public api means we can more easily make non breaking changes.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, maybe it's better to be more selective from the beginning.

For the moment I only picked the IndyDidSovResolver, which is the only class (among the existing ones) I think it will be used externally once an IndyVdrModule or so is created.

Signed-off-by: Ariel Gentile <[email protected]>
@genaris
Copy link
Contributor Author

genaris commented Jan 27, 2023

Some comments, but overall looks good. Nice!

Thanks! I think I covered all the feedback. Some other changes to be added in a further PR.

I noticed it's becoming a bit harder to make CI pass: I had to increase the timeout for some tests (create DID with endpoints and resolve takes ~30 secs) and also I've seen some OutOfMemory/Garbage Collector problems like:

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
 1: 0xb6b850 node::Abort() [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]
 2: 0xa806a6  [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]
 3: 0xd52140 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]
 4: 0xd524e7 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]
 5: 0xf2fbe5  [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]
 6: 0xf420cd v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]

Not sure if this could be related to the issue found in Askar. If it continues, maybe we can test with a patched ref-napi version to see how it performs.

@TimoGlastra
Copy link
Contributor

Some comments, but overall looks good. Nice!

Thanks! I think I covered all the feedback. Some other changes to be added in a further PR.

I noticed it's becoming a bit harder to make CI pass: I had to increase the timeout for some tests (create DID with endpoints and resolve takes ~30 secs) and also I've seen some OutOfMemory/Garbage Collector problems like:

FATAL ERROR: Reached heap limit Allocation failed - JavaScript heap out of memory
 1: 0xb6b850 node::Abort() [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]
 2: 0xa806a6  [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]
 3: 0xd52140 v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]
 4: 0xd524e7 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]
 5: 0xf2fbe5  [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]
 6: 0xf420cd v8::internal::Heap::CollectGarbage(v8::internal::AllocationSpace, v8::internal::GarbageCollectionReason, v8::GCCallbackFlags) [/opt/hostedtoolcache/node/18.13.0/x64/bin/node]

Not sure if this could be related to the issue found in Askar. If it continues, maybe we can test with a patched ref-napi version to see how it performs.

Hmm but Node 18 should work fine right?

@TimoGlastra
Copy link
Contributor

Is this ready to merge otherwise @genaris?

@genaris
Copy link
Contributor Author

genaris commented Jan 30, 2023

Is this ready to merge otherwise @genaris?

Yes, and if the issue with CI persists we can investigate it further to see where it is taking so much heap memory (and what's different from local node setup, as I haven't seen this particular issue locally).

@genaris genaris merged commit b5eb08e into openwallet-foundation:main Jan 30, 2023
@genaris genaris deleted the feat/indy-vdr-resolver branch February 19, 2023 20:32
TimoGlastra added a commit that referenced this pull request May 16, 2023
Co-authored-by: Karim Stekelenburg <[email protected]>
Co-authored-by: Ariel Gentile <[email protected]>
Co-authored-by: Timo Glastra <[email protected]>
Co-authored-by: Jim Ezesinachi <[email protected]>
Co-authored-by: Ry Jones <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Kim Ebert <[email protected]>
Co-authored-by: Grammatopoulos Athanasios Vasileios <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Victor Anene <[email protected]>
Co-authored-by: Jim Ezesinachi <[email protected]>
Co-authored-by: KolbyRKunz <[email protected]>
Co-authored-by: Berend Sliedrecht <[email protected]>
Co-authored-by: Jason C. Leach <[email protected]>
Co-authored-by: Martin Auer <[email protected]>
Co-authored-by: Niall Shaw <[email protected]>
Co-authored-by: Pritam Singh <[email protected]>
Co-authored-by: Mo <[email protected]>
Co-authored-by: NB-MikeRichardson <[email protected]>
Co-authored-by: Amit-Padmani <[email protected]>
Co-authored-by: DaevMithran <[email protected]>
Co-authored-by: Alexander Shenshin <[email protected]>
fix(openid4vc-client): set package to private (#1210)
fix: fix typing issues with typescript 4.9 (#1214)
Fixes #1205
resolver (#1247)
fix: set updateAt on records when updating a record (#1272)
fix(transport)!: added docs moved connection to connectionId (#1222)
fix(indy-vdr): export relevant packages from root (#1291)
fix(askar): generate nonce suitable for anoncreds (#1295)
resolver and registrar for did:indy (#1253)
fix: imports from core (#1303)
fix: thread id improvements (#1311)
fix: loosen base64 validation (#1312)
fix(samples): dummy module response message type (#1321)
fix: seed and private key validation and return type in registrars (#1324)
fix!: don't emit legacy did:sov prefix for new protocols (#1245)
fix(askar): anoncrypt messages unpacking (#1332)
fix: expose indy pool configs and action menu messages (#1333)
fix: create new socket if socket state is 'closing' (#1337)
fix(anoncreds): include prover_did for legacy indy (#1342)
fix(indy-sdk): import from core (#1346)
fix(anoncreds-rs): save revocation registry index (#1351)
fix: isNewSocket logic (#1355)
fix(tenant): Correctly configure storage for multi tenant agents (#1359)
Fixes hyperledger#1353
fix(anoncreds): Buffer not imported from core (#1367)
fix(core): repository event when calling deleteById (#1356)
fix(askar): custom error handling (#1372)
fix: return HTTP 415 if unsupported content type (#1313)
fix: remove named capture groups (#1378)
fix example usage of indy-sdk-react-native package (#1382)
fix: connection id in sessions for new connections (#1383)
fix: did cache key not being set correctly (#1394)
fix: incorrect type for anoncreds registration (#1396)
fix: reference to indyLedgers in IndyXXXNotConfiguredError (#1397)
fix: add reflect-metadata (#1409)
fix: various anoncreds revocation fixes (#1416)
fix: jsonld credential format identifier version (#1412)
fix: remove `deleteOnFinish` and added documentation (#1418)
fix(askar): default key derivation method (#1420)
fix(anoncreds): make revocation status list inline with the spec (#1421)
fix(anoncreds-rs): revocation status list as JSON (#1422)
fix: issuance with unqualified identifiers (#1431)
fix(connections): store imageUrl when using DIDExchange (#1433)
fix(indy-vdr): do not force indy-vdr version (#1434)
fix: small issues with migration and WAL files (#1443)
fix: migration of link secret (#1444)
fix: Emit RoutingCreated event for mediator routing record (#1445)
fix: small updates to cheqd module and demo (#1439)
fix: remove scope check from response (#1450)
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.

4 participants