Skip to content
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

Readme Improvements #33

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

kvnn
Copy link

@kvnn kvnn commented Jan 10, 2023

No description provided.

@kvnn kvnn requested review from OR13 and brentzundel as code owners January 10, 2023 05:51
@kvnn
Copy link
Author

kvnn commented Jan 10, 2023

  1. Updates "Request Credential" Message Format to include formats
  2. For "Request Credential", replaces "requests~attach" with "attachments"
  3. Fixes various typos

@kvnn kvnn changed the title Update "Request Credential" Message Format to include formats Readme Improvements Jan 10, 2023
@@ -219,6 +219,12 @@ Message Format:
"goal_code": "<goal-code>",
"comment": "some comment"
},
"formats": [

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a valid DIDComm message?
As far as I know, format is not a message header https://identity.foundation/didcomm-messaging/spec/#message-headers

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this formats mapping is extraneous as the format is found on the actual attachment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if needed, it should be included inside the body I think.

@@ -283,7 +289,7 @@ Description of fields:
- `comment` -- an optional field that provides human readable information about the issued credential, so it can be evaluated by human judgment. Follows [DIDComm conventions for l10n](https://github.com/hyperledger/aries-rfcs/tree/main/features/0043-l10n/README.md).
- `attachments` -- an array of attachments containing the issued credentials.

If the issuer wants an acknowledgement that he issued credential was accepted, this message must be decorated with the `~please-ack` decorator using the `OUTCOME` acknowledgement request. Outcome in the context of this protocol means the acceptance of the credential in whole, i.e. the credential is verified and the contents of the credential are acknowledged. Note that this is different from the default behavior as described in [0317: Please ACK Decorator](https://github.com/hyperledger/aries-rfcs/tree/main/features/0317-please-ack/README.md). It is then best practice for the new Holder to respond with an explicit `ack` message as described in the please ack decorator RFC.
If the issuer wants an acknowledgement that the issued credential was accepted, this message must be decorated with the `~please-ack` decorator using the `OUTCOME` acknowledgement request. Outcome in the context of this protocol means the acceptance of the credential in whole, i.e. the credential is verified and the contents of the credential are acknowledged. Note that this is different from the default behavior as described in [0317: Please ACK Decorator](https://github.com/hyperledger/aries-rfcs/tree/main/features/0317-please-ack/README.md). It is then best practice for the new Holder to respond with an explicit `ack` message as described in the please ack decorator RFC.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ~please-ack header will need to be changed as there is no notion of decorators in didcommv2 but that shouldn't hold up this PR.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should conform to ACKs in DIDComm spec

@@ -219,6 +219,12 @@ Message Format:
"goal_code": "<goal-code>",
"comment": "some comment"
},
"formats": [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this formats mapping is extraneous as the format is found on the actual attachment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants