Skip to content

Commit 4519035

Browse files
authored
Revert hack regarding Babbage→Conway ledger state translation (#1297)
Reverting the hacky approach of #366. Closes #1239 by superseding it. Addresses IntersectMBO/cardano-ledger#4635 (comment). # Justifying backwards-compatibility This PR touches the Cardano ledger rules, concretely the logic for translating a Babbage ledger state to a Conway ledger state. As the Conway HF already happend on mainnet, it is crucial to argue why this change retains backwards-compatibility with the historical chain. ## TL;DR - The original reason for #366 was resolved by the refactoring in IntersectMBO/cardano-ledger#4253, making the hack here in Consensus unnecessary. - The accidental side effects of #366 around pointer addresses were made "official" in IntersectMBO/cardano-ledger#4647. Therefore, it is fine to revert #366 without replacement. ## Detailed overview ### Context on HFC ledger ticking When the HFC ticks a ledger state across an era boundary from A to B, it does so via the "translate-then-tick" scheme: 1. First, the ledger state in A is translated into a ledger state in B. 2. Second, the ledger state is ticked to the target slot across the epoch/era boundary, using the logic of era B. For Cardano, the logic for these two operations lives in Ledger, or rather, it *should* live in Ledger. However, in #366, we introduced a temporary workaround/hack by modifying the translation logic from Babbage to Conway to resolve IntersectMBO/cardano-ledger#3491. This PR reverts the hack, such that we now again directly/transparently call Ledger logic. ### Chronology of changes to Babbage→Conway ticking 1. Mainnet era transitions are triggered by on-chain updates to the major ledger protocol version. The logic for updating the ledger protocol version lives, unsurprisingly, in the Ledger, and takes place while ticking across an epoch boundary. For `cardano-ledger-conway < 1.14` (that's significantly before any version used in a node that was mainnet-ready for Conway), this logic was broken on the era transition from Babbage to Conway, resulting in IntersectMBO/cardano-ledger#3491, ie the protocol version was *not* updated. Briefly[^1], the reason was that the governance schemes of Babbage and Conway are completely different, which caused issues because, as mentioned above, ticking across the Babbage→Conway era/epoch boundary uses the logic of Conway, which doesn't understand Babbage governance proposals, which were hence discarded during the translation step. 2. The Consensus team decided[^2] to fix this issue via #366, which updates the protocol version during the Babbage→Conway translation step in an ad-hoc fashion, by temporarily ticking the *Babbage* ledger step across the epoch/era boundary (yielding another Babbage ledger state), and then setting the `GovState` (an era-specific ledger concept deep in the ledger state, which in particular contains the current protocol parameters, and hence the protocol version) of the unticked Babbage ledger state to the one of the ticked Babbage ledger state, and then proceeding as before. Concretely, Babbage→Conway ticking now worked like this, starting with a Babbage ledger state `l0` and a target slot `s`. 1. Tick `l0` just across the era/epoch boundary to get `l1` (a Babbage ledger state). 2. Set the governance state of `l0` the the one of `l1` and get `l2` (a Babbage ledger state). 3. Translate `l2` into a Conway ledger state `l3`. 4. Tick `l4` to `s` to get the final result. 3. A few months later, for `cardano-ledger-conway-1.14`, @lehins changed in IntersectMBO/cardano-ledger#4253 how the way how protocol parameters are updated in Ledger in a way that is nicely compatible with the "translate-then-tick" scheme, see the [ADR](https://github.com/IntersectMBO/cardano-ledger/blob/a02dc6eae44287e8a1ac67ffafb8a1ecc492128f/docs/adr/2024-04-30_008-pparams-update.md) added in that PR for details[^3]. In particular, this would have allowed us to revert #366 immediately, but we didn't do so, probably because we saw now immediate motivation. (In retrospect, we should have done that immediately.) 4. A few months later, the Conway HF happened on mainnet. Due to investigating an unrelated serialization bug around pointer addresses (IntersectMBO/cardano-ledger#4589), I realized that not reverting #366 actually caused a slight difference in the ledger rules, namely regarding stake delegations from pointer addresses (also see IntersectMBO/cardano-ledger#4635 (comment)). Concretely, Ledger wants to get rid of pointer addresses as they are considered to be a misfeature and a potential liability for future projects like Leios (also see [this ADR](https://github.com/IntersectMBO/cardano-ledger/blob/master/docs/adr/2022-12-05_005-remove-ptr-addresses.md)). In Conway, stake delegations from pointer addresses are intentionally no longer considered. In particular, this happens during the SNAP rule while ticking, by invoking the [`forgoPointerAddressResolution`](https://github.com/IntersectMBO/cardano-ledger/blob/a02dc6eae44287e8a1ac67ffafb8a1ecc492128f/eras/shelley/impl/src/Cardano/Ledger/Shelley/HardForks.hs#L67) predicate on the current protocol version, branching on whether the current major protocol version is larger than `8` (the last Babbage major protocol version). - Using cardano-node 9.1 (i.e. the node that everyone was on to go to Conway), so with #366: When ticking the translated Conway ledger state into Conway, the current protocol version is `9` (the first Conway major protocol version), due to the previous ad-hoc patching of the `GovState` previously as part of the workaround from #366. Therefore, pointer addresses are *not* resolved while updating the stake distribution. - If we had reverted #366 for cardano-node 9.1: Because we directly translate the Babbage ledger state to Conway without doing the `GovState` patching before, the current protocol version while ticking is `8`, so pointer addresses *are* resolved. Altogether, the stake distribution used for the leader schedule starting in the second Conway epoch would have differed slightly (only very little stake, exactly 100 ADA, has been delegated via pointer addresses). Crucially, this difference had a chance to occur only because Ledger did *not* blank e.g. the `ptrMap` field in `IncrementalStake` during the Babbage→Conway translation. (This is actually what caused the serialization bug mentioned above.) There would have been another, less relevant difference: Because the current protocol parameters are updated *twice* with #366 (first during the Babbage tick, and then again during the Conway tick), the *previous* protocol parameters during the first Conway epoch are incorrectly equal to the *current* protocol parameters. However, the previous protocol parameters are only used for reward calculation, and reward calculation doesn't care whether the major protocol version is `8` or `9`. So this difference doesn't matter. 8. In a recent Ledger PR IntersectMBO/cardano-ledger#4647, @lehins modified the Babbage→Conway translation logic to blank out the pointer addresses, e.g. `ptrMap` in `IncrementalStake`. This change landed in Node 10.0. Therefore, the difference described in 4. does not matter anymore, as there no longer are any pointer addresses to resolve in Conway when ticking (which happens *after* translating). Crucially, this enables us to now revert #366 without replacement, because both before and after, no pointer addresses are resolved for the stake distribution while ticking from Babbage to Conway. ### Testing I tested this on mainnet by starting from a Babbage ledger state and evolving it via `db-analyser` to the first ledger state (slot `134092810`) in the second Conway epoch using full block validation, both with and without this PR. The resulting ledger states are identical. In the first Conway epoch, the ledger states differ, but only trivially in the previous protocol parameters which has no effect as explained above. We *could* also write a component-level test for the pointer address aspect, but that does not necessarily seem worth the cost/subtlety, as this is a legacy feature already. ### Concluding thoughts Generally, I think what we should take away from this is that we *really* need proper specification and testing of what exactly should happen at era boundaries, see #418 and IntersectMBO/cardano-ledger#4635, especially because certain esoteric parts of the ledger state (like pointer addresses) might not exist on any testnet. [^1]: See "Why the status quo is problematic" in #339 for the details (but ignore the rest of the issue). [^2]: After a long process that considered/prototyped various alternatives, but the details are not that relevant for this PR and the PR description is already very long. [^3]: Briefly, the logic that updates the protocol parameters on cross-epoch ticking is no longer era-dependent; rather, it just sets the protocol parameters to "future" ones that were decided on earlier by era-specific logic. The insight is that this set of future protocol parameters can be easily/cleanly translated from Babbage to Conway, and the Conway ticking logic can apply them despite having no idea *how* Babbage decided that these should be the next protocol parameters.
2 parents 82c5ebf + b1daa27 commit 4519035

File tree

2 files changed

+7
-64
lines changed

2 files changed

+7
-64
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
### Non-Breaking
2+
3+
- Removed now-redundant hack in Babbage→Conway translation.

ouroboros-consensus-cardano/src/ouroboros-consensus-cardano/Ouroboros/Consensus/Cardano/CanHardFork.hs

Lines changed: 4 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -33,27 +33,20 @@ import qualified Cardano.Chain.Genesis as CC.Genesis
3333
import qualified Cardano.Chain.Update as CC.Update
3434
import Cardano.Crypto.DSIGN (Ed25519DSIGN)
3535
import Cardano.Crypto.Hash.Blake2b (Blake2b_224, Blake2b_256)
36-
import qualified Cardano.Ledger.BaseTypes as SL
37-
import qualified Cardano.Ledger.Core as Core
3836
import Cardano.Ledger.Crypto (ADDRHASH, Crypto, DSIGN, HASH)
3937
import qualified Cardano.Ledger.Era as SL
4038
import qualified Cardano.Ledger.Genesis as SL
4139
import Cardano.Ledger.Hashes (EraIndependentTxBody)
4240
import Cardano.Ledger.Keys (DSignable, Hash)
4341
import qualified Cardano.Ledger.Shelley.API as SL
44-
import qualified Cardano.Ledger.Shelley.LedgerState as SL
4542
import Cardano.Ledger.Shelley.Translation
4643
(toFromByronTranslationContext)
4744
import qualified Cardano.Protocol.TPraos.API as SL
4845
import qualified Cardano.Protocol.TPraos.Rules.Prtcl as SL
4946
import qualified Cardano.Protocol.TPraos.Rules.Tickn as SL
50-
import Cardano.Slotting.EpochInfo (epochInfoFirst)
5147
import Control.Monad
5248
import Control.Monad.Except (runExcept, throwError)
53-
import qualified Control.State.Transition as STS
5449
import Data.Coerce (coerce)
55-
import Data.Function ((&))
56-
import Data.Functor.Identity
5750
import qualified Data.Map.Strict as Map
5851
import Data.Maybe (listToMaybe, mapMaybe)
5952
import Data.Proxy
@@ -62,11 +55,8 @@ import Data.SOP.InPairs (RequiringBoth (..), ignoringBoth)
6255
import qualified Data.SOP.Strict as SOP
6356
import Data.SOP.Tails (Tails (..))
6457
import qualified Data.SOP.Tails as Tails
65-
import Data.Void
6658
import Data.Word
6759
import GHC.Generics (Generic)
68-
import Lens.Micro ((.~))
69-
import Lens.Micro.Extras (view)
7060
import NoThunks.Class (NoThunks)
7161
import Ouroboros.Consensus.Block
7262
import Ouroboros.Consensus.Byron.Ledger
@@ -719,66 +709,16 @@ translateValidatedTxAlonzoToBabbageWrapper ctxt = InjectValidatedTx $
719709
-------------------------------------------------------------------------------}
720710

