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.

[amesgen]

(There still are a few unaddressed comments from the last review.)

In particular, once we have a self-contained overview of the change as mentioned above, we might want to ping people in the node diversity channel on Discord to give them a heads up and gather feedback, as any tool implementing LocalTxSubmission or TxSubmission2 will have to adapt to this serialization simplification.

Eg maybe this would be an opportunity to contribute to https://cardano-scaling.github.io/cardano-blueprint/mempool/txsubmission2.html cc @ch1bo

[amesgen]

Why do we need this?