diff --git a/.circleci/config.yml b/.circleci/config.yml index 3d14d6d9..cd2e77a4 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -23,6 +23,7 @@ jobs: go get -u golang.org/x/crypto/... go get -u github.com/loongy/covermerge go get -u github.com/mattn/goveralls + go get -u golang.org/x/lint/golint CI=true /go/bin/ginkgo -v --race --cover --coverprofile coverprofile.out ./... /go/bin/covermerge \ block/coverprofile.out \ diff --git a/process/process.go b/process/process.go index 21f359a0..fa420fcc 100644 --- a/process/process.go +++ b/process/process.go @@ -36,7 +36,7 @@ type Proposer interface { // A Validator validates a `block.Block` that has been proposed. type Validator interface { - IsBlockValid(block block.Block, checkHistory bool) bool + IsBlockValid(block block.Block, checkHistory bool) error } // An Observer is notified when note-worthy events happen for the first time. @@ -265,7 +265,8 @@ func (p *Process) handlePropose(propose *Propose) { // while currentStep = StepPropose if p.state.CurrentStep == StepPropose { var prevote *Prevote - if p.validator.IsBlockValid(propose.Block(), true) && (p.state.LockedRound == block.InvalidRound || p.state.LockedBlock.Equal(propose.Block())) { + err := p.validator.IsBlockValid(propose.Block(), true) + if err == nil && (p.state.LockedRound == block.InvalidRound || p.state.LockedBlock.Equal(propose.Block())) { prevote = NewPrevote( p.state.CurrentHeight, p.state.CurrentRound, @@ -278,7 +279,7 @@ func (p *Process) handlePropose(propose *Propose) { p.state.CurrentRound, block.InvalidHash, ) - p.logger.Debugf("prevoted= at height=%v and round=%v (invalid proposal)", propose.height, propose.round) + p.logger.Warnf("prevoted= at height=%v and round=%v (invalid propose: %v)", propose.height, propose.round, err) } p.state.CurrentStep = StepPrevote p.broadcaster.Broadcast(prevote) @@ -436,7 +437,8 @@ func (p *Process) checkProposeInCurrentHeightAndRoundWithPrevotes() { // while step = StepPropose and validRound >= 0 and validRound < currentRound if p.state.CurrentStep == StepPropose && propose.ValidRound() < p.state.CurrentRound { var prevote *Prevote - if p.validator.IsBlockValid(propose.Block(), true) && (p.state.LockedRound <= propose.ValidRound() || p.state.LockedBlock.Equal(propose.Block())) { + err := p.validator.IsBlockValid(propose.Block(), true) + if err == nil && (p.state.LockedRound <= propose.ValidRound() || p.state.LockedBlock.Equal(propose.Block())) { prevote = NewPrevote( p.state.CurrentHeight, p.state.CurrentRound, @@ -449,7 +451,7 @@ func (p *Process) checkProposeInCurrentHeightAndRoundWithPrevotes() { p.state.CurrentRound, block.InvalidHash, ) - p.logger.Debugf("prevoted= at height=%v and round=%v (invalid proposal)", prevote.height, prevote.round) + p.logger.Warnf("prevoted= at height=%v and round=%v (invalid propose: %v)", prevote.height, prevote.round, err) } p.state.CurrentStep = StepPrevote @@ -476,7 +478,8 @@ func (p *Process) checkProposeInCurrentHeightAndRoundWithPrevotesForTheFirstTime // and 2f+1 Prevote{currentHeight, currentRound, blockHash} while Validate(block) and step >= StepPrevote for the first time n := p.state.Prevotes.QueryByHeightRoundBlockHash(p.state.CurrentHeight, p.state.CurrentRound, propose.BlockHash()) if n > 2*p.state.Prevotes.F() { - if p.state.CurrentStep >= StepPrevote && p.validator.IsBlockValid(propose.Block(), true) { + err := p.validator.IsBlockValid(propose.Block(), true) + if p.state.CurrentStep >= StepPrevote && err == nil { p.state.ValidBlock = propose.Block() p.state.ValidRound = p.state.CurrentRound if p.state.CurrentStep == StepPrevote { @@ -491,6 +494,8 @@ func (p *Process) checkProposeInCurrentHeightAndRoundWithPrevotesForTheFirstTime p.logger.Debugf("precommitted=%v at height=%v and round=%v", precommit.blockHash, p.state.CurrentHeight, p.state.CurrentRound) p.broadcaster.Broadcast(precommit) } + } else { + p.logger.Warnf("nothing precommitted at height=%v, round=%v and step=%v (invalid block: %v)", propose.height, propose.round, p.state.CurrentStep, err) } } } @@ -508,7 +513,8 @@ func (p *Process) checkProposeInCurrentHeightWithPrecommits(round block.Round) { if n > 2*p.state.Precommits.F() { // while !BlockExistsAtHeight(currentHeight) if !p.blockchain.BlockExistsAtHeight(p.state.CurrentHeight) { - if p.validator.IsBlockValid(propose.Block(), false) { + err := p.validator.IsBlockValid(propose.Block(), false) + if err == nil { p.blockchain.InsertBlockAtHeight(p.state.CurrentHeight, propose.Block()) p.state.CurrentHeight++ p.state.Reset(p.state.CurrentHeight - 1) @@ -517,6 +523,8 @@ func (p *Process) checkProposeInCurrentHeightWithPrecommits(round block.Round) { } p.logger.Infof("✅ committed block=%v at height=%v", propose.BlockHash(), propose.height) p.startRound(0) + } else { + p.logger.Warnf("nothing committed at height=%v and round=%v (invalid block: %v)", propose.height, propose.round, err) } } } @@ -528,9 +536,10 @@ func (p *Process) syncLatestCommit(latestCommit LatestCommit) { return } - // Check the proposed block and previous block with checking historical data.. - // It needs the validator to store the previous execute state. - if !p.validator.IsBlockValid(latestCommit.Block, false) { + // Check the proposed block and previous block without historical data. It + // needs the validator to store the previous execute state. + if err := p.validator.IsBlockValid(latestCommit.Block, false); err != nil { + p.logger.Warnf("error syncing to height=%v and round=%v (invalid block: %v)", latestCommit.Block.Header().Height(), latestCommit.Block.Header().Round(), err) return } diff --git a/process/process_test.go b/process/process_test.go index 0dd2c031..3969b549 100644 --- a/process/process_test.go +++ b/process/process_test.go @@ -5,6 +5,7 @@ import ( "crypto/ecdsa" cRand "crypto/rand" "encoding/json" + "fmt" "math/rand" "time" @@ -138,7 +139,7 @@ var _ = Describe("Process", func() { privateKey := newEcdsaKey() scheduler := NewMockScheduler(id.NewSignatory(privateKey.PublicKey)) processOrigin.Scheduler = scheduler - processOrigin.Validator = NewMockValidator(false) + processOrigin.Validator = NewMockValidator(fmt.Errorf("")) process := processOrigin.ToProcess() // Generate a invalid proposal @@ -501,7 +502,7 @@ var _ = Describe("Process", func() { processOrigin.State.CurrentHeight = height processOrigin.State.CurrentRound = round processOrigin.State.CurrentStep = StepPropose - processOrigin.Validator = NewMockValidator(false) + processOrigin.Validator = NewMockValidator(fmt.Errorf("")) process := processOrigin.ToProcess() // Send the proposal diff --git a/replica/rebase.go b/replica/rebase.go index 382ad966..98595579 100644 --- a/replica/rebase.go +++ b/replica/rebase.go @@ -26,7 +26,7 @@ type BlockIterator interface { } type Validator interface { - IsBlockValid(block block.Block, checkHistory bool, shard Shard) bool + IsBlockValid(block block.Block, checkHistory bool, shard Shard) error } type Observer interface { @@ -110,31 +110,37 @@ func (rebaser *shardRebaser) BlockProposal(height block.Height, round block.Roun return block.New(header, data, prevState) } -func (rebaser *shardRebaser) IsBlockValid(proposedBlock block.Block, checkHistory bool) bool { +func (rebaser *shardRebaser) IsBlockValid(proposedBlock block.Block, checkHistory bool) error { rebaser.mu.Lock() defer rebaser.mu.Unlock() // Check the expected `block.Kind` if proposedBlock.Header().Kind() != rebaser.expectedKind { - return false + return fmt.Errorf("unexpected block kind: expected %v, got %v", rebaser.expectedKind, proposedBlock.Header().Kind()) } switch proposedBlock.Header().Kind() { case block.Standard: if proposedBlock.Header().Signatories() != nil { - return false + return fmt.Errorf("expected standard block to have nil signatories") } case block.Rebase: if !proposedBlock.Header().Signatories().Equal(rebaser.expectedRebaseSigs) { - return false + return fmt.Errorf("unexpected signatories in rebase block: expected %d, got %d", len(rebaser.expectedRebaseSigs), len(proposedBlock.Header().Signatories())) } + // TODO: Transactions are expected to be nil (the plan is not expected + // to be nil, because there are "default" computations that might need + // to be done every block). case block.Base: if !proposedBlock.Header().Signatories().Equal(rebaser.expectedRebaseSigs) { - return false + return fmt.Errorf("unexpected signatories in base block: expected %d, got %d", len(rebaser.expectedRebaseSigs), len(proposedBlock.Header().Signatories())) } if proposedBlock.Data() != nil { - return false + // TODO: Transactions are expected to be nil (the plan is not expected + // to be nil, because there are "default" computations that might need + // to be done every block). + return fmt.Errorf("expected base block to have nil data") } default: @@ -143,46 +149,46 @@ func (rebaser *shardRebaser) IsBlockValid(proposedBlock block.Block, checkHistor // Check the expected `block.Hash` if !proposedBlock.Hash().Equal(block.ComputeHash(proposedBlock.Header(), proposedBlock.Data(), proposedBlock.PreviousState())) { - return false + return fmt.Errorf("unexpected block hash for proposed block") } // Check against the parent `block.Block` if checkHistory { parentBlock, ok := rebaser.blockStorage.Blockchain(rebaser.shard).BlockAtHeight(proposedBlock.Header().Height() - 1) if !ok { - return false + return fmt.Errorf("block at height=%d not found", proposedBlock.Header().Height()-1) } if proposedBlock.Header().Timestamp() < parentBlock.Header().Timestamp() { - return false + return fmt.Errorf("expected timestamp for proposed block to be greater than parent block") } if proposedBlock.Header().Timestamp() > block.Timestamp(time.Now().Unix()) { - return false + return fmt.Errorf("expected timestamp for proposed block to be less than current time") } if !proposedBlock.Header().ParentHash().Equal(parentBlock.Hash()) { - return false + return fmt.Errorf("expected parent hash for proposed block to equal parent block hash") } // Check that the parent is the most recently finalised latestBlock := rebaser.blockStorage.LatestBlock(rebaser.shard) if !parentBlock.Hash().Equal(latestBlock.Hash()) { - return false + return fmt.Errorf("expected parent block hash to equal latest block hash") } if parentBlock.Hash().Equal(block.InvalidHash) { - return false + return fmt.Errorf("parent block hash should not be invalid") } } // Check against the base `block.Block` baseBlock := rebaser.blockStorage.LatestBaseBlock(rebaser.shard) if !proposedBlock.Header().BaseHash().Equal(baseBlock.Hash()) { - return false + return fmt.Errorf("expected base hash for proposed block to equal base block hash") } // Pass to the next `process.Validator` if rebaser.validator != nil { return rebaser.validator.IsBlockValid(proposedBlock, checkHistory, rebaser.shard) } - return true + return nil } func (rebaser *shardRebaser) DidCommitBlock(height block.Height) { diff --git a/replica/rebase_test.go b/replica/rebase_test.go index 2cc9198a..fb8a7d21 100644 --- a/replica/rebase_test.go +++ b/replica/rebase_test.go @@ -51,7 +51,7 @@ var _ = Describe("shardRebaser", func() { test := func(shard Shard) bool { store, initHeight, _ := initStorage(shard) iter := mockBlockIterator{} - validator := newMockValidator(true) + validator := newMockValidator(nil) rebaser := newShardRebaser(store, iter, validator, nil, shard) // Generate a valid propose block. @@ -64,7 +64,7 @@ var _ = Describe("shardRebaser", func() { header.Timestamp = block.Timestamp(time.Now().Unix()) proposedBlock := block.New(header.ToBlockHeader(), nil, nil) - return rebaser.IsBlockValid(proposedBlock, true) + return rebaser.IsBlockValid(proposedBlock, true) == nil } Expect(quick.Check(test, nil)).Should(Succeed()) @@ -166,7 +166,7 @@ var _ = Describe("shardRebaser", func() { header.Timestamp = block.Timestamp(time.Now().Unix() - 1) header.Signatories = sigs rebaseBlock := block.New(header.ToBlockHeader(), nil, nil) - Expect(rebaser.IsBlockValid(rebaseBlock, true)).Should(BeTrue()) + Expect(rebaser.IsBlockValid(rebaseBlock, true)).Should(BeNil()) // After the block been committed commitBlock(store, shard, rebaseBlock) @@ -182,7 +182,7 @@ var _ = Describe("shardRebaser", func() { baseHeader.Signatories = sigs baseBlock := block.New(baseHeader.ToBlockHeader(), nil, nil) - return rebaser.IsBlockValid(baseBlock, true) + return rebaser.IsBlockValid(baseBlock, true) == nil } Expect(quick.Check(test, nil)).Should(Succeed()) }) diff --git a/replica/replica_suite_test.go b/replica/replica_suite_test.go index 0b9933dd..10e76cc1 100644 --- a/replica/replica_suite_test.go +++ b/replica/replica_suite_test.go @@ -76,14 +76,14 @@ func (m mockBlockIterator) NextBlock(kind block.Kind, height block.Height, shard } type mockValidator struct { - valid bool + valid error } -func (m mockValidator) IsBlockValid(block.Block, bool, Shard) bool { +func (m mockValidator) IsBlockValid(block.Block, bool, Shard) error { return m.valid } -func newMockValidator(valid bool) Validator { +func newMockValidator(valid error) Validator { return mockValidator{valid: valid} } diff --git a/testutil/process.go b/testutil/process.go index 3f2756fb..11ad60ad 100644 --- a/testutil/process.go +++ b/testutil/process.go @@ -180,7 +180,7 @@ func NewProcessOrigin(f int) ProcessOrigin { BroadcastMessages: messages, Proposer: NewMockProposer(privateKey), - Validator: NewMockValidator(true), + Validator: NewMockValidator(nil), Scheduler: NewMockScheduler(sig), Broadcaster: NewMockBroadcaster(messages), Timer: NewMockTimer(1 * time.Second), @@ -296,14 +296,14 @@ func (m *MockProposer) BlockProposal(height block.Height, round block.Round) blo } type MockValidator struct { - valid bool + valid error } -func NewMockValidator(valid bool) process.Validator { +func NewMockValidator(valid error) process.Validator { return MockValidator{valid: valid} } -func (m MockValidator) IsBlockValid(block.Block, bool) bool { +func (m MockValidator) IsBlockValid(block.Block, bool) error { return m.valid } diff --git a/testutil/replica/replica.go b/testutil/replica/replica.go index 9db0b5b6..84e35197 100644 --- a/testutil/replica/replica.go +++ b/testutil/replica/replica.go @@ -80,23 +80,23 @@ func NewMockValidator(store *MockPersistentStorage) replica.Validator { } } -func (m *MockValidator) IsBlockValid(b block.Block, checkHistory bool, shard replica.Shard) bool { +func (m *MockValidator) IsBlockValid(b block.Block, checkHistory bool, shard replica.Shard) error { height := b.Header().Height() prevState := b.PreviousState() blockchain := m.store.MockBlockchain(shard) if !checkHistory { - return true + return nil } state, ok := blockchain.StateAtHeight(height - 1) if !ok { - return false + return fmt.Errorf("failed to get state at height %d", height-1) } if !bytes.Equal(prevState, state) { - return false + return fmt.Errorf("invalid previous state") } - return true + return nil } type MockObserver struct {