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

Update GrottoNetworking.json for additional endpoints #51

Merged
merged 5 commits into from
Aug 12, 2024

Conversation

Wind4Greg
Copy link
Contributor

Added ecdsa-jcs-2019 and eddsa-jcs-2022 endpoints.

Added ecdsa-jcs-2019 and eddsa-jcs-2022 endpoints.
@Wind4Greg Wind4Greg requested a review from aljones15 July 25, 2024 18:40
@PatStLouis
Copy link
Contributor

I see the endpoints use a localhost address, shouldn't this use a publicly available endpoint?

Replace localhost with correct URL.
Remove extra comma.
Added  additional endpoints  for Grotto Networking.
Copy link
Collaborator

@aljones15 aljones15 left a comment

Choose a reason for hiding this comment

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

it would help if you specified a couple more endpoint related settings

per endpoint

"supports": {
  "vc": ["2.0"]
}

that means your endpoint will only be run with the VC 2.0 test fixtures.

Two, you did not specify the supportedEcdsaKeyTypes as mentioned in the README: https://github.com/w3c/vc-di-ecdsa-test-suite?tab=readme-ov-file#implementation

"supportedEcdsaKeyTypes": ["P-256"],

the suite does not know the curve/keyType that your issuer or verifier supports.
A verifier can support multiple "supportedEcdsaKeyTypes", but an issuer should only support one, so please one issuer endpoint per curve/keyType.

Otherwise THANKS really appreciate the support here.

Added    ` "supports": {"vc": ["1.1", "2.0"]}` to issuer and  verifier entries.
@Wind4Greg
Copy link
Contributor Author

Hi @aljones15 I added "supports": {"vc": ["1.1", "2.0"]} to all issuer and verifier entries. It looks like all my ECDSA related entries have "supportedEcdsaKeyTypes".

Copy link
Collaborator

@aljones15 aljones15 left a comment

Choose a reason for hiding this comment

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

Sorry about the hold up and feel free to merge.

@BigBlueHat BigBlueHat merged commit ca50910 into main Aug 12, 2024
2 checks passed
@BigBlueHat BigBlueHat deleted the Wind4Greg-patch-1 branch August 12, 2024 17:08
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.

4 participants