-
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 deserializer for Catalyst Signed Documents #101
Conversation
✅ Test Report | |
1cd0243
to
96846f0
Compare
8e07f3e
to
b0aa90a
Compare
…t API * refactor code from mk_signed_doc example into src/lib.rs
…ocument * updates cat-signed-doc example to display deserialized cose sign documents.
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.
I see we are using coset
as a library here.
I am not asking that we change this as it would be too much work and we do not have the time, but the problem is, it relies on cborium which we decided earlier that we would not use.
As this crate now has a hard dependency on cborium, any and all decoding issues caused by that need to be 100% constrained to this crate.
Ultimately I would like us to not have any cborium dependency anywhere in our tree, which means we need an issue to stop using coset and either write our own cose encode/decode using minicbor or find another library. I don't think encoding/deciding cose is very challenging, especially as we only have a requirement for COSE_SIGN and none of the other COSE formats.
In future if any crate brings in a CBOR dependency other than minicbor we need to have a discussion before brining it in, as it creates a maintenance issue having multiple ways to encode/decode the same data to/from ultimately the exact same binary stream.
However this does mean that no types we have defined elsewhere can have either a cborium or coset dependency. That needs to be 100% constrained to this crate.
Also note for clarity, we should NOT add Type Encoding here to cbor for any type, we should be encdoing/decoding to/from vec and using minicbor for that, and then just using the appropriate higher level type.
I do not want to see multiple ways to encode/decode any type to the same binary formats because of differing dependent library choices made by upstream crates.
* wip: fix examples
98bb6d7
to
cd817a4
Compare
✅ 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 | |
Description
Thanks for contributing to the project!
Please fill out this template to help us review your changes.
Related Issue(s)
Closes input-output-hk/catalyst-voices#1364
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