Johnalanwoods/block diffusion pipelining #3579
Conversation
johnalanwoods
requested review from
bolt12,
coot,
EncodePanda,
karknu,
nfrisby and
njd42
as code owners
|
The changes added to the doc are all in this commit: 756fdd6 |
|
@johnalanwoods I think a better place for this document is |
Moved! :) |
| for Ouroboros chain diffusion that, if implemented, could facilitate higher block validation times | ||
| for the same choice of the $\Delta$~diffusion time. | ||
| Ergo, we would enjoy a faster $\Delta$~diffusion time in the context of our current block validation requirements. | ||
| This additional budget may be spent on increasing block-size, and/or increasing CPU \& memory limits for Plutus scripts. |
There was a problem hiding this comment.
Minor: I think no comma in "block-size, and/or"
| for the same choice of the $\Delta$~diffusion time. | ||
| Ergo, we would enjoy a faster $\Delta$~diffusion time in the context of our current block validation requirements. | ||
| This additional budget may be spent on increasing block-size, and/or increasing CPU \& memory limits for Plutus scripts. | ||
| This document will attempt to analyse the algorithm, and review its implications. The document also discusses which |
There was a problem hiding this comment.
Minor: I think no comma in "algorithm, and review"
| By propagating forward a block as soon as it has arrived locally (and its header is validated), the block | ||
| validation time is overlapped with the propagation time and the overall timeline becomes materially shorter. | ||
|
|
||
| \begin{center} |
There was a problem hiding this comment.
I think these pictures are hard to understand. In particular, the pictures in this chapter do not show that a node is simultaneously validating the block while also transmitting the header and body. I think we're only seeing the validation time and the tail part of the block transmission that ends after block validation completes (even though transmission might end before validation, yeah?). Relatedly: I think header transmission is not even shown (there are only 5 rectangles instead of 6 from the previous chatper's pictures).
Ideally, the picture could be enriched (eg with a thatched section implying an overlaid gray and white rectangles) (eg dividing each node's horizontal bar into three thinner lanes, one for header transmission, one for block validation, and one for block transmission -- the only rule is that block transmission comes after block validation header transmission; header transmission and block validation could and would likely begin simultaneously).
There was a problem hiding this comment.
yep I will try and add to this later
|
|
||
| The combination of checks 1.~and 2.~above should mean that the possibly-invalid | ||
| block is one which was signed by the block producer (KES validation), who (in-turn) was | ||
| authorised to create a block in this slot (VRF verification). This should mean that |
There was a problem hiding this comment.
This should mean that there cannot be a greater number of invalid blocks, in total, than in the case of valid blocks.
I'm not sure I follow this sentence. I'm guessing it's the "number of" and/or "in total" part I'm not grokking.
The proposal doesn't create new opportunities for minting invalid blocks. But it does -- for the first time ever -- allow honest nodes to transmit invalid blocks.
So, what is being counted in the words "greater number"?
There was a problem hiding this comment.
the leader schedule! which bounds block creation.
| authorised to create a block in this slot (VRF verification). This should mean that | ||
| there cannot be a greater number of invalid blocks, in total, than in the case of valid blocks. In | ||
| particular intermediate nodes cannot forge any extra or altered block(s) without | ||
| falling afoul of these checks. Thus, it should be the case that only the |
There was a problem hiding this comment.
Thus, it should be the case that only the authorised block producer can create many valid blocks, or many valid block headers with invalid block bodies.
Right, this is also something that was true before the proposal and remains true after the proposal.
| \section{Implementation changes} | ||
| The implementation changes required to deliver on the approach discussed in this document | ||
| are all to be made within the Consensus layer. The discussion in this section pertains specifically | ||
| to the "non-aggressive" diffusion pipelining variant, the specific novel behaviour here is, nodes now |
There was a problem hiding this comment.
Minor: I think the comma here "variant, the specific novel" should instead be a full stop
| \section{Implementation changes} | ||
| The implementation changes required to deliver on the approach discussed in this document | ||
| are all to be made within the Consensus layer. The discussion in this section pertains specifically | ||
| to the "non-aggressive" diffusion pipelining variant, the specific novel behaviour here is, nodes now |
There was a problem hiding this comment.
Minor: I think the comma here "here is, nodes" should be replaced by the word "that"
| \item With Praos, a node will adopt the chain which is most dense and whose intersection with the previous "best" known chain is less than $k$ blocks, where $k$ is currently set to 2160. | ||
| \item Cognisant of the above, the process continues as follows: | ||
| \begin{enumerate} | ||
| \item A node will notify interested peers that a new block is available by posting the new header to \lstinline{chainSync}, after block validation. |
There was a problem hiding this comment.
Minor: we tend to write ChainSync, camel case and without any font adjustments.
For broader audiences, I would tend to write "the ChainSync mini protocol".
| \begin{enumerate} | ||
| \item A node will notify interested peers that a new block is available by posting the new header to \lstinline{chainSync}, after block validation. | ||
| \item After validation of the header locally, the downstream peer will request/pull the corresponding block body. | ||
| \item The block is validated, and if it satisfies checks resulting in a new longer/better chain, it will be appended as the head of our "new" chain. |
There was a problem hiding this comment.
I would say "selected" instead of "appended", since selecting it might require a switch (ie some rollback), not just extension.
| \item We must of course, ensure faster than real-time syncing of the chain. This is not an issue for the proposed implementation herein, however it's something we must be cognisant of in the future. | ||
| \item The changes proposed herein don’t affect the structural properties of the chain. | ||
| \item Intuitively one may be inclined to suggest a change to minting policy, given that with pipelining an SPO now \emph{knows} that a potentially better chain exists to extend. However, knowledge of this reality (\emph{which existed prior to pipelining}), should not alter minting behaviour. | ||
| \item We should not expedite/pipeline a subsequent block until the previously pipelined block is validated, i.e. do not pipeline multiple blocks concurrently. |
There was a problem hiding this comment.
I think it'd help to have a second sentence here explaining why we don't think this would be worth the extra complexity.
ouroboros-consensus/docs/diffusion-pipelining-design/block-diffusion-pipelining.tex
Outdated
Show resolved
Hide resolved
… added additional context, refactored entire document
johnalanwoods
force-pushed
the
johnalanwoods/block-diffusion-pipelining
branch
from
06c3f52
to
4baf4ee
Compare
|
@nfrisby - all comments addressed, and code now rebased on latest master branch. |
| \maketitle | ||
|
|
||
| \section{Purpose} | ||
| The purpose of this document is to present a proposed change to the algorithm |
There was a problem hiding this comment.
| The purpose of this document is to present a proposed change to the algorithm | |
| The purpose of this document is to propose a change to the algorithm |
|
|
||
| \section{Purpose} | ||
| The purpose of this document is to present a proposed change to the algorithm | ||
| for Ouroboros chain diffusion that, if implemented, could facilitate higher block validation times |
There was a problem hiding this comment.
Doesn't "higher block validation times" sound negative? What about "could allow for more time to validate blocks".
| for the same choice of the $\Delta$~diffusion time. | ||
| Ergo, we would enjoy a faster $\Delta$~diffusion time in the context of our current block validation requirements. | ||
| This additional budget may be spent on increasing block-size and/or increasing CPU \& memory limits for Plutus scripts. | ||
| This document will attempt to analyse the algorithm and review its implications. The document also discusses which |
There was a problem hiding this comment.
| This document will attempt to analyse the algorithm and review its implications. The document also discusses which | |
| This document will analyse the algorithm and review its implications. The document also discusses which |
| Network and Consensus teams, these ideas multiplied into a range of plausible options. | ||
|
|
||
| A particularly radical idea, referred to by research as "heretic pipelining" (read: aggressive), is | ||
| a pipelined augmentation of the Praos algorithm which requires that we handle invalid/partially invalid blocks. |
There was a problem hiding this comment.
Might be worth clarifying what do we mean by "handle" in this context. We do handle invalid blocks nowadays, it's just that we don't do anything with them.
| design/implementation, without the need to handle invalid blocks, yet that nevertheless, may provide a substantial benefit. | ||
|
|
||
| \section{Introduction} | ||
| The key idea is to diffuse block bodies \emph{before} having fully validated |
There was a problem hiding this comment.
Why not simply "without validating them". The "fully" word seems to imply that we perform some validation of the bodies. Maybe it's the check that the header matches the body part of that validation "fully" seems to imply?
| \section{Risk analysis} | ||
| Before we analyse the risks it is important to be precise about which validations | ||
| will still be executed as normal, and which would be deferred. The following are | ||
| validations which should still be done: |
There was a problem hiding this comment.
Hmmm, this seems to imply that block body validation won't be done. What about "the following are validations that will not be deferred".
| to not consider any more blocks signed by this issuer in this slot. They | ||
| have demonstrated themselves to be adversarial by signing a bad block, and we | ||
| shouldn't spend any further resources validating blocks from them in the short term. | ||
| Note that for this purpose we must identify the block issuer by the hot key certificate and |
There was a problem hiding this comment.
and a guess we're ok with letting a compromised cold-key keep generating invalid blocks in the future (we will detect this, and blacklist the hot-key for that slot).
| The implementation changes required to deliver on the approach discussed in this document | ||
| are all to be made within the Consensus layer. The discussion in this section pertains specifically | ||
| to the "non-aggressive" diffusion pipelining variant. The specific novel behaviour is that nodes now | ||
| forward \emph{one unvalidated block}, so that multiple nodes can download and validate the latest |
There was a problem hiding this comment.
| forward \emph{one unvalidated block}, so that multiple nodes can download and validate the latest | |
| forward \emph{at most one unvalidated block}, so that multiple nodes can download and validate the latest |
|
|
||
| \begin{itemize} | ||
| \item With Ouroboros Praos, we want the block minted in a given slot to diffuse to the leader of the next slot early enough that they in-turn can extend the chain on-top of it. | ||
| \item With Praos, a node will adopt the chain which is most dense and whose intersection with the previous "best" known chain is less than $k$ blocks, where $k$ is currently set to 2160. |
There was a problem hiding this comment.
Aren't we going to consider density only when we move to Genesis? I thought at the moment we only considered chains length.
| \begin{enumerate} | ||
| \item A node will notify interested peers that a new block is available by posting the new header to \lstinline{ChainSync} mini-protocol, after block validation. | ||
| \item After validation of the header locally, the downstream peer will request/pull the corresponding block body. | ||
| \item The block is validated, and if it satisfies checks resulting in a new longer/better chain, it will be selected as the head of our "new" chain. |
There was a problem hiding this comment.
The blocks might arrive out of order, in which case a successful block validation might change the current chain, or extend it, but it does not necessarily mean that the newly validated block is the "head" of our new chain.
Also about the notation, I think we might prefer "tip" as opposed to "head" to refer to the newest element in a chain.
| \begin{itemize} | ||
| \item The aim is to expedite data propagation where appropriate, and to do so without destructive change. | ||
| \begin{enumerate} | ||
| \item Extend the current API to pre-notify subscribed threads when a new desirable block is seen, prior to block body validation. |
There was a problem hiding this comment.
Would it make sense to clarify which API are we referring to here?
|
Block diffusion pipelining (first version) has been implemented in #3688 |
|
@amesgen This branch contains the specification for pipelining. IMHO it would be better to have the doc updated and merged instead of having it forgotten. |
|
I agree! |
|
Thanks for pointing that out, just created IntersectMBO/ouroboros-consensus#559 to track that effort 👍 |
Completely refactored the architectural document on diffusion pipelining (non-heretic), and extended it to include a new implementation approach.