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

Versioned CBOR #3063

Merged
merged 2 commits into from
Oct 10, 2022
Merged

Versioned CBOR #3063

merged 2 commits into from
Oct 10, 2022

Conversation

lehins
Copy link
Collaborator

@lehins lehins commented Oct 5, 2022

Started new package cardano-ledger-binary:

This package introduces new Encoding and Decoder wrappers that can change their behavior depending on a version, which corresponds to major protocol version. This package introduces new conceptws and consolidates all the old ones that were spread around different packages:

  • Introduced Version in its own module and make it safe to construct
  • Switch from ReaderT to manual Version passing to recover representational role
  • All decoders and encoders as well as FromCBOR/ToCBOR classes from cardano-binary package
  • Re-exports Annotator functionality from cardano-binary package, with plans to migrate it completely in the future
  • Moved dropper from cardano-binary
  • Test suites from cardano-binary and cardano-data as vintage test suites
  • FromSharedCBOR from cardano-data package
  • Data.Coders module from cardano-data package
  • Data.Twiddler module from cardano-data package into a test-lib
  • Improved Roundtripping tests and sophisticated way to display CBOR encoded blobs that was transferred from ouroboros-consensus repo.
  • Custom decoders/encoders from Cardano.Ledger.Serialization module from cardano-ledger-core package
  • Implement round trip test for all To/FromCBOR instances. Discovered major bug with SlicedByteArray and fix it in cborg package: Fix SlicedByteArray well-typed/cborg#301

@lehins lehins marked this pull request as ready for review October 5, 2022 22:28
@lehins lehins force-pushed the lehins/vcbor-reader branch 2 times, most recently from 958efc5 to 44908aa Compare October 6, 2022 16:22
Copy link
Contributor

@TimSheard TimSheard left a comment

Choose a reason for hiding this comment

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

Aside from Twiddle, all this looks familiar, and in good shape.
Just a few comments need to be added, and a few mispelling typos to be fixed.

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.

Thank you @lehins , this is extremely exciting.

I was given a video tour of the PR on friday, and today I looked over the next instances. Looks great to me.

Note that this PR resolves #3014

@lehins lehins force-pushed the lehins/vcbor-reader branch 2 times, most recently from d5aa3b2 to 6091cb2 Compare October 10, 2022 21:33
This package introduces new `Encoding` and `Decoder` wrappers that can
change their behavior depending on a version, which corresponds to major
protocol version. This package introduces new conceptws and consolidates
all the old ones that were spread around different packages:

* Introduced `Version` in its own module and make it safe to construct
* Switch from `ReaderT` to manual Version passing to recover
  representational role
* All decoders and encoders as well as `FromCBOR`/`ToCBOR` classes
  from `cardano-binary` package
* Re-exports `Annotator` functionality from `cardano-binary` package,
  with plans to migrate it completely in the future
* Moved dropper from `cardano-binary`
* Test suites from `cardano-binary` and `cardano-data` as vintage test
  suites
* `FromSharedCBOR` from `cardano-data` package
* `Data.Coders` module from `cardano-data` package
* `Data.Twiddler` module from `cardano-data` package into a `test-lib`
* Improved Roundtripping tests and sophisticated way to display CBOR
  encoded blobs that was transferred from `ouroboros-consensus` repo.
* Custom decoders/encoders from `Cardano.Ledger.Serialization` module
  from `cardano-ledger-core` package
* Implement round trip test for all To/FromCBOR instances. Discovered
  major bug with `SlicedByteArray` and fix it in `cborg` package:
  well-typed/cborg#301
@lehins lehins force-pushed the lehins/vcbor-reader branch from 79d945f to 59f7c13 Compare October 10, 2022 22:09
@lehins
Copy link
Collaborator Author

lehins commented Oct 10, 2022

Note that this PR resolves #3014

Yes, it does, except only the first half of it. This PR only implements the library that is capable of versioned CBOR. A subsequent PR will be submitted that actually applies this new interface throughout ledger.

@lehins lehins merged commit 6546efb into master Oct 10, 2022
@iohk-bors iohk-bors bot deleted the lehins/vcbor-reader branch October 10, 2022 23:44
@lehins lehins mentioned this pull request Nov 21, 2022
3 tasks
@lehins lehins mentioned this pull request Jan 18, 2023
6 tasks
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

Successfully merging this pull request may close these issues.

4 participants