Remove LedgerView history #2546

mrBliss · 2020-08-26T14:57:24Z

Fixes #1935.

See the individual commits for detailed descriptions.

mrBliss · 2020-08-26T14:57:44Z

~~This includes #2543 until merged.~~

UPDATE: merged.

@coot

2548: Port #2525 and #2531 to master r=mrBliss a=mrBliss #2525 and #2531 were merged in the https://github.com/input-output-hk/ouroboros-network/tree/coot/connection-manager. However, that branch won't be merged for another few weeks. In the meantime, the merge conflicts with the changes made in those two PRs will accumulate. For example, I know that #2546 will cause a merge conflict. Now that @coot has kindly merged #2541 (which those PRs depended on) in master, we can port #2525 and #2532 to master so we can avoid the upcoming merge conflicts. After this PR is merged, the `coot/connection-manager` branch should be rebased onto master. Co-authored-by: Thomas Winant <thomas@well-typed.com> Co-authored-by: Nicolas Frisby <nick.frisby@iohk.io>

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.

Instead of O(n), it is now O(log n * log n) (binary search with at each step an operation logarithmic in the size). Since we're now much more often asking for past ledger snapshots in the ChainSyncClient, it has become more important to make this efficient. Microbenchmarks with size = 2160 (using `r = RealPoint` with a real hash): snapshot to lookup | before | after -------------------|------------|---------- oldest | 40. μs | 1.3 μs middle | 20. μs | 0.238 μs newest | 0.035 μs | 0.035 μs missing in middle | 40. μs | 1.3 μs Newest is a special case that is handled separately, so it didn't change. The other operations are now more than 10x faster. See the comments for more implementation details.

ouroboros-consensus-byron/src/Ouroboros/Consensus/Byron/Ledger/Ledger.hs

mrBliss · 2020-08-31T09:26:02Z

ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/Ledger.hs

+ futureLedgerView :: SlotNo -> Ticked (SL.LedgerView c)
+ futureLedgerView =
+ either
+ (\e -> error ("futureLedgerView failed: " <> show e))


Independent: let futureLedgerView use reapply instead of apply, then it's no longer partial.

ouroboros-consensus-shelley/src/Ouroboros/Consensus/Shelley/Ledger/Ledger.hs

ouroboros-consensus/src/Ouroboros/Consensus/MiniProtocol/ChainSync/Client.hs

nc6

Reviewed in a call with Thomas, looks good.

mrBliss · 2020-08-31T10:14:50Z

bors merge

iohk-bors · 2020-08-31T10:35:20Z

Build succeeded

nfrisby · 2020-08-31T13:12:12Z

https://buildkite.com/input-output-hk/ouroboros-network-nightly/builds/216 is full of errors due to the Issue that this PR fixes.

@mrBliss enqueued https://buildkite.com/input-output-hk/ouroboros-network-nightly/builds/217 to validate this PR, but unfortunately the build agent I would expect to run it is reported as "Disconnected", packet-ipxe-5-ci5-1: https://buildkite.com/organizations/input-output-hk/agents/2aa3ca09-f245-4015-b53e-2b9d2d171a43

nfrisby · 2020-08-31T14:17:32Z

@mrBliss Regarding a repro:

I lifted this from last night's nightlies' RealTPraos-artifact.log: https://buildkite.com/input-output-hk/ouroboros-network-nightly/builds/216#b50712a9-9306-4bfe-8133-62ba74682b73

repro :: TestSetup
repro = TestSetup
  { setupD = DecentralizationParam (1 / 5)
  , setupD2 = DecentralizationParam (3 / 5)
  , setupInitialNonce = Nonce (fromString "fee90546920fec9b2f4a3d22a8d3a47695967d222f79b0a6ecce36838348f7fc")
  , setupK = SecurityParam 8
  , setupTestConfig = TestConfig
    { initSeed = Seed (-378562860297457886)
    , nodeTopology = NodeTopology (Map.fromList [(CoreNodeId 0,Set.fromList []),(CoreNodeId 1,Set.fromList [CoreNodeId 0]),(CoreNodeId 2,Set.fromList [CoreNodeId 1]),(CoreNodeId 3,Set.fromList [CoreNodeId 0,CoreNodeId 2]),(CoreNodeId 4,Set.fromList [CoreNodeId 2])])
    , numCoreNodes = NumCoreNodes 5
    , numSlots = NumSlots 413
    }
  , setupVersion = (NodeToNodeV_1,Shelley.ShelleyNodeToNodeVersion1)
  }

That repro fails with HeaderError on the predecessor of this PR and passes with this PR. So that's good.

I think https://buildkite.com/input-output-hk/ouroboros-network-nightly/builds/217 will likely fail with Issue #2557, but we can inspect its list of failures after the fact to see if this PR removed the ones it should (ie the HeaderErrors).

mrBliss · 2020-08-31T14:21:38Z

@mrBliss Regarding a repro:

I lifted this from last night's nightlies' RealTPraos-artifact.log: https://buildkite.com/input-output-hk/ouroboros-network-nightly/builds/216#b50712a9-9306-4bfe-8133-62ba74682b73

