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.

Sign up for GitHub

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

Jump to bottom

Sovrin-specific DDO facade #871

Merged
merged 14 commits into from
Jun 20, 2023
Merged

Sovrin-specific DDO facade #871

merged 14 commits into from
Jun 20, 2023

Conversation

mirgee
Copy link
Contributor

@mirgee mirgee commented Jun 5, 2023

Introduces wrappers around DidDocument and Service in order to

  • handle logic specific to the Sovrin method, e.g.
    • setting and validating type and accept fields based on the composition of the service extra fields,
    • dereferencing service keys (?)
  • remove definition of the Sovrin extra fields from did_resolver_sov,
  • simplify common Sovrin DDO usage patterns (e.g. retrieving recipient / routing key from the first service in the service field, etc.).

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2023 •
edited
Loading

Codecov Report

Merging #871 (100c2bd) into main (fd0b8c7) will decrease coverage by 1.02%.
The diff coverage is n/a.

❗ Current head 100c2bd differs from pull request most recent head 87498b4. Consider uploading reports for the commit 87498b4 to get more accurate results

@@            Coverage Diff             @@
##             main     #871      +/-   ##
==========================================
- Coverage   49.89%   48.88%   -1.02%     
==========================================
  Files         432      432              
  Lines       35058    34337     -721     
  Branches     7610     7606       -4     
==========================================
- Hits        17493    16784     -709     
+ Misses      12288    12275      -13     
- Partials     5277     5278       +1
Flag Coverage Δ
unittests-aries-vcx 48.84% <ø> (-1.02%) ⬇️

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

see 43 files with indirect coverage changes

@mirgee mirgee force-pushed the feature/did-doc-sov branch 2 times, most recently from c6b6551 to abb2945 Compare June 6, 2023 15:23
@mirgee mirgee force-pushed the refactor/service-builder branch from d5ffa6c to d0279a6 Compare June 6, 2023 15:33
@mirgee mirgee force-pushed the feature/did-doc-sov branch from abb2945 to bb637aa Compare June 6, 2023 15:42
@mirgee mirgee marked this pull request as ready for review June 6, 2023 15:43
@mirgee mirgee force-pushed the refactor/service-builder branch from d0279a6 to 19d0d81 Compare June 7, 2023 05:46
@mirgee mirgee force-pushed the feature/did-doc-sov branch from bb637aa to acc3450 Compare June 7, 2023 05:46
@mirgee mirgee mentioned this pull request Jun 7, 2023
Merged
@mirgee mirgee requested a review from Patrik-Stas June 7, 2023 12:55
@mirgee mirgee force-pushed the refactor/service-builder branch from 19d0d81 to 4107359 Compare June 7, 2023 13:15
@mirgee mirgee force-pushed the feature/did-doc-sov branch from 95056f1 to 701dc8d Compare June 7, 2023 13:29
Base automatically changed from refactor/service-builder to main June 7, 2023 14:24
Patrik-Stas
Patrik-Stas reviewed Jun 8, 2023
View reviewed changes
Copy link
Contributor

@Patrik-Stas Patrik-Stas 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 comments

After this PR, perhaps it's really time to put all the did/ddo stuff into a directory, the number of crates is quite expanding, but I really like the separation of concerns in each of them

did_doc/src/schema/service.rs Outdated Show resolved Hide resolved
did_doc_sov/src/extra_fields/mod.rs
pub mod didcommv2;

#[derive(Serialize, Clone, Debug, PartialEq)]
pub enum AcceptType {
Copy link
Contributor

Choose a reason for hiding this comment

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

there's not much documentation about this even, is there?

Can't see much at https://sovrin-foundation.github.io/sovrin/spec/did-method-spec-template.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Found a mention here and here.

did_doc_sov/src/extra_fields/didcommv2.rs

#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Default)]
#[serde(rename_all = "camelCase")]
pub struct ExtraFieldsDidCommV2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

just wondering, what is this based on? How did you find out what this should look like?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Sovrin method spec you linked in the comment above.

did_doc_sov/src/extra_fields/mod.rs Show resolved Hide resolved
did_doc_sov/src/extra_fields/mod.rs Outdated Show resolved Hide resolved
did_doc_sov/src/error.rs Show resolved Hide resolved
@mirgee mirgee force-pushed the feature/did-doc-sov branch 2 times, most recently from 51392ce to b67aa3e Compare June 9, 2023 15:17
@mirgee mirgee changed the base branch from main to refactor/remove-unnecessary-bound June 9, 2023 15:17
Base automatically changed from refactor/remove-unnecessary-bound to main June 9, 2023 17:14
@mirgee mirgee force-pushed the feature/did-doc-sov branch 2 times, most recently from 820fef6 to 5431cdf Compare June 12, 2023 18:37
@mirgee mirgee requested a review from bobozaur June 13, 2023 05:42
mirgee
mirgee commented Jun 13, 2023
View reviewed changes
did_doc_sov/src/lib.rs Outdated Show resolved Hide resolved
@mirgee mirgee requested a review from Patrik-Stas June 13, 2023 08:51
mirgee added 5 commits June 20, 2023 11:17
@mirgee @bobozaur
WIP
2dda8b2 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Use extra fields from did_doc_sov in did_resolver_sov
3b73b97 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Change key type
7a1c4ad 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Remove extra fields on verification method
b6e40b6 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Use KeyKind for keys in service
2b05f04 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
mirgee added 9 commits June 20, 2023 11:17
@mirgee @bobozaur
Use enum for public key field in verification method
8755602 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Use enum for verification method types
704b6a8 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Add pubkey dereferencing
acdebf4 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Update did_resolver_sov
8469317 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
ExtraFields -> ExtraFieldsSov
5d1ff11 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Fail early - try converting all services
915278c 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Remove From<String> for KeyKind
fddb0f3 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Remove Default for KeyKind
fc3dc2f 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@mirgee @bobozaur
Remove ServicesList
87498b4 
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@bobozaur bobozaur force-pushed the feature/did-doc-sov branch from 100c2bd to 87498b4 Compare June 20, 2023 08:17
@bobozaur bobozaur merged commit 4d89351 into main Jun 20, 2023
@bobozaur bobozaur deleted the feature/did-doc-sov branch June 20, 2023 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobozaur bobozaur bobozaur approved these changes

@Patrik-Stas Patrik-Stas Awaiting requested review from Patrik-Stas

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mirgee @codecov-commenter @Patrik-Stas @bobozaur