fix: Revocation Index 0 anoncreds-rs support #2126
Conversation
Signed-off-by: George Mulhearn <[email protected]>
|
Signed-off-by: George Mulhearn <[email protected]>
| @@ -320,7 +320,7 @@ export class AnonCredsRsIssuerService implements AnonCredsIssuerService { | |||
| } | |||
|
|
|||
| let revocationConfiguration: CredentialRevocationConfig | undefined | |||
| if (revocationRegistryDefinitionId && revocationStatusList && revocationRegistryIndex) { | |||
| if (revocationRegistryDefinitionId && revocationStatusList && revocationRegistryIndex !== undefined) { | |||
There was a problem hiding this comment.
I just remembered it starts at one by convetion, so it will still throw later on i think. But this at least fixes your issue.
We might want to add an error saying index 0 is not allowed?
There was a problem hiding this comment.
oh damn! i didn't know this. i guess atleast with this change it will make it's way down into anoncreds-rs, where it will then throw an error - rather than current behaviour where it silently becomes non-revokable.
FYI i just tried running this fix with an index 0 input. i get the error:
AnoncredsError: Invalid state: Revocation index is outside of valid range
(as you linked)
There was a problem hiding this comment.
okay then let's merge this, because it improves a lot
There was a problem hiding this comment.
I'm a bit late into the conversation but yes, this came originally from the fact that an index = 0 is not allowed.
I think it would be great if you can add an specific check for revocationRegistryIndex === 0 with an appropriate error message, since this is confusing for those who are not familiar with this convention (which BTW is only shyly mentioned in the spec: "Each credential issued using the Revocation Registry is given its own index (1 to the capacity of the Revocation Registry into the array, the index of the point for that credential.")
|
I'm getting the same int test failure on my other PR: #2124 . seems unrelated? |
YEs we need to fix the CI, if it's only the unrelated issue i can merge it! |
TimoGlastra
merged commit 6989c65
into
openwallet-foundation:main
Closes #2125