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

Use EcdsaSecp256k1RecoveryMethod2020 v2 context #414

Merged
merged 2 commits into from Mar 31, 2022
Merged

Use EcdsaSecp256k1RecoveryMethod2020 v2 context #414

merged 2 commits into from Mar 31, 2022

Conversation

clehner
Copy link
Contributor

@clehner clehner commented Mar 29, 2022

Using the deprecate macro on the lazy_static pub static ref did not seem to work, so I wrote it as a docstring instead.

@clehner clehner marked this pull request as ready for review March 29, 2022 12:55
- Add the suite's v2 context.
- Deprecate the v1 and "extra" contexts.
- Only add context to proof if needed.
- Add fourth signature and verification method from upstream test vector
  VC and DID document.
- Signatures are as-is in the VC; the JSON-LD framing is changed to
  avoid loading the additional context.
- The DID document is changed to remove unneeded properties, and to use
  blockchainAccountId instead of ethereumAddress.
Copy link
Member

@sbihel sbihel left a comment

Choose a reason for hiding this comment

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

Not sure I fully understand the deprecation tags, is it to warn people who are building VCs with ssi/DIDKit? Wouldn't it also generate warning for any didkit build?

@clehner
Copy link
Contributor Author

clehner commented Mar 31, 2022

The deprecation tags are to alert anyone who uses the old context constants from Rust directly, and also to provide useful information in the rustdocs documentation. This shouldn't produce any warning in builds (as I added the ignores where the context is used internally) and it doesn't add any warning when processing JSON-LD documents using the old context.
Producing a runtime warning when the deprecated context is used might be useful though... It's not really bad if someone is using the old contexts, just that one was missing some fields and the other was only on Spruce's domain'; the new one is more following latest best practices and hopefully will be used more by other implementations.

@clehner clehner merged commit 693afbb into main Mar 31, 2022
@clehner clehner deleted the feat/esrs-v2 branch March 31, 2022 14:14
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.

None yet

2 participants