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

Specify cross-era ticking/forecasting for Cardano #4635

Open
nfrisby opened this issue Sep 17, 2024 · 3 comments
Open

Specify cross-era ticking/forecasting for Cardano #4635

nfrisby opened this issue Sep 17, 2024 · 3 comments
Labels
💳 technical-debt Issues related to technical debt we introduced 🤝 consensus Tickets that the fine folks over at ouroboros-consensus want to keep track of 📜 formal-spec

Comments

@nfrisby
Copy link
Contributor

nfrisby commented Sep 17, 2024

The Consensus Team needs assistance with Issue IntersectMBO/ouroboros-consensus#418.

Right now, there is no separate specification for how to tick/forecast ledger states across era boundaries, there is only code. Other than Byron-to-Shelley, it's "obvious for the most part". But there are plenty of subtleties buried in the implementation.

This Issue is to create a proper specification of the relation that must hold between two ledger states X and Tick(X, S) --- X is the ledger state that results from applying the final block of some era, and S is the slot of the first block of the next era, which will be validated against the Tick(X, S) ledger state.

@nfrisby nfrisby added 📜 formal-spec 💳 technical-debt Issues related to technical debt we introduced 🤝 consensus Tickets that the fine folks over at ouroboros-consensus want to keep track of labels Sep 17, 2024
@nfrisby
Copy link
Contributor Author

nfrisby commented Sep 17, 2024

For the record, translating txs etc from one era to the next also deserves a specification. But it's less error-prone, in our experience.

@amesgen
Copy link
Member

amesgen commented Sep 18, 2024

But there are plenty of subtleties buried in the implementation.

One such subtlety is on ticking from Babbage to Conway:

  • The Babbage->Conway hack (Minimal workaround for updating protocol params on Babbage→Conway ouroboros-consensus#366) became unnecessary once @lehins changed how ticking applies updates to protocol params (see the corresponding ADR for an explanation).
  • However, we didn't revert it before the Conway HF, as that didn't seem strictly necessary. (Probably should have done that immediately, I even tested that it works fine...)
  • However, there actually is a small semantic difference between them: Due to the hack, when ticking across the era/epoch boundary using the Conway logic, the ticking logic thinks that the current protocol version is 9 (instead of 8 without the hack). Therefore, it already un-delegates stake from pointer addresses, which would otherwise (without the hack) only have happened at the next epoch boundary.
  • Therefore, in order to make sure that there are no disagreements regarding the stake distribution, we need to preserve the current behavior. The most direct way would be to keep the current hack, but we don't have to: I opened Minimally re-implement additional logic on Babbage->Conway ledger state translation ouroboros-consensus#1239 that re-implements the hack in a minimal fashion. It could be made even more minimal/finegrained by upstreaming the logic into Ledger.
  • The actual effect on mainnet is pretty minimal: The pool abacadaba9f12a8b5382fc370e4e7e69421fb59831bb4ecca3a11d9b lost 100 ADA worth of delegation in epoch 508 instead of in epoch 509 due to the retirement of pointer addresses.

@amesgen
Copy link
Member

amesgen commented Sep 18, 2024

This Issue is to create a proper specification of the relation that must hold between two ledger states X and Tick(X, S) --- X is the ledger state that results from applying the final block of some era, and S is the slot of the first block of the next era, which will be validated against the Tick(X, S) ledger state.

We also need sth similar for forecasting. E.g. one weird thing at the moment is that forecasting and ticking work differently/symmetrically:

  • Ticking from era A to era B works by first translating the ledger state from A to B, and then ticking across the era/epoch boundary using the logic from era B.
  • Forecasting from era A to era B works by first forecasting from the ledger state in A to a ledger view from A (but for a slot in B), and then translating the ledger view from A to B.

Consensus requires that these things agree as long as forecasting is defined (the "forecasting lemma", see e.g. here), which isn't actually immediately obvious here.

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
Labels
💳 technical-debt Issues related to technical debt we introduced 🤝 consensus Tickets that the fine folks over at ouroboros-consensus want to keep track of 📜 formal-spec
Projects
None yet
Development

No branches or pull requests

2 participants