applyChainTick should update the Shelley ledger view history #2559
Comments
nfrisby
added
bug
|
The fix should also include a low-level test. I'm not quite sure yet (hence Issue 2558 is still open) but I think this particular failure path only occurs when pools end up with zero stake after an epoch transition, so I don't think we should rely on the ThreadNet tests to catch regressions here: that's an odd case. |
mrBliss
added a commit
that referenced
this issue
Aug 31, 2020
Fixes #1935, #2506, #2559, and #2562. Consider the following situation: A -> B -- current chain \ > B' -> C' -- fork To validate the header of C', we need a ledger view that is valid for the slot of C'. We can't always use the current ledger to produce a ledger view, because our chain might include changes to the ledger view that are not present in the fork (they were activated after the intersection A). So we must get a ledger view at the intersection, A, and use that to forecast the ledger view at C'. Previously, we only kept a ledger state in memory for every 100 blocks, so obtaining a ledger state at the intersection point might require reading blocks from disk and reapplying them. As this is expensive for us, but cheap to trigger by attackers (create headers and serve them to us), this would lead to DoS possibilities. For that reason, each ledger state stored snapshots of past ledger views. So to obtain a ledger view for A, we asked B for a past ledger view at the slot of A (and use that to forecast for C'). After #2459 we keep all `k` past ledgers in memory, which makes it cheap to ask for a past ledger and thus a past ledger view. This means we no longer need to store ledger view snapshots in each ledger state. This was awkward, because we had a double history: we stored snapshots of the ledger state and each ledger state stored snapshots of the ledger view. Both for Byron and Shelley we can remove the ledger view history from the ledger. To remain backwards binary compatible with existing ledger snapshots, we still allow the ledger view history in the decoders but ignore it, and don't encode it anymore. Consequently, `ledgerViewForecastAtTip` no longer needs to specify *at* which slot (i.e., A's slot) to make the forecast (i.e., which past ledger view snapshot to use). Instead, we first get the right past ledger with `getPastLedger` and use its ledger view to make forecasts. This results in some simplifications in the ChainSyncClient.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The fundamental cause of the error in Issue #2558 is -- at least as of 9480613 -- that the Shelley ledger view history is only updated when adding a block. It should also/instead be updated when ticking.
IIUC we always tick before/as part of adding a block. As a consequence the Shelley ledger view history is currently always empty: we tick, which updates the epoch state without taking a snapshot, and then we add a block. But the add-block logic now no longer sees an epoch change (since the tick already handled that) and so also doesn't take a snapshot (it would be the wrong snapshot anyway). That's what lead to the failure in Issue 2558.
This Issue is done when ticking the Shelley ledger state is responsible for adding snapshots to its ledger view history.
The text was updated successfully, but these errors were encountered: