-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat(rust/signed-doc): Add signing and verification logic to Catalyst Signed Docs #185
base: main
Are you sure you want to change the base?
Conversation
* verify signatures with public (verification) key * todo: verify UUIDs
a612907
to
70440b5
Compare
✅ Test Report | |
* verify doc type, id and ver
* verify extra fields
✅ Test Report | |
✅ Test Report | |
✅ Test Report | |
✅ Test Report | |
✅ Test Report | |
✅ Test Report | |
✅ Test Report | |
rust/signed_doc/src/builder.rs
Outdated
pub struct Builder { | ||
/// Document Metadata | ||
metadata: Option<Metadata>, | ||
pub(crate) metadata: Option<Metadata>, |
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.
Why we need these fields to be pub(crate)
. I think they should private even for this crate
rust/signed_doc/src/lib.rs
Outdated
/// Returns a signed document `Builder` pre-loaded with the current signed document's | ||
/// data. | ||
#[must_use] | ||
pub fn as_signed_doc_builder(&self) -> Builder { |
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.
pub fn as_signed_doc_builder(&self) -> Builder { | |
pub fn into_builder(self) -> Builder { |
rust/signed_doc/src/lib.rs
Outdated
#[must_use] | ||
pub fn as_signed_doc_builder(&self) -> Builder { | ||
Builder { | ||
metadata: Some(self.inner.metadata.clone()), |
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.
Seems it would be better to make all Builder
fields private and just call with_metadata
, with_signatures
etc.
rust/signed_doc/src/lib.rs
Outdated
where P: Fn(&KidUri) -> VerifyingKey { | ||
let error_report = ProblemReport::new("Catalyst Signed Document Verification"); | ||
|
||
if !self.doc_type().is_valid() { |
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.
We've updated a UuidV4
and UuidV7
decoding logic, so now we can assume that they always valid.
So such validation is not necessary at this place, because it is already done during decoding
✅ Test Report | |
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
✅ Test Report | |
✅ Test Report | |
b82ec2a
to
a2af6f1
Compare
✅ Test Report | |
Description
Thanks for contributing to the project!
Please fill out this template to help us review your changes.
Related Issue(s)
Closes #162
Description of Changes
Provide a clear and concise description of what the pull request changes.
Breaking Changes
Describe any breaking changes and the impact.
Screenshots
If applicable, add screenshots to help explain your changes.
Related Pull Requests
If applicable, list any related pull requests.
Please confirm the following checks