Repository navigation
Conversation
Duplicating forging logic is a maintenace burden, instead this change reuses 'Ouroboros.Consensus.NodeKernel.Forge.forge' function and instantiates a a Memmpool without a "sync thread" where transactions are 'addLocalTxs'd with the provided 'genTxs' transaction generator.
'ForgeBlockArgs' has a new field, 'fbEbTxs': the transactions that 'snapshotPartition' selects for the endorser block. 'forgeBlock' returns the forged block and a 'Maybe ForgedLeiosEb'. 'forge' takes a function that it calls with each forged endorser block, after the ChainDB adopts the ranking block. The hard fork combinator passes both transaction lists to the era of the ticked ledger state. It returns the endorser block of that era. The forge loop gives the endorser block a capacity of zero. As a result, 'fbEbTxs' is empty and every era returns 'Nothing'. No forged block changes. Part of input-output-hk/ouroboros-leios#1107.
The second part of 'snapshotPartition' is the longest prefix of the transactions that remain after the first part. The docs now name both parts in the same way. The test of the first part says "ranking-block part".
There was a problem hiding this comment.
More of this / aim higher!
I don't like that we are copying the prototype verbatim, but would be fine with it if it's self-contained. In this case here it's just plumbing that is not used. I would love to see small increment PRs that each does something.
Concretely, for this PR, I would say we should make base it on #2354 (or an alternative that introduces a new entry in CardanoBlock), that does forge an EB and at least traces it (if not even store it right away in the DB, which we already have on main).
That will give us something to actually discuss on, because right now nobody can tell whether one or the other thing is actually needed and we easily drift into bikeshedding about interfaces.
| -- | 'forge' calls this function after the ChainDB adopts the forged ranking | ||
| -- block. The arguments are the header of this block and the forged | ||
| -- endorser block. | ||
| (Header blk -> ForgedLeiosEb -> m ()) -> |
There was a problem hiding this comment.
What is this used for?
This is the only reason why an EB would need to be returned from the forgeBlock abstract interface.
| -- 'fbTxs', up to the endorser-block capacity. They are in mempool order. | ||
| -- | ||
| -- If you apply 'fbTxs' to 'fbCurrentTickedLedgerState', these transactions | ||
| -- then apply in order without errors. |
There was a problem hiding this comment.
Should: pass a MempoolSnapshot blk instead of fbTxs and fbEbTxs
For Leios-enabled eras it does not make a difference whether we "partition" the snapshot before in the abstract forge loop or within the concrete forgeBlock. For Non-Leios blocks/eras it may make a difference in terms of performance (or allocations).
Also, we likely want to have access to the MempoolMeasure's that went into an EB, that remain in the mempool etc.; at least for tracing.
You presented these reasons on the PR (opening a thread with focused responses here):
The hard fork combinator has no function that turns a MempoolSnapshot into one for a single era, but it can unwrap a list of transactions for the tip era.
Is this a problem or just that no such function exists today? Does the current transformation yield an empty list if the tag not matches? I would even claim that it is a programmer error if you try to use txs [GenTx blk] from a "non-matching blk" in forgeBlock :: ForgeBlockArgs blk -> m blk.
The Byron tests build ForgeBlockArgs without a snapshot.
Surely there is a MempoolSnapshot blk that can express "no transactions"?
For eras before Dijkstra, the EB capacity is zero, so the EB part is empty.
Yeah, that's fine. No difference between an empty fbEbTxs or a snapshot that will give you an empty list when you ask it for eb txs.
This branch has not been deployed
3 participants
Part of input-output-hk/ouroboros-leios#1107 (interface).
forgeBlockgets the transactions thatsnapshotPartitionselects for the endorser block (fbEbTxs).It returns the forged block and a
Maybe ForgedLeiosEb.forgetakes a function that it calls with each forged endorser block, after the ChainDB adopts the ranking block.The forge loop gives the endorser block a capacity of zero.
As a result, every era returns
Nothingand no forged block changes.Based on #2284.
Its commit (
cd4f81423) is in this branch until #2284 merges, so review only the last two commits.Alternative design: #2397. Comparison of both: input-output-hk/ouroboros-leios#1132.