-
Notifications
You must be signed in to change notification settings - Fork 55
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
feat: added support for indexing by legacy identifiers #335
feat: added support for indexing by legacy identifiers #335
Conversation
Signed-off-by: wadeking98 <[email protected]>
Signed-off-by: wadeking98 <[email protected]>
I’m slightly nervous about this because the unique identifier portion of an Indy So in theory — this fix should not work/be needed. That said, I think what is happening is that somewhere, someone’s code is just doing an inapporpriate (but understandable) text transformation for DID Indy, and so there is no real harm in doing this. The different identifier is generated at DID creation time, and once it is created, it can be put into a DID of either form and still be found. So — all good from my perspective. Others? |
If you don't think this should go in the main anoncreds package, I can just make a specific npm package for these changes that only bifold will use as a patch. I was planning on this only being a temporary fix anyway while projects transition to did:indy. |
I’m happy for it to go in — I think it makes good sense, a clever way to make this transition much easier. My thoughts on the topic changed as I wrote my previous comment — as reflected in hesitation at the start, go for it by the end :-). I would like @TimoGlastra @berendsliedrecht @andrewwhitehead to look at the code though, and weigh in on it. |
The identifier is determend based on the nym transaction right? So i think we can do the transformation two ways in this case as a did won't change:
|
Curious how this solves your issue in Bifold @wadeking98, as you still need to make the query logic try to fetch both identifiers new/legacy Also i'm not sure what will happen if i put a cred with qualified identifiers in a presentation to a wallet that only supports unqualified and vice versa. It would seem to me there's more logic needed for this |
Hi Timo, I think I need a bit more clarification on your proposed transformation method. I don't know if we can properly convert from legacy identifiers to the new did:indy ones because I don't know which ledger a legacy identifier is coming from. I think I'd have to import some functionality from indy-vdr in order to figure that out which seems likely to become messy. This solves my issue in bifold because after the credo update some objects were being saved using the new did:indy identifiers and then fetched using the legacy identifiers. This changes makes it so that legacy and did:indy identifiers can be used interchangeably while we're transitioning away from legacy to did:indy identifiers across the ecosystem. |
Wasn't this type of transformation already supported in AFJ 0.4.0? At that time, we were limited to use did:indy only for object creation, but not when it comes to storage/resolution. What has changed in Credo 0.5.0/anoncreds-rs to prevent that? Probably a good topic to discuss in tomorrow's Credo Contributors meeting! |
Is this PR still needed? Any progress on root cause discussions? |
I think the credo changes should cover it, I'll just do some testing once those credo changes are available and then I'll close this if it all works out |
Still running into the same issue after upgrading to 0.5.3 going to do some more testing to see if I can solve it in credo |
@genaris @TimoGlastra cc: @andrewwhitehead I'm not quite sure what changes you would like on this PR before we consider merging. Could you please clarify? |
Currently different parts of the verifiable credential ecosystem are being migrated from did:sov spec to did:indy. This PR adds support to index did:sov and did:indy by a common format.
ie this pr makes it so that identifiers such as these are equal:
did:indy:sovrin:F72i3Y3Q4i466efjYJYCHM/anoncreds/v0/SCHEMA/npdb/4.3.4
andF72i3Y3Q4i466efjYJYCHM:2:npdb:4.3.4
I've tested these changes in bifold and they fix the issue with the mobile verifier and accepting proofs