Did-exchange PR revival #1046
Merged
Did-exchange PR revival #1046
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Patrik-Stas
force-pushed
the
feature/did-exchange
branch
from
a821d0e to
e5eb72e
Compare
Patrik-Stas
force-pushed
the
feature/did-exchange-additions
branch
from
8d2c6cd to
e433820
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feature/did-exchange #1046 +/- ##
======================================================
Coverage ? 0.05%
======================================================
Files ? 471
Lines ? 24009
Branches ? 4307
======================================================
Hits ? 13
Misses ? 23995
Partials ? 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Patrik-Stas
force-pushed
the
feature/did-exchange-additions
branch
2 times, most recently
from
03c8f44 to
8c2fee1
Compare
Patrik-Stas
force-pushed
the
feature/did-exchange-additions
branch
from
23a4a2b to
266ee29
Compare
Patrik-Stas
force-pushed
the
feature/did-exchange
branch
from
e5eb72e to
79cc60a
Compare
Patrik-Stas
force-pushed
the
feature/did-exchange-additions
branch
2 times, most recently
from
f4c5b78 to
60fd307
Compare
Patrik-Stas
force-pushed
the
feature/did-exchange
branch
from
79cc60a to
41f85b9
Compare
Patrik-Stas
force-pushed
the
feature/did-exchange-additions
branch
from
14096f3 to
5be1a4c
Compare
Signed-off-by: Patrik Stas <[email protected]>
Patrik-Stas
force-pushed
the
feature/did-exchange
branch
from
41f85b9 to
dcd987c
Compare
mirgee
reviewed
Nov 16, 2023
aries_vcx/src/protocols/did_exchange/state_machine/generic/mod.rs
Outdated
Show resolved
Hide resolved
| // todo: no unwrap pls | ||
| let service = did_document.service().first()?; | ||
| // todo: will get cleaned up after service is de-generified | ||
| Some(match service { |
There was a problem hiding this comment.
This should be performed by ServiceSov
There was a problem hiding this comment.
moved as helper to aries agent, will probably look different after de-generification of did document
There was a problem hiding this comment.
It will look different, but does your idea include abolishment of ServiceSov?
Patrik-Stas
force-pushed
the
feature/did-exchange-additions
branch
from
5be1a4c to
a9829b7
Compare
- Generalize did_resolver_sov - Move methods - Remove did-exchange send_message method - Address more code review comments Signed-off-by: Patrik Stas <[email protected]>
Patrik-Stas
force-pushed
the
feature/did-exchange-additions
branch
from
36e00a9 to
1439ee2
Compare
mirgee
approved these changes
Nov 20, 2023
mirgee
reviewed
Nov 20, 2023
| .base58(); | ||
| EncryptionEnvelope::create( | ||
| wallet, | ||
| json!(message).to_string().as_bytes(), |
There was a problem hiding this comment.
It would be better to not use a potentially panicking macro
Patrik-Stas
added a commit
that referenced
this pull request
Nov 20, 2023
* Restore did-exchange * Address code review Signed-off-by: Patrik Stas <[email protected]>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
mainsince the original did-exchange PR DID Exchange Protocol #928oobinvitation fordid-exchange, drop support for others - the protocol is not supposed to handle legacy connection pairwise or connection public invitationspub async fn send_message<T>(&self, wallet: &impl BaseWallet, message: &AriesMessage, transport: &T)for did-exchange - this makes this a lot simpler in higher levels, eg. inaries-vcx-agentmediatoragent, remove localoob2didin favor of reusingaries_vcx'soob_invitation_to_legacy_did_docaries_vcx/src/common/ledger/transactions.rsmove functions from this file to more appropriate filearies_vcx/src/handlers/out_of_band/receiver.rsandaries_vcx/src/protocols/connection/invitee/mod.rsConstructRequestConfig- given we are now using generics rather than trait objects, these objecs would now start involving generics, as you can use syntax such as&impl IndyLedgerReadin struct definition. I have more arguments for the changerevoke_credential_local- first of all, workflows should not do asserts; secondly, the asserts didn't really make sense and was probably originating from lot older code after rounds of refactoringNext steps:
aries_vcxinto/ariesdirectory - before any new activity foraries_vcxemerges in PRsDidExchangeRequester<RequestSent>constructs requests messages - currently there's 2 functionsconstruct_request_pairwiseandconstruct_request_publicwith different set of arguments and certain assumptions. To keep the interface simple, and given we operate in Aries paradigm, the input should be an OOB Invitation message. Then, this processing function should ideally support variety of DIDs which might appear in the inivtation - for starter:did:peer:4anddid:sov.