Open wip-abramson opened 3 years ago
@wip-abramson do you see this being another helper class inside the https://github.com/OpenMined/PyDentity/blob/master/libs/aries-basic-controller/aries_basic_controller/helpers/ folder or by adding this functionality to https://github.com/OpenMined/PyDentity/blob/master/libs/aries-basic-controller/aries_basic_controller/helpers/utils.py directly?
@wip-abramson starting to take a stab at this in this branch https://github.com/OpenMined/PyDentity/tree/issue_60 Have a look and then let us discuss the plan of action, especially around errors and passing back the parsed results.
And on another note, don't think it is required to provide parsing for the whole verify object. Rather focus on the obvious and most relevant items.
HI guys,
I guess also the parsed JSON dumps could be also filtered more to allow easier forwarding in the flow.
On Wed, Feb 17, 2021 at 2:14 PM lohanspies notifications@github.com wrote:
And on another note, don't think it is required to provide parsing for the whole verify object. Rather focus on the obvious and most relevant items.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/OpenMined/PyDentity/issues/60#issuecomment-780547754, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWCAVAEPE7HBYADPPWKHZ3S7O6L5ANCNFSM4XU2OH2Q .
Hmm yes, where to put it is an interesting one. Which I am pretty open to ideas on. Helpers could be one place.
I am imagining this being used like:
from aries_basic_controller.helpers import Presentation
Is presentation a good name for it? Not sure.
I think most important parsing is just around accessing the attributes right?
Also definitely not within the utils file. Should be it's own file. I see utils as internal utils for the controller.
@wip-abramson the basic implementation is done. needs more work like error checking etc and maybe renaming/moving around the code. check the bottom of https://github.com/OpenMined/PyDentity/blob/issue_60/tutorials/aries-basic-controller/notebooks/alice/Part%2010%20-%20Revocation.ipynb for an example.
Example code of how to use it as per the notebook:
from aries_basic_controller.aries_controller import Presentation_Parser
presentation = Presentation_Parser(verify)
print(presentation.get_revealed())
print('\n')
print(presentation.get_presentation_request())
print('\n')
print(presentation.get_verified_state())
print('\n')
print(presentation.get_self_attested())
print('\n')
print(presentation.get_identifiers())
print('\n')
print(presentation.get_predicates())
print('\n')
print(presentation.get_role())
print('\n')
print(presentation.get_threadid())
print('\n')
print(presentation.get_unrevealed_attrs())
print('\n')
print(presentation.is_verified())
print('\n')
print(presentation.get_presxid())
Nice looks good!
I think lets call it Presentation. That is what it is on reflection, the basic controllers representation of a presentation.
I think we should throw error's when trying to fetch attributes of an unverified presentation.
It might be good to have a function around revocation. Not sure.
I think it would be good to handle this in a separate notebook aswell, we should test more complex scenarios such as presentations of attributes from multiple credentials.
Also, get_identifiers() needs to be a bit more meaningful.
Might it be worth having functions like get_cred_def_ids(), get_schema(). Then on the back of that, is it worth having queries like get_attrs_from_schema(schema_id). and cred_def?
Not sure, there is a lot to unpack here. I think it is something that can evolve as we learn how we want to use it.
Agree. Can we make a list of things we know must change now so that I can take a stab at it?
get_identifiers
and extract into fetching contents of identifiers. i.e. get_cred_defs
, get_schema
etcAnything else?
Not sure about error on instantiation.
I think it should be when accessing the attributes. We might want to instantiate a unsuccessfully verified presentation to interrogate why it failed. Not sure if that information is given but it should be.
Maybe a .from() function to get the connection_id also
Hey @wip-abramson have a look at the latest commit for this issue. Let me know what is still outstanding. From the checklist, it is to create a notebook explaining the helper functions and then to test the parser with a more complex presentation request. Can someone maybe pick-up these two tasks? Another task is for you to go over the code and make sure it is acceptable. ;-)
BTW - I am not throwing an error on verification failure but rather just print out a message to say it is unverified and then display the shared attributes. This could be done better.
@vineeth14 can you maybe pick this up and complete the two outstanding items?
Hey @lohanspies, I'll have a crack at it
Description
A presentation of a proof once received is left to the application to parse, checking it's status and retrieving the presented attributes. Currently, this involves having knowledge of the exact structure of the verification object which adds complexity for an application developer.
We should again abstract this behind some helper class and friendly API.
verify = await agent_controller.proofs.verify_presentation(presentation_exchange_id)
I imagine some class taking the verify object here. E.g.
Are you interested in working on this improvement yourself?
Additional Context
The present proof part 11 shows this parsing happening
https://github.com/OpenMined/PyDentity/blob/master/tutorials/aries-basic-controller/notebooks/alice/Part%206%20-%20Present%20Proof.ipynb