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

Aca-py not recording revoked status correctly #1823

Closed
ianco opened this issue Jun 18, 2022 · 4 comments · Fixed by #1827
Closed

Aca-py not recording revoked status correctly #1823

ianco opened this issue Jun 18, 2022 · 4 comments · Fixed by #1827
Assignees
Labels
bug Something isn't working

Comments

@ianco
Copy link
Contributor

ianco commented Jun 18, 2022

Aca-py no longer records the internal credential "revoked" state correctly.

Steps to reproduce:

  • run local von-network and tails server
  • start faber ./run_demo faber --revocation
  • start alice ./run_demo alice
  • accept invitation in alice agent
  • issue credential(s) from faber to alice
  • revoke a credential
  • check credential status using GET http://localhost:8021/api/doc#/revocation/<rev_reg_id>/issued/details

Expected - revoked credential will have revoked state
Actual - still in issued state

This breaks the new "revocation fix" endpoints

Also see integration tests @T002-TAA and @T003-TAA which fail against the latest release

e.g. run von-network as ./manage start --taa-sample --logs and then ./run_bdd -t @T002-TAA

@ianco ianco added bug Something isn't working 0.7.4 labels Jun 18, 2022
@swcurran
Copy link
Contributor

@ianco , some questions.

First, can you tell when this change was made? Was it part of 0.7.4?

Second, it should (at least in some/most cases) be the controller that tracks the revocations. I assume we’ll need too provide a way to support a fix for that model, right? Where we have to rationalize the controller set of revocations with the ledger set?

@andrewwhitehead - can you please look at this as a top priority?

@ianco
Copy link
Contributor Author

ianco commented Jun 18, 2022

First, can you tell when this change was made? Was it part of 0.7.4?

This changed in this PR: #1804 (I tested an early version of the PR and it worked ok so these was a change later on that broke this feature)

Second, it should (at least in some/most cases) be the controller that tracks the revocations. I assume we’ll need too provide a way to support a fix for that model, right? Where we have to rationalize the controller set of revocations with the ledger set?

Yes we should rationalize these calls. Right now the revocation status is tracked in both the indy-sdk wallet records as well as aca-py (as well the controller needs to track issue and revocation status vs its own "business" database). aca-py needs enough information to be able to verify the wallet/ledger consistency (unless we want to push this down to the controller as well).

@swcurran
Copy link
Contributor

I'm wondering if we should be tracking revocation status in the RevReg object in ACA-Py. If the credential object has been removed, we still need a way to batch update the RevReg, so I think this might be necessary. For example, could we have arrays pending_activation and pending_revocation that get added to when a credential is activated/revoked, and on successful publication, we move those to IDs to activated and revoked arrays, respectively.

@swcurran
Copy link
Contributor

@andrewwhitehead -- can you please take a look at #1804 again to see how it impacted this? Good to get RC4 or the final 0.7.4 out sooner than later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants