credo-ts icon indicating copy to clipboard operation
credo-ts copied to clipboard

Remaining work on DIDComm module

Open genaris opened this issue 10 months ago • 3 comments

Some ideas we got from #2127 that would be good to check before official 0.6.0 release:

  • [x] Fix build (core dependency on didcomm) (#2167 )
  • [ ] @TimoGlastra agent.didcomm possible (nesting is getting quite large) nest didcomm modules under agent.didcomm, also because agent.credentials / agent.modules.credentials is misleading, since it's didcomm specific. I think registerin
  • [ ] @TimoGlastra Adopting the conenctions / oob modules into the didcomm module by default, and you can configure it in the didcomm module config again. I think this simplifies setup.
    • might want an option to disable them in the didcomm module config
  • [ ] @TimoGlastra Registering custom didcomm protocols on the didcomm module rather than the agent? This may solve some of the initialization and feature registry issues. We can create a DidcommModule interface that has special feature registry etc.
  • [ ] @genaris Rename modules to have DIDcomm prefix (DidCommConnectionsModule, DidCommMessageHandlerRegistry). This will help with identifiying for what feature a class is
  • [ ] @GHkrishna Add inbound / outbound transports to the config? I always found it a bit weird you can't just add this to the didcomm config
  • [ ] @genaris Check Tenants dependency on DIDComm (connectionImageUrl, Routing, etc.)
    • [x] When a routing key is created for a tenant, it needs to be registered in the main agent (but this is done based on events)
    • Label is stored in the tenant record
    • Where to store DIDComm persisted config options?
      • options
        1. do not allow for didcomm persisted config options (provide label and image url to every API method)
        • animo and 2060 not using the global config, always overriding the global config.
        • what to do with v1 protocols (where the label is required) -> make it required in the API
        1. add a DIDCommXXXRecord that stores the didcomm config for this agent (like we have OpenID4VCIsssuerRecord)
        2. reuse the mechanism we use for askar, and store it in the TenantRecord - do we need some methods to make this reusable across modules?
        3. add a generic AgentContext config record, that could store persisted config options
    • preference for option 1. Remove label and image url from the module config completely.
  • [x] Remove MessageSender dependency on MessagePickupRepository injection symbol. Maybe this can be solved with events, so base DIDComm and MessagePickup modules are decoupled (https://github.com/openwallet-foundation/credo-ts/pull/2292)

genaris avatar Jan 28 '25 16:01 genaris

My proposal for agent.didcomm and agent.openid4vc:

  • You register then OpenId4vcModule, which has a config where you can enableIssuer, enableVerifier, enableHolder, (all on by default). Then you can do agent.openid4vc.issuer (instead of `agent.openID4VcIssuer (as it is now)).
  • For DIDComm it will be a bilt more complex, as we have to register modules dynamically. I think we can register the modules dynamically in the DIDcomm module register method, and then we do the same magic trick we do on the main agent, to add these modules to didcomm. So we get agent.didcomm.oob, etc..
  • We will just have to do a trick to add these to agent.XX instead of agent.modules.XX, but we can just do special handling for the openid4vc and didcomm keys?

If this approach looks good, i can take a stab at it. I feel like it's one the things blocking a 0.6 release, while otherwise we're getting in a good state to do another big release.

TimoGlastra avatar Jun 03 '25 07:06 TimoGlastra

I think that's a good idea! If you can take a look at that, it would be great!

genaris avatar Jun 03 '25 11:06 genaris

  • Where to store DIDComm persisted config options?

I think it would be better to store the configs in some records, just to avoid re-sending it every time. Options can be anything from the ones mentioned above, provided it is:

  • Updatable
  • Is lightweight (in terms we don't implement any heavy lifting to retrieve the records. Since we might often need it)

With this we could also possibly leverage it to store any agent level configs including the ones mentioned here https://github.com/openwallet-foundation/credo-ts/issues/2298

Because this way getting it for each request would be similar to how we handle auto Connection, issuance flags, default to global if not provided from request

GHkrishna avatar Jun 18 '25 06:06 GHkrishna