Merge pull request #297 from getamis/consensus_failed

In this PR, we fixed https://github.com/jpmorganchase/quorum/issues/289.
The root cause is we change the block header timestamp in Seal(). It will cause different root hash in proposer and validators' side.

We did the following updates:
1. remove `BlockPauseTime`
2. set block header timestamp in Prepare()

cmd/geth/main.go

@@ -122,7 +122,6 @@ var (
 		utils.EmitCheckpointsFlag,
 		utils.IstanbulRequestTimeoutFlag,
 		utils.IstanbulBlockPeriodFlag,
-		utils.IstanbulBlockPauseTimeFlag,
 	}
 
 	rpcFlags = []cli.Flag{

cmd/geth/usage.go

@@ -22,6 +22,8 @@ import (
 	"io"
 	"sort"
 
+	"strings"
+
 	"github.com/ethereum/go-ethereum/cmd/utils"
 	"github.com/ethereum/go-ethereum/internal/debug"
 	"gopkg.in/urfave/cli.v1"
@@ -225,7 +227,6 @@ var AppHelpFlagGroups = []flagGroup{
 		Flags: []cli.Flag{
 			utils.IstanbulRequestTimeoutFlag,
 			utils.IstanbulBlockPeriodFlag,
-			utils.IstanbulBlockPauseTimeFlag,
 		},
 	},
 }

cmd/utils/flags.go

@@ -534,11 +534,6 @@ var (
 		Usage: "Default minimum difference between two consecutive block's timestamps in seconds",
 		Value: eth.DefaultConfig.Istanbul.BlockPeriod,
 	}
-	IstanbulBlockPauseTimeFlag = cli.Uint64Flag{
-		Name:  "istanbul.blockpausetime",
-		Usage: "Pause time when zero tx in previous block, values should be larger than istanbul.blockperiod",
-		Value: eth.DefaultConfig.Istanbul.BlockPauseTime,
-	}
 )
 
 // MakeDataDir retrieves the currently requested data directory, terminating
@@ -967,9 +962,6 @@ func setIstanbul(ctx *cli.Context, cfg *eth.Config) {
 	if ctx.GlobalIsSet(IstanbulBlockPeriodFlag.Name) {
 		cfg.Istanbul.BlockPeriod = ctx.GlobalUint64(IstanbulBlockPeriodFlag.Name)
 	}
-	if ctx.GlobalIsSet(IstanbulBlockPauseTimeFlag.Name) {
-		cfg.Istanbul.BlockPauseTime = ctx.GlobalUint64(IstanbulBlockPauseTimeFlag.Name)
-	}
 }
 
 func checkExclusive(ctx *cli.Context, flags ...cli.Flag) {

consensus/istanbul/backend/engine.go

@@ -364,6 +364,12 @@ func (sb *backend) Prepare(chain consensus.ChainReader, header *types.Header) er
 		return err
 	}
 	header.Extra = extra
+
+	// set header's timestamp
+	header.Time = new(big.Int).Add(parent.Time, new(big.Int).SetUint64(sb.config.BlockPeriod))
+	if header.Time.Int64() < time.Now().Unix() {
+		header.Time = big.NewInt(time.Now().Unix())
+	}
 	return nil
 }
 
@@ -445,21 +451,7 @@ func (sb *backend) Seal(chain consensus.ChainReader, block *types.Block, stop <-
 
 // update timestamp and signature of the block based on its number of transactions
 func (sb *backend) updateBlock(parent *types.Header, block *types.Block) (*types.Block, error) {
-	// set block period based the number of tx
-	var period uint64
-	if len(block.Transactions()) == 0 {
-		period = sb.config.BlockPauseTime
-	} else {
-		period = sb.config.BlockPeriod
-	}
-
-	// set header timestamp
 	header := block.Header()
-	header.Time = new(big.Int).Add(parent.Time, new(big.Int).SetUint64(period))
-	time := now().Unix()
-	if header.Time.Int64() < time {
-		header.Time = big.NewInt(time)
-	}
 	// sign the hash
 	seal, err := sb.Sign(sigHash(header).Bytes())
 	if err != nil {

consensus/istanbul/backend/snapshot_test.go

@@ -352,7 +352,7 @@ func TestVoting(t *testing.T) {
 		for j, vote := range tt.votes {
 			headers[j] = &types.Header{
 				Number:     big.NewInt(int64(j) + 1),
-				Time:       big.NewInt(int64(j) * int64(config.BlockPauseTime)),
+				Time:       big.NewInt(int64(j) * int64(config.BlockPeriod)),
 				Coinbase:   accounts.address(vote.voted),
 				Difficulty: defaultDifficulty,
 				MixDigest:  types.IstanbulDigest,

consensus/istanbul/config.go

@@ -24,9 +24,8 @@ const (
 )
 
 type Config struct {
-	RequestTimeout uint64         `toml:",omitempty"` // The timeout for each Istanbul round in milliseconds. This timeout should be larger than BlockPauseTime
+	RequestTimeout uint64         `toml:",omitempty"` // The timeout for each Istanbul round in milliseconds.
 	BlockPeriod    uint64         `toml:",omitempty"` // Default minimum difference between two consecutive block's timestamps in second
-	BlockPauseTime uint64         `toml:",omitempty"` // Delay time if no tx in block, the value should be larger than BlockPeriod
 	ProposerPolicy ProposerPolicy `toml:",omitempty"` // The policy for proposer selection
 	Epoch          uint64         `toml:",omitempty"` // The number of blocks after which to checkpoint and reset the pending votes
 }
@@ -34,7 +33,6 @@ type Config struct {
 var DefaultConfig = &Config{
 	RequestTimeout: 10000,
 	BlockPeriod:    1,
-	BlockPauseTime: 2,
 	ProposerPolicy: RoundRobin,
 	Epoch:          30000,
 }