Add /rotate to revocation api. #2523
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added in handle full registry but there are some issues to discuss there... Signed-off-by: Jason Sherman <[email protected]>
|
Kudos, SonarCloud Quality Gate passed!
|
|
I am adding more context for the issues I hit dealing with rev list indexes/cred revocation id First, this is when I set the starting This is the holder side exception: |
|
Quick ack from me; I'll take a closer look when I can! |
|
And if I do not add 1 to the Most pertinent part of the error: Full trace: |
|
So it is an off-by-1 error? The RevId we’re getting back is 0 based and the update is 1 based? That seems very weird... |
|
Every rev list will be missing 1 slot; not a huge deal on larger sets. I am not quite sure where the logic needs to be updated; it's possible it needs to be changed in the It may be possible that it is a timing issue for the additional steps (upload tails file and register the revocation list), although I tried adding plenty of time (5 seconds) to see if that allowed those tasks to complete. |
|
Speaking from memory, I think I once asked @andrewwhitehead why the first index is unused. I remember there being a good reason but I don't recall the details unfortunately. For a time I thought that the generic interface would have to be 0-based and then we'd have to map it to Indy where it was strictly expected to be 1-based. I had logic that performed this mapping and then, while testing, I found it was actually causing issues. I believe the anoncreds library is expecting indexes to be 1-based for most operations but the full revocation status list is of course just a list making it subject to 0-based indexing for us in python. |
|
There's no technical reason why the counting has to start at 1, but that's the convention right now. The entries in the tails file are multiplied by powers of the secret |
|
I’m not clear what “it" is that is starting skipping 1. Could someone clarify? |
|
Near as I can tell, if I set everything to be zero-based, so pass 0 to the anoncreds library to Credential.create, the credential is created successfully but the HOLDER cannot store the credential. So something... the wallet? the spec? does not like zero-based, so the first credential is created using |
|
I added a couple changes here to verify the revocation index when a credential is created, as zero shouldn't be accepted: hyperledger/anoncreds-clsignatures-rs#24 In indy-credx (using deltas), the indexes are always 1-based. In anoncreds-rs, using status lists, I'm not sure at the moment if the first index is skipped or if it's interpreted as starting at 1. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added in
handle_full_registrybut seems to be some issues with how the counts work.Cannot set the
next_indexto start at 0 because the credential data is invalid.And because we don't track the index and current revocation id externally, we can't trigger the
handle_full_registryuntil after a credential is created. So for every list/index we lose 1 slot. The only way I could successfully have registries rollover AND create the credentials successfully was triggering 1 slot before the max. cred. num.