-
Notifications
You must be signed in to change notification settings - Fork 219
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
Clarity on nullability of credential_preview
in issue-credential-v2 offers (Aries RFC 0453)
#806
Comments
credential_preview
in issue-credential-v2 offers (Aries RFC 0453) credential_preview
in issue-credential-v2 offers (Aries RFC 0453)
@dbluhm — could you comment from the ACA-Py side? Should ACA-Py be changed to make it required/nullable? |
We've made it nullable as it doesn't make sense when issuing W3C credentials, and there's no disadvantage in making it optional AFAIK. |
Dumb PHB question time. Making it nullable mean that it is required but can be “[]”, correct? So we need to update ACA-Py to be the same — not optional, but default to “[]”? That gets us into alignment. And we can/should clarify the RFC accordingly. |
I believe the most correct answer is found in the second last paragraph of the OP's question.
I believe For reference, the attachment formats mentioned above are listed here. My reasoning: |
💯% agree with you @quinndevos-anonyome. The reason this wasn't done when e.g. the w3c attachment format was defined is that we didn't want to make a breaking change. Then v3 of the protocol was defined as part of WACI-PEX outside the Aries RFC ecosystem and so there wasn't really a chance to incorporate lessons learned in v3 of the protocol |
Credential Preview is not needed in AnonCreds and that is not why it is in the Issue Credentials protocol messages. It was added as a “general purpose” mechansim to allow an issuer and holder to negotiate about what data would be issued. As such, moving it to the AnonCreds (‘hlindy’) attachments does not make any sense. Its current place is where I think it makes sense — independent of any credential format, and optional, available for use if needed for negotiating an issuance. |
But as it has turned out, the general purpose structure caters very well to AnonCreds/Indy type of credentials (only one level, no complex value types) and doesn't transfer very well to any other credential format that I'm aware of. So saying it's a general purpose way to do things is misleading in my opinion, and I think it only adds confusion to label it as a general purpose way to do this. IMO previewing credential data is highly dependant on the actual credential format |
Got it — agreed. |
RFC: https://github.com/hyperledger/aries-rfcs/blob/main/features/0453-issue-credential-v2/README.md
For
credential_preview
in the issue-credential-v2 proposals, the following statement is made:Emphasis on optional.
For
credential_preview
in offers, the following statement is made:Emphasis on the lack of optional. Generally, every optional field is stated so in the Aries RFCs, and logically every other field is mandatory. Which leads me to believe that
credential_preview
in the issue-credential-v2 offer is indeed mandatory.However this does not match some of the practical implementations out there. The implementations state it as optional:
Note; I'm not really familiar with AFJ's approach to this, but i think AFJ will handle null/undefined credential_previews, but it prefers when it is defined. e.g. AFJ has this code when creating an offer:
In Aries-VCX we have currently implemented it as a non-nullable field (i was following the spec rather than implementation when making these Aries-VCX message structures), so we get deserialization issues when deserializing ACApy cred offers WITHOUT a cred preview.
My gut tells me that maybe the RFC should have the credential_preview as nullable. Since the credential_preview structure is not really applicable to some formats (e.g. for aries LDP / w3c it doesn't really make sense to have a credential_preview structure).
Also, coming from a perspective that credential_previews only really apply to hlindy/anoncreds credentials, I wonder in general if the credential_preview is better suited to be embedded in hlindy/anoncreds attachments rather than being a field on the base level of offers/proposals.
Anyway, i guess I'm seeking clarity on what the true type of credential_preview is in theory, and maybe some developer recommendations for what we should implement in practice. Cheers
The text was updated successfully, but these errors were encountered: