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

Support referrent-explicit format creating presentation request #844

Merged
merged 6 commits into from
May 15, 2023

Conversation

Patrik-Stas
Copy link
Contributor

@Patrik-Stas Patrik-Stas commented May 14, 2023

[
  { name: 'age', p_type: '>=', p_value: 18 }, 
  { name: 'balance', p_type: '>=', p_value: 10000 }
]

we transformed the input internally to anoncreds format

{
  predicate_0: { name: 'age', p_type: '>=', p_value: 18 }, 
  predicate_1: { name: 'balance', p_type: '>=', p_value: 10000 }
}
  • This PR enabled API called to simply supply entire requested_predicates object (as per anoncreds link above) and therefore letting the caller specify referents names eg.
{
  is_adult: { name: 'age', p_type: '>=', p_value: 18 }, 
  credit: { name: 'balance', p_type: '>=', p_value: 10000 }
}

This is useful capability when inspecting presentations.

Note

  • We should remove vector format, it's convenient but not very useful
  • String parsing should be happening on layer above aries-vcx
  • But these are for different PRs

@codecov-commenter
Copy link

codecov-commenter commented May 14, 2023

Codecov Report

Merging #844 (58d01a8) into main (f1314ef) will decrease coverage by 40.30%.
The diff coverage is 36.00%.

@@            Coverage Diff             @@
##             main    #844       +/-   ##
==========================================
- Coverage   49.00%   8.70%   -40.30%     
==========================================
  Files         414     408        -6     
  Lines       33694   32864      -830     
  Branches     7446    7200      -246     
==========================================
- Hits        16512    2862    -13650     
- Misses      12066   29191    +17125     
+ Partials     5116     811     -4305     
Flag Coverage Δ
unittests-aries-vcx 8.70% <36.00%> (-40.27%) ⬇️

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

Impacted Files Coverage Δ
aries_vcx/src/common/proofs/proof_request.rs 60.35% <36.00%> (-1.91%) ⬇️

... and 304 files with indirect coverage changes

@gmulhearn
Copy link
Contributor

had a passing browse of this; Rust changes LGTM! Keen for the typing changes in #845 . inspired me to make this ticket #847

Copy link
Contributor

@bobozaur bobozaur 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 about readability mainly.

aries_vcx/src/common/proofs/proof_request.rs Show resolved Hide resolved
aries_vcx/src/common/proofs/proof_request.rs Show resolved Hide resolved
aries_vcx/src/common/proofs/proof_request.rs Show resolved Hide resolved
@Patrik-Stas Patrik-Stas requested a review from bobozaur May 15, 2023 13:59
Copy link
Contributor

@bobozaur bobozaur left a comment

Choose a reason for hiding this comment

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

Had a short discussion with @Patrik-Stas who pointed out this is kind of legacy code and will most likely get refactored soon.

Since previous comments were mainly about readability, I have no problem with merging this as-is now.

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>
@bobozaur bobozaur force-pushed the verifier/support-referrent-format branch from 3b0ef07 to 58d01a8 Compare May 15, 2023 15:03
@bobozaur bobozaur merged commit e666981 into main May 15, 2023
39 checks passed
@bobozaur bobozaur deleted the verifier/support-referrent-format branch May 15, 2023 19:18
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.

None yet

4 participants