Merge pull request #155 from getamis/feature/consensus_failed

Fix consensus failed if the pause timestamp isn't 1

Author: markya0616

cmd/geth/main.go

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

cmd/geth/usage.go

@@ -22,10 +22,11 @@ import (
 	"io"
 	"sort"
 
+	"strings"
+
 	"github.com/ethereum/go-ethereum/cmd/utils"
 	"github.com/ethereum/go-ethereum/internal/debug"
 	"gopkg.in/urfave/cli.v1"
-	"strings"
 )
 
 // AppHelpTemplate is the test template for the default, global app help topic.
@@ -226,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
@@ -964,9 +959,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)
-	}
 }
 
 // checkExclusive verifies that only a single isntance of the provided flags was

consensus/istanbul/backend/backend.go

@@ -239,7 +239,7 @@ func (sb *backend) Verify(proposal istanbul.Proposal) (time.Duration, error) {
 
 // Sign implements istanbul.Backend.Sign
 func (sb *backend) Sign(data []byte) ([]byte, error) {
-	hashData := crypto.Keccak256([]byte(data))
+	hashData := crypto.Keccak256(data)
 	return crypto.Sign(hashData, sb.privateKey)
 }
 

consensus/istanbul/backend/backend_test.go

@@ -148,11 +148,8 @@ func TestCommit(t *testing.T) {
 	for _, test := range testCases {
 		expBlock := test.expectedBlock()
 		go func() {
-			select {
-			case result := <-backend.commitCh:
-				commitCh <- result
-				return
-			}
+			result := <-backend.commitCh
+			commitCh <- result
 		}()
 
 		backend.proposedBlockHash = expBlock.Hash()

consensus/istanbul/backend/engine.go

@@ -81,7 +81,7 @@ var (
 	// errEmptyCommittedSeals is returned if the field of committed seals is zero.
 	errEmptyCommittedSeals = errors.New("zero committed seals")
 	// errMismatchTxhashes is returned if the TxHash in header is mismatch.
-	errMismatchTxhashes = errors.New("mismatch transcations hashes")
+	errMismatchTxhashes = errors.New("mismatch transactions hashes")
 )
 var (
 	defaultDifficulty = big.NewInt(1)
@@ -366,6 +366,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
 }
 
@@ -454,21 +460,7 @@ func (sb *backend) CalcDifficulty(chain consensus.ChainReader, time uint64, pare
 
 // 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/engine_test.go

@@ -163,14 +163,12 @@ func TestSealStopChannel(t *testing.T) {
 	stop := make(chan struct{}, 1)
 	eventSub := engine.EventMux().Subscribe(istanbul.RequestEvent{})
 	eventLoop := func() {
-		select {
-		case ev := <-eventSub.Chan():
-			_, ok := ev.Data.(istanbul.RequestEvent)
-			if !ok {
-				t.Errorf("unexpected event comes: %v", reflect.TypeOf(ev.Data))
-			}
-			stop <- struct{}{}
+		ev := <-eventSub.Chan()
+		_, ok := ev.Data.(istanbul.RequestEvent)
+		if !ok {
+			t.Errorf("unexpected event comes: %v", reflect.TypeOf(ev.Data))
 		}
+		stop <- struct{}{}
 		eventSub.Unsubscribe()
 	}
 	go eventLoop()
@@ -189,14 +187,12 @@ func TestSealCommittedOtherHash(t *testing.T) {
 	otherBlock := makeBlockWithoutSeal(chain, engine, block)
 	eventSub := engine.EventMux().Subscribe(istanbul.RequestEvent{})
 	eventLoop := func() {
-		select {
-		case ev := <-eventSub.Chan():
-			_, ok := ev.Data.(istanbul.RequestEvent)
-			if !ok {
-				t.Errorf("unexpected event comes: %v", reflect.TypeOf(ev.Data))
-			}
-			engine.Commit(otherBlock, [][]byte{})
+		ev := <-eventSub.Chan()
+		_, ok := ev.Data.(istanbul.RequestEvent)
+		if !ok {
+			t.Errorf("unexpected event comes: %v", reflect.TypeOf(ev.Data))
 		}
+		engine.Commit(otherBlock, [][]byte{})
 		eventSub.Unsubscribe()
 	}
 	go eventLoop()
@@ -208,10 +204,8 @@ func TestSealCommittedOtherHash(t *testing.T) {
 
 	const timeoutDura = 2 * time.Second
 	timeout := time.NewTimer(timeoutDura)
-	select {
-	case <-timeout.C:
-		// wait 2 seconds to ensure we cannot get any blocks from Istanbul
-	}
+	<-timeout.C
+	// wait 2 seconds to ensure we cannot get any blocks from Istanbul
 }
 
 func TestSealCommitted(t *testing.T) {

consensus/istanbul/backend/snapshot.go

@@ -207,9 +207,9 @@ func (s *Snapshot) apply(headers []*types.Header) (*Snapshot, error) {
 		// Tally up the new vote from the validator
 		var authorize bool
 		switch {
-		case bytes.Compare(header.Nonce[:], nonceAuthVote) == 0:
+		case bytes.Equal(header.Nonce[:], nonceAuthVote):
 			authorize = true
-		case bytes.Compare(header.Nonce[:], nonceDropVote) == 0:
+		case bytes.Equal(header.Nonce[:], nonceDropVote):
 			authorize = false
 		default:
 			return nil, errInvalidVote

consensus/istanbul/backend/snapshot_test.go

@@ -354,7 +354,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,
@@ -417,7 +417,7 @@ func TestSaveAndLoad(t *testing.T) {
 			},
 		},
 		Tally: map[common.Address]Tally{
-			common.StringToAddress("1234567893"): Tally{
+			common.StringToAddress("1234567893"): {
 				Authorize: false,
 				Votes:     20,
 			},

consensus/istanbul/config.go

@@ -24,17 +24,15 @@ 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
 }
 
 var DefaultConfig = &Config{
 	RequestTimeout: 10000,
 	BlockPeriod:    1,
-	BlockPauseTime: 2,
 	ProposerPolicy: RoundRobin,
 	Epoch:          30000,
 }