repro :: TestSetup
repro = TestSetup
  { setupD = DecentralizationParam (1 / 5)
  , setupD2 = DecentralizationParam (3 / 5)
  , setupInitialNonce = Nonce (fromString "fee90546920fec9b2f4a3d22a8d3a47695967d222f79b0a6ecce36838348f7fc")
  , setupK = SecurityParam 8
  , setupTestConfig = TestConfig
    { initSeed = Seed (-378562860297457886)
    , nodeTopology = NodeTopology (Map.fromList [(CoreNodeId 0,Set.fromList []),(CoreNodeId 1,Set.fromList [CoreNodeId 0]),(CoreNodeId 2,Set.fromList [CoreNodeId 1]),(CoreNodeId 3,Set.fromList [CoreNodeId 0,CoreNodeId 2]),(CoreNodeId 4,Set.fromList [CoreNodeId 2])])
    , numCoreNodes = NumCoreNodes 5
    , numSlots = NumSlots 413
    }
  , setupVersion = (NodeToNodeV_1,Shelley.ShelleyNodeToNodeVersion1)
  }

That repro fails with HeaderError on the predecessor of this PR and passes with this PR. So that's good.

😌

I think https://buildkite.com/input-output-hk/ouroboros-network-nightly/builds/217 will likely fail with Issue #2557, but we can inspect its list of failures after the fact to see if this PR removed the ones it should (ie the HeaderErrors).

👍

This brings in <IntersectMBO/ouroboros-network#2546>, which fixes 3 (!) bugs related to Shelley's ledger view history.

1783: Update ouroboros-network r=mrBliss a=mrBliss This brings in <IntersectMBO/ouroboros-network#2546>, which fixes 3 (!) bugs related to Shelley's ledger view history. Co-authored-by: Thomas Winant <thomas@well-typed.com>

nfrisby · 2020-08-31T16:52:03Z

I think https://buildkite.com/input-output-hk/ouroboros-network-nightly/builds/217 will likely fail with Issue #2557, but we can inspect its list of failures after the fact to see if this PR removed the ones it should (ie the HeaderErrors).

As predicted, the 3 failures were all #2557 👍

mrBliss added the consensus issues related to ouroboros-consensus label Aug 26, 2020

mrBliss force-pushed the mrBliss/remove-ledger-view-history branch 2 times, most recently from 25f5aeb to bee8cdb Compare August 26, 2020 15:06

mrBliss mentioned this pull request Aug 26, 2020

Port #2525 and #2531 to master #2548

Merged

mrBliss force-pushed the mrBliss/remove-ledger-view-history branch from bee8cdb to 5c3bc95 Compare August 30, 2020 12:24

mrBliss requested review from nc6, nfrisby and dcoutts August 31, 2020 08:30

mrBliss marked this pull request as ready for review August 31, 2020 08:31

This was linked to issues Aug 31, 2020

Incorrect trimming of the Shelley Ledger View History #2562

Closed

Off-by-one error in TPraos history #2506

Closed

applyChainTick should update the Shelley ledger view history #2559

Closed

mrBliss added 2 commits August 31, 2020 10:56

mrBliss force-pushed the mrBliss/remove-ledger-view-history branch from 5c3bc95 to 5c74ff0 Compare August 31, 2020 08:56

mrBliss commented Aug 31, 2020

View reviewed changes

nc6 approved these changes Aug 31, 2020

View reviewed changes

iohk-bors bot merged commit f0eb6e4 into master Aug 31, 2020

iohk-bors bot deleted the mrBliss/remove-ledger-view-history branch August 31, 2020 10:35

mrBliss added a commit to IntersectMBO/cardano-node that referenced this pull request Aug 31, 2020

Update ouroboros-network

e7c4f2a

This brings in <IntersectMBO/ouroboros-network#2546>, which fixes 3 (!) bugs related to Shelley's ledger view history.

mrBliss mentioned this pull request Aug 31, 2020

Update ouroboros-network IntersectMBO/cardano-node#1783

Merged

This was referenced Sep 2, 2020

Off-by-one error in TPraos history #2506

Closed

Remove rewindChainDepState #2571

Closed

Debug new nightly failure in RealTPraos #2574

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove LedgerView history #2546

Remove LedgerView history #2546

mrBliss commented Aug 26, 2020

mrBliss commented Aug 26, 2020 •

edited

Loading

mrBliss Aug 31, 2020

nc6 left a comment

mrBliss commented Aug 31, 2020

iohk-bors bot commented Aug 31, 2020

nfrisby commented Aug 31, 2020 •

edited

Loading

nfrisby commented Aug 31, 2020 •

edited

Loading

mrBliss commented Aug 31, 2020

nfrisby commented Aug 31, 2020

Remove LedgerView history #2546

Remove LedgerView history #2546

Conversation

mrBliss commented Aug 26, 2020

mrBliss commented Aug 26, 2020 • edited Loading

mrBliss Aug 31, 2020

Choose a reason for hiding this comment

nc6 left a comment

Choose a reason for hiding this comment

mrBliss commented Aug 31, 2020

iohk-bors bot commented Aug 31, 2020

Build succeeded

nfrisby commented Aug 31, 2020 • edited Loading

nfrisby commented Aug 31, 2020 • edited Loading

mrBliss commented Aug 31, 2020

nfrisby commented Aug 31, 2020

mrBliss commented Aug 26, 2020 •

edited

Loading

nfrisby commented Aug 31, 2020 •

edited

Loading

nfrisby commented Aug 31, 2020 •

edited

Loading