consensus/istanbul: add block lock tests

consensus/istanbul/backend/backend_test.go

@@ -177,6 +177,44 @@ func TestCommit(t *testing.T) {
 	}
 }
 
+func TestHasBlock(t *testing.T) {
+	chain, engine := newBlockChain(1)
+	block := makeBlockWithoutSeal(chain, engine, chain.Genesis())
+	finalBlock, _ := engine.Seal(chain, block, nil)
+	chain.InsertChain(types.Blocks{finalBlock})
+	if engine.HasBlock(block.Hash(), finalBlock.Number()) {
+		t.Errorf("error mismatch: have true, want false")
+	}
+	if !engine.HasBlock(finalBlock.Hash(), finalBlock.Number()) {
+		t.Errorf("error mismatch: have false, want true")
+	}
+}
+
+func TestGetProposer(t *testing.T) {
+	chain, engine := newBlockChain(1)
+	block := makeBlock(chain, engine, chain.Genesis())
+	chain.InsertChain(types.Blocks{block})
+	expected := engine.GetProposer(1)
+	actual := engine.Address()
+	if actual != expected {
+		t.Errorf("proposer mismatch: have %v, want %v", actual.Hex(), expected.Hex())
+	}
+}
+
+func TestParentValidators(t *testing.T) {
+	chain, engine := newBlockChain(1)
+	block := makeBlock(chain, engine, chain.Genesis())
+	chain.InsertChain(types.Blocks{block})
+	expected := engine.Validators(block).List()
+	//Block without seal will make empty validator set
+	block = makeBlockWithoutSeal(chain, engine, block)
+	chain.InsertChain(types.Blocks{block})
+	actual := engine.ParentValidators(block).List()
+	if len(expected) != len(actual) || expected[0] != actual[0] {
+		t.Errorf("validator set mismatch: have %v, want %v", actual, expected)
+	}
+}
+
 /**
  * SimpleBackend
  * Private key: bb047e5940b6d83354d9432db7c449ac8fca2248008aaa7271369880f9f11cc1

consensus/istanbul/core/commit_test.go

@@ -178,6 +178,9 @@ OUTER:
 				if err != test.expectedErr {
 					t.Errorf("error mismatch: have %v, want %v", err, test.expectedErr)
 				}
+				if r0.current.IsHashLocked() {
+					t.Errorf("block should not be locked")
+				}
 				continue OUTER
 			}
 		}
@@ -191,7 +194,9 @@ OUTER:
 			if r0.current.Commits.Size() > 2*r0.valSet.F() {
 				t.Errorf("the size of commit messages should be less than %v", 2*r0.valSet.F()+1)
 			}
-
+			if r0.current.IsHashLocked() {
+				t.Errorf("block should not be locked")
+			}
 			continue
 		}
 
@@ -214,6 +219,9 @@ OUTER:
 		if signedCount <= 2*r0.valSet.F() {
 			t.Errorf("the expected signed count should be larger than %v, but got %v", 2*r0.valSet.F(), signedCount)
 		}
+		if !r0.current.IsHashLocked() {
+			t.Errorf("block should be locked")
+		}
 	}
 }
 

consensus/istanbul/core/core.go

@@ -188,14 +188,14 @@ func (c *core) startNewRound(newView *istanbul.View, roundChange bool) {
 	// Clear invalid round change messages
 	c.roundChangeSet = newRoundChangeSet(c.valSet)
 	// New snapshot for new round
-	c.updateRoundState(newView, c.valSet)
+	c.updateRoundState(newView, c.valSet, roundChange)
 	// Calculate new proposer
 	c.valSet.CalcProposer(c.lastProposer, newView.Round.Uint64())
 	c.waitingForRoundChange = false
 	c.setState(StateAcceptRequest)
 	if roundChange && c.isProposer() {
 		// If it is locked, propose the old proposal
-		if c.current.IsHashLocked() {
+		if c.current != nil && c.current.IsHashLocked() {
 			r := &istanbul.Request{
 				Proposal: c.current.Proposal(), //c.current.Proposal would be the locked proposal by previous proposer, see updateRoundState
 			}
@@ -218,17 +218,17 @@ func (c *core) catchUpRound(view *istanbul.View) {
 	c.waitingForRoundChange = true
 
 	//Needs to keep block lock for round catching up
-	c.updateRoundState(view, c.valSet)
+	c.updateRoundState(view, c.valSet, true)
 	c.roundChangeSet.Clear(view.Round)
 	c.newRoundChangeTimer()
 
 	logger.Trace("Catch up round", "new_round", view.Round, "new_seq", view.Sequence, "new_proposer", c.valSet)
 }
 
 // updateRoundState updates round state by checking if locking block is necessary
-func (c *core) updateRoundState(view *istanbul.View, validatorSet istanbul.ValidatorSet) {
-	// Lock only if both keepLock is true and it is locked
-	if c.current.IsHashLocked() {
+func (c *core) updateRoundState(view *istanbul.View, validatorSet istanbul.ValidatorSet, roundChange bool) {
+	// Lock only if both roundChange is true and it is locked
+	if roundChange && c.current != nil && c.current.IsHashLocked() {
 		c.current = newRoundState(view, validatorSet, c.current.GetLockedHash(), c.current.Preprepare)
 	} else {
 		c.current = newRoundState(view, validatorSet, common.Hash{}, nil)

consensus/istanbul/core/prepare_test.go

@@ -201,6 +201,9 @@ OUTER:
 				if err != test.expectedErr {
 					t.Errorf("error mismatch: have %v, want %v", err, test.expectedErr)
 				}
+				if r0.current.IsHashLocked() {
+					t.Errorf("block should not be locked")
+				}
 				continue OUTER
 			}
 		}
@@ -214,6 +217,9 @@ OUTER:
 			if r0.current.Prepares.Size() > 2*r0.valSet.F() {
 				t.Errorf("the size of prepare messages should be less than %v", 2*r0.valSet.F()+1)
 			}
+			if r0.current.IsHashLocked() {
+				t.Errorf("block should not be locked")
+			}
 
 			continue
 		}
@@ -246,6 +252,9 @@ OUTER:
 		if !reflect.DeepEqual(m, expectedSubject) {
 			t.Errorf("subject mismatch: have %v, want %v", m, expectedSubject)
 		}
+		if !r0.current.IsHashLocked() {
+			t.Errorf("block should be locked")
+		}
 	}
 }
 

consensus/istanbul/core/preprepare_test.go

@@ -39,6 +39,7 @@ func TestHandlePreprepare(t *testing.T) {
 		system          *testSystem
 		expectedRequest istanbul.Proposal
 		expectedErr     error
+		existingBlock   bool
 	}{
 		{
 			// normal case
@@ -56,6 +57,7 @@ func TestHandlePreprepare(t *testing.T) {
 			}(),
 			newTestProposal(),
 			nil,
+			false,
 		},
 		{
 			// future message
@@ -84,6 +86,7 @@ func TestHandlePreprepare(t *testing.T) {
 			}(),
 			makeBlock(1),
 			errFutureMessage,
+			false,
 		},
 		{
 			// non-proposer
@@ -105,6 +108,7 @@ func TestHandlePreprepare(t *testing.T) {
 			}(),
 			makeBlock(1),
 			errNotFromProposer,
+			false,
 		},
 		{
 			// ErrInvalidMessage
@@ -124,6 +128,27 @@ func TestHandlePreprepare(t *testing.T) {
 			}(),
 			makeBlock(1),
 			errOldMessage,
+			false,
+		},
+		{
+			// ErrInvalidMessage
+			func() *testSystem {
+				sys := NewTestSystemWithBackend(N, F)
+
+				for i, backend := range sys.backends {
+					c := backend.engine.(*core)
+					c.valSet = backend.peers
+					if i != 0 {
+						c.state = StatePreprepared
+						c.current.SetSequence(big.NewInt(10))
+						c.current.SetRound(big.NewInt(10))
+					}
+				}
+				return sys
+			}(),
+			makeBlock(5), //only height 5 will retrun true on backend.HasBlock, see testSystemBackend.HashBlock
+			nil,
+			true,
 		},
 	}
 
@@ -167,7 +192,7 @@ OUTER:
 				t.Errorf("state mismatch: have %v, want %v", c.state, StatePreprepared)
 			}
 
-			if !reflect.DeepEqual(c.current.Subject().View, curView) {
+			if !test.existingBlock && !reflect.DeepEqual(c.current.Subject().View, curView) {
 				t.Errorf("view mismatch: have %v, want %v", c.current.Subject().View, curView)
 			}
 
@@ -178,17 +203,116 @@ OUTER:
 				t.Errorf("error mismatch: have %v, want nil", err)
 			}
 
-			if decodedMsg.Code != msgPrepare {
-				t.Errorf("message code mismatch: have %v, want %v", decodedMsg.Code, msgPrepare)
+			expectedCode := msgPrepare
+			if test.existingBlock {
+				expectedCode = msgCommit
+			}
+			if decodedMsg.Code != expectedCode {
+				t.Errorf("message code mismatch: have %v, want %v", decodedMsg.Code, expectedCode)
 			}
+
 			var subject *istanbul.Subject
 			err = decodedMsg.Decode(&subject)
 			if err != nil {
 				t.Errorf("error mismatch: have %v, want nil", err)
 			}
-			if !reflect.DeepEqual(subject, c.current.Subject()) {
+			if !test.existingBlock && !reflect.DeepEqual(subject, c.current.Subject()) {
 				t.Errorf("subject mismatch: have %v, want %v", subject, c.current.Subject())
 			}
+
+		}
+	}
+}
+
+func TestHandlePreprepareWithLock(t *testing.T) {
+	N := uint64(4) // replica 0 is primary, it will send messages to others
+	F := uint64(1) // F does not affect tests
+	proposal := newTestProposal()
+	mismatchProposal := makeBlock(10)
+	newSystem := func() *testSystem {
+		sys := NewTestSystemWithBackend(N, F)
+
+		for i, backend := range sys.backends {
+			c := backend.engine.(*core)
+			c.valSet = backend.peers
+			if i != 0 {
+				c.state = StateAcceptRequest
+			}
+			c.roundChangeSet = newRoundChangeSet(c.valSet)
+		}
+		return sys
+	}
+
+	testCases := []struct {
+		system       *testSystem
+		proposal     istanbul.Proposal
+		lockProposal istanbul.Proposal
+	}{
+		{
+			newSystem(),
+			proposal,
+			proposal,
+		},
+		{
+			newSystem(),
+			proposal,
+			mismatchProposal,
+		},
+	}
+
+	for _, test := range testCases {
+		test.system.Run(false)
+		v0 := test.system.backends[0]
+		r0 := v0.engine.(*core)
+		curView := r0.currentView()
+		preprepare := &istanbul.Preprepare{
+			View:     curView,
+			Proposal: test.proposal,
+		}
+		lockPreprepare := &istanbul.Preprepare{
+			View:     curView,
+			Proposal: test.lockProposal,
+		}
+
+		for i, v := range test.system.backends {
+			// i == 0 is primary backend, it is responsible for send preprepare messages to others.
+			if i == 0 {
+				continue
+			}
+
+			c := v.engine.(*core)
+			c.current.SetPreprepare(lockPreprepare)
+			c.current.LockHash()
+			m, _ := Encode(preprepare)
+			_, val := r0.valSet.GetByAddress(v0.Address())
+			if err := c.handlePreprepare(&message{
+				Code:    msgPreprepare,
+				Msg:     m,
+				Address: v0.Address(),
+			}, val); err != nil {
+				t.Errorf("error mismatch: have %v, want nil", err)
+			}
+			if test.proposal == test.lockProposal {
+				if c.state != StatePrepared {
+					t.Errorf("state mismatch: have %v, want %v", c.state, StatePrepared)
+				}
+				if !reflect.DeepEqual(curView, c.currentView()) {
+					t.Errorf("view mismatch: have %v, want %v", c.currentView(), curView)
+				}
+			} else {
+				// Should stay at StateAcceptRequest
+				if c.state != StateAcceptRequest {
+					t.Errorf("state mismatch: have %v, want %v", c.state, StateAcceptRequest)
+				}
+				// Should have triggered a round change
+				expectedView := &istanbul.View{
+					Sequence: curView.Sequence,
+					Round:    big.NewInt(1),
+				}
+				if !reflect.DeepEqual(expectedView, c.currentView()) {
+					t.Errorf("view mismatch: have %v, want %v", c.currentView(), expectedView)
+				}
+			}
 		}
 	}
 }

consensus/istanbul/core/roundstate_test.go

@@ -17,8 +17,11 @@
 package core
 
 import (
+	"math/big"
 	"sync"
+	"testing"
 
+	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/consensus/istanbul"
 )
 
@@ -33,3 +36,39 @@ func newTestRoundState(view *istanbul.View, validatorSet istanbul.ValidatorSet)
 		mu:          new(sync.RWMutex),
 	}
 }
+
+func TestLockHash(t *testing.T) {
+	sys := NewTestSystemWithBackend(1, 0)
+	rs := newTestRoundState(
+		&istanbul.View{
+			Round:    big.NewInt(0),
+			Sequence: big.NewInt(0),
+		},
+		sys.backends[0].peers,
+	)
+	if !common.EmptyHash(rs.GetLockedHash()) {
+		t.Errorf("error mismatch: have %v, want empty", rs.GetLockedHash())
+	}
+	if rs.IsHashLocked() {
+		t.Error("IsHashLocked should return false")
+	}
+
+	// Lock
+	expected := rs.Proposal().Hash()
+	rs.LockHash()
+	if expected != rs.GetLockedHash() {
+		t.Errorf("error mismatch: have %v, want %v", rs.GetLockedHash(), expected)
+	}
+	if !rs.IsHashLocked() {
+		t.Error("IsHashLocked should return true")
+	}
+
+	// Unlock
+	rs.UnlockHash()
+	if !common.EmptyHash(rs.GetLockedHash()) {
+		t.Errorf("error mismatch: have %v, want empty", rs.GetLockedHash())
+	}
+	if rs.IsHashLocked() {
+		t.Error("IsHashLocked should return false")
+	}
+}