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

Fix IssuerCredRevRecord state update on revocation publish #1827

Merged

Conversation

andrewwhitehead
Copy link
Contributor

@andrewwhitehead andrewwhitehead commented Jun 20, 2022

Fixes #1823 broken in #1804

Ran against integration tests T002-TAA and T003-TAA which were previously failing.

@swcurran
Copy link
Contributor

@shaangill025 @ianco --- would really appreciate your review and assessment of this change. Working?

@andrewwhitehead
Copy link
Contributor Author

Looks like Github webhooks are currently in degraded status

@codecov-commenter
Copy link

Codecov Report

Merging #1827 (213d2fa) into main (4240fa9) will increase coverage by 0.01%.
The diff coverage is 73.07%.

@@            Coverage Diff             @@
##             main    #1827      +/-   ##
==========================================
+ Coverage   93.70%   93.71%   +0.01%     
==========================================
  Files         534      534              
  Lines       33871    33879       +8     
==========================================
+ Hits        31738    31750      +12     
+ Misses       2133     2129       -4     

@shaangill025

This comment was marked as resolved.

@ianco
Copy link
Contributor

ianco commented Jun 20, 2022

If you're running with a ledger that requires TAA acceptance you need to run faber with --accept-taa (or something like that)

@shaangill025
Copy link
Contributor

@T002-TAA and @T003-TAA are passing, but with @T003-TAA I get the following:

Aries      | Credential: state = done, cred_ex_id = ba2c035d-bd42-4854-8008-1803f8caf13a
    And "Faber" attempts to revoke the credential                                                                     # features/steps/0453-issue-credential.py:150
INFO:aiohttp.access:172.17.0.1 [21/Jun/2022:06:46:20 +0000] "POST /webhooks/topic/issuer_cred_rev/ HTTP/1.1" 200 149 "-" "Python/3.6 aiohttp/3.8.1"
INFO:aiohttp.access:172.17.0.1 [21/Jun/2022:06:46:20 +0000] "POST /webhooks/topic/issue_credential_v2_0/ HTTP/1.1" 200 149 "-" "Python/3.6 aiohttp/3.8.1"
Aries      | Credential: state = credential-revoked, cred_ex_id = ba2c035d-bd42-4854-8008-1803f8caf13a
Aries      | 2022-06-21 06:46:20,187 aries_cloudagent.admin.server ERROR Handler error with exception: Ledger rejected transaction request: client request invalid: InvalidClientTaaAcceptanceError('KHhfaCWgMjuqexPTNh56Hn', 1655793980145993232, 'Incorrect Txn Author Agreement(digest=e76a2894d2d7fb19cc269d4756401061b478fed5e80c4ed6c6ff7653aaf1c00d) in the request', 202).

@shaangill025
Copy link
Contributor

Tested with Alice-Faber demo, now after revocation and publication, the credential state is revoked as expected.

@andrewwhitehead
Copy link
Contributor Author

@T002-TAA and @T003-TAA are passing, but with @T003-TAA I get the following:

Aries      | Credential: state = done, cred_ex_id = ba2c035d-bd42-4854-8008-1803f8caf13a
    And "Faber" attempts to revoke the credential                                                                     # features/steps/0453-issue-credential.py:150
INFO:aiohttp.access:172.17.0.1 [21/Jun/2022:06:46:20 +0000] "POST /webhooks/topic/issuer_cred_rev/ HTTP/1.1" 200 149 "-" "Python/3.6 aiohttp/3.8.1"
INFO:aiohttp.access:172.17.0.1 [21/Jun/2022:06:46:20 +0000] "POST /webhooks/topic/issue_credential_v2_0/ HTTP/1.1" 200 149 "-" "Python/3.6 aiohttp/3.8.1"
Aries      | Credential: state = credential-revoked, cred_ex_id = ba2c035d-bd42-4854-8008-1803f8caf13a
Aries      | 2022-06-21 06:46:20,187 aries_cloudagent.admin.server ERROR Handler error with exception: Ledger rejected transaction request: client request invalid: InvalidClientTaaAcceptanceError('KHhfaCWgMjuqexPTNh56Hn', 1655793980145993232, 'Incorrect Txn Author Agreement(digest=e76a2894d2d7fb19cc269d4756401061b478fed5e80c4ed6c6ff7653aaf1c00d) in the request', 202).

I think this is part of the test. The ledger transaction is rejected, but it checks the revoked state of the credential anyway.

@ianco
Copy link
Contributor

ianco commented Jun 21, 2022

I think this is part of the test. The ledger transaction is rejected, but it checks the revoked state of the credential anyway.

That's correct, the test puts the wallet into a state where the ledger write will fail, to test the recovery method.

Copy link
Contributor

@ianco ianco left a comment

Choose a reason for hiding this comment

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

lgtm

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.

Aca-py not recording revoked status correctly
5 participants