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

Endorser protocol configuration, automation and demo integration #1422

Merged
merged 15 commits into from
Oct 1, 2021

Conversation

ianco
Copy link
Contributor

@ianco ianco commented Sep 23, 2021

See issue #1238

This PR adds automation support for requesting, endorsing and writing transactions, and for executing the workflow of creating a cred def that supports revocation.

The next stage of work will re-factor the code to use the event bus and eliminate the code duplication (see https://hackmd.io/51flx8CVS1ypyOjEqVl1tg?view#Implementation-Details)

@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2021

Codecov Report

Merging #1422 (d0cb97a) into main (9bea1d2) will decrease coverage by 0.73%.
The diff coverage is 28.61%.

@@            Coverage Diff             @@
##             main    #1422      +/-   ##
==========================================
- Coverage   95.30%   94.57%   -0.74%     
==========================================
  Files         483      483              
  Lines       29236    29518     +282     
==========================================
+ Hits        27864    27916      +52     
- Misses       1372     1602     +230     

@ianco ianco marked this pull request as ready for review September 30, 2021 22:29
@ianco ianco requested a review from swcurran September 30, 2021 23:03
@swcurran
Copy link
Contributor

swcurran commented Oct 1, 2021

I didn't find anything in testing this using alice/faber, but I couldn't figure out how to activate it in the demo. Do you have instructions?

We also need some tweaks to the run_demo help handling. I'm going to put in some suggestions in a separate issue.

@ianco
Copy link
Contributor Author

ianco commented Oct 1, 2021

I didn't find anything in testing this using alice/faber, but I couldn't figure out how to activate it in the demo. Do you have instructions?

The demo is in an "interim" state right now, endorser support is added but not fully added. The following is how it works now (sets up the endorser roles to allow manual testing using the swagger page) but not ultimately how I see the demo demonstrating endorsement ...

Run the following in 2 separate shells (make sure you are running von-network and the tails server first):

./run_demo faber --endorser-role endorser --revocation
./run_demo alice --endorser-role author --revocation

Copy the invitation from faber to alice to complete the connection.

Then in the alice shell, select option "D" and copy faber's DID to alice (it is the DID displayed on faber agent startup).

This starts up the aca-py agents with the endorser role set (via the new command-line args) and sets up the connection between the 2 agents with appropriate configuration.

Then, in the alice swagger page (http://localhost:8031) you can create a schema and cred def, and all the endorser steps will happen automatically. You don't need to specify a connection id or explicitly request endorsement (aca-py does it all automatically based on the startup args).

@swcurran
Copy link
Contributor

swcurran commented Oct 1, 2021

Nice -- that worked.

I'm thinking we need to have a "von-network"-like Endorser so that we can start it when needed for testing. Perhaps it should be built into von-network? We should brainstorm a bit how to do that. We also need to have one that we can put into production, ahead of a BPA-based Endorser.

I think an "Endorser.md" file is needed for this to document how to use it both in real usage, and a section on how to use it in the demo. Seems like a fun thing to do on a Friday, no? :-). I'm thinking that should be done before merging.

I'm hoping you can demo this on Tuesday morning at the ACA-Pug meeting. Doable?

Signed-off-by: Ian Costanzo <[email protected]>
@ianco
Copy link
Contributor Author

ianco commented Oct 1, 2021

I think an "Endorser.md" file is needed for this to document how to use it both in real usage, and a section on how to use it in the demo. Seems like a fun thing to do on a Friday, no? :-). I'm thinking that should be done before merging.

I added some docs both for ACA-Py as well as the demo. These docs will need to be updated as the Endorser code is updated & re-factored.

@@ -0,0 +1,61 @@
# Transaction Endorser Support

Note that the ACA-Py transaciton support is in the process of code refactor and cleanup. The following documents the current state, but is subject to change.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you anticipate the API (e.g. Admin API and Startup Parameters) changing, or are those more or less set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't anticipate any API changes but can't guarantee we won't need any

Copy link
Contributor

Choose a reason for hiding this comment

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

Typo "transaction"


Note that the ACA-Py transaciton support is in the process of code refactor and cleanup. The following documents the current state, but is subject to change.

ACA-Py supports an [Endorser Protocol](https://github.com/hyperledger/aries-rfcs/pull/586), that allows an un-privieged agent (an "Author") to request another agent (the "Endorser") to sign their transactions so they can write these transactions to the ledger.
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo "un-privileged"

Copy link
Contributor

Choose a reason for hiding this comment

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

Should have a reference to this being an Indy feature/requirement.


Since endorsement involves message exchange between two agents, these agents must establish and configure a connection before any endorsements can be provided or requested.

Once the connection is established and `active`, the "role" (either Author or Endorser) is attached to the connection using the `/transactions/{conn_id}/set-endorser-role` endpoint. For Authors, they must additionally configure the DID of the Endorser as this is required when the Author signs the transaction (prior to sending to the Endorser for endorsement) - this is done using the `/transactions/{conn_id}/set-endorser-info` endpoint.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be possible to have the DID specified on the command line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree this is a command line parameter but not currently used (the auto-connection-setup support is still TBD)

Copy link
Contributor

@swcurran swcurran left a comment

Choose a reason for hiding this comment

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

Approved -- but you might want to clean up the typos first. Or just merge.

@ianco
Copy link
Contributor Author

ianco commented Oct 1, 2021

Approved -- but you might want to clean up the typos first. Or just merge.

I'll include the doc updates/fixes in the next PR

@ianco ianco merged commit 33389d7 into openwallet-foundation:main Oct 1, 2021
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.

3 participants