feat: find existing connection based on invitation did #698
Conversation
Signed-off-by: Jakub Koci <[email protected]>
Signed-off-by: Jakub Koci <[email protected]>
packages/core/src/modules/dids/domain/createPeerDidFromServices.ts
Outdated
Show resolved
Hide resolved
|
|
||
| import { DidDocumentBuilder, Key } from '.' | ||
|
|
||
| export function createDidDocumentFromServices(services: DidCommService[]) { |
There was a problem hiding this comment.
Let's verify this approach in the Aries WG, and if we have an answer we can add some documentation to the RFC.
There was a problem hiding this comment.
Yes. But we don't have to keep this PR open until then right?
| if (connections.length === 1) { | ||
| const [firstConnection] = connections | ||
| return firstConnection | ||
| } else if (connections.length > 1) { | ||
| this.logger.warn(`There is more than one connection created from invitationDid ${did}. Taking the first one.`) | ||
| const [firstConnection] = connections | ||
| return firstConnection | ||
| } |
There was a problem hiding this comment.
| if (connections.length === 1) { | |
| const [firstConnection] = connections | |
| return firstConnection | |
| } else if (connections.length > 1) { | |
| this.logger.warn(`There is more than one connection created from invitationDid ${did}. Taking the first one.`) | |
| const [firstConnection] = connections | |
| return firstConnection | |
| } | |
| if (connections.length > 1) { | |
| this.logger.warn(`There is more than one connection created from invitationDid ${did}. Taking the first one.`) | |
| } | |
| // firstConnection will be undefined if length of connections array is 0 | |
| const [firstConnection] = connections | |
| return firstConnection |
There was a problem hiding this comment.
I'd rather be more explicit in the code than add a comment.
| if (recipientKey.startsWith('did:key')) { | ||
| didKey = DidKey.fromDid(recipientKey) | ||
| } else { | ||
| const publicKeyBase58 = recipientKey | ||
| const ed25519Key = Key.fromPublicKeyBase58(publicKeyBase58, KeyType.Ed25519) | ||
| didKey = new DidKey(ed25519Key) | ||
| } |
There was a problem hiding this comment.
Maybe we can add this to the DidCommService class? I think we'll have to do this more often and if we can just add recipientKeysPublicKeyBase58 and also an recipientKeysDids (how to name this?) we don't have to guess if the value starts with did:key anywhere else in the codebase
There was a problem hiding this comment.
Hmm, maybe, it seems there is just one place, so I don't see significant benefits.
There was a problem hiding this comment.
It might be more related to the PR #700
There was a problem hiding this comment.
See my comments on your other PR. To mee it looks like we're doing it quite often.
There was a problem hiding this comment.
Let's work on it there then.
Signed-off-by: Jakub Koci <[email protected]>
* Extract creation of did doc from services * Create peer did from service and store it as invitation did Signed-off-by: Jakub Koci <[email protected]>
* Extract creation of did doc from services * Create peer did from service and store it as invitation did Signed-off-by: Jakub Koci <[email protected]>
* Extract creation of did doc from services * Create peer did from service and store it as invitation did Signed-off-by: Jakub Koci <[email protected]>
I added a function
serviceToNumAlgo2Didbut I'm testing it inDidPeer.test, because I was curious if it's possible to create regular PeerDid and then takedidDocumentfrom it. As you can see it's not possible right now so I commented the code in the test.I described what might be a cause of the problem in issue here RFC 0434: Ambiguous description of Peer DID numalgo 2 service encoding.
There is also a simplification of when and what invitation did we store in the connection record. OOB invitation can contain more dids (or service blocks encoded as peer did numalgo2) and we should store only the one we successfully use to send connection request message to.