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

Possibly confusing binary formats #263

Open
rooooooooob opened this issue Nov 19, 2021 · 1 comment
Open

Possibly confusing binary formats #263

rooooooooob opened this issue Nov 19, 2021 · 1 comment

Comments

@rooooooooob
Copy link
Contributor

For most types we have the to_bytes() and from_bytes() formats using the direct CBOR format in the alonzo.cddl binary spec.

This causes some confusion for various types that are essentially defined as just bytes (possibly with size restrictions). These types are mostly crypto related and consist of all the hash (Ed25519KeyHash, TransactionHash etc) and key (PublicKey, Bip32PublicKey, etc) types as well as Address. I believe these are the only ones but there could be more.

These types already had well-defined binary formats, whereas others simply had those to_bytes()/from_bytes() blanket-implemented off the CBOR format and the concern was just being self-consistent primarily, not with having every single to_bytes()/from_bytes() all be valid CBOR or not.

It wasn't as big of an issue worth breaking the API over before but now with increasing use-cases directly building off of the CDDL format it has already become a source of trouble and confusion. One example is how the CIP-30 dApp connector defines most types as hex-encoded CBOR based off of alonzo.cddl for types used directly in the API. This poses an issue with Address in particular where according to the spec, for consistency against alonzo.cddl (since we don't want to tie any CIP to a specific library) you would not be following the spec if you use our Address::to_bytes()/Address::from_bytes().

While this could be a local fix for just Address, I'm sure this will come up again somewhere else and it just is a source of slight confusion waiting to catch random unsuspecting users.

My first thoughts are to do the following three things:

  1. keep to_bytes()/from_bytes() as is everywhere and document that this is simply a self-consistent (e.g. A::from_bytes(a.to_bytes()) should run correctly and return essentially a copy of a) format and may or may not be CBOR bytes. These could also possibly be marked as deprecated.
  2. add in to_raw_bytes() and from_raw_bytes() to those binary-wrapper types above which would be identical to their current to_bytes()/from_bytes()
  3. add in to_cbor_bytes()and from_cbor_bytes() to all types and have them just call to_bytes()/from_bytes() for most types, but have them include the CBOR binary wrapper around those special hash/key/address types above.

but we should figure out the best approach first since there could be better approaches. We must balance both backwards compatability with consistency.

@rooooooooob
Copy link
Contributor Author

At the very least we'll want to better document this quirk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant