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: patch anoncreds to fix partially deleted creds issue #1999

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

bryce-mcmath
Copy link
Contributor

After this patch, credentials issued in 0.4.2 and before will be successfully deleted in current build

Signed-off-by: Bryce McMath <bryce.j.mcmath@gmail.com>
@bryce-mcmath bryce-mcmath requested a review from jleach June 3, 2024 21:47
Copy link

sonarcloud bot commented Jun 3, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

+ relatedCredentialExchangeRecord.setTags({
+ anonCredsRevocationRegistryId: (0, indyIdentifiers_1.getQualifiedDidIndyDid)(legacyTags.revocationRegistryId, indyNamespace),
+ anonCredsUnqualifiedRevocationRegistryId: (0, indyIdentifiers_1.getUnQualifiedDidIndyDid)(legacyTags.revocationRegistryId),
+ anonCredsUnqualifiedRevocationRegistryId: (0, indyIdentifiers_1.getUnqualifiedRevocationRegistryDefinitionId)(namespaceIdentifier, schemaSeqNo, credentialDefinitionTag, revocationRegistryTag),
Copy link
Member

Choose a reason for hiding this comment

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

Should this fix be added to Credo or anoncrets-rs so that we can eventually not relay on the patch?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's partly in a PR and partly merged into Credo main after 0.5.3, so will likely make it into Credo 0.5.4 or 0.5.5. I didn't want to wait until then though

@jleach
Copy link
Member

jleach commented Jun 3, 2024

@bryce-mcmath In the description to you mean 0.4.2 or 0.5.2? What was special about 0.4.3 and after that fixes things? Also, does this fix need to go into anoncreds-rs?

@bryce-mcmath
Copy link
Contributor Author

@bryce-mcmath In the description to you mean 0.4.2 or 0.5.2? What was special about 0.4.3 and after that fixes things? Also, does this fix need to go into anoncreds-rs?

@jleach 0.4.2 was just the last version of AFJ we used before we upgraded to Credo 0.5.x. I guess more generally I should say, credentials issued in 0.4.x and before, can be successfully deleted after upgrading to 0.5.3 with this patch.

This fix is not needed in anoncreds-rs, just in Credo, but the fix for the mobile verifier issue may be required in anoncreds-rs and the wrappers. That's the last outstanding issue with the upgrade AFAIK and Ariel from the Credo team is looking into it. Wade may have already fixed it as well, we'll see after his vdr and anoncreds PRs get merged.

@bryce-mcmath bryce-mcmath requested a review from jleach June 3, 2024 22:07
@bryce-mcmath bryce-mcmath merged commit c58b7ca into main Jun 3, 2024
7 checks passed
@bryce-mcmath bryce-mcmath deleted the fix/anoncreds-patch branch June 3, 2024 22:15
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