Change serialization of OneEraGenTxId to treat it as though it's simply a ShortByteString

[fraser-iohk]

Description

This PR changes the serialization of OneEraGenTxId so that it's serialized and deserialized as an era-independent ShortByteString rather than as an era-tag-prefixed ShortByteString. As part of this, I've had to make a breaking change to the representation of TxId (GenTx ByronBlock), since there is now no way to identify which kind of Byron-era transaction ID we're deserializing – TxId, CertificateId, UpId, or VoteId – and these are all now represented simply in ByronGenTxId as a Hash.

As this is a breaking change to the wire format, there's a new protocol version and a corresponding branch on ouroboros-network here: fraser-iohk/one-era-gen-tx-id-protocol-version-bump

[fraser-iohk]

what should happen in the case that the blessed GenTxId era is disabled in the config? should we pick another era arbitrarily? should there be a way to order fallbacks?

          *** Failed! Falsified (after 15 tests):
          WithVersion (HardForkNodeToClientEnabled HardForkSpecificNodeToClientVersion4 (EraNodeToClientEnabled ByronNodeToClientVersion1 :* EraNodeToClientEnabled ShelleyNodeToClientVersion6 :* EraNodeToClientEnabled ShelleyNodeToClientVersion9 :* EraNodeToClientEnabled ShelleyNodeToClientVersion1 :* EraNodeToClientEnabled ShelleyNodeToClientVersion5 :* EraNodeToClientDisabled :* EraNodeToClientEnabled ShelleyNodeToClientVersion5 :* Nil)) (HardForkGenTxId {getHardForkGenTxId = S (S (S (Z (WrapGenTxId {unwrapGenTxId = txid: TxId {unTxId = SafeHash "fb4ea1312616d1a61645991aa8012a3aca2178e121759fb833177201080c5673"}}))))})
          DeserialiseFailure 0 "HardForkEncoderDisabledEra (SingleEraInfo {singleEraName = \"Babbage\"})"
          When deserialising 5820fb4ea1312616d1a61645991aa8012a3aca2178e121759fb833177201080c5673
          Use --quickcheck-replay="(SMGen 1921722396195416107 8896093690400693515,14)" to reproduce.
          Use -p '/Roundtrip.SerialiseNodeToClient.roundtrip GenTxId/' to rerun this test only.

[amesgen]

what should happen in the case that the blessed GenTxId era is disabled in the config? should we pick another era arbitrarily? should there be a way to order fallbacks?

What about picking an era that is supported by all CardanoNodeToXVersions, eg Shelley should work I think?

Related: The next Network release (#1314) will have IntersectMBO/ouroboros-network#5002, such that we can remove all old CardanoNodeToClientVersions that do not support all currently known eras.

[fraser-iohk]

yeah, I saw the changes in network and kinda feel like this is irrelevant since we're dropping support for older NodeToClientVersions. does picking Shelley have any other risks that would be relevant? I did think about messing with the Arbitrary instances to always pick a sensible era instead

[amesgen]

does picking Shelley have any other risks that would be relevant?

I can't think of anything; everything that uses our code shouldn't care about this at all due to #1017; and I don't think that external services (like custom tx submission clients) do either.

[fraser-iohk]

added the no changelog label for -diffusion, whose only change is a new instance for HasBlessedGenTxIdEra

[amesgen]

Looks good! (Sorry for not sending this review earlier, just noticed it while closing tabs)

I think it makes sense to also give a self-contained overview of what changes here for external implementations of mini-protocols involving tx ids, such that they can easily adapt to this change without having to read the code (ideally, we would have CDDL specs as tracked by #7, but we aren't there yet).

[amesgen]

Can we share the NodeToClient and NodeToNode logic?

[amesgen]

Just brainstorming an alternative: Instead of having a typeclass for this, we could also "hardcode" this choice in the serialization logic (e.g. sth like: "use the first index when xs has length 1, otherwise, use the second index). The downside is that one can't customize this for a hypothetical different HardForkBlock that would need to customize this, but that seems unlikely.