Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove didcore generics, update did-exchange #1097

Merged
merged 40 commits into from
Jan 18, 2024

Conversation

Patrik-Stas
Copy link
Contributor

@Patrik-Stas Patrik-Stas commented Jan 8, 2024

  • Remove unused DidDocumentSovError
  • Implement new methods for DidDocument: get_service_of_type, get_key_agreement_of_type - this is handy upon using the did doc for didcomm. For example in agent code, we
    • find service of DIDCommV1 type
    • instruct "encryption envelope" layer to encrypt msg specifically for that service, specified by its id
    • in encryption envelope code, we make use of get_key_agreement_of_type to resolve former "recipient keys", but now using first compatible key from key_agreement section
  • Refactor DidExchangeRequester<RequestSent>::receive_response
  • Revert peer did decocoding: use PeerDidResolver in favor of peer_did.to_did_doc(). Make the to_did_doc available only locally within peer_did crate
  • Modify EncryptionEnvelope ::create such that it requires additional argument their_service_id: &Uri. This is because msg encryption should be intended for specific service, using its routing keys. Without specifying the service, EncryptionEnvelope would need to make additional assumption about which service to encrypt for.
  • Trim down variants of DidDocumentBuilderError enumeration - merge various decoding error variants into single KeyDecodingError
  • Reorganize ExtraFieldsDidComm* types inside did_doc crate. Replace its custom builders by TypedBuilder annotations.
  • Attempted to hide away OneOrList from DidDocument and Service consumers. However this is not entirely possible without giving up on certain properties. In order for did_peer to abbraviate and de-abbreviate service back to its original form, did_peer crates need to work with "representation aware" OneOrList, rather than Vector.
  • Tweaked "typed" service ServiceDidCommV1, ServiceDidCommV2 to hold single, hardecoded, value of types. The type itself represents DidCore Service object of particular DidCore service type value, therefore it makes no sense for this object to possibly hold list of type values, neither to hold value didcore service type different than the Rust type inherently represents.
    • On this note, there was ask to extract ServiceDidCommV1, ServiceDidCommV2 and related structures to separate crate, as these previously resided in did_doc_sov crate. The main purpose of that crate was previously to fill-in the underlying generics into concrete types. This would no longer be the purpose of the crate - these "concrete type" service types are now rather just supporting structures for particular use-cases (1. building service objects of specific type; 2. converting general Service of known type to its respective Rust type representation). Hence I see now much less justification to take these out of did_doc crate.
  • Fix: did:peer:3 validation regex did not allow DIDs which contains Service only. But all fields in Did Document are optional - except id, did:peer:3.S.* is valid DID too.
  • Fix: did peer deabbreviation did not preserve id value, if it was originally set.

@codecov-commenter
Copy link

codecov-commenter commented Jan 8, 2024

Codecov Report

Attention: 281 lines in your changes are missing coverage. Please review.

Comparison is base (4a3a300) 0.05% compared to head (49b3a10) 0.05%.

Files Patch % Lines
...re/did_doc/src/schema/verification_method/error.rs 0.00% 36 Missing ⚠️
.../did_doc/src/schema/service/service_accept_type.rs 0.00% 26 Missing ⚠️
did_core/did_doc/src/schema/utils/mod.rs 0.00% 26 Missing ⚠️
aries/aries_vcx/src/utils/didcomm_utils.rs 0.00% 21 Missing ⚠️
...core/did_doc/src/schema/service/typed/didcommv1.rs 0.00% 20 Missing ⚠️
aries/aries_vcx/tests/test_did_exchange.rs 0.00% 17 Missing ⚠️
did_core/did_doc/src/schema/types/jsonwebkey.rs 0.00% 14 Missing ⚠️
...ore/did_doc/src/schema/service/service_key_kind.rs 0.00% 10 Missing ⚠️
...core/did_doc/src/schema/service/typed/didcommv2.rs 0.00% 10 Missing ⚠️
did_core/did_doc/src/schema/types/multibase.rs 0.00% 9 Missing ⚠️
... and 28 more
Additional details and impacted files
@@                  Coverage Diff                   @@
##           diddoc/remove-generics   #1097   +/-   ##
======================================================
  Coverage                    0.05%   0.05%           
======================================================
  Files                         480     481    +1     
  Lines                       24234   24180   -54     
  Branches                     4373    4367    -6     
======================================================
  Hits                           13      13           
+ Misses                      24220   24166   -54     
  Partials                        1       1           
Flag Coverage Δ
unittests-aries-vcx 0.05% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Patrik-Stas Patrik-Stas force-pushed the diddoc/remove-generics-updates branch 2 times, most recently from 5f29bb8 to 9a5e54a Compare January 8, 2024 15:07
@mirgee
Copy link
Contributor

mirgee commented Jan 10, 2024

Is this really ready for review? It doesn't seem to be...?

@Patrik-Stas
Copy link
Contributor Author

@mirgee yes, what am I missing?

@Patrik-Stas
Copy link
Contributor Author

I didn't reworked all of the errors stuff we discussed. I'd like to do that as separate PR

@mirgee
Copy link
Contributor

mirgee commented Jan 10, 2024

How about we make all the changes in one PR instead of expecting the reviewers to keep track of the changes in #1075, the changes in this PR in the context of changes in #1075 and the changes which were not yet made but are planned to be made in the future?

Copy link
Contributor

@mirgee mirgee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some brief comments, flagging the points discussed verbally in a private conversation.

did_core/did_methods/did_peer/src/resolver/mod.rs Outdated Show resolved Hide resolved
did_core/did_doc/src/schema/verification_method/error.rs Outdated Show resolved Hide resolved
did_core/did_doc/src/schema/utils/mod.rs Outdated Show resolved Hide resolved
did_core/did_doc/src/schema/types/multibase.rs Outdated Show resolved Hide resolved
aries/misc/shared/src/http_client.rs Outdated Show resolved Hide resolved
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
…only used for destructuring pattern)

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
@Patrik-Stas
Copy link
Contributor Author

@mirgee I believe after merging #1106 into this, this is also ready for merge, all comments has been addressed.

Copy link
Contributor

@mirgee mirgee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGETM 👍

aries/aries_vcx/src/errors/mapping_others.rs Outdated Show resolved Hide resolved
Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
@Patrik-Stas Patrik-Stas merged commit 5dcd014 into diddoc/remove-generics Jan 18, 2024
29 checks passed
@Patrik-Stas Patrik-Stas deleted the diddoc/remove-generics-updates branch January 18, 2024 13:52
Patrik-Stas added a commit that referenced this pull request Jan 18, 2024
* Enhancement: find first comptabile key agreement verification method

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Used TypedBulder for didcomm-service data models

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Do not leak DidDocumentBuilder out of did_doc crate

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Refactor construct_request didexchange transition

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Reformat

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Rename resolve_their_ddo -> resolve_ddo_from_request

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Reorganize typed didcomm service models

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Make encryption envelope api more general and safer

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Change MissingField to wrap 'str rather than String

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Reduce use of OneOrList

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Remove DidDocumentSovError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Add negative test cases for verification method

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Do not return DidDocumentBuilderError from JsonWebKey methods

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Multibase wrapper use custom error instead of DidDocumentBuilderError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Add todo about using ? with Jwk decoding

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Handle jwk error explicitly

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Remove now unnecessary serde:error->DidDocumentBuilderError mapping

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Uri wrapper use custom error instead of DidDocumentBuilderError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Create custom error for KeyDecoding errors

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Remove unused InvalidInput error variant

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Use PeerDid resolver in favor of peer_did.to_did_doc

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Typed service object represents service with particular value of types attribute

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Fix didpeer test, fix did:peer:2 regex validation

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Fix didpeer regex, fix test test_peer_did_2_encode_decode, fix service id deabbreviation

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Add todo note

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Fix invalid peer did fixture, remove comments

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Remove forgotten logs

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Use global uri error mapping in aries-vcx

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Create DidDocumentLookupError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Remove getters of DidResolutionOutput in favor of public fields (commonly used for destructuring pattern)

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Replace for cycles by find()

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Tweak send_message to take Url by reference

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Change to_did_doc to to_did_doc_builder

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Add source attribute to MultibaseWrapperError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Create JsonWebKeyError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Turn KeyDecodingError enum into struct with source err as trait object

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Define peer2 resolution on peer_did itself

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Refactor service_types()

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Fix formatting

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* refactor: keep single way to resolve peer dids (#1106)

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

---------

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
Patrik-Stas added a commit that referenced this pull request Jan 31, 2024
* Enhancement: find first comptabile key agreement verification method

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Used TypedBulder for didcomm-service data models

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Do not leak DidDocumentBuilder out of did_doc crate

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Refactor construct_request didexchange transition

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Reformat

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Rename resolve_their_ddo -> resolve_ddo_from_request

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Reorganize typed didcomm service models

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Make encryption envelope api more general and safer

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Change MissingField to wrap 'str rather than String

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Reduce use of OneOrList

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Remove DidDocumentSovError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Add negative test cases for verification method

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Do not return DidDocumentBuilderError from JsonWebKey methods

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Multibase wrapper use custom error instead of DidDocumentBuilderError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Add todo about using ? with Jwk decoding

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Handle jwk error explicitly

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Remove now unnecessary serde:error->DidDocumentBuilderError mapping

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Uri wrapper use custom error instead of DidDocumentBuilderError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Create custom error for KeyDecoding errors

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Remove unused InvalidInput error variant

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Use PeerDid resolver in favor of peer_did.to_did_doc

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Typed service object represents service with particular value of types attribute

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Fix didpeer test, fix did:peer:2 regex validation

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Fix didpeer regex, fix test test_peer_did_2_encode_decode, fix service id deabbreviation

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Add todo note

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Fix invalid peer did fixture, remove comments

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Remove forgotten logs

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Use global uri error mapping in aries-vcx

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Create DidDocumentLookupError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Remove getters of DidResolutionOutput in favor of public fields (commonly used for destructuring pattern)

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Replace for cycles by find()

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Tweak send_message to take Url by reference

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Change to_did_doc to to_did_doc_builder

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Add source attribute to MultibaseWrapperError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Create JsonWebKeyError

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Turn KeyDecodingError enum into struct with source err as trait object

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Define peer2 resolution on peer_did itself

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Refactor service_types()

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* Fix formatting

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

* refactor: keep single way to resolve peer dids (#1106)

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>

---------

Signed-off-by: Patrik Stas <patrik.stas@absa.africa>
@Patrik-Stas Patrik-Stas changed the title Diddoc/remove generics updates Remove didcore generics, update did-exchange Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants