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

Regression(0.12.X): Revocation Notifications not getting emitted #2915

Closed
dbluhm opened this issue Apr 25, 2024 · 6 comments · Fixed by #2916
Closed

Regression(0.12.X): Revocation Notifications not getting emitted #2915

dbluhm opened this issue Apr 25, 2024 · 6 comments · Fixed by #2916

Comments

@dbluhm
Copy link
Contributor

dbluhm commented Apr 25, 2024

It looks like #2558 introduced a bug where revocation notifications are no longer getting emitted when using the /revocation/publish-revocations endpoint. It seems to be fine when revocation is immediately published on revoke but if the issuer desires to batch them together and publish all pending revocations at once, no notifications are emitted to the holder connection.

This seems to be an issue in both 0.12.0 and 0.12.1rc0. Not sure why we didn't catch this one sooner 🤔

@dbluhm
Copy link
Contributor Author

dbluhm commented Apr 25, 2024

Actually no, it wasn't #2558; it looks like it was #2812. @jamshale it looks like we might have deleted some notification emissions from aries_cloudagent/revocation/manager.py so they could be handled in Askar-AnonCreds but maybe didn't account for the plain old Askar wallet case?

@dbluhm
Copy link
Contributor Author

dbluhm commented Apr 25, 2024

@jamshale
Copy link
Contributor

I thought I removed this on purpose because I changed the flow to only notify after the revocation id's were written to the ledger instead of right when the endpoint was called. If this isn't happening, I must have messed something up. I can look. I thought I had covered this case.

dbluhm added a commit to dbluhm/aries-cloudagent-python that referenced this issue Apr 25, 2024
Fixes openwallet-foundation#2915.

Ensure revocation notifications are emitted when publishing batched
pending revocations.

Signed-off-by: Daniel Bluhm <dbluhm@pm.me>
@dbluhm
Copy link
Contributor Author

dbluhm commented Apr 25, 2024

@jamshale I think conditions of the deleted lines were such that it would only send that notification only if not using an endorser. It makes sense to only notify after the transactions are actually published in the endorser case but the deletion of these lines was preventing the self-endorsed revocations to ever trigger a notification, I think.

See #2916; the changes in that PR get things working for me again

@dbluhm
Copy link
Contributor Author

dbluhm commented Apr 25, 2024

In case it's useful, I used this minimal example to reproduce and diagnose the issue: https://github.com/Indicio-tech/acapy-minimal-example/tree/main/examples/presenting_revoked_credential

With the image of alice pointing to 0.12.1rc0. docker-compose run example to actually run it.

@jamshale
Copy link
Contributor

jamshale commented Apr 25, 2024

Ughh. I think that must have been the issue. I was trying to fix the problem where using an endorser didn't send revocation notifications. This was introduced and then fixed for 0.12.0. I must have overlooked the self-endorsed case and got trigger happy here.

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 a pull request may close this issue.

2 participants