feat(anoncreds): add anoncreds API

[TimoGlastra]

No description provided.

[TimoGlastra]

revocationList -> revocationStatusList

[genaris]

I know this is still a draft but I've been checking it for an integration with anoncreds-rs and I left some comments, mostly some questions to make sure I understood correctly the way these interfaces should be used.

[genaris]

So this means that each IssuerService implementation would define its own format for storing private part, by either creating specific records or attaching it to CredentialDefinitionRecord metadata? In this way, when it needs to retrieve private part (e.g. in createCredentialOffer) it will need to fetch it using CredentialDefinitionRepository (or its custom repo). It will need to access CredentialDefinitionRecord anyway to retrieve its schemaId (as it is not being passed in CreateCredentialOfferOptions).

I'm wondering what would happen if we switch from an implementation to another. Suppose we are using anoncreds-rs and we want to use another implementation: how could we export the private part in such a way that such data can be interpreted by it? I guess migration scripts for those particular transitions will be needed. But could get a bit complicated given that we are now StorageService-agnostic.

[TimoGlastra]

My idea is that we add records for this in the AnonCreds package and we instruct that the same repositories are used by implementations that can use it. However, for Indy SDK we can't use the records and therefore it's part of the specific issuer/holder/verifier services.

This way there also won't be migration scripts needed, as they'll be using the same records. The idea is to create a separate record for the private parts so it's not exposed to the end user (unless they specifically fetch the private record).

See #1193 and #1164 for extra context.

Do you think this approach will work? Happy to go other directions

[genaris]

Yes, I think it will be better that way, especially because we are talking about records that are aligned with AnonCreds specification, so any implementation should be fine by using it (and if it needs something else, it can probably use an internal metadata value).

The same would apply to AnonCredsCredentialRecord, right? I mean there will be a common Record/Repository and each AnonCredsHolderService implementation will use it. In such case, wouldn't be the methods getCredential / deleteCredential / storeCredential nearly the same for every implementation (except of indy-sdk of course)?

[TimoGlastra]

Yes probably! I think storeCredential in indy-sdk verifies the credential so that will be different.

[TimoGlastra]

interface AnonCredsCredentialInfo {
        credentialId: string
        attributes: Record<string, string>
        schemaId: string
        credentialDefinitionId: string
        revocationRegistryId?: string
        credentialRevocationId?: string
}

getCredential(credentialId: string): Promise<AnonCredsCredentialInfo>
deleteCredential(credentialId: string): Promise<void>

[TimoGlastra]

interface AnonCredsCredentialInfo {
        credentialId: string
        attributes: Record<string, string>
        schemaId: string
        credentialDefinitionId: string
        revocationRegistryId?: string
        credentialRevocationId?: string
}

getCredential(credentialId: string): Promise<AnonCredsCredentialInfo>
deleteCredential(credentialId: string): Promise<void>

[genaris]

What about MasterSecret creation and storage? It is part of AnonCredsApi, isn't it? If that's the case, will it be passed in CreateCredentialRequestOptions and CreateProofOptions?

[TimoGlastra]

Yeah it should be handled by the AnonCreds Api / services (prob the holder service), and removed from the wallet interface.

[genaris]

I figured out that it's only needed for CreateCredentialRequestOptions and can even be optional, using a default one as in 99% of the cases it will be a single master secret. I've implemented some draft classes in my PR TimoGlastra#18

[TimoGlastra]

I know this is still a draft but I've been checking it for an integration with anoncreds-rs and I left some comments, mostly some questions to make sure I understood correctly the way these interfaces should be used.

Thanks for the feedback, comments!

[genaris]

Not completely related with this PR but it seems that packages/anoncreds/src/formats/__tests__/LegacyIndyCredentialFormatService.test.ts takes quite a significant time in CI (between 20 and 30 seconds). Not sure where it is getting stuck but maybe we'll need to increase the timeout for anoncreds suite.

[TimoGlastra]

Not completely related with this PR but it seems that packages/anoncreds/src/formats/__tests__/LegacyIndyCredentialFormatService.test.ts takes quite a significant time in CI (between 20 and 30 seconds). Not sure where it is getting stuck but maybe we'll need to increase the timeout for anoncreds suite.

It's actually doing integration tests (while the indy-sdk was mocked before) so that's prob what's taking so long. I've been a bit more focused on the integration tests as unit tests take A LOT of time to write and often don't really test if something actually works. But it does mean tests take longer to run ....

[TimoGlastra]

This is now ready for review. I've renamed master secret to link secret and opened a new issue for adding the getCredential/getCredentials methods.

I've added end to end tests for the functionality in anoncreds.test.ts which uses an in memory anoncreds registry to register/resolve all anoncreds objects

[codecov-commenter]

Codecov Report

Merging #1232 (bfb900f) into main (f1e4937) will increase coverage by 0.70%.
The diff coverage is 63.84%.

@@            Coverage Diff             @@
##             main    #1232      +/-   ##
==========================================
+ Coverage   75.08%   75.78%   +0.70%     
==========================================
  Files         657      701      +44     
  Lines       15896    17054    +1158     
  Branches     2698     2921     +223     
==========================================
+ Hits        11935    12925     +990     
- Misses       3958     4125     +167     
- Partials        3        4       +1     

Impacted Files	Coverage Δ
...s/anoncreds/src/services/AnonCredsHolderService.ts	100.00% <ø> (ø)
...s/anoncreds/src/services/AnonCredsIssuerService.ts	100.00% <ø> (ø)
.../src/services/registry/AnonCredsRegistryService.ts	90.00% <0.00%> (ø)
...tory/AnonCredsCredentialDefinitionPrivateRecord.ts	25.00% <25.00%> (ø)
.../repository/AnonCredsCredentialDefinitionRecord.ts	25.00% <25.00%> (ø)
...c/repository/AnonCredsKeyCorrectnessProofRecord.ts	25.00% <25.00%> (ø)
...s/anoncreds/src/error/AnonCredsStoreRecordError.ts	50.00% <50.00%> (ø)
...ds/src/repository/AnonCredsLinkSecretRepository.ts	57.14% <57.14%> (ø)
.../AnonCredsCredentialDefinitionPrivateRepository.ts	60.00% <60.00%> (ø)
...ository/AnonCredsCredentialDefinitionRepository.ts	60.00% <60.00%> (ø)
... and 95 more

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

[genaris]

I left some very minor comments. There are a few more details (like defining KeyCorrectnessProof and CredentialDefinitionPrivate structure) but we can handle in another PR.

[genaris]

Seems that passing anoncreds test is still hard due to timing issues... Based on the run for node 14 it took around 23 secs to pass. Maybe would be neeeded to increase the timeout to 30000?