Reorganize our documentation #1542
Conversation
- Update the README to reflect the new structure. - Update the frontpage to reflect the contents of the documentation. - Delete all the content. It'll be gradually reintroduced and redistributed when appropriate.
dnadales
requested review from
nfrisby,
jasagredo,
amesgen,
fraser-iohk and
geo2a
as code owners
Most of the information was taken from https://ouroboros-consensus.cardano.intersectmbo.org/docs/for-developers/QueryVersioning/
| @@ -55,7 +55,7 @@ Custom implementations of the Cardano node client are free to bypass this check | |||
|
|
|||
| Our code does not use the negotiated [`NodeToClientVersion`][n2c] directly, but translates them first to a [`CardanoNodeToClientVersion`][cardano-n2c] and then to [`ShelleyNodeToClientVersion`][shelley-n2c]. | |||
|
|
|||
| - The [`querySupportedVersion`][query-supported-version] function assigns a [`ShelleyNodeToClientVersion`][shelley-n2c] to each Shelley-based query. | |||
| - The [`querySupportedVersion`][query-supported-version] function assigns a [`ShelleyNodeToClientVersion`][shelley-n2c] to each Shelley-based query, indicating the minimum *Shelley* version that supports the query. | |||
There was a problem hiding this comment.
-
querySupportedVersionwas removed (or rather, generalized) in ExposequerySupportedVersions#1437, so it makes sense to link to that one instead. -
In general, it might not just be a minimum version, but also a maximum version, or even any subset of versions. Concrete example:
with an explanation of `querySupportedVersions`.
... and a subsection that explains the `ConsensusProtocol` class.
|
The added text looks good and nice to me. |
... explaining the checks that are performed.
|
|
||
| Its main function is [`preferCandidate`](https://github.com/intersectmbo/ouroboros-consensus/blob/010b374c54f4d2f485ab114f702db6ec3b7a8f95/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Protocol/Abstract.hs#L262), which checks if a candidate chain is strictly better than the node’s current chain. This function is used during [initial chain selection](https://github.com/intersectmbo/ouroboros-consensus/blob/010b374c54f4d2f485ab114f702db6ec3b7a8f95/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/ChainDB/Impl/ChainSel.hs#L180) and in [chain selection](https://github.com/intersectmbo/ouroboros-consensus/blob/010b374c54f4d2f485ab114f702db6ec3b7a8f95/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Storage/ChainDB/Impl/ChainSel.hs#L692). | ||
|
|
||
| `ChainOrder` also requires a total order on the `SelectView` type. This allows candidate chains to be [sorted](https://github.com/intersectmbo/ouroboros-consensus/blob/010b374c54f4d2f485ab114f702db6ec3b7a8f95/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Util/AnchoredFragment.hs#L126) for prioritization. |
There was a problem hiding this comment.
The section title says ChainOrdering but this text says ChainOrder.
|
|
||
| The core rule in Ouroboros protocols (including Praos) is to prefer longer chains over shorter ones. This assumes that the honest majority of stake will produce denser chains. | ||
|
|
||
| - A chain that extends the current one is always preferred. |
There was a problem hiding this comment.
Without this change, the sentence could be misinterpreted.
| - A chain that extends the current one is always preferred. | |
| - A chain that extends the current one is always preferable to it. |
|
|
||
| - 2. **VRF Tiebreaker**: | ||
| If the `opcert` check is inconclusive (which is common when two pools are elected in the same slot), the chain with the lower VRF value at its tip is preferred. | ||
| This avoids always picking the first block to arrive, which could encourage centralization to reduce latency (see ["The Frankfurt Problem"](https://github.com/IntersectMBO/ouroboros-consensus/blob/40d77fdd74a9b2b2a1d112a0b836b5cb8026c88c/ouroboros-consensus-protocol/src/ouroboros-consensus-protocol/Ouroboros/Consensus/Protocol/Praos/Common.hs#L227)). The VRF value used for tiebreaking (non-range extended) is uncorrelated to the leader VRF value and typically results in a uniformly random decision. Depending on the `ChainOrderConfig` for Praos, there are two [flavors](https://github.com/intersectmbo/ouroboros-consensus/blob/010b374c54f4d2f485ab114f702db6ec3b7a8f95/ouroboros-consensus-protocol/src/ouroboros-consensus-protocol/Ouroboros/Consensus/Protocol/Praos/Common.hs#L75) that determine when this tie-breaker comparison takes place. |
There was a problem hiding this comment.
centralization -> geographic centralization?
centralization -> network centralization?
centralization -> physical collocation?
There was a problem hiding this comment.
Is there a link we could provide if someone wants to better understand the "non-range extended" qualifier?
There was a problem hiding this comment.
From the three options above, I'd go with either "geographic centralization" or "physical collocation". I think it's better to mention centralization since the contrast with the aspiration of desentralization is more evident (or immediate).
There was a problem hiding this comment.
There was a problem hiding this comment.
Yes, also see footnote 3 in https://hackmd.io/hX7q5s8JSKSP-j3525J0bA
| This avoids always picking the first block to arrive, which could encourage centralization to reduce latency (see ["The Frankfurt Problem"](https://github.com/IntersectMBO/ouroboros-consensus/blob/40d77fdd74a9b2b2a1d112a0b836b5cb8026c88c/ouroboros-consensus-protocol/src/ouroboros-consensus-protocol/Ouroboros/Consensus/Protocol/Praos/Common.hs#L227)). The VRF value used for tiebreaking (non-range extended) is uncorrelated to the leader VRF value and typically results in a uniformly random decision. Depending on the `ChainOrderConfig` for Praos, there are two [flavors](https://github.com/intersectmbo/ouroboros-consensus/blob/010b374c54f4d2f485ab114f702db6ec3b7a8f95/ouroboros-consensus-protocol/src/ouroboros-consensus-protocol/Ouroboros/Consensus/Protocol/Praos/Common.hs#L75) that determine when this tie-breaker comparison takes place. | ||
| - `UnrestrictedVRFTiebreaker`: With this flavor, VRF tiebreakers are always compared. This has been the standard behavior for all eras before Conway. | ||
| - `RestrictedVRFTiebreaker`: This flavor restricts the VRF tiebreaker comparison to situations where the slot numbers of the chain tips differ by at most a specified maximum distance (`maxDist`). | ||
| The primary motivation for this restriction is to favor blocks that were diffused earlier (in earlier slots) over those diffused later, even if the later block has a "better" VRF tiebreaker value. This aims to mitigate [issues](https://github.com/IntersectMBO/ouroboros-network/issues/2913) caused by poorly configured or resource-constrained pools that might diffuse blocks later. |
There was a problem hiding this comment.
The primary motivation for this restriction is to favor blocks that were diffused earlier (in earlier slots) over those diffused later
The "in earlier slots" clarification seems wrong to me, but I haven't thought about this in a while.
- Suppose I'm elected in X and somebody else is elected in Y, such that Y > X + maxDist.
- If a node selects the other person's block before the receive mine, then they would not switch to mine even though mine is from an earlier slot.
| | `opcert` number | 2 | j | 1 | | ||
| | VRF | 3 | 2 | 1 | | ||
|
|
||
| Lower-case letters stand for arbitrary values (two letters designate the same value if and only if they are same letter). |
There was a problem hiding this comment.
two letters designate the same value if and only if they are same letter
It's an irrelevant distraction, but: it'd be harmless if different letters in different rows had the same value.
There was a problem hiding this comment.
Makes sense, I'll remove it 👍
|
|
||
| - `B` is preferred over `A`, since `B` has lower VRF than `A`. | ||
| - `C` is preferred over `B`, since `C` has lower VRF than `B`. | ||
| - However `C` is **not** preferred over `A`, since they have the same issuer and slot, and therefore we prefer the chain with the highest `opcert` number (2), therefore `A` is preferred over `C`. |
There was a problem hiding this comment.
If you have the same issuer and the same slot, then the VRF value must be equivalent.
There was a problem hiding this comment.
Right 😬 Changing the slot in the example should work right?
There was a problem hiding this comment.
I am not actually sure what exactly you want to demonstrate here, given that you are not using the example from #1075 (comment). It (or variants thereof) are the only source of non-transitivity of UnrestrictedVRFTiebreaker (and we should just fix that).
|
|
||
|
|
||
| We have that: | ||
| - `E` is preferred over `D`, since `E` has lower VRF than `D` and `|0 - 3| < 5`. |
There was a problem hiding this comment.
If the name is maxFoo, then I assume that's an inclusive upper bound, and so it should be <= instead of <.
Edit: However you're chosen the numbers such that the = case is irrelevant. 🤷
There was a problem hiding this comment.
Yes, good point, I can change it to <=.
| - `F` is preferred over `E`, since `F` has lower VRF than `E` and `|3 - 6| < 5` | ||
| - However, `D` is **not** preferred over `F`, but instead `D` and `F` are equally preferred since `|0 - 6| > 5` which implies that the VRF values of `D` and `F` are not used. | ||
|
|
||
| Despite the non-transitivity, the fundamental Consensus properties, such as [Common Prefix](TODO-ref!), are not affected. This is because the primary factor for chain selection for a [caught-up](TODO-ref!) node remains the chain length, with longer chains always being preferred. However, a non-transitive chain ordering brings the following complications: |
There was a problem hiding this comment.
This paragraph is where we'd normally say something like "the Ouroboros Praos authors make no assumptions about how nodes resolve ties". That's been stated colloquially as long as I can remember. However, if I recall correctly, the rules that the paper proves the security argument for does use a specific tiebreaker: favor first to arrive.
It's folklore, though, that "the proof itself doesn't really depend on that particular choice". And it's pretty clear that "favor first to arrive" is not transitive/objective/etc.
There was a problem hiding this comment.
Is it too terse if I add the sentence above in this paragraph?
Despite the non-transitivity, the fundamental Consensus properties, such as Common Prefix, are not affected. This is because the primary factor for chain selection for a caught-up node remains the chain length, with longer chains always being preferred.
The Ouroboros Praos authors make no assumptions about how nodes resolve ties.
However, a non-transitive chain ordering brings the following complications:
There was a problem hiding this comment.
However, if I recall correctly, the rules that the paper proves the security argument for does use a specific tiebreaker: favor first to arrive.
The proofs in the Praos paper actually don't assume anything about the tiebreaker (so in particular work when the ties are broken by the adversary):
Function
$\mathrm{maxvalid}(C, C)$ : Returns the longest chain from$C \cup \{C\}$ . Ties are broken in favor of$C$ , if it has maximum length, or arbitrarily otherwise.
(There are other papers that assume specific tiebreakers, eg this one which proves security in systems where there only are either empty or multi-leader slots.)
|
|
||
| Despite the non-transitivity, the fundamental Consensus properties, such as [Common Prefix](TODO-ref!), are not affected. This is because the primary factor for chain selection for a [caught-up](TODO-ref!) node remains the chain length, with longer chains always being preferred. However, a non-transitive chain ordering brings the following complications: | ||
|
|
||
| - **Implementation**: The use of `sortBy` from base in `chain` selection, which is typically expected to work with transitive relations, could become a concern. While preliminary property tests suggest it works for the current non-transitive order, there's a theoretical risk that future GHC implementations might interact non-trivially. |
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks. For the last suggestion, I used "might behave in unexpected ways". Let me know if you prefer your version.
| Despite the non-transitivity, the fundamental Consensus properties, such as [Common Prefix](TODO-ref!), are not affected. This is because the primary factor for chain selection for a [caught-up](TODO-ref!) node remains the chain length, with longer chains always being preferred. However, a non-transitive chain ordering brings the following complications: | ||
|
|
||
| - **Implementation**: The use of `sortBy` from base in `chain` selection, which is typically expected to work with transitive relations, could become a concern. While preliminary property tests suggest it works for the current non-transitive order, there's a theoretical risk that future GHC implementations might interact non-trivially. | ||
| - **Reasoning**: The non-transitive order can be very confusing for reasoning about anything related to chain order, as transitivity is an implicitly assumed property of orders. |
There was a problem hiding this comment.
suggestion: transitivity is most often an implicitly assumed property when working with "orders"
There was a problem hiding this comment.
What about:
The non-transitive order can be very confusing for reasoning about anything related to chain order, as transitivity is an implicitly assumed property when working with order relations.
| - **Implementation**: The use of `sortBy` from base in `chain` selection, which is typically expected to work with transitive relations, could become a concern. While preliminary property tests suggest it works for the current non-transitive order, there's a theoretical risk that future GHC implementations might interact non-trivially. | ||
| - **Reasoning**: The non-transitive order can be very confusing for reasoning about anything related to chain order, as transitivity is an implicitly assumed property of orders. | ||
| This, in turn, leads to "obvious" properties failing to hold. For instance, the expectation that observing a node's selection over time yields a strictly improving sequence may not hold, as different observers could disagree on whether each selection is "strictly better" than the previous one. This non-objectivity can have practical effects, particularly for diffusion pipelining, which relies on a clear, consistent chain order. | ||
| - **Potential for Cycles**: The non-transitivity can conceptually give rise to "cycles" in preference, such as `A < B < C = A`. However, in practice, the node's implementation guarantees that it will never end up changing its selection in such a cycle because blocks already in the VolatileDB are not added again |
There was a problem hiding this comment.
Missing a final full stop . in this sentence.
|
|
||
| ### On the Transitivity of Praos Chain Order | ||
|
|
||
| Praos chain ordering is **not transitive**, regardless of the VRF tiebreaker flavor. |
There was a problem hiding this comment.
This is technically true because of #1075 (comment), but this is just a bug that should be fixed.
I think it is important to mention two aspects of the chain order here:
- The
Ordinstance onPraosChainSelectViewmust be a total order (and in particular transitive). The fact that is currently isn't in a weird edge case (Fix innocent non-transitivity of chain order related to issue numbers #1075) is just a bug. preferCandidateis not defined in terms of theOrdinstance for Praos, and is not transitive when usingRestrictedVRFTiebreaker, but this is fine.
| The Ouroboros Praos authors make no assumptions about how nodes resolve ties. | ||
| However, a non-transitive chain ordering brings the following complications: | ||
|
|
||
| - **Implementation**: The use of `sortBy` from [`base`](https://hackage.haskell.org/package/base) in chain selection, which is typically expected to work with transitive relations, could become a concern. While preliminary property tests suggest it works for the current non-transitive order, there's a potential risk that future GHC implementations might behave in unexpected ways. |
There was a problem hiding this comment.
This is no longer a concern due to #1063, also see #891 (comment)
| Also, the `RestrictedVRFTiebreaker` flavour breaks the transitivity of chain ordering. To see this consider the following example where chains `D`, `E`, and `F` have the same length and different issuers, and assume `maxDist = 5` slots: | ||
|
|
||
| | | D | E | F | | ||
| | ------------ | - | - | - | | ||
| | Slot | 0 | 3 | 6 | | ||
| | VRF | 3 | 2 | 1 | | ||
|
|
||
|
|
||
| We have that: | ||
| - `E` is preferred over `D`, since `E` has lower VRF than `D` and `|0 - 3| <= 5`. | ||
| - `F` is preferred over `E`, since `F` has lower VRF than `E` and `|3 - 6| <= 5` | ||
| - However, `D` is **not** preferred over `F`, but instead `D` and `F` are equally preferred since `|0 - 6| > 5` which implies that the VRF values of `D` and `F` are not used. |
There was a problem hiding this comment.
I think it makes sense to point out explicitly that RestrictedVRFTiebreaker is used only for preferCandidate, but not for Ord PraosChainSelectView.
| - **Implementation**: The use of `sortBy` from [`base`](https://hackage.haskell.org/package/base) in chain selection, which is typically expected to work with transitive relations, could become a concern. While preliminary property tests suggest it works for the current non-transitive order, there's a potential risk that future GHC implementations might behave in unexpected ways. | ||
| - **Reasoning**: The non-transitive order can be very confusing for reasoning about anything related to chain order, as transitivity is an implicitly assumed property when working with order relations. | ||
| This, in turn, leads to "obvious" properties failing to hold. For instance, the expectation that observing a node's selection over time yields a strictly improving sequence may not hold, as different observers could disagree on whether each selection is "strictly better" than the previous one. This non-objectivity can have practical effects, particularly for diffusion pipelining, which relies on a clear, consistent chain order. | ||
| - **Potential for Cycles**: The non-transitivity can conceptually give rise to "cycles" in preference, such as `A < B < C = A`. However, in practice, the node's implementation guarantees that it will never end up changing its selection in such a cycle because blocks already in the `VolatileDB` are not added again. |
There was a problem hiding this comment.
It might be useful to mention here that the "Consistency with Ord" requirement of the ChainOrder class guarantees that we never can have preferCandidate A B, preferCandidate B C and preferCandidate C A (so no "cycles of preference").
|
|
||
| - `B` is preferred over `A`, since `B` has lower VRF than `A`. | ||
| - `C` is preferred over `B`, since `C` has lower VRF than `B`. | ||
| - However `C` is **not** preferred over `A`, since they have the same issuer and slot, and therefore we prefer the chain with the highest `opcert` number (2), therefore `A` is preferred over `C`. |
There was a problem hiding this comment.
I am not actually sure what exactly you want to demonstrate here, given that you are not using the example from #1075 (comment). It (or variants thereof) are the only source of non-transitivity of UnrestrictedVRFTiebreaker (and we should just fix that).
|
|
||
| In this example we have: | ||
| To see why `RestrictedVRFTiebreaker` flavour breaks the transitivity of chain ordering, consider the following example where chains `D`, `E`, and `F` have the same length and different issuers, and assume `maxDist = 5` slots: |
There was a problem hiding this comment.
| To see why `RestrictedVRFTiebreaker` flavour breaks the transitivity of chain ordering, consider the following example where chains `D`, `E`, and `F` have the same length and different issuers, and assume `maxDist = 5` slots: | |
| To see why `RestrictedVRFTiebreaker` flavour breaks the transitivity of chain ordering, consider the following example where chains `A`, `B`, and `C` have the same length and different issuers, and assume `maxDist = 5` slots: |
|
|
||
| The [`blockForgingController`](https://github.com/intersectmbo/ouroboros-consensus/blob/6c1f0e293b50b898e69116df0355595004432077/ouroboros-consensus-diffusion/src/ouroboros-consensus-diffusion/Ouroboros/Consensus/NodeKernel.hs#L375) function uses [`BlockChainTime`](https://github.com/intersectmbo/ouroboros-consensus/blob/6c1f0e293b50b898e69116df0355595004432077/ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/BlockchainTime/API.hs#L23) to monitor the current slot. | ||
|
|
||
| Block forging invokes `forecastFor` on the `LedgerView`. If this operation takes too long, it can delay block production. Note that if it’s not possible to forecast to the current slot, then forging a block is not possible. |
There was a problem hiding this comment.
| Block forging invokes `forecastFor` on the `LedgerView`. If this operation takes too long, it can delay block production. Note that if it’s not possible to forecast to the current slot, then forging a block is not possible. | |
| Block forging invokes `forecastFor` for the current slot on the `LedgerView`. If this operation takes too long, it can delay block production. Note that if it’s not possible to forecast to the current slot, then forging a block is not possible. |
|
|
||
| Block forging invokes `forecastFor` on the `LedgerView`. If this operation takes too long, it can delay block production. Note that if it’s not possible to forecast to the current slot, then forging a block is not possible. | ||
|
|
||
| If `checkLeader` confirms the node is a leader, a block is forged, extending the current [selected chain](#chain-selection). Transactions are selected from a [mempool snapshot](TODO-ref!), which is a sequence of valid transactions consistent with the ledger state upon which the block will be built. For the purposes of block forging and the consensus protocol, these transactions are irrelevant as long as they are _valid_. |
There was a problem hiding this comment.
| If `checkLeader` confirms the node is a leader, a block is forged, extending the current [selected chain](#chain-selection). Transactions are selected from a [mempool snapshot](TODO-ref!), which is a sequence of valid transactions consistent with the ledger state upon which the block will be built. For the purposes of block forging and the consensus protocol, these transactions are irrelevant as long as they are _valid_. | |
| If `checkLeader` confirms the node is a leader, a block is forged, extending the current [selected chain](#chain-selection). Transactions are selected from a [mempool snapshot](TODO-ref!), which is a sequence of valid transactions consistent with the ledger state upon which the block will be built. For the purposes of block forging and the consensus protocol, these transactions are irrelevant as long as they are _valid from the Ledger point of view_. |
|
|
||
| The `k` parameter, often referred to as the security parameter, is a fundamental constant in the Ouroboros consensus protocols. On Cardano mainnet, `k` is set to 2160 blocks. This parameter underpins various aspects of a Cardano node's operation, from chain selection and storage to security guarantees and performance. | ||
|
|
||
| For Ouroboros Praos, used in Shelley-based eras, theoretical proofs guarantee that within `3k/f` slots, the chain will grow by at least `k` blocks. |
There was a problem hiding this comment.
Mention or link the Chain Growth Property
|
|
||
| ### Chain Selection and Rollbacks | ||
|
|
||
| The `ouroboros-consensus` implementation enforces a strict rule: a node will never switch to a chain that forks more than `k` blocks deep from its current tip. |
There was a problem hiding this comment.
Mention again the CP property
|
|
||
| When a node is [caught up](TODO-ref), any candidate chains requiring a rollback exceeding `k` blocks are excluded from consideration. This constraint limits the number of ledger states that need to be retained to evaluate forks. Additionally, it prevents arbitrary-length rollbacks thus protecting against potential [attacks](#network-security-and-performance) that rely on this. | ||
|
|
||
| When a node is [syncing](TODO-ref) with the chain, an adversary may present it with an alternative chain that forks more than `k` blocks from the current tip. However, the [Genesis](TODO-ref) syncing protocol ensures that a node joining the network does not switch to an adversarial chain. |
There was a problem hiding this comment.
| When a node is [syncing](TODO-ref) with the chain, an adversary may present it with an alternative chain that forks more than `k` blocks from the current tip. However, the [Genesis](TODO-ref) syncing protocol ensures that a node joining the network does not switch to an adversarial chain. | |
| When a node is [syncing](TODO-ref) with the chain, an adversary may present it with an alternative chain that forks more than `k` blocks from the server's current tip, which would be rejected by a caught up node. However, the [Genesis](TODO-ref) syncing protocol ensures that a node joining the network does not switch to an adversarial chain. |
or something similar
| However, Ouroboros involves short forks and potentially invalid blocks. | ||
| On the other hand, even for a single chain, the Consensus Layer would forecast across the same slots multiple times: it does it each time the wall clock enters a new slot as part of the leadership check, and it does so each time any peer sends it a header. | ||
|
|
||
| Ideally all ticks would be inexpensive, but the occasional spike is managable, since short forks are short lived. |
| ## Bounding Ticking and Forecasting Computations | ||
|
|
||
| On a single valid chain, the Consensus Layer will never tick across the same slots multiple times. | ||
| However, Ouroboros involves short forks and potentially invalid blocks. |
There was a problem hiding this comment.
However, Ouroboros necessarily involves both short forks and potentially invalid blocks, and even for a single chain, [...]
| However, Ouroboros involves short forks and potentially invalid blocks. | ||
| On the other hand, even for a single chain, the Consensus Layer would forecast across the same slots multiple times: it does it each time the wall clock enters a new slot as part of the leadership check, and it does so each time any peer sends it a header. | ||
|
|
||
| Ideally all ticks would be inexpensive, but the occasional spike is managable, since short forks are short lived. |
| It could plausibly cause problems within the ledger rules if an entire epoch has no blocks, for example, so there's definitely no guarantee that the resulting ledger state would actually be useful. | ||
| But again: the Consensus Layer will never request such a tick. |
There was a problem hiding this comment.
Note
can these lines be moved into a [!note] block?
| ## Query versioning | ||
|
|
||
| Queries are versioned to ensure compatibility between nodes and clients running different software versions. | ||
| As the ledger gets more features and new use cases are explored, teams will add new queries that allow to get the necessary data. |
There was a problem hiding this comment.
"that allow getting the necessary data" or (probably better) "allow the client to get the necessary data"
| @@ -0,0 +1,37 @@ | |||
| # How to version a new query | |||
|
|
|||
| 1. Determine whether the query is supposed to be experimental. | |||
There was a problem hiding this comment.
"should be experimental" or "is intended to be experimental"
|
|
||
| -- | The 'ChainOrder' type class's 'preferCandidate' function | ||
| -- consumes both the 'SelectView's of competing chain tips and the | ||
| -- 'ChainOrderConfig' to make the final decision on which chain is | ||
| -- strictly preferred. This design allows for flexible and | ||
| -- protocol-specific tie-breaking rules that go beyond simple chain | ||
| -- length comparisons. |
There was a problem hiding this comment.
this extra comment is good but it's made me unsure about whether the design of the ChainOrder class makes sense -- should it instead have projectBlockNo and tiebreakCandidates functions, and preferCandidates could be removed from the class and separately enforce the "longer chains are always preferred" property?
(this is a bit unrelated to the actual documentation changes, so it might be better discussed somewhere else)
There was a problem hiding this comment.
Yeah, I think this has come up in the past; I think everyone agreed that this would be neat (I don't remember why we didn't follow through with it; probably because it was relatively minor and SelectView containing the BlockNo has been the case forever). Eg concretely
data SelectView proto = SelectView {
svBlockNo :: BlockNo
, svTiebreaker :: TiebreakerView proto
}where TiebreakerView is an associated type family of ConsensusProtocol with ChainOrder (TiebreakerView proto) as a superclass constraint, and instances
Ord (TiebreakerView proto) => Ord (SelectView proto).ChainOrder (TiebreakerView proto) => ChainOrder (SelectView proto).
I was actually considering doing sth like this in the context of Peras, as doing weighted comparisons between chains is naturally modeled by adding the additional weight to the BlockNo component, and it is a bit weird that it currently isn't directly accessible.
EDIT: #1591
| Ensuring the thorough testability of the consensus layer is a critical design goal. | ||
| As a core component of the Cardano Node, which manages the cryptocurrency, the consensus layer must adhere to strict correctness standards. | ||
| Currently, we extensively employ property-based testing. | ||
| Whenever possible, we should abstract over IO, enabling simulations of various failures (such as disk or network errors) to verify system recovery capabilities. |
There was a problem hiding this comment.
probably worth mentioning IOSim and io-classes here specifically
This PR does not change any behavior; most of the diff is purely
mechanical.
Previously, the `SelectView`s of all `ConsensusProtocol`s contained the
`BlockNo` as the most important criterion for comparing chains, with
only tiebreaking behavior (among chains of equal length) being different
across different protocols.
This PR makes this explicit: `SelectView` is now longer an associated
type family of `ConsensusProtocol`, but rather an ordinary data type
```haskell
data SelectView p = SelectView
{ svBlockNo :: !BlockNo
, svTiebreakerView :: !(TiebreakerView p)
}
```
where `TiebreakerView` is now an associated type family of
`ConsensusProtocol`.
This PR also removes the `WithBlockNo` type that the HFC was already
using to always attach a `BlockNo` to its `HardForkSelectView`. The code
can be simpler now that `SelectView`s always automatically contain an
explicit `BlockNo`.
Also see
#1542 (comment)
## Primary motivation
The main motivation for this is to enable weighted chain selection in
Peras (tweag/cardano-peras#62), which can now
be easily modeled by adding the weight of a chain to the block number
component, at least morally; we probably want to eventually have more
type safety here to make it hard to "forget" to account for the weight,
such as defining
```haskell
data WeightedSelectView p = WeightedSelectView
{ wsvWeight :: !PerasWeight
, wsvTiebreakerView :: !(TiebreakerView p)
}
```
If the only goal were to minimize the diff, then it would also be
possible to add a function `(BlockNo -> BlockNo) -> SelectView p ->
SelectView p`, but it seems more honest to "properly" extract out the
block number.
This PR reorganizes the Consensus documentation, integrating information from various sources such as the Consensus report, Haddock comments, and existing documentation. The documentation is structured using the Diátaxis framework. Implementation-specific concepts will be moved to the Cardano Blueprints.