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

Restore en-/decoding compatibility for GetCurrentPParams #95

Merged
merged 1 commit into from
May 23, 2023

Conversation

amesgen
Copy link
Member

@amesgen amesgen commented May 22, 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.

@amesgen amesgen changed the title Scaffolding for version-dependent en-/decoding of GetCurrentPParams Restore en-/decoding compatibility for GetCurrentPParams May 22, 2023
@amesgen amesgen force-pushed the amesgen/pparams-encoding-compat branch from f81b66d to 01c0ceb Compare May 23, 2023 14:35
@amesgen amesgen marked this pull request as ready for review May 23, 2023 15:15
@amesgen amesgen requested a review from a team as a code owner May 23, 2023 15:15
By introducing a newtype `LegacyPParams` we can create different
`ToCBOR`/`FromCBOR` instances that matchup with the old and buggy
serialization.

We can then switch the en-/decoders used based on the negotiated
protocol version in order to support older clients.

Co-authored-by: Alexey Kuleshevich <[email protected]>
@amesgen amesgen force-pushed the amesgen/pparams-encoding-compat branch from 01c0ceb to f8072e9 Compare May 23, 2023 16:00
@amesgen amesgen enabled auto-merge May 23, 2023 16:01
@amesgen amesgen added this pull request to the merge queue May 23, 2023
Merged via the queue into main with commit 79a2422 May 23, 2023
@amesgen amesgen deleted the amesgen/pparams-encoding-compat branch May 23, 2023 19:20
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.

2 participants