feat(rust/signed-doc): Catalyst signed document encoding using minicbor #353
Conversation
Mr-Leshiy
changed the title
| coset::ContentType::Assigned(CoapContentFormat::Cbor) => ContentType::Cbor, | ||
| _ => { | ||
| match value { | ||
| coset::ContentType::Assigned(CoapContentFormat::Json) => Ok(ContentType::Json), |
There was a problem hiding this comment.
The aim is to remove coset, is it not?
There was a problem hiding this comment.
Inside this PR, I've just removing coset for encoding functionality. Decoding would be done under the different PR
| @@ -12,10 +12,11 @@ pub(crate) mod utils; | |||
| use catalyst_types::{problem_report::ProblemReport, uuid::UuidV7}; | |||
| pub use content_encoding::ContentEncoding; | |||
| pub use content_type::ContentType; | |||
| use coset::{cbor::Value, iana::CoapContentFormat, CborSerializable}; | |||
| use coset::CborSerializable; | |||
There was a problem hiding this comment.
It will be eliminated under the next PR
| fn encode<W: minicbor::encode::Write>( | ||
| &self, e: &mut minicbor::Encoder<W>, _ctx: &mut (), | ||
| ) -> Result<(), minicbor::encode::Error<W::Error>> { | ||
| e.str(self.to_string().as_str())?; |
There was a problem hiding this comment.
Can we figure out what's up with the mismatch between Section spec and its impl?
Catalyst spec – RFC6901 Standard JSON Pointer.
Impl in this file – RFC9535 JSONPath.
I've brought up this issue before, but it hasn't been resolved. Either the spec or the impl should change.
There was a problem hiding this comment.
create pls a separate ticket under this feature ticket, #330.
Under this PR, not going to change it, just keep the original behaviour that we have
| | SupportedField::Parameters(document_ref) => document_ref.encode(e, ctx), | ||
| SupportedField::Type(doc_type) => doc_type.encode(e, ctx), | ||
| SupportedField::Collabs(collabs) => { | ||
| if !collabs.is_empty() { |
There was a problem hiding this comment.
Can the cddl spec be changed from
collaborators = [ * catalyst_id_kid ]
to
collaborators = [ + catalyst_id_kid ]
Because it's not deterministic encoding if this field is both optional and can be an empty array. This impl follows the optional + rule.
| <minicbor::bytes::ByteVec>::from(minicbor::to_vec(metadata)?), | ||
| <minicbor::bytes::ByteVec>::from(protected_header_bytes(kid)?), | ||
| minicbor::bytes::ByteArray::from([]), | ||
| <minicbor::bytes::ByteVec>::from(content.encoded_bytes(metadata.content_encoding())?), |
There was a problem hiding this comment.
Neither Catalyst spec nor RFC 8152 Section 2 explicitly says whether an empty payload should nil or empty bstr. RFC says – nil if the signature is for a detached content.
From what I understand, it wouldn't make sense for Catalyst Singed Document to have a detached content signature.
So could the spec be changed to forbid nil? This impl follows that rule.
Co-authored-by: Artur Helmanau <[email protected]>
This reverts commit 5b209bb.
|
✅ Test Report | |
Description
Partially migrating
signed-doccomponents encoding fromcosettominicbor.As the next step, COSE helpers from #351 are intended to be salvaged too.
Related Issue(s)
Closes #332
Description of Changes
ContentType,ContentEncoding,DocumentRef,Sectionand othersigned-doccomponents tominicbor.cosettraits now wrap around theminiciborimplementation as opposed to the other way around it's been before.serdeimplementations, whichcosetdepends on throughciborium, now also made to wrap theminicborimplementation where possible.Breaking Changes
None.
Related Pull Requests
Please confirm the following checks