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

Cardano-cli reports protocol version 8 in Conway era #3491

Closed
johnalotoski opened this issue Jun 20, 2023 · 1 comment · Fixed by IntersectMBO/ouroboros-consensus#366 or IntersectMBO/ouroboros-consensus#1297
Assignees
Labels
conway SanchoNet Minimal working version of Governance

Comments

@johnalotoski
Copy link

When in Babbage protocol version 8 and doing a proposal update to Conway protocol version 9, cardano-cli query tip will show the expected switch from Babbage to Conway era at the next epoch transition.

Now, while in Conway era, cardano-cli querying protocol-parameters still shows protocol version as 8.

# While in Conway era -- check query tip output:
> cardano-cli query tip
{
    "block": 24516,
    "epoch": 5,
    "era": "Conway",
    "hash": "86a514fce42ad660ed587b0afe4a731bd01090d82de2225c5a9e15517a8ca068",
    "slot": 492091,
    "slotInEpoch": 60091,
    "slotsToEpochEnd": 26309,
    "syncProgress": "100.00"
}

# Then query protocol parameters, however still show version 8:
> cardano-cli query protocol-parameters | jq .protocolVersion
{
  "major": 8,
  "minor": 0
}

# The proposal update logged in node does show pvMajor Version 9 for the hard fork:
Event: LedgerUpdate (HardForkUpdateInEra S (S (S (S (S (Z (WrapLedgerUpdate {unwrapLedgerUpdate = ShelleyUpdatedProtocolUpdates [ProtocolUpdate {protocolUpdateProposal = UpdateProposal {proposalParams = PParamsUpdate (BabbagePParams {bppMinFeeA = SNothing, bppMinFeeB = SNothing, bppMaxBBSize = SNothing, bppMaxTxSize = SNothing, bppMaxBHSize = SNothing, bppKeyDeposit = SNothing, bppPoolDeposit = SNothing, bppEMax = SNothing, bppNOpt = SNothing, bppA0 = SNothing, bppRho = SNothing, bppTau = SNothing, bppProtocolVersion = SJust (ProtVer {pvMajor = Version 9, pvMinor = 0}), bppMinPoolCost = SNothing, bppCoinsPerUTxOByte = SNothing, bppCostModels = SNothing, bppPrices = SNothing, bppMaxTxExUnits = SNothing, bppMaxBlockExUnits = SNothing, bppMaxValSize = SNothing, bppCollateralPercentage = SNothing, bppMaxCollateralInputs = SNothing}), proposalVersion = Just (ProtVer {pvMajor = Version 9, pvMinor = 0}), proposalEpoch = EpochNo 4}, protocolUpdateState = UpdateState {proposalVotes = [KeyHash "c1ad22cabb342cbb83ce3859708232f4945ccb669e9b5f932cffc0ed",KeyHash "c264bca994a3a5deee5a1d9b92a3d7e9d6cbdb81f2f6989bb7f7b437",KeyHash "d4bf7eb45b72dffa5ac33d5c902fe409e4e611f2e9a52fb0d09784c3"], proposalReachedQuorum = True}}]})))))))
@teodanciu
Copy link
Contributor

The current hypothesis is that the protocol update doesn't actually happen when transitioning to Conway, because Translation is losing the update proposals from the LedgerState. The explanation and plan is in this consensus ticket

github-merge-queue bot pushed a commit to IntersectMBO/ouroboros-consensus that referenced this issue Sep 25, 2023
Closes IntersectMBO/cardano-ledger#3491 in
an ad-hoc but minimal fashion.

Primary motivation is to fix this bug on Sanchonet without having to
wait for the full HFC refactoring (see #345) that we will probably end
up with.
github-merge-queue bot pushed a commit to IntersectMBO/ouroboros-consensus that referenced this issue Nov 4, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment