UTxO-HD targeting main

[jasagredo]

Description

The changes from UTxO-HD span over ouroboros-consensus, ouroboros-consensus-diffusion and ouroboros-consensus-cardano. The core change is:

• The UTxO set is extracted from the LedgerState in the form of LedgerTables.
• These tables are stored in the LedgerDB, which can keep them in memory or on disk.
• When performing an action that requires UTxOs, we have to ask the LedgerDB for those. This might perform IO.

Here I will explain how I would review this enormous PR. Instead of listing files I will describe concepts, and my suggestion is to go look at the mentioned files (or search for the concepts) then mark the file as viewed to offload it from the brain.

The ledger tables

• The first step would be to understand the concept of LedgerTables, see Ouroboros.Consensus.Ledger.Tables.* modules. The LedgerTables are parametrized by l (in the end it will be by blk) and by mk (or MapKinds). MapKinds are just types parametrized by the Key and Value of l. These will be TxIn|TxOut for unitary blocks and CanonicalTxIn|HardForkTxOut for hard fork blocks.
• LedgerTables are barbies-like, see Ouroboros.Consensus.Ledger.Tables.Combinators.
• LedgerTables are (most commonly) empty (EmptyMK), a (possibly restricted) UTxO set (ValuesMK), a set of TxIns (KeysMK), a sequence of differences (DiffMK) or a combination of values + diffs (TrackingMK). The only non-obvious one is DiffMK which is a map of sequences of changes to a value (in the UTxO case values don't change, they are created and destroyed, so there will be at most 2 elements there). On top of that there is a DiffSeqMK which is a fingertree of differences. Only used in V1 (see below).
• The LedgerState is itself parametrized by this same mk. The data instances will then make use of that mk to define tables associated with the block. So the byron ledger state ignores it, the shelley ledger state has a new field with the tables and the hard fork ledger state will propagate the mk through the telescope, therefore having an mk of the particular state in the Telescope.
• The LedgerTables can live on their own, which for unitary blocks don't make a difference, but for the Cardano Block, we go from an mk passed to the Telescope (therefore tables at the tip of the Telescope) to CardanoLedgerTables, in which each value is a HardForkTxOut. This cost is non-trivial and we only want to pay it when applying a new block/transaction.
• LedgerTables can be extracted and injected into the ledger state via (un)stowLedgerTables.
• The ledger tables of the Extended ledger state are the same as the ones form the LedgerState.
• A very important bit that maybe was not clear above is that the HardForkBlock has no canonical tables because our definitions are not compositional for the HF block, only the CardanoBlock has "hard fork tables". See the constraints of HasHardForkLedgerTables.

Applying and ticking (Ouroboros.Consensus.Ledger.Abstract/Basics)

When ticking a block, some differences might be created, and no values are needed. So the types go from l EmptyMK to Ticked1 l DiffMK. This is the case at least in two moments: when going from Byron to Shelley (all values are created here) and when going from Shelley to Allegra (avvm addresses are deleted). See the relevant functions: translateLedgerStateByronToShelley and translateLedgerStateShelleyToAllegra.

When applying a block, we get the inputs needed (getBlockKeySets then read those from the LedgerDB), tick the ledger state without tables (possibly creating diffs), apply those diffs on the values from the LedgerDB, then call the ledger rules. We then diff the input and output tables to get a set of differences from applying a block, to which we will prepend the ones from ticking. See applyBlockResult and the Shelley functions for applying blocks.

The story with transactions is pretty similar.

The LedgerDB versions (Ouroboros.Consensus.Storage.LedgerDB)

There are two flavors of the LedgerDB, each one having two implementations:

• V1 (Ouroboros.Consensus.Storage.LedgerDB.V1): we keep a sequence of EmptyMK ledger states and dump the values into a BackingStore. We can get back values from the backing store at any ledger state, by opening a BackingStoreValueHandle and reading from it. The BackingStore consists of a "complete" UTxO set at some anchor and then a sequence of differences. To get values at a given point we have to read the anchor, then reapply the differences up to the desired point. This is "wasteful" if done in memory (why keep diffs and have to reapply them every time if we can just apply them in place?) but it is useful on the on-disk implementation which puts the "complete" UTxO set on the disk, offloading it from memory. There are two implementations:
  • OnDisk: It uses LMDB underneath. See the Ouroboros.Consensus.Storage.LedgerDB.V1.BackingStore.Impl.LMDB.* modules.
  • InMemory: Not intended for real use. As mentioned above it is wasteful. It serves as a reference impl for the OnDisk implementation.
• V2 (Ouroboros.Consensus.Storage.LedgerDB.V2): We keep a sequence of StateRefs, which are EmptyMK ledger states together with a tables handle from which we can read values monadically. This is very similar to the previous LedgerDB, in which we kept a sequence of (complete) LedgerStates. There are two implementations:
  • InMemory
  • LSM: still a WIP

Evaluating forks

In order to evaluate forks, we created the concept of Forkers, where each LedgerDB implementation has their own concept. They are just an abstract interface that allows to query for values and push differences that eventually can be dumped back into the LedgerDB (only by ChainSelection, others use ReadOnlyForkers). Note that they allocate resources so there is some juggling with ResourceRegistries there.

Ledger queries (Ouroboros.Consensus.Ledger.Query)

Some queries will have to look at the UTxO set, in particular GetUtxoByAddress, GetUtxoWhole and GetUtxoByTxin. We categorize them by the means of QueryFootprint. We will process each one of them differently.

Other queries use QFNoTables, GetUtxoByTxIn uses QFLookupTables and will have to read a single value from the tables, and GetUtxoWhole and GetUtxoByAddress use QFTraverseTables as they will have to scan the whole UTxO set.

For the HardForkBlock there is another class Ouroboros.Consensus.HardFork.Combinator.Ledger.Query.BlockSupportsHFLedgerQuery which has faster implementations than projecting the tables into the particular tip of the Telescope, because we can usually judge whether we want the result without upgrading the TxOut to the latest era.

In essence, queries are now monadic. Queries that don't look at the UTxO set are artificially monadic (just a pure of the already existing logic).

The mempool

The mempool in essence will have to acquire (read only) forkers on the LedgerDB at the tip, then read values for the incoming transactions and apply them. The returned diffs are appended to the ones in the mempool, which keeps a TrackingMK with the current values and past diffs.

When revalidating transactions we cannot know if the UTxO set changed so we will have to re-read the values from the (new) forker.

The internal state is now a TMVar because we need to acquire >> read tables >> update where read tables is in IO and the others are in STM.

The snapshots

We now store snapshots in a new format:

• V1-OnDisk: a copy of the lmdb database and a (Haskell-CBOR) serialization of the LedgerState.
• V*-InMemory: a (Haskell-CBOR) serialization of the UTxO set and a (Haskell-CBOR) serialization of the LedgerState.

Note that for V2 we can take snapshots at any time of the immutable tip, but for v1 we have to take flush some differences from the BackingStore into the anchor to advance it to the immutable tip.

This is abstracted by either implementation in Ouroboros.Consensus.Storage.LedgerDB.V*...tryTakeSnapshot

The forging loop

The forging loop didn't change much. Each iteration runs with a resource registry (to allocate the forkers). Then we use the forker to provide values for the mempool snapshot acquisition, in case of a revalidation.

Changes in Byron/Shelley/Cardano

The changes here are mostly fulfilling everything that was described above, to make all the types match. There are some specific things which are interesting to look at because they might be non-trivial:

• Translation functions (with the two examples I already mentioned)
• The TxIn|TxOut data instances, the LedgerState data instance and the HasLedgerTables instances
• applyBlock for shelley. The cardano one is just the HFC one, which injects the CardanoTables into the tip of the Telescope (here is where we do the costly step, but it usually won't be that costly because the UTxO set for a block is small).
• The Cardano.Ledger module which defines the CardanoTxIn and CardanoTxOut.

Other changes

The rest of the changes are mainly just following GHC adjusting the types here and there. Most other code doesn't use tables so an abstract mk or EmptyMK is used to make the kind well-formed.

[jasagredo]

I did a pass over the non-testing libraries.

[nfrisby]

This is the result of my first pass on the Ouroboros.Consensus.Ledger.Tables.* modules.

[nfrisby]

Another round of comments. This is all of the *Hard* files, except for Query.hs.

[nfrisby]

Maybe write "ignoring the DiffMK" instead of "ignoring the differences" to avoid alarm/confusion.

[amesgen]

This code is luckily gone in #1297, so rebasing will resolve this

[jasagredo]

Rewrote it as Nick suggests, will deal with it when I do a rebase.

I'm leaving this thread open to see it again in the future.

[nfrisby]

This implementation allocates a list of txs for each previous era.

I wonder if an approach similar to composeTxOutTranslations $ ipTranslateTxOut hardForkEraTranslation from Combinator/Ledger.hs might make the logic easier to follow and also avoid allocating extraneous lists.

[jasagredo]

Perhaps lets chat about this on a call, I'm not sure what you mean here.

[nfrisby]

reifying a TODO about skipping traversals if the key/value translations are id

[nfrisby]

Might also consider combining the two traversals into a single traversal. And maybe also something about monotonicity.

[nfrisby]

All *Query*hs files, except:

• ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/ChainDB/Impl/Query.hs
• ouroboros-consensus/test/consensus-test/Test/Consensus/MiniProtocol/LocalStateQuery/Server.hs
• ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/Query.hs

[nfrisby]

Some suspicious duplication with answerBlockQueryLookup right above.

[jasagredo]

Yes, these are pretty similar but the footprint is concretized on each. I tried to unify them with extremely cryptic GHC errors. I will leave this open to revisit but maybe we have to live with this.

answerMonadicQueryVia ::
  forall m xs footprint result. ( MonadSTM m
  , All SingleEraBlock xs
  , HardForkHasLedgerTables xs
  , BlockSupportsHFLedgerQuery xs
  , CanHardFork xs
  )
  => (   forall result'. NP ExtLedgerCfg xs
      -> QueryIfCurrent xs footprint result'
      -> ReadOnlyForker' m (HardForkBlock xs)
      -> m result'
     )
  -> ExtLedgerCfg (HardForkBlock xs)
  -> BlockQuery (HardForkBlock xs) footprint result
  -> ReadOnlyForker' m (HardForkBlock xs)
  -> m result
answerMonadicQueryVia answerVia (ExtLedgerCfg cfg)
    qry
    forker = do
      st@(HardForkLedgerState hardForkState)  <- ledgerState <$> atomically (roforkerGetLedgerState forker)
      let ei   = State.epochInfoLedger lcfg hardForkState
          cfgs = hmap ExtLedgerCfg $ distribTopLevelConfig ei cfg
      case qry of
        QueryIfCurrent (queryIfCurrent :: QueryIfCurrent xs footprint result') ->
          answerVia
                cfgs
                queryIfCurrent
                forker
        -- We only call this with QFLookupTables or QFTraverseTables, so these
        -- two matches below are effectively dead.
        QueryAnytime queryAnytime (EraIndex era) ->
          pure $ interpretQueryAnytime
            lcfg
            queryAnytime
            (EraIndex era)
            hardForkState
        QueryHardFork queryHardFork ->
          pure $ interpretQueryHardFork
            lcfg
            queryHardFork
            st
    where
      lcfg = configLedger cfg

src\ouroboros-consensus\Ouroboros\Consensus\HardFork\Combinator\Ledger\Query.hs:262:17: error: [GHC-25897]
    • Couldn't match type ‘result1’
                     with ‘Either (MismatchEraInfo xs) result1’
      Expected: QueryIfCurrent xs footprint result
        Actual: QueryIfCurrent xs footprint result1
      ‘result1’ is a rigid type variable bound by
        a pattern with constructor:
          QueryIfCurrent :: forall (xs :: [*]) (footprint :: QueryFootprint)
                                   result.
Failed, 215 modules loaded.
                            QueryIfCurrent xs footprint result
                            -> BlockQuery
                                 (HardForkBlock xs) footprint (HardForkQueryResult xs result),
        in a case alternative
        at src\ouroboros-consensus\Ouroboros\Consensus\HardFork\Combinator\Ledger\Query.hs:259:9-78
    • In the second argument of ‘answerVia’, namely ‘queryIfCurrent’
      In the expression: answerVia cfgs queryIfCurrent forker
      In a case alternative:
          QueryIfCurrent (queryIfCurrent :: QueryIfCurrent xs footprint result')
            -> answerVia cfgs queryIfCurrent forker
    • Relevant bindings include
        queryIfCurrent :: QueryIfCurrent xs footprint result1
          (bound at src\ouroboros-consensus\Ouroboros\Consensus\HardFork\Combinator\Ledger\Query.hs:259:25)
        cfgs :: NP ExtLedgerCfg xs
          (bound at src\ouroboros-consensus\Ouroboros\Consensus\HardFork\Combinator\Ledger\Query.hs:257:11)
        hardForkState :: State.HardForkState (Flip LedgerState EmptyMK) xs
          (bound at src\ouroboros-consensus\Ouroboros\Consensus\HardFork\Combinator\Ledger\Query.hs:255:31)
        st :: LedgerState (HardForkBlock xs) EmptyMK
          (bound at src\ouroboros-consensus\Ouroboros\Consensus\HardFork\Combinator\Ledger\Query.hs:255:7)
        lcfg :: LedgerConfig (HardForkBlock xs)
          (bound at src\ouroboros-consensus\Ouroboros\Consensus\HardFork\Combinator\Ledger\Query.hs:278:7)
        forker :: ReadOnlyForker' m (HardForkBlock xs)
          (bound at src\ouroboros-consensus\Ouroboros\Consensus\HardFork\Combinator\Ledger\Query.hs:254:5)
        qry :: BlockQuery (HardForkBlock xs) footprint result
          (bound at src\ouroboros-consensus\Ouroboros\Consensus\HardFork\Combinator\Ledger\Query.hs:253:5)
        (Some bindings suppressed; use -fmax-relevant-binds=N or -fno-max-relevant-binds)
    |
262 |                 queryIfCurrent
    |                 ^^^^^^^^^^^^^^

Perhaps I just need to stare at it a bit more...

[nfrisby]

*LedgerDB* files, except I stopped when I got to the LMDB impl. I'll pick up there tomorrow.

[nfrisby]

My previous review was the *LedgerDB* files up to but excluding LMDB.

This review picks up there and stops before V2.

[nfrisby]

This is the LedgerDB*V2 files

[nfrisby]

Could share a control HOF with destroySnapshots.

[nfrisby]

Some of these impl* functions share a worrying amount of code with the V1 impl.

[jasagredo]

Yes, but they use different types. Perhaps we should hide this in some typeclass? We will eventually delete V1 so I'm unsure how worth this is.

[nfrisby]

This is the LedgerDB files after V2, ie the tests.

[nfrisby]

Which of these tests V2?

[nfrisby]

I would suggest removing verboseShrinking

[nfrisby]

reifying a TODO

[nfrisby]

alignment

[nfrisby]

Stale haddock: There is no prefixBackToAnchor function.

[nfrisby]

Please comment these fields, at least the pending and consumed ones.

[amesgen]

This code is luckily gone in #1297, so rebasing will resolve this

[amesgen]

I guess this could also be shared with the Cardano block and the two-era Shelley blocks.

[jorisdral]

Is it worthwhile to have a combined operation that maps both keys and values in one pass?

Makes sense, though I'm not sure Data.Map exposes anything to perform the mapping of both keys and values at the same time. So we'd probably end up defining

mapKeysValuesMK fkeys fvalues = Map.mapKeys fkeys . Map.map fvalues

The above is probably more susceptible to optimisations than mapKeysMK . mapMK, so that might be an argument in favour of mapKeysValuesMK

If so, is it also worthwhile to require the key mapping is monotone?

This would rely on injectCanonicalTxIn and distribCanonicalTxIn being monotonic. I suppose they are monotonic in practice, but if we want to use a monotonic key mapping here then we should make it a requirement on the HasCanonicalTxIn class

[jorisdral]

Not that I know of

[jorisdral]

What do you mean by more flexible?

[amesgen]

I could see that one would want to adapt the LMDB live dir to a non-default subdir, especially when analyzing an existing ChainDB. (Doesn't necessarily have to be implemented here, can also create a follow-up issue with potential improvements/polishing.)

[jasagredo]

This is in fact for the chaindb of which we get blocks from. Not for the current live chaindb.

[jasagredo]

Ah no I'm wrong. It actually points to the LedgerDB in which we will perform operations.

[amesgen]

So we have a duplicate reapply here, right? Could we avoid this, for example by creating a forker and pushing + committing the diff from newLedger via the forker?

Same point about the occurrences of reapplyThenPushNOW further below (maybe apart from reproMempoolForge)

[nfrisby]

This is every *Mempool* file, except ouroboros-consensus/test/consensus-test/Test/Consensus/Mempool/StateMachine.hs (it's new, big, and involved, so I'll review it separately).

[nfrisby]

Why do we pull the produced keys as well? So that we can check whether they already exist?

If so, that is not immediately obvious, so I think deserves a comment on the definition of the getTransactionKeySets method in SupportsMempool.hs and getBlockKeySets in Abstract.hs.

---

OK, maybe it's just a bug here. Because the upstream definition in Ledger doesn't obviously do it:

-- | The validity of any individual block depends only on a subset
-- of the UTxO stored in the ledger state. This function returns
-- the transaction inputs corresponding to the required UTxO for a
-- given Block.
--
-- This function will be used by the consensus layer to enable storing
-- the UTxO on disk. In particular, given a block, the consensus layer
-- will use 'neededTxInsForBlock' to retrieve the needed UTxO from disk
-- and present only those to the ledger.
neededTxInsForBlock ::
  forall h era.
  EraSegWits era =>
  Block h era ->
  Set (TxIn (EraCrypto era))
neededTxInsForBlock (Block' _ txsSeq _) = Set.filter isNotNewInput allTxIns
  where
    txBodies = map (^. bodyTxL) $ toList $ fromTxSeq txsSeq
    allTxIns = Set.unions $ map (^. allInputsTxBodyF) txBodies
    newTxIds = Set.fromList $ map txIdTxBody txBodies
    isNotNewInput (TxIn txID _) = txID `Set.notMember` newTxIds

[jorisdral]

I agree it's just a bug.

This bug may be indicative of a general weakness in our code. getTransactionKeySets and getBlockKeySets can declare more txins than strictly necessary for block/tx (re-)application, which won't make our test suites fail, but it does mean that there will be more disk traffic than strictly necessary. The reverse (fewer txins than necessary) will probably make our tests fail. Still, I'd suggest we put some tests in place that check whether getTransactionKeySets and getBlockKeySets declare precisely those txins that are required, no more and no less

[nfrisby]

reifying TODO

[nfrisby]

The rest of the files I had claimed. So I only need to review the new Mempool Statemachine tests now.

[nfrisby]

reifying a TODO

[nfrisby]

Reviewed the utxo-hd.md file

[nfrisby]

Do we intend/hope for the Ledger codebase to ever add an mk to its types, so that this warning wouldn't be necessary? Perhaps an upstream Issue for that should be linked here?

[jorisdral]

It's not something I've considered much, though I'm not sure it would be worth it for the ledger to include it. It would be a pretty pervasive change, and it's arguably a consensus implementation detail that we manipulate the UTXO set inside the ledger state

I think we currently have a reasonably safe API that prevent misuse. It could possibly be improved using moar types, e.g.:

• Introducing something like StowedMK / UnstowedMK instead of just EmptyMK
• Be more restrictive about when it is okay to extract a NewEpochState from a consensus LedgerState. For example, it's only okay to extract a NewEpochState when the consensus ledger state has type LedgerState _ StowedMK

[jasagredo]

Probably this should be StowedMK / EmptyMK instead. EmptyMK meaning that the Ledger must have an empty UTxO map inside, StowedMK means that there are values in the UTxO set inside.

[jorisdral]

From what I can tell, this benchmark was used to compare performance before and after changes made in #823. For comparative benchmarks, it does not matter much whether we use trivially empty tables or not, and it makes sense to use trivially empty tables here here since it's the smallest change to make here. If this benchmark were to be used in the future as an "absolute" measure of performance, then we should reconsider using actual tables. Though arguably we should use Cardano blocks instead of TestBlock if realistic measurements were to be the goal.

[jorisdral]

Thinking along the same lines: maybe the goal of this benchmark should be specified somewhere. We can do that in a direct PR to main instead of in this PR

[jorisdral]

I agree it's just a bug.

This bug may be indicative of a general weakness in our code. getTransactionKeySets and getBlockKeySets can declare more txins than strictly necessary for block/tx (re-)application, which won't make our test suites fail, but it does mean that there will be more disk traffic than strictly necessary. The reverse (fewer txins than necessary) will probably make our tests fail. Still, I'd suggest we put some tests in place that check whether getTransactionKeySets and getBlockKeySets declare precisely those txins that are required, no more and no less

[amesgen]

Because it occurs rather often in testing code: What about having a combinator for \st -> applyDiffs st $ tickApplyLike st?

[jorisdral]

Should this instance generate:

• A ledger state with no ledger tables, without stowed UTXOs
• A ledger state with no ledger tables, with stowed UTXOs

Right now it's generating the former. My guess is that the latter would be more interesting

Maybe this ambiguity is an argument in favour of having two types of EmptyMK: StowedEmptyMK and UnstowedEmptyMK, or something along those lines

[amesgen]

I replaced forgetLedgerTables with with stowLedgerTables, and the mock-test suite passes fine 👍

[jasagredo]

I don't think it is worth concretizing this more until we introduce StowedMK and UnstowedMK.

I would favor names that explain not what happened with the data but instead what data is there, so something like NothingMK and LedgerMK

[amesgen]

Don't we need to cleanup validatedChainDiff here, just as in the ValidPrefix case below?

(However, this code will be removed with #1269 anyways.)

[jasagredo]

Lets ignore it for now, waiting for a rebase

[amesgen]

Is there already an issue for this?

[jasagredo]

@jorisdral and the comment below

[jorisdral]

Hmm, unfortunately changing the tests to work with the new LedgerDB went under the radar :P

When designing the LedgerDB API, the getLedgerDB function was removed from the ChainDB API, because LedgerDB was no longer a pure datastructure. It was replaced by more concrete functions, amongst which: getCurrentLedger, getImmutableLedger, getPastLedger, getHeaderStateHistory, getReadOnlyForkerAtPoint, getLedgerTablesAtFor, getStatistics.

Options I can think of for how to restore testing:

• Test (a subset of) the concrete functions by creating a command for each.
• Don't test the concrete functions, but use TestInternals to output some pure information about the LedgerDB. Maybe we could reconstruct a pure sequence of ledger states from the concrete LedgerDB impls (reading tables from disk too), and compare those against the model?

And no, there is no issue for it, but there probably should be

[amesgen]

Does this comment actually apply ATM given that the GetLedgerDB command is disabled?

[jorisdral]

It does not apply currently, but it might if we decide to re-enable LedgerDB commands. Depending on which commands we include and how, we might still run into the problem that the V1.LedgerDB has a different set of states than the model has. However, I'd argue that the proper way to handle this discrepancy between the model and the real impl is to change the Eq instance for the result of flushing. We do a similar thing with IsValidResult

[amesgen]

I replaced forgetLedgerTables with with stowLedgerTables, and the mock-test suite passes fine 👍

[amesgen]

Changes to this file are superfluous

[amesgen]

What is taking so long that we need this? (I suppose that it's for Hydra?)

[jasagredo]

Hydra was complaining because there was too much silence. Not sure if this will still be the case.

[jorisdral]

This module seems to be LedgerDB.V1 specific, by the way, so maybe we should move it into there.

[jorisdral]

Related to my previous comment, but we can probably move these into DiffSeq as well, because these functions are only used in conjunction with a DiffSeq