refactor(credentials): generic credentials module

[TimoGlastra]

This PR overhauls the credentials module to make it ready for modularization. This PR became a lot bigger than anticipated, but it are mostly consistency and cosmetic changes that don't change the behaviour. It also doesn't change the public api of the agent, except for a few interface fixes (e.g. removing a property that shouldn't have been exposed in the first place)

Main item addressed in this PR:

Dynamic Interfaces for credential formats

Instead of creating interfaces for each method defining the supported credential formats.

interface ProposeCredentialOptions {
  credentialFormats: {
    indy: IndyProposePayload
  }
}

We can now dynamically configure the services and credential formats. This will make it possible to extract the formats and services out of core while still keeping a fully typed interface.

You can declare which services and formats you use like this (currently not needed as it has defaults, this will later be automatically detected based on your agent configuration):

// v1 and v2 for indy
CredentialsModule<[IndyCredentialFormatService], [V1CredentialService, V2CredentialService]>

Now when I use the agent it will restrict which protocol versions I can use and which credentialFormats

agent.credentials.proposeCredentials({
  /* other options */
  protocolVersion: 'v3' // will error
  credentialFormats: {
    // indy is allowed and will have typing because we registered the interface.
    indy: {
    }
  }
})

So even though everything is still in core, it's fullly decoupled now.

You define a format like this:

export interface IndyCredentialFormat extends CredentialFormat {
  formatKey: 'indy'
  credentialRecordType: 'indy'
  credentialFormats: {
    createProposal: IndyProposeCredentialFormat
    acceptProposal: IndyAcceptProposalFormat
    createOffer: IndyOfferCredentialFormat
    acceptOffer: IndyAcceptOfferFormat
    createRequest: never // cannot start from createRequest
    acceptRequest: Record<string, never> // empty object
    createCredential: IndyIssueCredentialFormat
  }
}

To make sure this won't cause issues with extensions later on I've extended the api a bit to make a bit more generic and less focused on the indy use case (adding interface method we don't need right now).

In addition I made some smaller changes:

• Remove CredentialProtocolVersion. You now need to use string values. Because we can dynamically register new versions, an enum doesn't really work.
• move all indy related models to the indy format directory
• create separate interfaces for each method in the module, service and format service. The interfaces were reused between layers, which helps with duplication, but makes consistency and overview harder. This will also help with separating into multiple packages and will keep us sharper on responsiblity of each layer.
• order all methods in the module,service,format service based on the public/private interface order and the topic order (group all proposal methods at top, then all offer methods, etc..).
• removed requirement to pass values to accept methods (this should
• removed all places where we modify the input options object to pass to a lower serivce. Options are now always unique.
• move all tests to the correct folder (v1 and v2 tests were being mixed in the v1 and v2 directories)
• extract revocation notification into separate revocation-notification protocol directory.

[2byrds]

Awesome!

[codecov-commenter]

Codecov Report

Merging #841 (e48e64d) into main (ca6c1ce) will decrease coverage by 0.04%.
The diff coverage is 91.16%.

@@            Coverage Diff             @@
##             main     #841      +/-   ##
==========================================
- Coverage   87.59%   87.55%   -0.05%     
==========================================
  Files         458      463       +5     
  Lines       11176    11064     -112     
  Branches     1897     1820      -77     
==========================================
- Hits         9790     9687     -103     
+ Misses       1324     1315       -9     
  Partials       62       62              

Impacted Files	Coverage Δ
...s/core/src/modules/credentials/CredentialEvents.ts	100.00% <ø> (ø)
...les/credentials/models/CredentialAutoAcceptType.ts	100.00% <ø> (ø)
.../src/modules/credentials/models/CredentialState.ts	100.00% <ø> (ø)
...cation/messages/V1RevocationNotificationMessage.ts	100.00% <ø> (ø)
...cation/messages/V2RevocationNotificationMessage.ts	100.00% <ø> (ø)
...src/modules/indy/services/IndyRevocationService.ts	58.90% <0.00%> (ø)
packages/core/src/types.ts	100.00% <ø> (ø)
...s/protocol/v1/handlers/V1OfferCredentialHandler.ts	67.74% <60.00%> (-3.69%)	⬇️
...s/protocol/v2/messages/V2IssueCredentialMessage.ts	91.30% <60.00%> (-8.70%)	⬇️
...protocol/v2/messages/V2RequestCredentialMessage.ts	91.30% <60.00%> (-8.70%)	⬇️
... and 68 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca6c1ce...e48e64d. Read the comment docs.

[berendsliedrecht]

I did not do a code review yet, but would this not be the point to rename some stuff to anoncreds instead of indy? Or will that happen in another PR?

[NB-MikeRichardson]

These changes look very good but I wonder if we should not merge this PR until after the w3c/JsonLd issue credentials v2 PR is merged.

Otherwise that is going to create a large number of conflicts.

So I am wondering if this PR should go into 0.3.0-pre rather than main??

[TimoGlastra]

I did not do a code review yet, but would this not be the point to rename some stuff to anoncreds instead of indy? Or will that happen in another PR?

With the already large number of changes doing that in a follow up PR would be nicer I think. We can then rename all indy related services/objects etc to anoncreds

[TimoGlastra]

These changes look very good but I wonder if we should not merge this PR until after the w3c/JsonLd issue credentials v2 PR is merged.

Otherwise that is going to create a large number of conflicts.

So I am wondering if this PR should go into 0.3.0-pre rather than main??

I would rather get this in 0.2.0 mainly because its a lot of changes and keeping that up to date with 0.3.0 branch is a hassle. Also there are numerous improvements here that would be good to have in 0.2.0.

I'll be happy to address the merge conflicts in your w3c/jsonld branch @NB-MikeRichardson! I think that will be easier than merging this into 0.3.0.

So proposed plan from my side would be:

1. merge this into main
2. update 0.3.0-pre with changes from main
3. I'll update the 0.3.0 jsonld credentials branch from @NB-MikeRichardson to integrate with these changes and create a PR to his branch

Does that sound good?

[NB-MikeRichardson]

These changes look very good but I wonder if we should not merge this PR until after the w3c/JsonLd issue credentials v2 PR is merged.
Otherwise that is going to create a large number of conflicts.
So I am wondering if this PR should go into 0.3.0-pre rather than main??

I would rather get this in 0.2.0 mainly because its a lot of changes and keeping that up to date with 0.3.0 branch is a hassle. Also there are numerous improvements here that would be good to have in 0.2.0.

I'll be happy to address the merge conflicts in your w3c/jsonld branch @NB-MikeRichardson! I think that will be easier than merging this into 0.3.0.

So proposed plan from my side would be:

1. merge this into main
2. update 0.3.0-pre with changes from main
3. I'll update the 0.3.0 jsonld credentials branch from @NB-MikeRichardson to integrate with these changes and create a PR to his branch

Does that sound good?

Yes I'm ok with that, but there is no reason why the jsonld credentials can't go into 0.3.0-pre before you merge these changes across from main. I think that PR is pretty much ready to be merged (I made some changes last week, I think all comments are resolved)

[JamesKEbert]

There's a lot here Timo! Thanks for the hard work

I'm okay with this, but I am a little worried about the increase in complexity added here.
I'm also fairly concerned about the typing that the end user receives, as my code editor was unable to give me a clear indication of what should go in my call, for instance in agent.credentials.acceptOffer(). In order to figure it out I had to dive into AFJ code to look at the interface.

[TimoGlastra]

I'm also fairly concerned about the typing that the end user receives, as my code editor was unable to give me a clear indication of what should go in my call, for instance in agent.credentials.acceptOffer()

This shouldn't happen, I'll take a look at this.

I am a little worried about the increase in complexity added here.

That's understandable. It's just really complex to serve both the "I want simplicity" and the "I want modularity" camps. The approach I took is making the internal code a bit complexer so we achieve modularity without sacrificing simplicity in the public API. Happy to look at other paths, but IMO this is a good trade-off to make

[TimoGlastra]

@JamesKEbert could you show me what it looks like for you? I have full auto complete for the credentialFormats