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: anoncreds revocation notification when revoking #3226

Conversation

thiagoromanos
Copy link
Contributor

@thiagoromanos thiagoromanos commented Sep 11, 2024

When revoking a credential using anoncreds, if you set publish: true and notify:true, it wasn't sending the notification to the holder, because the publish part was happening before the notification "enqueue" part. Changing the order of things fixes the problem.

Copy link

Copy link
Contributor

@jamshale jamshale left a comment

Choose a reason for hiding this comment

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

I'm not sure how this got messed up because it's in the correct order for non-anoncreds, and looks like it's always been this way.

A side note. This scenario should get a new scenario integration test. I'm going to create a ticket for it. This should be tested with automated testing.

Thanks for fixing it. Your fix makes sense.

@thiagoromanos
Copy link
Contributor Author

@jamshale , can you merge this? I don't have permission on this repo

@jamshale jamshale merged commit e12fc05 into openwallet-foundation:main Sep 11, 2024
11 checks passed
@thiagoromanos thiagoromanos deleted the fix/anoncreds-revocation-notification branch September 18, 2024 17:18
ff137 pushed a commit to ff137/acapy that referenced this pull request Oct 22, 2024
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.

2 participants