-
Notifications
You must be signed in to change notification settings - Fork 135
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
Remove II's direct dependency on identity.rs library for handling VCs #2378
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks. Two minor comments.
src/vc_util/src/lib.rs
Outdated
Ok(encoder.into_jws(sig)) | ||
} | ||
|
||
/// Extracts the canister signature public key from the given signing_input. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe expand the comment a bit on what signing input
is? I.e. a JWT without signature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
src/vc_util/src/lib.rs
Outdated
Ok(encoder.into_jws(sig)) | ||
} | ||
|
||
/// Extracts the canister signature public key from the given signing_input. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Until now II's code was depending on a fork of
identity.rs
in two ways: directly, to create id-alias VCs, and indirectly, via the dependency onvc_util
-crate. This PR removes the first (direct) dependency, by extendingvc_util
, and changing the format of the id-alias VCs, to follow the recommended convention for general VCs. This completes the removal of direct dependency onidentity.rs
for II and VC-issuers -- now the dependency is encapsulated invc_util
-crate.NOTE: a side-effect of this PR is that once it is deployed in production, II will start issuing id-alias VCs in the new format, so any dapp that verifies these VCs should be updated as well. If a dapp is using
vc_util
to verify these VCs, it should be re-compiled with the updated version ofvc_util
.🟡 Some screens were changed