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

Update to the newest ledger #4349

Merged
merged 6 commits into from
Mar 3, 2023
Merged

Update to the newest ledger #4349

merged 6 commits into from
Mar 3, 2023

Conversation

lehins
Copy link
Contributor

@lehins lehins commented Feb 8, 2023

Description

Fixes invalid ExtLedgerState serialization. Produced CBOR encoding contained a list without a length, thus it was not decodable as arbitrary CBOR.

Integration of latest cardano-ledger and cardano-base.

  • Update iohk-nix sources to get latest libsodium needed for cardano-crypto-praos
  • Account for introduction of versioned CBOR in ledger. The new interface EncCBOR/DecCBOR is very similar to the plain ToCBOR/FromCBOR, with helper functions to go between them.
  • Remove usage of MemoBytes for Header, since Header cannot be parameterized on era and MemoBytes now requires it.
  • Account for addition of Shelley translation context: FromByronTranslationContext
  • Account for change of parametrization of ShelleyGenesis from era to crypto
  • Account for switch of PParams and other types from HasFields to lenses
  • Account for various fields getting renamed
  • Account for the fact that ConwayGenesis has to also include AlonzoGenesis fields, because otherwise CanStartFromGenesis can't work. We need to figure out the whole business of TranslationContext and Genesis, it is a bit messy right now IMHO.
  • Update golden tests, which were needed due to:
    • Fixes to PParams serialization
    • Changes of default protocol versions in emptyPParams in order eras
    • Fixes to test examples
    • Addition of a new field with deposits to the ledger state
    • Fix to ExtLedgerState serialization
    • Changes to Conway types
  • Remove some arbitrary instances (such as Slotting for example), since they are now imported from ledger. Notable instances are also UTCTime and NominalDiffTime, that now are imported from quickcheck-instances. In order to preserve older semantics standalone generators for those types were retained and used in delayNextSlot property test. Also guarded that against an "infinite" loop with within.

Blocked by IntersectMBO/cardano-ledger#3282 and CHaPS releases

Checklist

  • Branch
    • Commit sequence broadly makes sense
    • Commits have useful messages
    • The documentation has been properly updated
    • New tests are added if needed and existing tests are updated
    • Any changes affecting Consensus packages must have an entry in the appropriate changelog.d directory created using scriv. If in doubt, see the Consensus release process.
    • If this branch changes Network and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
  • Pull Request
    • Self-reviewed the diff
    • Useful pull request description at least containing the following information:
      • What does this PR change?
      • Why these changes were needed?
      • How does this affect downstream repositories and/or end-users?
      • Which ticket does this PR close (if any)? If it does, is it linked?
    • Reviewer requested

@lehins lehins force-pushed the lehins/update-ledger branch 2 times, most recently from 2bac21b to aa8ec69 Compare February 14, 2023 21:27
@lehins lehins changed the title Lehins/update ledger Update to the newest ledger Feb 15, 2023
cabal.project Outdated Show resolved Hide resolved
@lehins lehins force-pushed the lehins/update-ledger branch 2 times, most recently from dc85175 to 85d20b3 Compare February 16, 2023 19:26
@lehins lehins force-pushed the lehins/update-ledger branch from 9ad99e1 to db06e19 Compare February 16, 2023 20:29
@lehins lehins marked this pull request as ready for review February 16, 2023 20:30
@lehins lehins requested review from a team, nfrisby, dnadales, coot and bolt12 as code owners February 16, 2023 20:30
@lehins lehins force-pushed the lehins/update-ledger branch from db06e19 to f8247e7 Compare February 16, 2023 21:36
Copy link
Contributor

@disassembler disassembler left a comment

Choose a reason for hiding this comment

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

I've reviewed this and while long, i don't see anything controversial. It's mainly adopting to the new pparams syntax and the changes to enc/Dec serialization functions for the versioned cbor. While i agree not touching the odd protver in tools is smart, please do open a GH issue to track the oddity so the team can fix it at a later date or clarify why the numbers vary in older eras.

Copy link
Member

@dnadales dnadales 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 for the great PR description 🙌

LGTM. However, I can't answer @lehins 's questions.

