refactor!: didcomm module #2127

genaris · 2024-12-05T14:01:55Z

Initial work on extracting DIDComm out of core. This PR moves all DIDComm-related modules to a dedicated didcomm package, where they are located alongside a base DidCommModule that exposes and API and config to set up basic DIDComm features (e.g. register inbound/outbound transports, endpoints and DIDComm flags).

To not overcomplicate this big refactoring, no further API and usage changes where applied, so most changes to use DIDComm in an Agent are:

We need to explicitly create in Agent constructor all modules we use (apart from base DidCommModule): ProofsModule, ConnectionsModule, BasicMessagesModule, etc. The exposed getDefaultDidcommModules() can help to facilitate typical Agent creation before doing a further refactoring
Move DIDComm-related AgentConfig parameters to the DidCommModuleConfig
Update calls to Agent's registerInboundTransport/registerOutboundTransport, endpoints, features and other DIDComm-related properties to agent.modules.didcomm.xxx
Add .modules to all calls to DIDComm related modules (e.g. agent.proofs.method() to agent.modules.proofs.method())

And those are pretty much the changes required to use it coming from previous code base.

Some notes I took:

Since DIDComm is not initialized at BaseAgent.initialize() anymore, its dependant modules cannot access objects like Feature Registry at module register(). So protocol and other features are actually registered at each module's initialize()` method. Not a big problem I think but it is a new measure to take into account
Due to the way it is implemented, mediationRecipient needs to be explicitly initialized (i.e. call its initialize() API method). Otherwise, provisioning and pickup will not work
Tenant records only consider base Agent config. Therefore, it is not possible to specify a particular connection image URL for each tenant in a multi-tenant configuration (or at least that's what I think)
Some types were duplicated (e.g. EncryptedMessage, PlaintextMessage, DidCommService) to avoid a dependency cycle between core and didcomm
For the moment, the very minimum setup needs to instantiate at least DIDComm, Connections, OOB and Message Pickup. Dependency on the latter might be easy to remove but we will probably need to embed most functionality from connections and oob in the base module.
node package depends on didcomm, due to the definition of HTTP and WebSocket Inbound transports. We can create a separate package for that, or leave this work for a full refactor of this package (and react-native), since most of them are used on DIDComm and AnonCreds related modules
I didn't want to make this PR heavier than it is, so tests have been mostly adapted to just work. But we'll need to reorganize them and also simplify them a bit, since there are a good amount of overlap and this makes the test suite to be slower and harder to maintain

changeset-bot · 2024-12-05T14:02:00Z

🦋 Changeset detected

Latest commit: ba42e98

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages

Name	Type
@credo-ts/question-answer	Minor
@credo-ts/bbs-signatures	Minor
@credo-ts/action-menu	Minor
@credo-ts/anoncreds	Minor
@credo-ts/indy-vdr	Minor
@credo-ts/didcomm	Minor
@credo-ts/tenants	Minor
@credo-ts/askar	Minor
@credo-ts/cheqd	Minor
@credo-ts/core	Minor
@credo-ts/drpc	Minor
@credo-ts/node	Minor
@credo-ts/indy-sdk-to-askar-migration	Minor
@credo-ts/openid4vc	Minor
@credo-ts/react-native	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Signed-off-by: Ariel Gentile <[email protected]>

…t < ES2020) Signed-off-by: Ariel Gentile <[email protected]>

Signed-off-by: Ariel Gentile <[email protected]>

TimoGlastra · 2025-01-27T10:32:24Z

@genaris I see tests etc are passing. Do you think we could already merge a (partial) version of this soon, so we can get most of the large file changes merged, and also start work on some other tickets (such as #2145 and #2157).

Signed-off-by: Ariel Gentile <[email protected]>

genaris · 2025-01-27T18:18:48Z

@genaris I see tests etc are passing. Do you think we could already merge a (partial) version of this soon, so we can get most of the large file changes merged, and also start work on some other tickets (such as #2145 and #2157).

I've updated the description and marked this PR as ready for review. Both tests and demos are working, so I think it is in a status where it can be safe to merge. There is a good number of things to do yet before the official 0.6.0 release but I think it is a good start.

…ns that now all DIDComm related modules (e.g. proofs, credentials) must be explicitly added when creating an `Agent` instance. Therefore, their API will be accesable under `agent.modules.[moduleAPI]` instead of `agent.[moduleAPI]`. Some `Agent` DIDComm-related properties and methods where also moved to the API of a new DIDComm module (e.g. `agent.registerInboundTransport` turned into `agent.modules.didcomm.registerInboundTransport`). **Example of DIDComm Agent** Previously: ```ts const config = { label: name, endpoints: ['https://myendpoint'], walletConfig: { id: name, key: name, }, } satisfies InitConfig const agent = new Agent({ config, dependencies: agentDependencies, modules: { connections: new ConnectionsModule({ autoAcceptConnections: true, }) }) this.agent.registerInboundTransport(new HttpInboundTransport({ port })) this.agent.registerOutboundTransport(new HttpOutboundTransport()) ``` Now: ```ts ```ts const config = { label: name, walletConfig: { id: name, key: name, }, } satisfies InitConfig const agent = new Agent({ config, dependencies: agentDependencies, modules: { ...getDefaultDidcommModules({ endpoints: ['https://myendpoint'] }), connections: new ConnectionsModule({ autoAcceptConnections: true, }) }) this.agent.modules.didcomm.registerInboundTransport(new HttpInboundTransport({ port })) this.agent.modules.didcomm.registerOutboundTransport(new HttpOutboundTransport()) ``` Signed-off-by: Ariel Gentile <[email protected]>

Signed-off-by: Ariel Gentile <[email protected]>

TimoGlastra · 2025-01-28T07:21:55Z

.changeset/purple-donkeys-camp.md

+'@credo-ts/node': minor
+---
+
+DIDComm has been extracted out of the Core. This means that now all DIDComm related modules (e.g. proofs, credentials) must be explicitly added when creating an `Agent` instance. Therefore, their API will be accesable under `agent.modules.[moduleAPI]` instead of `agent.[moduleAPI]`. Some `Agent` DIDComm-related properties and methods where also moved to the API of a new DIDComm module (e.g. `agent.registerInboundTransport` turned into `agent.modules.didcomm.registerInboundTransport`).


This is not for this PR, but I think before 0.6 we should look at making:

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

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.

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.

Rename modules to have DIDcomm prefix (DidCommConnectionsModule, DidCommMessageHandlerRegistry). This will help with identifiying for what feature a class is

Add inbound / outbound transports to the config? I always found it a bit weird you can't just add this to the didcomm config

Yeah good ideas there! Initially I made a "DidCommModule" but I decided to leave it for a further PR to not overcomplicate it. But for sure it is something that makes sense, especially to have a tidy Agent API.

Same thing about the transport config, which is something not so evident for developers.

packages/core/src/modules/dids/__tests__/DidsApi.test.ts

packages/core/src/modules/dids/methods/peer/createPeerDidDocumentFromServices.ts

TimoGlastra · 2025-01-28T07:42:54Z

tests/transport/SubjectInboundTransport.ts

@@ -19,18 +18,19 @@ export class SubjectInboundTransport implements InboundTransport {
    this.ourSubject = ourSubject
  }

-  public async start(agent: Agent) {
+  public async start(agent: AgentContext) {


Is it not possible to provide the agent here anymore?

The problem is that we start transports at DIDComm module's initialize(), which receives the AgentContext as an argument. Before this PR we started transport in Agent so we were able to call transport.start(this).

Do you see a potential issue on this change, besides the fact that it breaks the API?

TimoGlastra · 2025-01-28T08:32:24Z

packages/tenants/src/services/__tests__/TenantService.test.ts

@@ -28,18 +28,20 @@ describe('TenantRecordService', () => {
    jest.clearAllMocks()
  })

+  // FIXME: connectionImageUrl is now part of DIDComm module. Tenants records do not currently


For the new crypto api i've also been looking to solve this problem, and how i tried to solve it is that each module can hook into the the storing process of the tenant config. It's a bit tricky though, as it makes all modules somewhat intertwined with the tenants module ...

Well, maybe tenants could become a core feature? 🤔

packages/core/src/storage/migration/updates/0.3-0.3.1/index.ts

packages/core/src/constants.ts

TimoGlastra · 2025-01-28T08:40:05Z

Left some comments, but all are fine to fix later. Very nice work!! 🙌

Maybe we can open an issue and add your + mine comments so we can keep track of todos?

Signed-off-by: Ariel Gentile <[email protected]>

Signed-off-by: Ariel Gentile <[email protected]> Signed-off-by: Sai Ranjit Tummalapalli <[email protected]>

refactor: move to separate didcomm package

ededb58

Signed-off-by: Ariel Gentile <[email protected]>

genaris force-pushed the refactor/didcomm-module branch from d26f24e to ededb58 Compare December 19, 2024 12:35

genaris added 7 commits December 19, 2024 10:08

Merge branch 'main' into refactor/didcomm-module

89f8d62

Signed-off-by: Ariel Gentile <[email protected]>

refactor: remove didcomm packages from BaseAgent

524a07c

Signed-off-by: Ariel Gentile <[email protected]>

fix: move luxon dependency to didcomm

73fafcf

Signed-off-by: Ariel Gentile <[email protected]>

fix: fixed cheqd SDK to 2.5.1 because otherwise it won't build (targe…

851cf8a

…t < ES2020) Signed-off-by: Ariel Gentile <[email protected]>

test: fix some core tests

17b2674

Signed-off-by: Ariel Gentile <[email protected]>

test: more fixed tests

618b975

Signed-off-by: Ariel Gentile <[email protected]>

test: fixes

0e46629

Signed-off-by: Ariel Gentile <[email protected]>

TimoGlastra mentioned this pull request Jan 16, 2025

Switch from prettier + eslint to Biome #2145

Open

1 task

genaris added 9 commits January 22, 2025 16:07

fix: update pnpm lock

17386a4

Signed-off-by: Ariel Gentile <[email protected]>

test: fix most tests

bb6917a

Signed-off-by: Ariel Gentile <[email protected]>

Merge branch 'main' into refactor/didcomm-module

add1645

Signed-off-by: Ariel Gentile <[email protected]>

fix: several types, format and lint errors

d1143ad

Signed-off-by: Ariel Gentile <[email protected]>

fix: upgrade asn1/sd-swt libraries to pass x509/oidc tests

5b40e63

Signed-off-by: Ariel Gentile <[email protected]>

fix: demo and extension module

c902a37

Signed-off-by: Ariel Gentile <[email protected]>

test: storage tests

6d21374

Signed-off-by: Ariel Gentile <[email protected]>

fix: types

02e8d90

Signed-off-by: Ariel Gentile <[email protected]>

fix: sphereon lib as didcomm dev dep

bc1ba33

Signed-off-by: Ariel Gentile <[email protected]>

TimoGlastra mentioned this pull request Jan 27, 2025

Make Credo a pure ESM package #2157

Open

genaris added 2 commits January 27, 2025 15:15

feat: add some basic documentation and expose convenience function

17e4fe2

Signed-off-by: Ariel Gentile <[email protected]>

Merge branch 'main' into refactor/didcomm-module

d50f20e

genaris marked this pull request as ready for review January 27, 2025 18:30

genaris requested a review from a team as a code owner January 27, 2025 18:30

genaris added 3 commits January 27, 2025 15:33

fix: changeset comment

30a937d

Signed-off-by: Ariel Gentile <[email protected]>

style: fix prettier

e64dfb6

Signed-off-by: Ariel Gentile <[email protected]>

TimoGlastra reviewed Jan 28, 2025

View reviewed changes

packages/core/src/modules/dids/__tests__/DidsApi.test.ts Outdated Show resolved Hide resolved

TimoGlastra reviewed Jan 28, 2025

View reviewed changes

packages/core/src/modules/dids/methods/peer/createPeerDidDocumentFromServices.ts Show resolved Hide resolved

TimoGlastra reviewed Jan 28, 2025

View reviewed changes

packages/core/src/storage/migration/updates/0.3-0.3.1/index.ts Outdated Show resolved Hide resolved

TimoGlastra reviewed Jan 28, 2025

View reviewed changes

packages/core/src/constants.ts Show resolved Hide resolved

TimoGlastra approved these changes Jan 28, 2025

View reviewed changes

genaris added 3 commits January 28, 2025 10:29

fix: address PR feedback

728aa33

Signed-off-by: Ariel Gentile <[email protected]>

Merge branch 'main' into refactor/didcomm-module

a93c6ba

fix: no updateConfig currently used

ba42e98

Signed-off-by: Ariel Gentile <[email protected]>

genaris mentioned this pull request Jan 28, 2025

Remaining work on DIDComm module #2160

Open

genaris merged commit 897c834 into openwallet-foundation:main Jan 28, 2025
20 checks passed

genaris deleted the refactor/didcomm-module branch January 28, 2025 16:28

genaris mentioned this pull request Jan 29, 2025

Move didcomm specific config options to DIDComm module #1762

Closed

sairanjit pushed a commit to sairanjit/credo-ts that referenced this pull request Feb 7, 2025

refactor!: didcomm module (openwallet-foundation#2127)

cdd6245

Signed-off-by: Ariel Gentile <[email protected]> Signed-off-by: Sai Ranjit Tummalapalli <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: didcomm module #2127

refactor!: didcomm module #2127

genaris commented Dec 5, 2024 •

edited

Loading

changeset-bot bot commented Dec 5, 2024 •

edited

Loading

TimoGlastra commented Jan 27, 2025

genaris commented Jan 27, 2025

TimoGlastra Jan 28, 2025 •

edited

Loading

genaris Jan 28, 2025

TimoGlastra Jan 28, 2025

genaris Jan 28, 2025

TimoGlastra Jan 28, 2025

genaris Jan 28, 2025

TimoGlastra commented Jan 28, 2025

refactor!: didcomm module #2127

refactor!: didcomm module #2127

Conversation

genaris commented Dec 5, 2024 • edited Loading

changeset-bot bot commented Dec 5, 2024 • edited Loading

🦋 Changeset detected

TimoGlastra commented Jan 27, 2025

genaris commented Jan 27, 2025

TimoGlastra Jan 28, 2025 • edited Loading

Choose a reason for hiding this comment

genaris Jan 28, 2025

Choose a reason for hiding this comment

TimoGlastra Jan 28, 2025

Choose a reason for hiding this comment

genaris Jan 28, 2025

Choose a reason for hiding this comment

TimoGlastra Jan 28, 2025

Choose a reason for hiding this comment

genaris Jan 28, 2025

Choose a reason for hiding this comment

TimoGlastra commented Jan 28, 2025

genaris commented Dec 5, 2024 •

edited

Loading

changeset-bot bot commented Dec 5, 2024 •

edited

Loading

TimoGlastra Jan 28, 2025 •

edited

Loading