Deprecate serialiseTxLedgerCddl

[palas]

Changelog

- description: |
    Added deprecation warning to function `serialiseTxLedgerCddl`
  type:
   - breaking

Context

In the past, we had an intermediate tx body format (sometimes called the intermediate cli format). This format did not adhere to the CDDL format in the ledger. So serialiseTxLedgerCddl was introduced to remedy this, with the aim of dropping the "intermediate cli format".

We have completely dropped the "intermediate cli format" so both functions will serialize txs that adhere to the ledger's CDDL format.

Therefore serialiseTxLedgerCddl is no longer necessary.

This PR deprecates it and reimplements serialiseTxLedgerCddl to use serialiseToTextEnvelope, which causes slight changes to the formats, see How to trust this PR below for more detail.

cardano-api

• Main PR: Deprecate serialiseTxLedgerCddl #534
• Branch: cardano-api/deprecate-serialiseTxLedgerCddl
• Backported branch: cardano-api/deprecate-serialiseTxLedgerCddl-testing

cardano-cli

• PR: Deprecate serialiseTxLedgerCddl cardano-cli#772
• In the end, done in PR: Update hackage, CHaP; and cardano-api to 8.47.0.0 cardano-cli#786
• Branch: cardano-cli/deprecate-serialiseTxLedgerCddl-testing

cardano-node

• PR: Deprecate serialiseTxLedgerCddl cardano-node#5867
• In the end, done in PR: Updated dependencies for the Node 8.12 cardano-node#5879
• Branch: cardano-node/deprecate-serialiseTxLedgerCddl-testing

How to trust this PR

Make sure that the message makes sense, that the functions deprecated are the one we want to deprecate, and that there are no other functions associated that we want to deprecate. Also that we want to override the deprecation in the tests (which makes sense to me, but maybe we want to remove or adapt the tests instead).

Also make sure the amount of backwards compatibility adaptors is adequate and in the right places.

Migrating to using serialiseToTextEnvelope changes the serialization format in three ways:

• description: is provided by current functions but is a parameter to serialiseToTextEnvelope:
  • This is not a problem for deserialiseFromTextEnvelope because it ignores the field, so we get compatibility for free.
  • We pass the old descriptions to serialiseToTextEnvelope to ensure old functions keep producing the same description just to avoid breaking golden tests at this point.
• type: is deduced from the HasTextEnvelope instance of the serialised type, but serialiseTxLedgerCddl has hardcoded types which do not match HasTextEnvelop instances:
  • This PR modified deserialiseToTextEnvelope to loosely compare types (see legacyComparison function) allowing for support for the serialiseTxLedgerCddl versions.
  • We keep the old types in serialiseTxLedgerCddl by wrapping serialiseToTextEnvelope and patch the output to produce the old types too. This is, again, just to avoid breaking golden tests at this point.
• rawCBOR: is serialised pretty much identically by serialiseToTextEnvelope with one exception:
  • serialiseWitnessLedgerCddl encodes KeyWitness slightly differently than serialiseToTextEnvelope. In fact, deserialiseWitnessLedgerCddl relies on description to decide on the constructor to deserialise to of KeyWitness. This is a breaking change in the format and is not parsable by deserialiseFromTextEnvelope because it ignores the description. Changing deserialiseFromTextEnvelope to support this would be very patchy. So, this PR:
    • Modifies serialiseWitnessLedgerCddl to directly use serialiseToTextEnvelope (while keeping descriptions).
    • Modifies deserialiseWitnessLedgerCddl to use deserialiseFromTextEnvelope and to use the old decoding when that fails (this is done by the sub-function legacyDecoding).

These decisions provide a fair level of backwards compatibility but, at the same time, CBOR produced after the PR will be compatible with deserialiseFromTextEnvelope so functions in SerialiseLedgerCddl will become less necessary over time.

Also, to check these claims, you can see the results of running the tests in cardano-testnet and cardano-cli that are in the PRs linked in the Context section above.

Checklist

• Commit sequence broadly makes sense and commits have useful messages
• New tests are added if needed and existing tests are updated. See Running tests for more details
• Self-reviewed the diff

[Jimbo4350]

Looking good but we should take this a little further.

[Jimbo4350]

We can go further here. We can have serialiseTxLedgerCddl call serialiseToTextEnvelope and remove some of the additional unnecessary types e.g TextEnvelopeCddl

[palas]

Like in this commit?

It looks like this is likely a breaking change. Is that ok?

[carbolymer]

LGTM. Can you make sure that tests in cardano-testnet and cardano-cli pass prior to merge?

[Jimbo4350]

Nice work 👍

[Jimbo4350]

Nice 👍 . What we want to do is complain in the cli when a "legacy" text envelope type is used. The complaint would instruct them to convert the type field to the "correct" type. However before we do this, we should decide on a type format. I.e should we use the Witnesses vs Unwitnessed distinction or use the previous Tx $era format? You could start a discussion in discord on this.