@lehins lehins force-pushed the lehins/update-ledger branch 2 times, most recently from 4975d7a to 8981498 Compare February 21, 2023 15:46
@lehins lehins force-pushed the lehins/update-ledger branch 2 times, most recently from dad1400 to 3214721 Compare February 23, 2023 18:54
@andreabedini andreabedini mentioned this pull request Feb 24, 2023
@lehins lehins force-pushed the lehins/update-ledger branch from 240bbf0 to 31a34a3 Compare February 28, 2023 12:46
lehins added 4 commits March 2, 2023 16:56
* Update iohk-nix sources to get latest `libsodium` needed for `cardano-crypto-praos`
* Account for introduction of versioned CBOR in ledger. The new interface `EncCBOR`/`DecCBOR` is very similar to the plain `ToCBOR`/`FromCBOR`, with helper functions to go between them.
* Remove usage of `MemoBytes` for `Header`, since `Header` cannot be parameterized on `era` and `MemoBytes` now requires it.
* Account for addition of Shelley translation context: `FromByronTranslationContext`
* Account for change of parametrization of `ShelleyGenesis` from `era` to `crypto`
* Account for switch of `PParams` and other types from `HasFields` to lenses
* Account for various fields getting renamed
* Account for the fact that ConwayGenesis has to also include AlonzoGenesis fields, because otherwise `CanStartFromGenesis` can't work. We need to figure out the whole business of TranslationContext and Genesis, it is a bit messy right now IMHO.
* Update golden tests, which were needed due to:
  * Fixes to PParams serialization
  * Changes of default protocol versions in emptyPParams in order eras
  * Fixes to test examples
  * Addition of a new field with deposits to the ledger state
  * Fix to `ExtLedgerState` serialization
  * Changes to Conway types
* Remove some arbitrary instances (such as Slotting for example), since they are now imported from ledger. Notable instances are also `UTCTime` and `NominalDiffTime`, that now are imported from `quickcheck-instances`. In order to preserve older semantics standalone generators for those types were retained and used in `delayNextSlot` property test. Also guarded that against an "infinite" loop with `within`.
@lehins lehins force-pushed the lehins/update-ledger branch from dc3c027 to cb15887 Compare March 2, 2023 14:00
@lehins
Copy link
Contributor Author

lehins commented Mar 2, 2023

bors r+

iohk-bors bot added a commit that referenced this pull request Mar 2, 2023
4349: Update to the newest ledger r=lehins a=lehins

# Description

Fixes invalid `ExtLedgerState` serialization. Produced CBOR encoding contained a list without a length, thus it was not decodable as arbitrary CBOR.

Integration of latest `cardano-ledger` and `cardano-base`.

* Update iohk-nix sources to get latest `libsodium` needed for `cardano-crypto-praos`
* Account for introduction of versioned CBOR in ledger. The new interface `EncCBOR`/`DecCBOR` is very similar to the plain `ToCBOR`/`FromCBOR`, with helper functions to go between them.
* Remove usage of `MemoBytes` for `Header`, since `Header` cannot be parameterized on `era` and `MemoBytes` now requires it.
* Account for addition of Shelley translation context: `FromByronTranslationContext`
* Account for change of parametrization of `ShelleyGenesis` from `era` to `crypto`
* Account for switch of `PParams` and other types from `HasFields` to lenses
* Account for various fields getting renamed
* Account for the fact that ConwayGenesis has to also include AlonzoGenesis fields, because otherwise `CanStartFromGenesis` can't work. We need to figure out the whole business of TranslationContext and Genesis, it is a bit messy right now IMHO.
* Update golden tests, which were needed due to:
  * Fixes to PParams serialization
  * Changes of default protocol versions in emptyPParams in order eras
  * Fixes to test examples
  * Addition of a new field with deposits to the ledger state
  * Fix to `ExtLedgerState` serialization
  * Changes to Conway types
* Remove some arbitrary instances (such as Slotting for example), since they are now imported from ledger. Notable instances are also `UTCTime` and `NominalDiffTime`, that now are imported from `quickcheck-instances`. In order to preserve older semantics standalone generators for those types were retained and used in `delayNextSlot` property test. Also guarded that against an "infinite" loop with `within`.
 
Blocked by IntersectMBO/cardano-ledger#3282 and CHaPS releases



Co-authored-by: Alexey Kuleshevich <alexey.kuleshevich@iohk.io>
Co-authored-by: teodanciu <teodora.danciu@tweag.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 3, 2023

Timed out.

@lehins
Copy link
Contributor Author

lehins commented Mar 3, 2023

bors r+

@iohk-bors iohk-bors bot merged commit 17889be into master Mar 3, 2023
@iohk-bors iohk-bors bot deleted the lehins/update-ledger branch March 3, 2023 05:49
github-merge-queue bot pushed a commit to IntersectMBO/ouroboros-consensus that referenced this pull request May 23, 2023
IntersectMBO/ouroboros-network#4349
changed/fixed the encoding of `PParams`, which broke compatibility of
older clients with Node 8.0. This PR restores compatibility, by making
the en-/decoding version-dependent.

See the commit message for some implementation details. Also, note how
the golden files changed due to this PR:

 - Pre-Alonzo serialization did not change.
- Alonzo and Babbage changed, but only for `CardanoNodeToClientVersion
<= 10`; these are enabled by `NodeToClient <= 14`, which are the
currently released node-to-client versions.
- Note that no golden files changed for
`CardanoNodeToClientVersion{11,12}` (which are enabled by
`NodeToClientV_{15,16}`). `NodeToClientV_15` will be released in Node
8.1, and indeed, we want to use the new and fixed encoding when this
version is negotiated.
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