• refactored Forks interface to reflect new version

• minor simplification of tests

Author: Alexander Hentschel

consensus/hotstuff/forks.go

@@ -44,7 +44,7 @@ type Forks interface {
 	// blocks beyond the finalized root block it was initialized with.
 	FinalityProof() (*FinalityProof, bool)
 
-	// AddProposal appends the given block to the tree of pending
+	// AddValidatedBlock appends the validated block to the tree of pending
 	// blocks and updates the latest finalized block (if applicable). Unless the parent is
 	// below the pruning threshold (latest finalized view), we require that the parent is
 	// already stored in Forks. Calling this method with previously processed blocks
@@ -69,7 +69,7 @@ type Forks interface {
 	//     breaking the safety guarantees of HotStuff (or there is a critical bug / data
 	//     corruption). Forks cannot recover from this exception.
 	//   - All other errors are potential symptoms of bugs or state corruption.
-	AddProposal(proposal *model.Block) error
+	AddValidatedBlock(proposal *model.Block) error
 
 	// AddCertifiedBlock appends the given certified block to the tree of pending
 	// blocks and updates the latest finalized block (if finalization progressed).

consensus/hotstuff/forks/block_builder_test.go

@@ -75,9 +75,9 @@ func (bb *BlockBuilder) AddVersioned(qcView uint64, blockView uint64, qcVersion
 	return bb
 }
 
-// Blocks returns a list of all blocks added to the BlockBuilder.
+// Proposals returns a list of all proposals added to the BlockBuilder.
 // Returns an error if the blocks do not form a connected tree rooted at genesis.
-func (bb *BlockBuilder) Blocks() ([]*model.Proposal, error) {
+func (bb *BlockBuilder) Proposals() ([]*model.Proposal, error) {
 	blocks := make([]*model.Proposal, 0, len(bb.blockViews))
 
 	genesisBlock := makeGenesis()
@@ -124,6 +124,16 @@ func (bb *BlockBuilder) Blocks() ([]*model.Proposal, error) {
 	return blocks, nil
 }
 
+// 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 (bb *BlockBuilder) Blocks() ([]*model.Block, error) {
+	proposals, err := bb.Proposals()
+	if err != nil {
+		return nil, fmt.Errorf("BlockBuilder failed to generate proposals: %w", err)
+	}
+	return toBlocks(proposals), nil
+}
+
 func makePayloadHash(view uint64, qc *flow.QuorumCertificate, blockVersion int) flow.Identifier {
 	return flow.MakeID(struct {
 		View         uint64
@@ -165,3 +175,12 @@ func makeGenesis() *model.CertifiedBlock {
 	}
 	return &certifiedGenesisBlock
 }
+
+// toBlocks converts the given proposals to slice of blocks
+func toBlocks(proposals []*model.Proposal) []*model.Block {
+	blocks := make([]*model.Block, 0, len(proposals))
+	for _, b := range proposals {
+		blocks = append(blocks, b.Block)
+	}
+	return blocks
+}

consensus/hotstuff/forks/forks.go

@@ -15,7 +15,7 @@ import (
 // The same approach has later been adopted by the Diem team resulting in DiemBFT v4:
 // https://developers.diem.com/papers/diem-consensus-state-machine-replication-in-the-diem-blockchain/2021-08-17.pdf
 // Forks is NOT safe for concurrent use by multiple goroutines.
-type Forks2 struct {
+type Forks struct {
 	finalizationCallback module.Finalizer
 	notifier             hotstuff.FinalizationConsumer
 	forest               forest.LevelledForest
@@ -26,19 +26,14 @@ type Forks2 struct {
 	finalityProof *hotstuff.FinalityProof
 }
 
-// TODO:
-//   - update `hotstuff.Forks` interface to represent Forks2
-//   - update business logic to of consensus participant and follower to use Forks2
-//
-// As the result, the following should apply again
-var _ hotstuff.Forks = (*Forks2)(nil)
+var _ hotstuff.Forks = (*Forks)(nil)
 
-func NewForks2(trustedRoot *model.CertifiedBlock, finalizationCallback module.Finalizer, notifier hotstuff.FinalizationConsumer) (*Forks2, error) {
+func NewForks(trustedRoot *model.CertifiedBlock, finalizationCallback module.Finalizer, notifier hotstuff.FinalizationConsumer) (*Forks, error) {
 	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")
 	}
 
-	forks := Forks2{
+	forks := Forks{
 		finalizationCallback: finalizationCallback,
 		notifier:             notifier,
 		forest:               *forest.NewLevelledForest(trustedRoot.Block.View),
@@ -56,15 +51,15 @@ func NewForks2(trustedRoot *model.CertifiedBlock, finalizationCallback module.Fi
 }
 
 // FinalizedView returns the largest view number where a finalized block is known
-func (f *Forks2) FinalizedView() uint64 {
+func (f *Forks) FinalizedView() uint64 {
 	if f.finalityProof == nil {
 		return f.trustedRoot.Block.View
 	}
 	return f.finalityProof.Block.View
 }
 
 // FinalizedBlock returns the finalized block with the largest view number
-func (f *Forks2) FinalizedBlock() *model.Block {
+func (f *Forks) FinalizedBlock() *model.Block {
 	if f.finalityProof == nil {
 		return f.trustedRoot.Block
 	}
@@ -75,13 +70,13 @@ func (f *Forks2) FinalizedBlock() *model.Block {
 // the subsequent view, which proves finality.
 // CAUTION: method returns (nil, false), when Forks has not yet finalized any
 // blocks beyond the finalized root block it was initialized with.
-func (f *Forks2) FinalityProof() (*hotstuff.FinalityProof, bool) {
+func (f *Forks) FinalityProof() (*hotstuff.FinalityProof, bool) {
 	return f.finalityProof, f.finalityProof != nil
 }
 
 // GetBlock returns (BlockProposal, true) if the block with the specified
 // id was found and (nil, false) otherwise.
-func (f *Forks2) GetBlock(blockID flow.Identifier) (*model.Block, bool) {
+func (f *Forks) GetBlock(blockID flow.Identifier) (*model.Block, bool) {
 	blockContainer, hasBlock := f.forest.GetVertex(blockID)
 	if !hasBlock {
 		return nil, false
@@ -90,7 +85,7 @@ func (f *Forks2) GetBlock(blockID flow.Identifier) (*model.Block, bool) {
 }
 
 // GetBlocksForView returns all known blocks for the given view
-func (f *Forks2) GetBlocksForView(view uint64) []*model.Block {
+func (f *Forks) GetBlocksForView(view uint64) []*model.Block {
 	vertexIterator := f.forest.GetVerticesAtLevel(view)
 	blocks := make([]*model.Block, 0, 1) // in the vast majority of cases, there will only be one proposal for a particular view
 	for vertexIterator.HasNext() {
@@ -101,7 +96,7 @@ func (f *Forks2) GetBlocksForView(view uint64) []*model.Block {
 }
 
 // IsKnownBlock checks whether block is known.
-func (f *Forks2) IsKnownBlock(blockID flow.Identifier) bool {
+func (f *Forks) IsKnownBlock(blockID flow.Identifier) bool {
 	_, hasBlock := f.forest.GetVertex(blockID)
 	return hasBlock
 }
@@ -113,7 +108,7 @@ func (f *Forks2) IsKnownBlock(blockID flow.Identifier) bool {
 //   - the block already exists in the consensus state
 //
 // UNVALIDATED: expects block to pass Forks.EnsureBlockIsValidExtension(block)
-func (f *Forks2) IsProcessingNeeded(block *model.Block) bool {
+func (f *Forks) IsProcessingNeeded(block *model.Block) bool {
 	if block.View < f.FinalizedView() || f.IsKnownBlock(block.BlockID) {
 		return false
 	}
@@ -148,7 +143,7 @@ func (f *Forks2) IsProcessingNeeded(block *model.Block) bool {
 //     forest (but is above the pruned view). Represents violation of condition 3.
 //   - model.InvalidBlockError if the block violates condition 1. or 2.
 //   - generic error in case of unexpected bug or internal state corruption
-func (f *Forks2) EnsureBlockIsValidExtension(block *model.Block) error {
+func (f *Forks) EnsureBlockIsValidExtension(block *model.Block) error {
 	if block.View < f.forest.LowestLevel { // exclusion (i)
 		return nil
 	}
@@ -200,7 +195,7 @@ func (f *Forks2) EnsureBlockIsValidExtension(block *model.Block) error {
 //     breaking the safety guarantees of HotStuff (or there is a critical bug / data
 //     corruption). Forks cannot recover from this exception.
 //   - All other errors are potential symptoms of bugs or state corruption.
-func (f *Forks2) AddCertifiedBlock(certifiedBlock *model.CertifiedBlock) error {
+func (f *Forks) AddCertifiedBlock(certifiedBlock *model.CertifiedBlock) error {
 	if !f.IsProcessingNeeded(certifiedBlock.Block) {
 		return nil
 	}
@@ -227,7 +222,7 @@ func (f *Forks2) AddCertifiedBlock(certifiedBlock *model.CertifiedBlock) error {
 	return nil
 }
 
-// AddProposal appends the given block to the tree of pending
+// AddValidatedBlock appends the validated block to the tree of pending
 // blocks and updates the latest finalized block (if applicable). Unless the parent is
 // below the pruning threshold (latest finalized view), we require that the parent is
 // already stored in Forks. Calling this method with previously processed blocks
@@ -252,7 +247,7 @@ func (f *Forks2) AddCertifiedBlock(certifiedBlock *model.CertifiedBlock) error {
 //     breaking the safety guarantees of HotStuff (or there is a critical bug / data
 //     corruption). Forks cannot recover from this exception.
 //   - All other errors are potential symptoms of bugs or state corruption.
-func (f *Forks2) AddProposal(proposal *model.Block) error {
+func (f *Forks) AddValidatedBlock(proposal *model.Block) error {
 	if !f.IsProcessingNeeded(proposal) {
 		return nil
 	}
@@ -299,7 +294,7 @@ func (f *Forks2) AddProposal(proposal *model.Block) error {
 //   - model.ByzantineThresholdExceededError if conflicting QCs have been detected.
 //     Forks cannot recover from this exception.
 //   - All other errors are potential symptoms of bugs or state corruption.
-func (f *Forks2) checkForByzantineEvidence(block *model.Block) error {
+func (f *Forks) checkForByzantineEvidence(block *model.Block) error {
 	err := f.EnsureBlockIsValidExtension(block)
 	if err != nil {
 		return fmt.Errorf("consistency check on block failed: %w", err)
@@ -325,7 +320,7 @@ func (f *Forks2) checkForByzantineEvidence(block *model.Block) error {
 //   - model.ByzantineThresholdExceededError if conflicting QCs have been detected.
 //     Forks cannot recover from this exception.
 //   - All other errors are potential symptoms of bugs or state corruption.
-func (f *Forks2) checkForConflictingQCs(qc *flow.QuorumCertificate) error {
+func (f *Forks) checkForConflictingQCs(qc *flow.QuorumCertificate) error {
 	it := f.forest.GetVerticesAtLevel(qc.View)
 	for it.HasNext() {
 		otherBlock := it.NextVertex() // by construction, must have same view as qc.View
@@ -352,7 +347,7 @@ func (f *Forks2) checkForConflictingQCs(qc *flow.QuorumCertificate) error {
 // checkForDoubleProposal checks if the input proposal is a double proposal.
 // A double proposal occurs when two proposals with the same view exist in Forks.
 // If there is a double proposal, notifier.OnDoubleProposeDetected is triggered.
-func (f *Forks2) checkForDoubleProposal(block *model.Block) {
+func (f *Forks) checkForDoubleProposal(block *model.Block) {
 	it := f.forest.GetVerticesAtLevel(block.View)
 	for it.HasNext() {
 		otherVertex := it.NextVertex() // by construction, must have same view as block
@@ -377,7 +372,7 @@ func (f *Forks2) checkForDoubleProposal(block *model.Block) {
 //     (weighted by stake) in the network, breaking the safety guarantees of HotStuff (or there
 //     is a critical bug / data corruption). Forks cannot recover from this exception.
 //   - generic error in case of unexpected bug or internal state corruption
-func (f *Forks2) checkForAdvancingFinalization(certifiedBlock *model.CertifiedBlock) error {
+func (f *Forks) checkForAdvancingFinalization(certifiedBlock *model.CertifiedBlock) error {
 	// We prune all blocks in forest which are below the most recently finalized block.
 	// Hence, we have a pruned ancestry if and only if either of the following conditions applies:
 	//    (a) If a block's parent view (i.e. block.QC.View) is below the most recently finalized block.
@@ -463,7 +458,7 @@ func (f *Forks2) checkForAdvancingFinalization(certifiedBlock *model.CertifiedBl
 //     (weighted by stake) in the network, breaking the safety guarantees of HotStuff (or there
 //     is a critical bug / data corruption). Forks cannot recover from this exception.
 //   - generic error in case of bug or internal state corruption
-func (f *Forks2) collectBlocksForFinalization(qc *flow.QuorumCertificate) ([]*model.Block, error) {
+func (f *Forks) collectBlocksForFinalization(qc *flow.QuorumCertificate) ([]*model.Block, error) {
 	lastFinalized := f.FinalizedBlock()
 	if qc.View < lastFinalized.View {
 		return nil, model.ByzantineThresholdExceededError{Evidence: fmt.Sprintf(