Merge #4190

4190: Alex/use qc of uncertified block optimization r=AlexHentschel a=AlexHentschel

Implements part 2 of #4154
> 2. Forks can ingest `Proposals` as well as `CertifiedBlock`s. The finality logic in Forks accounts for the certifying QC.

## Approach taken in this PR

* I added a revised implementation of Forks, which I called `Forks2`
   * `Forks2` is not yet used by any business logic (will be done in subsequent PR)
   * extended tests to cover logic for adding `CertifiedBlock`s
* the old `Forks` and its unit tests remain in the code base, as they are still used
  * to avoid conflicts, I have moved the tests into the testing package `forks_test`
  * both `Forks` and its unit tests will be removed in subsequent PR


Co-authored-by: Alexander Hentschel <[email protected]>

Author: bors[bot]

consensus/hotstuff/forks/blockQC.go

@@ -42,51 +42,54 @@ func NewBlockBuilder() *BlockBuilder {
 	}
 }
 
-// Add adds a block with the given qcView and blockView.
-func (f *BlockBuilder) Add(qcView uint64, blockView uint64) {
-	f.blockViews = append(f.blockViews, &BlockView{
+// Add adds a block with the given qcView and blockView. Returns self-reference for chaining.
+func (bb *BlockBuilder) Add(qcView uint64, blockView uint64) *BlockBuilder {
+	bb.blockViews = append(bb.blockViews, &BlockView{
 		View:   blockView,
 		QCView: qcView,
 	})
+	return bb
 }
 
 // GenesisBlock returns the genesis block, which is always finalized.
-func (f *BlockBuilder) GenesisBlock() *model.Block {
-	return makeGenesis().Block
+func (bb *BlockBuilder) GenesisBlock() *model.CertifiedBlock {
+	return makeGenesis()
 }
 
 // AddVersioned adds a block with the given qcView and blockView.
-// In addition the version identifier of the QC embedded within the block
+// In addition, the version identifier of the QC embedded within the block
 // is specified by `qcVersion`. The version identifier for the block itself
 // (primarily for emulating different payloads) is specified by `blockVersion`.
-// [3,4] denotes a block of view 4, with a qc of view 3
-// [3,4'] denotes a block of view 4, with a qc of view 3, but has a different BlockID than [3,4]
-// [3,4'] can be created by AddVersioned(3, 4, 0, 1)
-// [3',4] can be created by AddVersioned(3, 4, 1, 0)
-func (f *BlockBuilder) AddVersioned(qcView uint64, blockView uint64, qcVersion int, blockVersion int) {
-	f.blockViews = append(f.blockViews, &BlockView{
+// [(◄3) 4] denotes a block of view 4, with a qc for view 3
+// [(◄3) 4'] denotes a block of view 4 that is different than [(◄3) 4], with a qc for view 3
+// [(◄3) 4'] can be created by AddVersioned(3, 4, 0, 1)
+// [(◄3') 4] can be created by AddVersioned(3, 4, 1, 0)
+// Returns self-reference for chaining.
+func (bb *BlockBuilder) AddVersioned(qcView uint64, blockView uint64, qcVersion int, blockVersion int) *BlockBuilder {
+	bb.blockViews = append(bb.blockViews, &BlockView{
 		View:         blockView,
 		QCView:       qcView,
 		BlockVersion: blockVersion,
 		QCVersion:    qcVersion,
 	})
+	return bb
 }
 
 // Blocks returns a list of all blocks added to the BlockBuilder.
 // Returns an error if the blocks do not form a connected tree rooted at genesis.
-func (f *BlockBuilder) Blocks() ([]*model.Proposal, error) {
-	blocks := make([]*model.Proposal, 0, len(f.blockViews))
+func (bb *BlockBuilder) Blocks() ([]*model.Proposal, error) {
+	blocks := make([]*model.Proposal, 0, len(bb.blockViews))
 
-	genesisBQ := makeGenesis()
+	genesisBlock := makeGenesis()
 	genesisBV := &BlockView{
-		View:   genesisBQ.Block.View,
-		QCView: genesisBQ.QC.View,
+		View:   genesisBlock.Block.View,
+		QCView: genesisBlock.CertifyingQC.View,
 	}
 
 	qcs := make(map[string]*flow.QuorumCertificate)
-	qcs[genesisBV.QCIndex()] = genesisBQ.QC
+	qcs[genesisBV.QCIndex()] = genesisBlock.CertifyingQC
 
-	for _, bv := range f.blockViews {
+	for _, bv := range bb.blockViews {
 		qc, ok := qcs[bv.QCIndex()]
 		if !ok {
 			return nil, fmt.Errorf("test fail: no qc found for qc index: %v", bv.QCIndex())
@@ -145,6 +148,7 @@ func makeBlockID(block *model.Block) flow.Identifier {
 	})
 }
 
+// constructs the genesis block (identical for all calls)
 func makeGenesis() *model.CertifiedBlock {
 	genesis := &model.Block{
 		View: 1,

consensus/hotstuff/forks/block_builder_test.go

@@ -21,3 +21,17 @@ func (b *BlockContainer) Level() uint64             { return b.Proposal.Block.Vi
 func (b *BlockContainer) Parent() (flow.Identifier, uint64) {
 	return b.Proposal.Block.QC.BlockID, b.Proposal.Block.QC.View
 }
+
+// BlockContainer wraps a block proposal to implement forest.Vertex
+// so the proposal can be stored in forest.LevelledForest
+type BlockContainer2 model.Block
+
+var _ forest.Vertex = (*BlockContainer2)(nil)
+
+func ToBlockContainer2(block *model.Block) *BlockContainer2 { return (*BlockContainer2)(block) }
+func (b *BlockContainer2) Block() *model.Block              { return (*model.Block)(b) }
+
+// Functions implementing forest.Vertex
+func (b *BlockContainer2) VertexID() flow.Identifier         { return b.BlockID }
+func (b *BlockContainer2) Level() uint64                     { return b.View }
+func (b *BlockContainer2) Parent() (flow.Identifier, uint64) { return b.QC.BlockID, b.QC.View }

consensus/hotstuff/forks/blockcontainer.go

@@ -47,7 +47,7 @@ type Forks struct {
 var _ hotstuff.Forks = (*Forks)(nil)
 
 func New(trustedRoot *model.CertifiedBlock, finalizationCallback module.Finalizer, notifier hotstuff.FinalizationConsumer) (*Forks, error) {
-	if (trustedRoot.Block.BlockID != trustedRoot.QC.BlockID) || (trustedRoot.Block.View != trustedRoot.QC.View) {
+	if (trustedRoot.Block.BlockID != trustedRoot.CertifyingQC.BlockID) || (trustedRoot.Block.View != trustedRoot.CertifyingQC.View) {
 		return nil, model.NewConfigurationErrorf("invalid root: root QC is not pointing to root block")
 	}
 
@@ -307,7 +307,7 @@ func (f *Forks) updateFinalizedBlockQC(blockContainer *BlockContainer) error {
 	if ancestryChain.oneChain.Block.View != b.Block.View+1 {
 		return nil
 	}
-	return f.finalizeUpToBlock(b.QC)
+	return f.finalizeUpToBlock(b.CertifyingQC)
 }
 
 // getTwoChain returns the 2-chain for the input block container b.