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

Create Helper for Parsing a Presentation object #60

Open
wip-abramson opened this issue Feb 15, 2021 · 15 comments
Open

Create Helper for Parsing a Presentation object #60

wip-abramson opened this issue Feb 15, 2021 · 15 comments
Assignees
Labels
Type: Improvement 📈 Performance improvement not introducing a new feature or requiring a major refactor

Comments

@wip-abramson
Copy link
Member

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.

presentation = Presentation(verify)
presentation.get_self_attested()
presentation.get_revealed()
etc...

Defining this API should be the first task. Note this should also include understanding any errors in the presentation.

Are you interested in working on this improvement yourself?

  • No but happy to support

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

@wip-abramson wip-abramson added the Type: Improvement 📈 Performance improvement not introducing a new feature or requiring a major refactor label Feb 15, 2021
@lohanspies
Copy link
Member

@lohanspies
Copy link
Member

@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.

@lohanspies
Copy link
Member

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.

@frogman
Copy link

frogman commented Feb 17, 2021 via email

@wip-abramson
Copy link
Member Author

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?

@wip-abramson
Copy link
Member Author

Also definitely not within the utils file. Should be it's own file. I see utils as internal utils for the controller.

@lohanspies
Copy link
Member

lohanspies commented Feb 17, 2021

@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())

@wip-abramson
Copy link
Member Author

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.

@wip-abramson
Copy link
Member Author

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.

@lohanspies
Copy link
Member

lohanspies commented Feb 17, 2021

Agree. Can we make a list of things we know must change now so that I can take a stab at it?

  • Rename to Class to be Presentation
  • Throw error on instantiation if verification failed for the presentation
  • Create notebook to explain helper functions
  • Unpack get_identifiers and extract into fetching contents of identifiers. i.e. get_cred_defs, get_schema etc
  • Test with complex proof presentation (multiple credentials, predicates, restrictions etc)
  • .from() to return connection_id

Anything else?

@wip-abramson
Copy link
Member Author

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.

@wip-abramson
Copy link
Member Author

Maybe a .from() function to get the connection_id also

@lohanspies
Copy link
Member

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.

@lohanspies
Copy link
Member

@vineeth14 can you maybe pick this up and complete the two outstanding items?

@vineeth14
Copy link
Collaborator

Hey @lohanspies, I'll have a crack at it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement 📈 Performance improvement not introducing a new feature or requiring a major refactor
Projects
None yet
Development

No branches or pull requests

4 participants