-
Notifications
You must be signed in to change notification settings - Fork 87
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
Incorrect trimming of the Shelley Ledger View History #2562
Milestone
Comments
mrBliss
added
bug
Something isn't working
consensus
issues related to ouroboros-consensus
shelley ledger integration
labels
Aug 31, 2020
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
LedgerViewHistory
maintained by the ShelleyLedgerState
is trimmed after a snapshot of the old ledger view history is made.Call stack:
https://github.com/input-output-hk/ouroboros-network/blob/0934a3cb1e24ecbfcbab4e40522d99ffcd60feaf/ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/History.hs#L70
https://github.com/input-output-hk/ouroboros-network/blob/0934a3cb1e24ecbfcbab4e40522d99ffcd60feaf/ouroboros-consensus/src/Ouroboros/Consensus/Ledger/History.hs#L103
This last module,
Ouroboros.Consensus.Ledger.History
is reused by the Byron ledger to maintain a history of the delegation state (= the Byron ledger view).In
Ouroboros.Consensus.Ledger.History.trim
, snapshots older than "the earliest slot we might roll back to" are trimmed. This "earliest slot ..." is defined as "now -2k
". This is correct for Byron, but not for Shelley! For Shelley, it should be "now -3k/f
". This means that the effective rollback in the Shelley era is much shorter than it should be.The text was updated successfully, but these errors were encountered: