feat: add out-of-band and did exchange

[jakubkoci]

Todo

• API
  • refactor(api): Change acceptInvitation param from oob record to record id (directly pushed to this branch)
  • feat(api): Expose remove, find, getAll oob methods (directly pushed to this branch)
• Implement handshake reuse handler and event emitter to react on handshake (pushed directly to this branch)
• Store the old did in metadata (this can have impact to migration scripts) (feat: 0.2.0 connection migration script #731)
• Connection record migration script @TimoGlastra (feat: 0.2.0 connection migration script #731)
• Reuse oob invitation keys also for multiuse invitation (?)
  • @TimoGlastra Currently, I'm still creating new keys when it's multiuse invitation. I assume we don't want to do that, right?
• Add oob record state and role checks
• Emit oob state change event
  • TODO: may need some extra tests in the connection/didexchange handlers to test whether updateState is called
• Resolve key from reference (?)
• Align connection and did-exchange protocol states @TimoGlastra Could you look at this, please? I don't see it as entirely beneficial and you already have an idea of how to do it in your mind. (refactor: unify connection record state and role #732)
• API
  • refactor(api): Rename outOfBandMessage to outOfBandInvitation (directly pushed to this branch)
  • feat(api): store more from receiveInvitation config into a record and allow override in acceptInvitation
• refactor(core): Change did to unqualifiedSovDid or maybe unqualifiedIndyDid
• refactor(core): Extract connection protocol methods from service to protocol class
• Throw an error if there is more then one rule resolved by state machine
  • But I actually didn’t find beneficial the way how I implemented the did exchange state machine
• Fix naming caused by DecryptedMessageContext vs. UnpackedMessageContext
• refactor: Rename XxxMessageOptions to XxxMessageProps (useful outside of oob, extracted to separate issue: refactor: Rename XxxMessageOptions to XxxMessageProps #750)

I ordered it approximately by priority. The first half before the second API point seems to be quite important, and the rest is more a nice-to-have.

I assigned names based on notes from our AFJ call last week https://wiki.hyperledger.org/display/ARIES/2022-04-14+Aries+Framework+JS+Meeting+notes, please raise your hand if it doesn't suit you. I can look at the rest of the tasks. I welcome some feedback on what you think is important and what is not.

Is there any task from the Todo list I should look at even before merging?

[TimoGlastra]

Really great work @jakubkoci. I've left quite some comments, interested to hear what you think!

Will take a look at the todo's over the next couple of days

[TimoGlastra]

Nit, but I think using connection was probably a mistake and we should use outOfBandRecord and connectionRecord everywhere. I think outOfBand as property name is not quite explicit of what it is (a record).

Suggested change

	const { connection, outOfBand, payload } = outboundMessage
	const { connection, outOfBandRecord, payload } = outboundMessage

[TimoGlastra]

Would be better to do this using an event listener

[TimoGlastra]

Should this be true? It's the opposite of what we've done in the past where the default is always false.

In addition, we should maybe also take the agentConfig autoAcceptInivtation into account here?

[TimoGlastra]

I think this one is important to change, or we should otherwise at least be very clear in documentation that this is the inverse of what we've done in the past. Any comments on this one @jakubkoci?

[TimoGlastra]

Not for now, but we should update this to an event listener so it survives an agent shutdown.

[codecov-commenter]

Codecov Report

Merging #717 (d7fd2d2) into main (cbdff28) will increase coverage by 0.46%.
The diff coverage is 87.45%.

@@            Coverage Diff             @@
##             main     #717      +/-   ##
==========================================
+ Coverage   86.92%   87.39%   +0.46%     
==========================================
  Files         393      430      +37     
  Lines        9227    10488    +1261     
  Branches     1624     1822     +198     
==========================================
+ Hits         8021     9166    +1145     
- Misses       1147     1259     +112     
- Partials       59       63       +4     

Impacted Files	Coverage Δ
packages/core/src/agent/Dispatcher.ts	80.70% <0.00%> (-2.94%)	⬇️
packages/core/src/agent/Events.ts	100.00% <ø> (ø)
...s/core/src/modules/connections/ConnectionEvents.ts	100.00% <ø> (ø)
...les/connections/repository/ConnectionRepository.ts	83.33% <ø> (+5.55%)	⬆️
.../core/src/modules/credentials/CredentialsModule.ts	91.70% <ø> (+1.03%)	⬆️
...s/protocol/v1/handlers/V1IssueCredentialHandler.ts	92.59% <ø> (ø)
...s/protocol/v1/handlers/V1OfferCredentialHandler.ts	71.05% <ø> (ø)
...protocol/v1/handlers/V1RequestCredentialHandler.ts	97.67% <ø> (ø)
...s/protocol/v2/handlers/V2IssueCredentialHandler.ts	93.10% <ø> (ø)
...s/protocol/v2/handlers/V2OfferCredentialHandler.ts	71.79% <ø> (ø)
... and 104 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 cbdff28...d7fd2d2. Read the comment docs.

[TimoGlastra]

@TimoGlastra Currently, I'm still creating new keys when it's multiuse invitation. I assume we don't want to do that, right?

Not sure I fully understand. Where are you creating new keys? I think we should create new keys for each new connection, but we can reuse the same keys for everyone that uses the invitation (we must because otherwise the invitation becomes invalid).

[TimoGlastra]

While writing the migration scripts I found some issues, I'll start working on them one by one, but just to let you know (maybe you have some comments on the points):

• update messageId to invitationId
• OOB state should be set to done once we send or receive the first non-oob message (so after sending or receiving the didexchange / connection request message)
  • Currently done in response / complete handlers
• convertToNewInvitation should transform the verkeys into did:key (directly pushed to feat: add out-of-band and did exchange #717)
• key encoding in recipientKeys and routingKeys is not in line with the RFCs: (fix: always encode keys according to RFCs #733)
  • According to rfc:
    • oob service should always use did:key
    • did-communication service: should always use did:example:123#key-id references (or #key-id references in peer did doc)
      • for routingKeys if we don't have the did, we transform the raw key into a did:key with key reference
    • IndyAgent: raw publicKeyBase58 for both routing and recipient keys
    • 0160 invitation service: raw publicKeyBase58 for both routing and recipient keys
  • update recipientKeys in the did document
    • Should normalize the values (to a key?)
      • as you can see above there's three different ways to encode keys into a didcomm service, we should normailze the value before storing it as recipient key (otherwise it will be really hard to know which format to query in later on)
      • if the key in the service is a local reference we should actually extract the key from the verificationMethod / authentication array objects
      • if the key is an external reference we should probably resolve it? Not sure we need this as we only ned to store the recipientKeys and those will always be local key references I think
    • What to do if DIDComm v2 service is present? Should we store all recipientKeys?
      • Probably not because in didcomm v2 the other agent will indicate the did they're using to send the message. Let's skip this for now
• convertToNewDidDoc (directly pushed to branch)
  • Should convert the id and controller values
  • May be other items that should be converted
• Store legacy did and did document string in did record after converting to new did document (needs changes from feat: 0.2.0 connection migration script #731)
• We can probably remove some props from the connection record over time? E.g. multiUseInvitation (in oob record) and mediatorId (in oob record)
• out of band record only storing first recipient key as tag
  • STILL TODO: Also, if we're connecting using a public did we should store the key of the did public did also probably? In that case we should probably set the tag outside of the oob record as we need to resolve the did document.
• associating out of band recor with session and finding it based on that can maybe be insecure, as there's multiple entities that can act on the same oob record when using multi use invitations. Is this true?
• differentiate between stored and resolved version of peer did document
• demo is broken
• connectionless should be integrated with oob, also we should allow for connection reuse when doing connectionless.
  • the oob offer/request will not have a connection id when we receive the first reply. we must set it on the record
• creating an out of band offer/request will currently create keys, and the oob invitation will also create keys. I propose the following flow:
  1. create oob offer/request no keys will be created, no ~service is present
  2. call oob.createInvitation to create oob invitation containing the offer/request
  3. OR call oob.createLegacyConnectionlessInvitation (naming not final yet, but you get the idea) that will generate a key and set the ~service decorator. This makes the process two steps, but I think the api is nice enough to do that.

[TimoGlastra]

Already merged #736 into this branch because it contains fixes needed for this branch to work with pickup v2

[TimoGlastra]

What do you mean by this task @jakubkoci?

Resolve key from reference (?)

[jakubkoci]

What do you mean by this task @jakubkoci?

Resolve key from reference (?)

I think I meant if there is a recipient key in a form of reference in service recipient keys array, then we must resolve the key.

[TimoGlastra]

I think I meant if there is a recipient key in a form of reference in service recipient keys array, then we must resolve the key.

This has been solved in this PR I think: #733

[TimoGlastra]

Also, I'm not 100% sure what you mean by this:

Reuse oob invitation keys also for multiuse invitation (?)
@TimoGlastra Currently, I'm still creating new keys when it's multiuse invitation. I assume we don't want to do that, right?

I think we should use new keys for each connection, but the invitation will use the same invitation keys for everyone connecting

[TimoGlastra]

Wohoo 🎉

[TimoGlastra]

I'll write a release notes / breaking changes message in a bit and then merge