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 ASN.1 DER instead of MessagePack #111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fjarri
Copy link
Contributor

@fjarri fjarri commented Jan 15, 2023

This PR is supposed to replace MessagePack with ASN.1 DER as the preferred binary format.

There are several problems though:

  • serde_asn1_der is semi-abandoned and reports the format as human-readable, causing storage inefficiencies.
  • picky-asn1-der, a fork of serde_asn1_der, which is used in this PR, does not support Optional fields (or rather, it does not serialize None, so a subsequent deserialization fails - let's wait for the resolution on Support for Optional in ASN.1 Devolutions/picky-rs#201)
  • Also picky-asn1-der does not support mappings, which is not critical for this crate, but nucypher-core has those in its data structures.

Since the whole point of this change is to use a "standard" format, we should establish whether the lack of support for Optional and mappings is a limitation of the standard, or just a lack of a feature in picky-asn1-der. In the latter case we can make a PR to it, adding that support. Evidently, mappings do occur in formats that use ASN.1, e.g. VarBind here encoded as a sequence of 2-tuples.

@fjarri fjarri added the ABI Changes the format of serialized objects label Jan 15, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jan 15, 2023

Codecov Report

Merging #111 (aaeb755) into master (5080d6d) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master     #111   +/-   ##
=======================================
  Coverage   48.44%   48.44%           
=======================================
  Files          17       17           
  Lines         997      997           
=======================================
  Hits          483      483           
  Misses        514      514           
Impacted Files Coverage Δ
umbral-pre/src/capsule_frag.rs 79.68% <ø> (ø)
umbral-pre/src/key_frag.rs 84.94% <ø> (ø)
umbral-pre/src/serde_bytes.rs 41.86% <ø> (ø)
umbral-pre/src/traits.rs 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@KizzyCode
Copy link

Just as a side note: serde_asn1_der is not really abandoned – currently I'm working on a v1 release of the underlying asn1_der-crate (which however may take some time since I don't really want to soften it's no-panic approach and subsequent "guarantees").

Since serde_asn1_der serves as a simple interface to asn1_der, it cannot really progress independently (apart from bug-fixes obviously). However, I'm always open for feature-requests or PRs, especially if I missed a useful feature that can be implemented on top of the current asn1_der crate version 😊

Since DER's type system is not fully compatible to serde's type system, there is probably still a lot of room for improvements I have missed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ABI Changes the format of serialized objects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants