wip

Author: Alexander Hentschel

consensus/follower.go

@@ -6,7 +6,6 @@ import (
 	"github.com/rs/zerolog"
 
 	"github.com/onflow/flow-go/consensus/hotstuff"
-	"github.com/onflow/flow-go/consensus/hotstuff/follower"
 	"github.com/onflow/flow-go/consensus/hotstuff/validator"
 	"github.com/onflow/flow-go/consensus/recovery"
 	"github.com/onflow/flow-go/model/flow"
@@ -16,9 +15,16 @@ import (
 
 // TODO: this needs to be integrated with proper configuration and bootstrapping.
 
-func NewFollower(log zerolog.Logger, committee hotstuff.DynamicCommittee, headers storage.Headers, updater module.Finalizer,
-	verifier hotstuff.Verifier, notifier hotstuff.FinalizationConsumer, rootHeader *flow.Header,
-	rootQC *flow.QuorumCertificate, finalized *flow.Header, pending []*flow.Header,
+func NewFollower(log zerolog.Logger,
+	committee hotstuff.DynamicCommittee,
+	headers storage.Headers,
+	updater module.Finalizer,
+	verifier hotstuff.Verifier,
+	notifier hotstuff.FinalizationConsumer,
+	rootHeader *flow.Header,
+	rootQC *flow.QuorumCertificate,
+	finalized *flow.Header,
+	pending []*flow.Header,
 ) (*hotstuff.FollowerLoop, error) {
 
 	forks, err := NewForks(finalized, headers, updater, notifier, rootHeader, rootQC)
@@ -35,14 +41,8 @@ func NewFollower(log zerolog.Logger, committee hotstuff.DynamicCommittee, header
 		return nil, fmt.Errorf("could not recover hotstuff follower state: %w", err)
 	}
 
-	// initialize the follower logic
-	logic, err := follower.New(log, validator, forks)
-	if err != nil {
-		return nil, fmt.Errorf("could not create follower logic: %w", err)
-	}
-
 	// initialize the follower loop
-	loop, err := hotstuff.NewFollowerLoop(log, logic)
+	loop, err := hotstuff.NewFollowerLoop(log, forks)
 	if err != nil {
 		return nil, fmt.Errorf("could not create follower loop: %w", err)
 	}

consensus/hotstuff/follower/follower.go

@@ -1,6 +1,7 @@
 package hotstuff
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/rs/zerolog"
@@ -18,24 +19,28 @@ import (
 // Concurrency safe.
 type FollowerLoop struct {
 	*component.ComponentManager
-	log           zerolog.Logger
-	followerLogic FollowerLogic
-	proposals     chan *model.Proposal
+	log             zerolog.Logger
+	certifiedBlocks chan *model.CertifiedBlock
+	forks           Forks
 }
 
 var _ component.Component = (*FollowerLoop)(nil)
 var _ module.HotStuffFollower = (*FollowerLoop)(nil)
 
-// NewFollowerLoop creates an instance of EventLoop
-func NewFollowerLoop(log zerolog.Logger, followerLogic FollowerLogic) (*FollowerLoop, error) {
+// NewFollowerLoop creates an instance of HotStuffFollower
+func NewFollowerLoop(log zerolog.Logger, forks Forks) (*FollowerLoop, error) {
+	// We can't afford to drop messages since it undermines liveness, but we also want to avoid blocking
+	// the compliance layer. Generally, the follower loop should be able to process inbound blocks faster
+	// than they pass through the compliance layer. Nevertheless, in the worst case we will fill the
+	// channel and block the compliance layer's workers. Though, that should happen only if compliance
+	// engine receives large number of blocks in short periods of time (e.g. when catching up for).
 	// TODO(active-pacemaker) add metrics for length of inbound channels
-	// we will use a buffered channel to avoid blocking of caller
-	proposals := make(chan *model.Proposal, 1000)
+	certifiedBlocks := make(chan *model.CertifiedBlock, 1000)
 
 	fl := &FollowerLoop{
-		log:           log,
-		followerLogic: followerLogic,
-		proposals:     proposals,
+		log:             log.With().Str("hotstuff", "FollowerLoop").Logger(),
+		certifiedBlocks: certifiedBlocks,
+		forks:           forks,
 	}
 
 	fl.ComponentManager = component.NewComponentManagerBuilder().
@@ -45,27 +50,36 @@ func NewFollowerLoop(log zerolog.Logger, followerLogic FollowerLogic) (*Follower
 	return fl, nil
 }
 
-// SubmitProposal feeds a new block proposal (header) into the FollowerLoop.
-// This method blocks until the proposal is accepted to the event queue.
+// AddCertifiedBlock appends the given certified block to the tree of pending
+// blocks and updates the latest finalized block (if finalization progressed).
+// Unless the parent is below the pruning threshold (latest finalized view), we
+// require that the parent has previously been added.
 //
-// Block proposals must be submitted in order, i.e. a proposal's parent must
-// have been previously processed by the FollowerLoop.
-func (fl *FollowerLoop) SubmitProposal(proposal *model.Proposal) {
+// Notes:
+//   - Under normal operations, this method is non-blocking. The follower internally
+//     queues incoming blocks and processes them in its own worker routine. However,
+//     when the inbound queue is, we block until there is space in the queue. This
+//     behaviours is intentional, because we cannot drop blocks (otherwise, we would
+//     cause disconnected blocks). Instead we simply block the compliance layer to
+//     avoid any pathological edge cases.
+//   - Blocks whose views are below the latest finalized view are dropped.
+//   - Inputs are idempotent (repetitions are no-ops).
+func (fl *FollowerLoop) AddCertifiedBlock(certifiedBlock *model.CertifiedBlock) {
 	received := time.Now()
 
 	select {
-	case fl.proposals <- proposal:
+	case fl.certifiedBlocks <- certifiedBlock:
 	case <-fl.ComponentManager.ShutdownSignal():
 		return
 	}
 
 	// the busy duration is measured as how long it takes from a block being
 	// received to a block being handled by the event handler.
 	busyDuration := time.Since(received)
-	fl.log.Debug().Hex("block_id", logging.ID(proposal.Block.BlockID)).
-		Uint64("view", proposal.Block.View).
-		Dur("busy_duration", busyDuration).
-		Msg("busy duration to handle a proposal")
+	fl.log.Debug().Hex("block_id", logging.ID(certifiedBlock.ID())).
+		Uint64("view", certifiedBlock.View()).
+		Dur("wait_time", busyDuration).
+		Msg("wait time to queue inbound certified block")
 }
 
 // loop will synchronously process all events.
@@ -83,12 +97,15 @@ func (fl *FollowerLoop) loop(ctx irrecoverable.SignalerContext, ready component.
 		}
 
 		select {
-		case p := <-fl.proposals:
-			err := fl.followerLogic.AddBlock(p)
+		case b := <-fl.certifiedBlocks:
+			err := fl.forks.AddCertifiedBlock(b)
+			if err != nil {
+			}
 			if err != nil { // all errors are fatal
+				err = fmt.Errorf("finalization logic failes to process certified block %v: %w", b.ID(), err)
 				fl.log.Error().
-					Hex("block_id", logging.ID(p.Block.BlockID)).
-					Uint64("view", p.Block.View).
+					Hex("block_id", logging.ID(b.ID())).
+					Uint64("view", b.View()).
 					Err(err).
 					Msg("irrecoverable follower loop error")
 				ctx.Throw(err)

consensus/hotstuff/follower_loop.go

@@ -5,7 +5,16 @@ import (
 	"github.com/onflow/flow-go/model/flow"
 )
 
-// Forks maintains an in-memory data-structure of all proposals whose view-number is larger or equal to
+// FinalityProof represents a finality proof for a Block. By convention, a FinalityProof
+// is immutable. Finality in Jolteon/HotStuff is determined by the 2-chain rule:
+//
+//	There exists a _certified_ block C, such that Block.View + 1 = C.View
+type FinalityProof struct {
+	Block          *model.Block
+	CertifiedChild model.CertifiedBlock
+}
+
+// Forks maintains an in-memory data-structure of all blocks whose view-number is larger or equal to
 // the latest finalized block. The latest finalized block is defined as the finalized block with the largest view number.
 // When adding blocks, Forks automatically updates its internal state (including finalized blocks).
 // Furthermore, blocks whose view number is smaller than the latest finalized block are pruned automatically.
@@ -16,29 +25,71 @@ import (
 // and ignore the block.
 type Forks interface {
 
-	// GetProposalsForView returns all BlockProposals at the given view number.
-	GetProposalsForView(view uint64) []*model.Proposal
+	// GetBlocksForView returns all known blocks for the given view
+	GetBlocksForView(view uint64) []*model.Block
 
-	// GetProposal returns (BlockProposal, true) if the block with the specified
-	// id was found (nil, false) otherwise.
-	GetProposal(id flow.Identifier) (*model.Proposal, bool)
+	// GetBlock returns (BlockProposal, true) if the block with the specified
+	// id was found and (nil, false) otherwise.
+	GetBlock(blockID flow.Identifier) (*model.Block, bool)
 
 	// FinalizedView returns the largest view number where a finalized block is known
 	FinalizedView() uint64
 
 	// FinalizedBlock returns the finalized block with the largest view number
 	FinalizedBlock() *model.Block
 
-	// NewestView returns the largest view number of all proposals that were added to Forks.
-	NewestView() uint64
-
-	// AddProposal adds the block proposal to Forks. This might cause an update of the finalized block
-	// and pruning of older blocks.
-	// Handles duplicated addition of blocks (at the potential cost of additional computation time).
-	// PREREQUISITE:
-	// Forks must be able to connect `proposal` to its latest finalized block
-	// (without missing interim ancestors). Otherwise, an exception is raised.
-	// Expected errors during normal operations:
-	//  * model.ByzantineThresholdExceededError - new block results in conflicting finalized blocks
-	AddProposal(proposal *model.Proposal) error
+	// FinalityProof returns the latest finalized block and a certified child from
+	// 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.
+	FinalityProof() (*FinalityProof, bool)
+
+	// AddProposal appends the given 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
+	// leaves the consensus state invariant (though, it will potentially cause some
+	// duplicate processing).
+	// Notes:
+	//   - Method `AddCertifiedBlock(..)` should be used preferably, if a QC certifying
+	//     `block` is already known. This is generally the case for the consensus follower.
+	//     Method `AddProposal` is intended for active consensus participants, which fully
+	//     validate blocks (incl. payload), i.e. QCs are processed as part of validated proposals.
+	//
+	// Possible error returns:
+	//   - model.MissingBlockError if the parent does not exist in the forest (but is above
+	//     the pruned view). From the perspective of Forks, this error is benign (no-op).
+	//   - model.InvalidBlockError if the block is invalid (see `Forks.EnsureBlockIsValidExtension`
+	//     for details). From the perspective of Forks, this error is benign (no-op). However, we
+	//     assume all blocks are fully verified, i.e. they should satisfy all consistency
+	//     requirements. Hence, this error is likely an indicator of a bug in the compliance layer.
+	//   - model.ByzantineThresholdExceededError if conflicting QCs or conflicting finalized
+	//     blocks have been detected (violating a foundational consensus guarantees). This
+	//     indicates that there are 1/3+ Byzantine nodes (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.
+	//   - All other errors are potential symptoms of bugs or state corruption.
+	AddProposal(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).
+	// 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 leaves the consensus state invariant (though,
+	// it will potentially cause some duplicate processing).
+	//
+	// Possible error returns:
+	//   - model.MissingBlockError if the parent does not exist in the forest (but is above
+	//     the pruned view). From the perspective of Forks, this error is benign (no-op).
+	//   - model.InvalidBlockError if the block is invalid (see `Forks.EnsureBlockIsValidExtension`
+	//     for details). From the perspective of Forks, this error is benign (no-op). However, we
+	//     assume all blocks are fully verified, i.e. they should satisfy all consistency
+	//     requirements. Hence, this error is likely an indicator of a bug in the compliance layer.
+	//   - model.ByzantineThresholdExceededError if conflicting QCs or conflicting finalized
+	//     blocks have been detected (violating a foundational consensus guarantees). This
+	//     indicates that there are 1/3+ Byzantine nodes (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.
+	//   - All other errors are potential symptoms of bugs or state corruption.
+	AddCertifiedBlock(certifiedBlock *model.CertifiedBlock) error
 }