Update block validation interface to provide additional context

.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      \

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=<nil> at height=%v and round=%v (invalid proposal)", propose.height, propose.round)
+					p.logger.Warnf("prevoted=<nil> 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=<nil> at height=%v and round=%v (invalid proposal)", prevote.height, prevote.round)
+					p.logger.Warnf("prevoted=<nil> 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
 	}
 

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

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) {

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())
 		})

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}
 }
 

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
 }
 

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 {