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

consolidate appendix examples #159

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

Conversation

toteto
Copy link

@toteto toteto commented Oct 26, 2023

This PR consolidates the examples stated in the Appendix section of the spec. Check comments on files for what has been wrong and what has changed in them.

Fixes #154

Copy link
Author

Choose a reason for hiding this comment

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

Note

Updated the JWT representations with their new values from their appropriate appendix-jwt.json

Copy link
Author

Choose a reason for hiding this comment

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

Note

The previous value was completely incorrect and didn't represent the JWT representation counterpart. Some changes include the correct placement and value of manifest_id and correct values for descriptor_map[x].id to have the same values as those in the manifest input_descriptors[x].id.
Also exported the VC payload out the JWT

Copy link
Author

Choose a reason for hiding this comment

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

Note

Used the credential-application/appendix-jwt.json to update the values so they more closely resemble the examples in the section, these include also the descriptor IDs.

Copy link
Author

@toteto toteto Oct 26, 2023

Choose a reason for hiding this comment

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

Note

As it can be seen, the previous value was for credential_application object. Updated the value from the JWT in the spec. No other changes to the JWT were made.
This change fixes #154

Copy link
Author

Choose a reason for hiding this comment

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

Note

Update the values of the descriptor_map[x].id to match those of the manifest's output_descriptors[x].id

Copy link
Author

Choose a reason for hiding this comment

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

Note

Just fixed the incorrect manifest_id

@toteto toteto marked this pull request as ready for review October 26, 2023 12:48
@bnonni
Copy link

bnonni commented Aug 29, 2024

ACKing in support

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.

The Embed Target Example for Credential Manifest is wrong
2 participants