Abstract common consensus functions #8
Conversation
|
@dnkolegov, do you mind having a look at the last commit also? I parametrized the block reward so we can use alternative approaches for Mir. We should also figure out what to do with CI (Testground keeps hiding the important checks) |
There was a problem hiding this comment.
Having looked through the code, I feel quite unable to say almost anything about it. It would really help me to have some description of the interface (in English) either in form of comments in the code or in some other document. Currently I only see the function signatures defined in the Consensus interface in chain/consensus/iface.go, but I'm pretty clueless about their semantics. I assume that each of those functions is called by Lotus at some point when it needs to communicate with the consensus layer. But when does this happen and what is the meaning of the parameters?
|
@matejpavlovic, I added comments to the interface and a brief description of how I imagine the integration to work in the description of the PR. @dnkolegov, please also jump in if you don't agree with something. Let me know if you think it would be useful to discuss this sync. Thanks 🙏 |
There was a problem hiding this comment.
Thanks for the clarifications @adlrocha , they really helped a lot!
Now I was able to formulate some more concrete questions around the interface. Some of them are rather broad and open, so I suggest a quick sync call today to answer them together, if you guys are free.
| // CreateBlock implements all the logic required to propose and assemble a new Filecoin block. | ||
| // | ||
| // This function encapsulate all the consensus-specific actions to propose a new block | ||
| // such as the ordering of transactions, the inclusion of consensus proofs, the signature | ||
| // of the block, etc. |
There was a problem hiding this comment.
Who calls CreateBlock and when? Can the consensus layer count on this being called by Lotus code, the contract being that on each call of CreateBlock a new block has to be proposed (and (much later) delivered and returned from CreateBlock)? Is the returned block supposed to be ready for execution?
Currently we're working with the idea that the consensus algorithm chooses itself when it proposes new blocks. I.e., it has access to the mempool and pulls requests ("messages") from it whenever it wants. It then "announces" the delivered blocks to Lotus (still Eudico in our case) through an API, and Lotus (Eudico) then takes care of the execution.
I don't have a principal problem if the triggers for proposing the block came from outside (from the point of view of the consensus protocol) instead, I'd just like to know from where and how. I guess it's the miner that calls CreateBlock, right? In this case the miner will also need to run consensus-specific logic (as we do in Eudico now), and maybe it will even end up completely circumventing this function...
| // ValidateBlockHeader is called by peers when they receive a new block through the network. | ||
| // | ||
| // This is a fast sanity-check validation performed by the PubSub protocol before delivering | ||
| // it to the syncer. It checks that the block has the right format and it performs | ||
| // other consensus-specific light verifications like ensuring that the block is signed by | ||
| // a valid miner, or that it includes all the data required for a full verification. |
There was a problem hiding this comment.
By "through the network" you mean through gossipsub, right?
If I understood your PR description correctly, nodes that participate in the BFT protocol itself would not use this. That makes sense, since for a BFT-style protocol like the Mir-based one we have, this only seems meaningful (possibly) a part of some state synchronization sub-protocol (or for "learner" nodes, if we introduce such a thing within the protocol).
For this to be useful, we need to define what exactly a block header must look like, how it will be validated, by whom, and what information is necessary to validate the block (e.g. the membership?).
| // ValidateBlock is called by the syncer to determine if to accept a block or not. | ||
| // | ||
| // It performs all the checks needed by the syncer to accept | ||
| // the block (signature verifications, VRF checks, message validity, etc.) |
There was a problem hiding this comment.
This is just a more heavy-weight version of ValidateBlockHeader, but I assume it is performed by the same nodes for the same purpose, and is just separated for performance optimization purposes. Please correct me if I'm wrong.
There was a problem hiding this comment.
Correct. The first one needs to be really light because it is done by the network protocol to prevent DDoS attacks (so it shouldn't be computationally expensive). ValidateBlock is the one you do to fully verify the block before executing it.
There was a problem hiding this comment.
Can the implementation assume that b's header was already validated with ValidateBlockHeader?
There was a problem hiding this comment.
Unfortunately, not. GossipSub is not the only way in which you can be receiving a new block. The syncer may also request blocks directly from known peers, in which case you will want to also do first the header (light) verification and then the full-block validation.
chain/consensus/iface.go
Outdated
|
|
||
| // SignBlock determines how should blocks be signed for them to be considered valid by the consensus | ||
| // engine. |
There was a problem hiding this comment.
I don't quite understand how the signing works here, especially since only an error is returned. Is SignBlock supposed to modify next in place and add the local node's signature to it?
I find it rather unlikely that the Mir-based ordering subsystem would need an explicit trigger from the outside to know when to sign blocks, so probably SignBlock (and VerifyBlockSignature) won't really be used by us. If we do need to produce and verify signatures, we'd probably produce them somewhere deep within the protocol and verify them in ValidateBlock[Header]. Does that make sense?
There was a problem hiding this comment.
Total sense, in that case we can remove those. I will do that and remove those two.
|
@matejpavlovic, I removed the two functions that we mentioned. If we are happy with this I will start the PR to the |
|
PR superseded by the one done to the As long as no one strongly opposes, I suggest following this approach for BuilderNet:
|
Related Issues
#1
Proposed Changes
This PR extends the consensus interface in order to prepare it for the implementation of other consensus algorithms, and it extracts from
FilecoinECcommon logic that will be shared by all consensus implementations.ValidatedPubSubas a common function.compute_stateas common code for all consensus algorithms.Integration of the interface
To illustrate how can this interface be integrated for the implementation of alternative consensus algorithm, I will share how I imagine it to work and be implemented for Mir:
CreateBlockruns the block proposal logic for Mir in Lotus. InCreateBlockLotus validators pull unverified messages from the mempool, and send them to Mir for ordering. Once the ordered batch is output by Mir, a new Filecoin block is assembled and broadcast through GossipSub to the network.CreateBlockcallsSignBlockif the block needs to be signed.ValidateBlockHeaderbefore delivering the block to the node. If the validation succeeds, the block is passed to the syncer which runsValidateBlockto perform a full block validation to determine if the block is valid and can be executed. As part of these verifications,VerifyBlockSignaturemay be called.RewardFuncthat parametrized how should block rewards be distributed among validators.