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

Anoncreds-RS update revocation endpoints for TAA #2493

Closed
usingtechnology opened this issue Sep 14, 2023 · 4 comments
Closed

Anoncreds-RS update revocation endpoints for TAA #2493

usingtechnology opened this issue Sep 14, 2023 · 4 comments
Assignees
Labels
AnonCreds Ledger Agnostic AnonCreds

Comments

@usingtechnology
Copy link
Contributor

See #2383 and #2432

There are some endpoints used in the TAA tests that need to be updated to use anoncreds-rs.

  • GET /revocation/registry/<id>/issued
  • GET /revocation/registry/<id>/issued/details
  • GET /revocation/registry/<id>/issued/indy_recs
  • PUT /revocation/registry/<id>/fix-revocation-entry-state

These are mentioned in Troubleshooting.md.

@usingtechnology
Copy link
Contributor Author

@dbluhm - can you comment on if we need to implement these or remove them. It appears they were brought in to address a specific need - is that ongoing?

@dbluhm
Copy link
Contributor

dbluhm commented Sep 14, 2023

  • GET /revocation/registry/<id>/issued
    • Yes, this would require updates; it is currently dependent on the IssuerRevRegRecord which has been replaced. The only thing it's using it for though is to make sure it's a known rev reg id that was passed in. AnonCredsRevocation.get_created_revocation_list would serve the same purpose. the IssuerCredRevRecords are still in use.
  • GET /revocation/registry/<id>/issued/details
    • Same story as pervious
  • GET /revocation/registry/<id>/issued/indy_recs
    • This one basically goes straight to performing a ledger call. It technically doesn't need to be changed but it may be better to translate it to using the AnonCreds interface to retrieve the rev list and then translate the rev list back into an Indy rev reg delta so that we can eliminate usage of the ledger outside of the Indy registry implementations. This feels like a low priority to me though.
  • PUT /revocation/registry/<id>/fix-revocation-entry-state
    • This one will need updates. It's also using the IssuerRevRegRecord and also using RevocationManager.update_rev_reg_revoked_state in the old style (it should accept a rev_reg_def_id, apply_ledger_update, genesis_transactions). The RevocationManager.update_rev_reg_revoked_state` is currently hardcoded to use the LegacyIndyRegistry to perform the ledger state correction. I alluded to this method in Refactor revocation recovery to be generic #2370 -- I'm still not certain whether this recovery mechanism is relevant outside of the context of indy or not. To sum up, the endpoint shouldn't be too bad to update but there's a separate question of whether there should be a generic way to do this for other AnonCreds methods besides Indy based ones.

@dbluhm
Copy link
Contributor

dbluhm commented Sep 14, 2023

Re: addressing a specific need, I haven't personally used these before but I know they were at least used recently in that IDIM issue I got tagged in 🙂 So I think they are "necessary" from the perspective that this out of sync issue can occur at least for Indy. But, as to whether they're necessary in the general case, I'm not sure. I'm inclined to just move the implementations into something specific to the Indy registry and expose them from that package (in plugin style) and let them continue to be specific to Indy until it's clear they have use for other methods.

@usingtechnology
Copy link
Contributor Author

Closing this work was done in #2458.

The 4 documented APIs are in place and updated to AnonCreds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AnonCreds Ledger Agnostic AnonCreds
Projects
None yet
Development

No branches or pull requests

2 participants