WIP Spec: Draft for top-level p2p spec #80
Conversation
Co-authored-by: Lasaro Camargos <lasaro@informal.systems>
Co-authored-by: Lasaro Camargos <lasaro@informal.systems>
…int into josef/i9573-josef-p2p
…ated things I will fix later
| client of the blockchain but has not yet been committed to the blockchain. | ||
| The mempool is thus fed with client transactions, | ||
| that a priori can be submitted to any node in the network. | ||
| And it is consumed by the consensus protocol, more specifically by validator nodes, |
There was a problem hiding this comment.
Consensus shouldn't have to know about transactions, requiring only that the mempool return a potential block payload. The fact that consensus knows transactions should be a detail of our implementation.
There was a problem hiding this comment.
I'm not sure if I agree. Transactions are values for consensus. Blockchain consensus protocol do batching, a block is a batch of values.
There was a problem hiding this comment.
I understand but I don't see the value in that as the application itself can do the batching, as the SDK does, specially with ABCI 1 and the application's ability to redo the whole block. But I guess a better point is that the mempool shouldn't have to know about who is consuming its transactions, only that they may be reaped and become invalid due to the transactions being executed or the app state changing.
Co-authored-by: Lasaro <lasaro@informal.systems>
Co-authored-by: Lasaro <lasaro@informal.systems>
|
|
||
| #### [CM-REQ-MEMP-GOSSIP.0] | ||
|
|
||
| 1. every transaction submitted to a correct full node must eventually be included in the mempool of a correct validator |
There was a problem hiding this comment.
We should figure out whether we want the same guarantees for transactions submitted to any fullnode, of e.g. for MEV, you submit your transaction to a special full node (or a set of full nodes) and you get better guarantees. Best effort transactions vs. transactions with guarantees.
Might be a question for the mempool spec.
There was a problem hiding this comment.
Daniel: Define stable nodes vs. transient nodes is important. We can give guarantees wrt stable nodes, while we allow transient nodes.
| on the mempool, e.g., every transaction that is submitted to a full node should | ||
| eventually be present at the mempool of a validator/proposer. |
There was a problem hiding this comment.
Thanks for looking at the document and giving feedback. Please note that this is a working document that should describe requirements one may expect from the reactors in order to formalize, in turn, what the reactors expect from the p2p system below.
By describing stronger properties (than those we actually need) here, and deriving properties for p2p from them, we can eventually specify a p2p system that may do a bit more than is needed and thus suffices for what the reactors need in reality. We had noted that on line 104, which says
Note that in this document, we want to capture (an overapproximation of) the requirements the mempool puts on p2p.
We will try to make this clearer in a revision to avoid future confusion.
There was a problem hiding this comment.
That's all fine, I think. But it is not the case that "every transaction that is submitted to a full node should eventually be present at the mempool of a validator/proposer".
| into requirements on the p2p layer that are similar to those of the | ||
| consensus reactor. | ||
|
|
||
| - end-to-end requirement: "every transaction submitted should be eventually put into a block" |
| - current solution to address this requirement: | ||
| - the mempool of correct full nodes is organized as a list. | ||
| This list can be understood as a shared data structure with relaxed semantics, that is, | ||
| not all nodes need to agree on the order of the elements, but we have a property that | ||
| approximately says "if at time *t* a transaction tx1 is in the list of **all** nodes, and a | ||
| client submits transaction tx2 after time t, then it is never the case the tx2 is before | ||
| tx1 in a list of a correct validator. |
There was a problem hiding this comment.
This property is not provided by the current spec or implementation. It is not possible to guarantee in a weakly-connected, eventually-consistent, gossip-based network as is defined by the p2p layer.
There was a problem hiding this comment.
Not sure I understand the comment. Are you worried about the "if" part or the "then" part here? My understanding is that the property is actually quite week as the "if" part might be hard to be satisfied, so this condition holds quite easily.
There was a problem hiding this comment.
I think this may boil down to the definition of a "correct" node. If "correct" means that a node will reliably receive any message gossiped from some other "correct" node within some time bound, then "correct" is relative to an observer, it has no well-defined value in general, and it can't be used to assert stuff like what is asserted here.
If you want to define "correct" in this way, and use that definition to say things like "it is never the case that tx2 is before tx1 among correct nodes", then you're defining a node that suffers any kind of network error to be incorrect, which is unknowable by the protocol as evaluated at any given node.
It is entirely possible that a transaction arrives in the mempool of only (say) 70% of the validators in a network, and if the proposer is one of those 70% of validators that transaction gets put into a proposed block, and that block is validated and committed, all without that transaction ever entering the mempool of (say) 30% of the validators of that network. Those 30% validators can fail to receive the gossip messages that would put the transaction in their mempool, while at the same time successfully receiving (and approving) the consensus messages required to commit a block which included that transaction to the chain.
So "if at time t1 a transaction tx1 is in the list of all nodes" is never possible to assume, because there is no situation where any given transaction is guaranteed to be known by all validators, unless it is put through consensus (and even then it is only guaranteed to be known to the approving quorum); "and a client submits transaction tx2 after time t1" is not something that can even be evaluated, as times t1 and t2 are not reliably comparable, and even if they were that comparison can only be made by nodes which know about both tx1 and tx2, which are not guaranteed to be a quorum (or even a majority) of nodes at any time; and finally if "it is never the case that tx2 is before tx1 in a list of a correct validator" defines order as time order, which is not necessarily true (app side mempools, and alternative prepare proposal hooks, can re-order mempool tx order by any criteria) even if time is reliable (which it isn't, afaiu).
|
|
||
| #### [CM-REQ-MEMP-GOSSIP.0] | ||
|
|
||
| 1. every transaction submitted to a correct full node must eventually be included in the mempool of a correct validator |
There was a problem hiding this comment.
It is definitely not true that every transaction submitted to a correct full node will, or can, be included in the mempool of any other arbitrary "correct" validator.
There was a problem hiding this comment.
It is important to point out that gossipped information has no delivery guarantees whatsoever. No message that goes through the p2p layer is, by itself, reliably delivered to any specific destination. Gossip does not guarantee that a message is reliably delivered to any/every node in any time frame. Only the information which goes through consensus can be assumed as true.
There was a problem hiding this comment.
See this comment.
Let me add the we need to capture what mempool is doing and what it should do. Just saying there are "no guarantees" in the extreme argument leads to the question why there is a mempool after all? My current understanding is that the design of the gossiping in mempool is trying to achieve a very strong end-to-end semantics, although its actual guarantees depend on the connectivity provided by the p2p, which, in practice seems to be quite strong actually, but I agree with you the connectivity cannot be guaranteed all the time in all cases.
There was a problem hiding this comment.
Gossip provides no guarantees about delivery that can be assumed by the protocol at any specific node or time. It guarantees eventual delivery, over an unbounded time window, yes, but that doesn't allow the protocol as executed at any specific node to assert specific assumptions like a GST, or that the mempool will definitely contain any specific set of elements. Gossipping message M1 from node N1 at time T1 does not guarantee that M1 will be received by node N2 at time T2 within any time limit. T2 could be T1 plus 1s, or T1 plus 1h, or T1 plus 1d, or T1 plus 1w.
Let me add the we need to capture what mempool is doing and what it should do. Just saying there are "no guarantees" in the extreme argument leads to the question why there is a mempool after all?
"The mempool" is not some thing that has any specific definition at a network level, it is a set of transactions in each full node which is entirely independent between full nodes, at least from the perspective of the protocol.
Gossiping a message to one node does not guarantee that the message will exist in the mempool of any other node by any specific time bounds. It is likely that the mempools of validators, and probably a lot of full nodes, have transaction sets with a lot of overlap. But you can't assume any specific properties of those transaction sets when making decisions at the protocol level.
I understand that it would be very useful if you could do that! But that desire doesn't change reality 🙃 There are no strong end-to-end semantics in the mempool. Stochastically, at a system level, maybe yes, but those properties can't be treated as invariants by protocol code that runs on individual nodes.
| #### [CM-REQ-MEMP-GOSSIP.0] | ||
|
|
||
| 1. every transaction submitted to a correct full node must eventually be included in the mempool of a correct validator | ||
| 2. if a transaction is included in the mempool of a correct validator eventually it is included in the mempool of all correct validators |
| 1. An evidence transaction reaches all validator nodes as "fast as possible". | ||
| 1. Evidence is not written into a block after it has expired | ||
|
|
||
| > With more control over the four points, in principle we might define a real-time property which formalizes "if evidence is submitted at least a day before it expires, then evidence is written into a block before it expires". |
There was a problem hiding this comment.
Nothing guarantees that evidence submitted at some wall-clock time X will be written into a block before some later wall-clock time Y.
There was a problem hiding this comment.
I agree. The statement also doesn't contradict this.
There was a problem hiding this comment.
Does the statement not assert that evidence submitted at wall-clock time X (where X is at least a day before the expiration time of that evidence) will be written into a block before some later wall-clock time Y (where Y is the expiration time of the evidence)"?
|
I close as the target location of the text is unclear. I have moved relevant parts into our new knowledge base. |
…… (backport cometbft#75) (cometbft#80) * perf: Micro optimization to save one allocation per packet (backport cometbft#3018) (cometbft#3023) This PR is a slight optimization to save one allocation per packet. We have much more worthwhile performance improvements to pursue, just driveby noticed it as I was reading through the code. (Though I am surprised this did add up to 1 second in total -- 4% of the processing time) This re-uses the byte reader's allocation across all ReadMsg's. There is no concurrenct access possible under safe usage (also implied by the reader) This is the cause of the 1s time on the far right:  --- #### PR checklist - [x] Tests written/updated - covered by existing tests - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec <hr>This is an automatic backport of pull request cometbft#3018 done by [Mergify](https://mergify.com). --------- Co-authored-by: Dev Ojha <ValarDragon@users.noreply.github.com> Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> (cherry picked from commit ad8f851) * changelog (cherry picked from commit dd19cb0) # Conflicts: # CHANGELOG.md --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Dev Ojha <dojha@berkeley.edu> Co-authored-by: PaddyMc <paddymchale@hotmail.com>
Original PR: tendermint/tendermint#9732
--
This is the beginning of a high-level p2p spec addressing #20 for discussions. I started top down trying to understand what p2p is supposed to do rather than how p2p should do things. This collects findings from several discussion.