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.

Sign up for GitHub

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

Jump to bottom

Improve cardano-binary and account for migration to cardano-ledger-binary #361

Merged
merged 13 commits into from
Feb 15, 2023
Merged

Improve cardano-binary and account for migration to cardano-ledger-binary #361

merged 13 commits into from
Feb 15, 2023

Conversation

lehins
Copy link
Collaborator

@lehins lehins commented Jan 19, 2023 •
edited
Loading

cardano-ledger-binary now implements versioned serialization that is suitable for cardano-ledger and possibly some users of thereof. For that reason cardano-binary will be mostly used by tools and libraries outside of cardano-ledger and can be simplified a bit.

This PR also includes various minor improvements as well, which are needed for cardano-ledger-binary as well, since it reuses some parts of cardano-binary:

  • Generalized cborError to MonadFail
  • Add ToCBOR instance for Tokens -> Tokens
  • Add To/FromCBOR instances for Term and ToCBOR for Encoding
  • Add To/FromCBOR instances for 6-tuples and 8-tuples
  • Remove FromCBOR instance for Ratio in favor of Rational.
  • Add To/FromCBOR instances for Double.
  • Rename toCBORMaybe -> encodeMaybe with deprecation.
  • Rename decCBORMaybe -> decodeMaybe with deprecation.
  • Add encodeNullMaybe and decodeNullMaybe.
  • Add ToFromCBOR instances for Seq and StrictSeq
  • Deprecate serializeEncoding and serializeEncoding' in favor of serialize and
    serialize' respectfully, since Encoding now has the ToCBOR instance.
  • Add decodeFullDecoder' that accepts strict ByteString.

@lehins lehins force-pushed the lehins/binary branch 3 times, most recently from aff148d to e4c33c8 Compare February 1, 2023 10:27
@lehins lehins force-pushed the lehins/binary branch 2 times, most recently from 7271258 to ba2651a Compare February 14, 2023 23:32
@lehins lehins changed the title Account for migration to cardano-ledger-bianry Improve cardano-binary and account for migration to cardano-ledger-binary Feb 14, 2023
@lehins lehins marked this pull request as ready for review February 14, 2023 23:57
@lehins
Remove Annotated, Drop and Raw modules
40c4285 
These modules are used exclusively by ledger, therefore they have been moved
into `cardano-ledger-binary` and `cardano-crypto-wrapper`
teodanciu
teodanciu reviewed Feb 15, 2023
View reviewed changes
binary/CHANGELOG.md Outdated Show resolved Hide resolved
teodanciu
teodanciu approved these changes Feb 15, 2023
View reviewed changes
Copy link
Contributor

@teodanciu teodanciu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just some minor comments

JaredCorduan
JaredCorduan approved these changes Feb 15, 2023
View reviewed changes
Copy link
Contributor

@JaredCorduan JaredCorduan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great, thank you @lehins 🙌

@lehins lehins merged commit 9cc0123 into master Feb 15, 2023
@lehins lehins deleted the lehins/binary branch February 21, 2023 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JaredCorduan JaredCorduan JaredCorduan approved these changes

@teodanciu teodanciu teodanciu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lehins @JaredCorduan @teodanciu