Support connection re-use for did:peer:2/4 #2823
Conversation
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
|
My thoughts on the questions:
The requirement for reuse in the spec. is that the Does that help?
Perhaps this should also allow flagging ledger-based (published/public) DIDs? E.g. any DID that supports "resuse"?
Sounds necessary to me. |
|
Is there any way we can pull out the create methods for did:peer:2/4 into a separate but mergeable PR?. It would be useful to have for integration testing, but I can also patch in the change from this PR. |
It's already used for flagging public DID's, so I'm just piggybacking on an existing feature. |
OK this is clear, I'll update to check for a DID, and remove the specific |
|
For vs ... |
Signed-off-by: Ian Costanzo <[email protected]>
|
@dbluhm and @TelegramSam — can you please weigh in on this one to help Ian complete the implementation? Thanks! |
Signed-off-by: Ian Costanzo <[email protected]>
I'd say short form unless there's a strong reason not to (there isn't an obvious reason that comes to mind for me). |
|
10 minute scan didn't reveal any red flags for me. I'll have to set aside some time to look deeper. |
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
Signed-off-by: Ian Costanzo <[email protected]>
|
Integration tests have failed twice in a row, so I ran them locally and everything passed. |
Signed-off-by: Ian Costanzo <[email protected]>
aries_cloudagent/wallet/askar.py
Outdated
| other_did_info = other_did.value_json | ||
| if len(other_did_info["did"]) < len(ret_did_info["did"]): | ||
| ret_did = other_did | ||
| ret_did_info = ret_did.value_json |
There was a problem hiding this comment.
Shouldn't this be ret_did_info = other_did.value_json?
There was a problem hiding this comment.
Could be either I guess, it's the same value
There was a problem hiding this comment.
Could we just keep it the same to alleviate confusion? I know it's a bit nitpicky but from an outside perspective I would assume there's a reason they're not referencing the same object.
| @@ -279,6 +284,99 @@ async def create_invitation( | |||
| routing_keys=[], | |||
| ).serialize() | |||
|
|
|||
| elif did_peer_4 or did_peer_2: | |||
There was a problem hiding this comment.
This function is getting quite long and is difficult to track logic. I would suggest at some point we refactor this out into smaller and more digestible blocks.
There was a problem hiding this comment.
Yes I agree, I think that's a task for the next time a DID method needs to be supported for connection reuse
I chatted with Stephen about this general topic and we agreed to put it off. The next DID(s) - that we know of - that need to support connection reuse are all public, so we should be good for now ...
Signed-off-by: Ian Costanzo <[email protected]>
|
See issue #2703
This PR is READY FOR REVIEW!, reuse working for public DID and did:peer:2/4.
GET /wallet/didresponse--public-did-connections- use the inviter's public DID in invitations, and allow use of implicit invitations--reuse-connections- support connection re-use (invitee will reuse an existing connection if it uses the same DID as in the new invitation)--multi-use-invitations- inviter will issue multi-use invitations--reuse-connectionsflag needs to be enabled in both Faber and Alice for the demo to enable connection reuseAlso added a few new integration tests with the
--connection-reuseflag.