721711
translateLedgerStateBabbageToConwayWrapper ::
722-
forall c. (Praos.PraosCrypto c)
712+
(Praos.PraosCrypto c)
723713
=> RequiringBoth
724714
WrapLedgerConfig
725715
(Translate LedgerState)
726716
(ShelleyBlock (Praos c) (BabbageEra c))
727717
(ShelleyBlock (Praos c) (ConwayEra c))
728718
translateLedgerStateBabbageToConwayWrapper =
729-
RequireBoth $ \cfgBabbage cfgConway ->
730-
Translate $ \epochNo ->
731-
let -- It would be cleaner to just pass in the entire 'Bound' instead of
732-
-- just the 'EpochNo'.
733-
firstSlotNewEra = runIdentity $ epochInfoFirst ei epochNo
734-
where
735-
ei =
736-
SL.epochInfoPure
737-
$ shelleyLedgerGlobals
738-
$ unwrapLedgerConfig cfgConway
739-
740-
-- HACK to make sure protocol parameters get properly updated on the
741-
-- era transition from Babbage to Conway. This will be replaced by a
742-
-- more principled refactoring in the future.
743-
--
744-
-- Pre-Conway, protocol parameters (like the ledger protocol
745-
-- version) were updated by the UPEC rule, which is executed while
746-
-- ticking across an epoch boundary. If sufficiently many Genesis
747-
-- keys submitted the same update proposal, it will update the
748-
-- governance state accordingly.
749-
--
750-
-- Conway has a completely different governance scheme (CIP-1694),
751-
-- and thus has no representation for pre-Conway update proposals,
752-
-- which are hence discarded by 'SL.translateEra'' below. Therefore,
753-
-- we monkey-patch the governance state by ticking across the
754-
-- era/epoch boundary using Babbage logic, and set the governance
755-
-- state to the updated one /before/ translating.
756-
patchGovState ::
757-
LedgerState (ShelleyBlock proto (BabbageEra c))
758-
-> LedgerState (ShelleyBlock proto (BabbageEra c))
759-
patchGovState st =
760-
st { shelleyLedgerState = shelleyLedgerState st
761-
& SL.newEpochStateGovStateL .~ newGovState
762-
}
763-
where
764-
newGovState =
765-
view SL.newEpochStateGovStateL
766-
. tickedShelleyLedgerState
767-
. applyChainTick
768-
(unwrapLedgerConfig cfgBabbage)
769-
firstSlotNewEra
770-
$ st
771-
772-
-- The UPEC rule emits no ledger events, hence this hack is not
773-
-- swallowing anything.
774-
_upecNoLedgerEvents ::
775-
STS.Event (Core.EraRule "UPEC" (BabbageEra c)) :~: Void
776-
_upecNoLedgerEvents = Refl
777-
778-
in unComp
779-
. SL.translateEra' (getConwayTranslationContext cfgConway)
780-
. Comp
781-
. patchGovState
719+
RequireBoth $ \_cfgBabbage cfgConway ->
720+
Translate $ \_epochNo ->
721+
unComp . SL.translateEra' (getConwayTranslationContext cfgConway) . Comp
782722

783723
getConwayTranslationContext ::
784724
WrapLedgerConfig (ShelleyBlock (Praos c) (ConwayEra c))

0 commit comments

Comments
 (0)