From 09ea54001825cb586b675f26c9843dbe41a5e96e Mon Sep 17 00:00:00 2001 From: Yute Lin Date: Wed, 19 Jul 2017 16:50:43 +0800 Subject: [PATCH] consensus/istanbul: add block lock tests --- consensus/istanbul/backend/backend_test.go | 38 ++++++ consensus/istanbul/core/commit_test.go | 10 +- consensus/istanbul/core/core.go | 12 +- consensus/istanbul/core/prepare_test.go | 9 ++ consensus/istanbul/core/preprepare_test.go | 132 +++++++++++++++++++- consensus/istanbul/core/roundstate_test.go | 39 ++++++ consensus/istanbul/core/testbackend_test.go | 13 ++ 7 files changed, 242 insertions(+), 11 deletions(-) diff --git a/consensus/istanbul/backend/backend_test.go b/consensus/istanbul/backend/backend_test.go index 9c6bfd3dd823..52cbca7ead7a 100644 --- a/consensus/istanbul/backend/backend_test.go +++ b/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 diff --git a/consensus/istanbul/core/commit_test.go b/consensus/istanbul/core/commit_test.go index 2e8e5e254956..02107dd8136b 100644 --- a/consensus/istanbul/core/commit_test.go +++ b/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") + } } } diff --git a/consensus/istanbul/core/core.go b/consensus/istanbul/core/core.go index fccb94534085..e1ccfbdcf26a 100644 --- a/consensus/istanbul/core/core.go +++ b/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,7 +218,7 @@ 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() @@ -226,9 +226,9 @@ func (c *core) catchUpRound(view *istanbul.View) { } // 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) diff --git a/consensus/istanbul/core/prepare_test.go b/consensus/istanbul/core/prepare_test.go index a41c22d12c72..deb7705a267e 100644 --- a/consensus/istanbul/core/prepare_test.go +++ b/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") + } } } diff --git a/consensus/istanbul/core/preprepare_test.go b/consensus/istanbul/core/preprepare_test.go index eb86e550fca8..3abe0069d841 100644 --- a/consensus/istanbul/core/preprepare_test.go +++ b/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) + } + } } } } diff --git a/consensus/istanbul/core/roundstate_test.go b/consensus/istanbul/core/roundstate_test.go index 0e3ee0ef0f9d..14a2c9c2c797 100644 --- a/consensus/istanbul/core/roundstate_test.go +++ b/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") + } +} diff --git a/consensus/istanbul/core/testbackend_test.go b/consensus/istanbul/core/testbackend_test.go index 2cc66e7ac7d6..a2ed8dd804ec 100644 --- a/consensus/istanbul/core/testbackend_test.go +++ b/consensus/istanbul/core/testbackend_test.go @@ -133,6 +133,19 @@ func (self *testSystemBackend) NewRequest(request istanbul.Proposal) { }) } +// Only block height 5 will return true +func (self *testSystemBackend) HasBlock(hash common.Hash, number *big.Int) bool { + return number.Cmp(big.NewInt(5)) == 0 +} + +func (self *testSystemBackend) GetProposer(number uint64) common.Address { + return common.Address{} +} + +func (self *testSystemBackend) ParentValidators(proposal istanbul.Proposal) istanbul.ValidatorSet { + return self.peers +} + // ============================================== // // define the struct that need to be provided for integration tests.