-
Notifications
You must be signed in to change notification settings - Fork 24
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
Determine what to do with the prover_did property #107
Comments
IIUC, you are right that it does not need to be a DID in Ursa and that the format is actually not checked. I understand that the In terms of backwards compatibility, is there something ACA-Py does with this field other than validating the format? |
I just checked in the ACA-Py Alice-Faber demo and the field is populated by the holder's DID in its connection with the issuer -- which makes no sense at all. The DIDComm DIDs have nothing to do with the issuance of an AnonCreds verifiable credential. I suspect that when the code was developed, the ACA-Py developer did not know what AnonCreds meant to be put in the field, so they used the only DID the holder had in context. And since it didn't break anything, it was ignored. In searching ACA-Py for My recommendation is to try to do a bit of archeology into the indy-sdk AnonCreds (and perhaps anoncreds-rs) code and see if the purpose of the field can be determined. Perhaps it is meant as a unique identifier for the holder that the issuer can hold on to -- but it has lots of such values to use. My bet is that it is never used in AnonCreds. @TimoGlastra -- if I'm right that it is never used in AnonCreds, I suggest we flag it to be deprecated and plan on dropping it vs. renaming it. As you note, controller (business) code above the Aries Framework may be using it. |
Tried to do a little digging into the Indy project JIRA board and found this issue: IS-1224: The prover create_cred_request function requires a "prover_did" that (likely) does not make sense. I was hoping that it would have something useful for us... Sadly, I was the submitter of the report (in March 2019 no less) and the issue did not receive a response. There are a couple of links to some code, but that's it. Oh well. |
Do we have a path for what we want to do with the |
AFAIK, we don't have a solution on this. We are looking for a report of how the |
In libursa the Since the |
@whalelephant -- how is |
@swcurran to take a pass at updating the spec. with information about this item. |
To reiterate what was discussed (at least how I understood it):
Is that correct? cc @blu3beri |
After reviewing the code in anoncreds.rs and Ursa (specifically, this module), I think we should keep the The Holder is able to verify that the value of m2, by hashing the combined |
Discussed at the 2023.02.06 meeting -- update the PR to change the item to @blu3beri |
It is not required to use AnonCreds with dids. And as I understand it the prover_did doesn't actually have to be a did. It's used internally by ursa as a context string. So I think it just needs to be unique.
@whalelephant you know more about the exact working of this :)
My suggestion would be to rename it to something else that represents a unique identifier, but doesn't necessarily have to be a did. One downside of changing this is that's it is sent to the issuer, meaning it will break backwards compability.
We could also deprecate but allow it, and add a new property that doesn't have to be a did. this way we keep backwards compat. Or we keep the property but allow it to not be a did, but this will break e.g. ACA-Py that currently vaildates whether it is a did.
Thoughts?
The text was updated successfully, but these errors were encountered: