Pipelining

[leandernikolaus]

This implements pipelining for chained hotstuff.
Pipelined blocks still form a single chain, but need not include the certificate for their direct parent.

Commit rule is updated to commit requests if there is a depth 3 confirmation chain and all intermediate blocks are confirmed as well.

[meling]

Looks good. I've requested some changes. Looks like you already refer to the number of views as the type hotstuff.View:

	// PipelinedViews returns the number of concurrently executed views.
 	PipelinedViews() hotstuff.View

This makes it seem reasonable that the Synchronizer struct should also represent it as a hotstuff.View instead of a uint8.

[meling]

Looks like some of the tests are failing now. Please take a look.

[meling]

This should not be done in the loop, and it is also more complex than necessary. Adding this at line 124 should suffice:

	if pipelinedViews > 1 && consensusName != "chainedhotstuff" {
		return fmt.Errorf("pipelining currently only supported for chainedhotstuff")
	}
	if pipelinedViews < 1 {
		pipelinedViews = 1
	}

[leandernikolaus]

fixed in fb04815
I also added a similar error in internal/cli/run.go

func checkFlags() error {
	consensus := viper.GetString("consensus")
	pipelinedViews := viper.GetInt("pipelined-views")
	if pipelinedViews > 1 && consensus != "chainedhotstuff" {
		return errors.New("piplining only supported for chainedhotstuff consensus")
	}
	return nil
}

[meling]

Missing newline

[leandernikolaus]

fixed in fb04815

[leandernikolaus]

In 7e59d24 I revised the approach. Now the synchronizer has both a currentView and nextView.
currentView is advanced on receiving a TC or QC, while nextView is advanced on receiving a block if it lies within the pipeline.
This makes the system run more stable since it no longer reacts to individual timeouts.
However, the consensus module now no longer proposes blocks only in the current view but needs to receive a view, in which it should propose.
This also required updates to Twins.
Based on my analysis and testing, without pipelining, nextView and currentView should always coincide.

[meling]

@leandernikolaus I tried to merge master into this branch (locally) in an attempt to resolve the conflicts. However, the current synchronizer no longer has a leafBlock field and LeafBlock() method, it wasn't clear to me what needed to be done. This was removed in this commit. Could you try to resolve the conflicts, or let me know what needs to be done to get this back into a good shape. Or let me know if this no longer makes sense to merge.