Merge pull request ethereum#13870 from karalabe/miners-fixes

all: clean up various error handling in core and the miner

cmd/geth/main.go

@@ -161,12 +161,6 @@ func init() {
 		// Start system runtime metrics collection
 		go metrics.CollectProcessMetrics(3 * time.Second)
 
-		// This should be the only place where reporting is enabled
-		// because it is not intended to run while testing.
-		// In addition to this check, bad block reports are sent only
-		// for chains with the main network genesis block and network id 1.
-		eth.EnableBadBlockReporting = true
-
 		utils.SetupNetwork(ctx)
 		return nil
 	}

consensus/errors.go

@@ -0,0 +1,41 @@
+// Copyright 2017 The go-ethereum Authors
+// This file is part of the go-ethereum library.
+//
+// The go-ethereum library is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Lesser General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// The go-ethereum library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public License
+// along with the go-ethereum library. If not, see <http://www.gnu.org/licenses/>.
+
+package consensus
+
+import "errors"
+
+var (
+	// ErrUnknownAncestor is returned when validating a block requires an ancestor
+	// that is unknown.
+	ErrUnknownAncestor = errors.New("unknown ancestor")
+
+	// ErrLargeBlockTime is returned if the value of the timestamp is beyond
+	// any reasonable value.
+	ErrLargeBlockTime = errors.New("timestamp too big")
+
+	// ErrZeroBlockTime is returned if the block's timestamp is the same as the one
+	// its parent has.
+	ErrZeroBlockTime = errors.New("timestamp equals parent's")
+
+	// ErrFutureBlock is returned when a block's timestamp is in the future according
+	// to the current node.
+	ErrFutureBlock = errors.New("block in the future")
+
+	// ErrInvalidNumber is returned if a block's number doesn't equal it's parent's
+	// plus one.
+	ErrInvalidNumber = errors.New("invalid block number")
+)

consensus/ethash/consensus.go

@@ -42,20 +42,15 @@ var (
 )
 
 var (
-	ErrInvalidChain        = errors.New("invalid header chain")
-	ErrParentUnknown       = errors.New("parent not known locally")
-	ErrFutureBlock         = errors.New("block in the future")
-	ErrLargeBlockTimestamp = errors.New("timestamp too big")
-	ErrZeroBlockTime       = errors.New("timestamp equals parent's")
-	ErrInvalidNumber       = errors.New("invalid block number")
-	ErrTooManyUncles       = errors.New("too many uncles")
-	ErrDuplicateUncle      = errors.New("duplicate uncle")
-	ErrUncleIsAncestor     = errors.New("uncle is ancestor")
-	ErrDanglingUncle       = errors.New("uncle's parent is not ancestor")
-	ErrNonceOutOfRange     = errors.New("nonce out of range")
-	ErrInvalidDifficulty   = errors.New("non-positive difficulty")
-	ErrInvalidMixDigest    = errors.New("invalid mix digest")
-	ErrInvalidPoW          = errors.New("invalid proof-of-work")
+	ErrInvalidChain      = errors.New("invalid header chain")
+	ErrTooManyUncles     = errors.New("too many uncles")
+	ErrDuplicateUncle    = errors.New("duplicate uncle")
+	ErrUncleIsAncestor   = errors.New("uncle is ancestor")
+	ErrDanglingUncle     = errors.New("uncle's parent is not ancestor")
+	ErrNonceOutOfRange   = errors.New("nonce out of range")
+	ErrInvalidDifficulty = errors.New("non-positive difficulty")
+	ErrInvalidMixDigest  = errors.New("invalid mix digest")
+	ErrInvalidPoW        = errors.New("invalid proof-of-work")
 )
 
 // VerifyHeader checks whether a header conforms to the consensus rules of the
@@ -72,7 +67,7 @@ func (ethash *Ethash) VerifyHeader(chain consensus.ChainReader, header *types.He
 	}
 	parent := chain.GetHeader(header.ParentHash, number-1)
 	if parent == nil {
-		return ErrParentUnknown
+		return consensus.ErrUnknownAncestor
 	}
 	// Sanity checks passed, do a proper verification
 	return ethash.verifyHeader(chain, header, parent, false, seal)
@@ -125,7 +120,7 @@ func (ethash *Ethash) VerifyHeaders(chain consensus.ChainReader, headers []*type
 				case chain.GetHeader(headers[index].Hash(), headers[index].Number.Uint64()-1) != nil:
 					outputs <- result{index: index, err: nil}
 				case parent == nil:
-					failure = ErrParentUnknown
+					failure = consensus.ErrUnknownAncestor
 					outputs <- result{index: index, err: failure}
 				default:
 					failure = ethash.verifyHeader(chain, headers[index], parent, false, seals[index])
@@ -254,15 +249,15 @@ func (ethash *Ethash) verifyHeader(chain consensus.ChainReader, header, parent *
 	// Verify the header's timestamp
 	if uncle {
 		if header.Time.Cmp(math.MaxBig256) > 0 {
-			return ErrLargeBlockTimestamp
+			return consensus.ErrLargeBlockTime
 		}
 	} else {
 		if header.Time.Cmp(big.NewInt(time.Now().Unix())) > 0 {
-			return ErrFutureBlock
+			return consensus.ErrFutureBlock
 		}
 	}
 	if header.Time.Cmp(parent.Time) <= 0 {
-		return ErrZeroBlockTime
+		return consensus.ErrZeroBlockTime
 	}
 	// Verify the block's difficulty based in it's timestamp and parent's difficulty
 	expected := CalcDifficulty(chain.Config(), header.Time.Uint64(), parent.Time.Uint64(), parent.Number, parent.Difficulty)
@@ -282,7 +277,7 @@ func (ethash *Ethash) verifyHeader(chain consensus.ChainReader, header, parent *
 	}
 	// Verify that the block number is parent's +1
 	if diff := new(big.Int).Sub(header.Number, parent.Number); diff.Cmp(big.NewInt(1)) != 0 {
-		return ErrInvalidNumber
+		return consensus.ErrInvalidNumber
 	}
 	// Verify the engine specific seal securing the block
 	if seal {
@@ -449,7 +444,7 @@ func (ethash *Ethash) VerifySeal(chain consensus.ChainReader, header *types.Head
 func (ethash *Ethash) Prepare(chain consensus.ChainReader, header *types.Header) error {
 	parent := chain.GetHeader(header.ParentHash, header.Number.Uint64()-1)
 	if parent == nil {
-		return ErrParentUnknown
+		return consensus.ErrUnknownAncestor
 	}
 	header.Difficulty = CalcDifficulty(chain.Config(), header.Time.Uint64(),
 		parent.Time.Uint64(), parent.Number, parent.Difficulty)

core/block_validator.go

@@ -54,15 +54,15 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error {
 	// Check whether the block's known, and if not, that it's linkable
 	if v.bc.HasBlock(block.Hash()) {
 		if _, err := state.New(block.Root(), v.bc.chainDb); err == nil {
-			return &KnownBlockError{block.Number(), block.Hash()}
+			return ErrKnownBlock
 		}
 	}
 	parent := v.bc.GetBlock(block.ParentHash(), block.NumberU64()-1)
 	if parent == nil {
-		return ParentError(block.ParentHash())
+		return consensus.ErrUnknownAncestor
 	}
 	if _, err := state.New(parent.Root(), v.bc.chainDb); err != nil {
-		return ParentError(block.ParentHash())
+		return consensus.ErrUnknownAncestor
 	}
 	// Header validity is known at this point, check the uncles and transactions
 	header := block.Header()
@@ -82,10 +82,10 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error {
 // transition, such as amount of used gas, the receipt roots and the state root
 // itself. ValidateState returns a database batch if the validation was a success
 // otherwise nil and an error is returned.
-func (v *BlockValidator) ValidateState(block, parent *types.Block, statedb *state.StateDB, receipts types.Receipts, usedGas *big.Int) (err error) {
+func (v *BlockValidator) ValidateState(block, parent *types.Block, statedb *state.StateDB, receipts types.Receipts, usedGas *big.Int) error {
 	header := block.Header()
 	if block.GasUsed().Cmp(usedGas) != 0 {
-		return ValidationError(fmt.Sprintf("invalid gas used (remote: %v local: %v)", block.GasUsed(), usedGas))
+		return fmt.Errorf("invalid gas used (remote: %v local: %v)", block.GasUsed(), usedGas)
 	}
 	// Validate the received block's bloom with the one derived from the generated receipts.
 	// For valid blocks this should always validate to true.

core/blockchain.go

@@ -831,7 +831,7 @@ func (self *BlockChain) WriteBlock(block *types.Block) (status WriteStatus, err
 	// Calculate the total difficulty of the block
 	ptd := self.GetTd(block.ParentHash(), block.NumberU64()-1)
 	if ptd == nil {
-		return NonStatTy, ParentError(block.ParentHash())
+		return NonStatTy, consensus.ErrUnknownAncestor
 	}
 	// Make sure no inconsistent state is leaked during insertion
 	self.mu.Lock()
@@ -918,9 +918,8 @@ func (self *BlockChain) InsertChain(chain types.Blocks) (int, error) {
 		}
 		// If the header is a banned one, straight out abort
 		if BadHashes[block.Hash()] {
-			err := BadHashError(block.Hash())
-			self.reportBlock(block, nil, err)
-			return i, err
+			self.reportBlock(block, nil, ErrBlacklistedHash)
+			return i, ErrBlacklistedHash
 		}
 		// Wait for the block's verification to complete
 		bstart := time.Now()
@@ -930,25 +929,25 @@ func (self *BlockChain) InsertChain(chain types.Blocks) (int, error) {
 			err = self.Validator().ValidateBody(block)
 		}
 		if err != nil {
-			if IsKnownBlockErr(err) {
+			if err == ErrKnownBlock {
 				stats.ignored++
 				continue
 			}
 
-			if err == BlockFutureErr {
+			if err == consensus.ErrFutureBlock {
 				// Allow up to MaxFuture second in the future blocks. If this limit
 				// is exceeded the chain is discarded and processed at a later time
 				// if given.
 				max := big.NewInt(time.Now().Unix() + maxTimeFutureBlocks)
-				if block.Time().Cmp(max) == 1 {
-					return i, fmt.Errorf("%v: BlockFutureErr, %v > %v", BlockFutureErr, block.Time(), max)
+				if block.Time().Cmp(max) > 0 {
+					return i, fmt.Errorf("future block: %v > %v", block.Time(), max)
 				}
 				self.futureBlocks.Add(block.Hash(), block)
 				stats.queued++
 				continue
 			}
 
-			if IsParentErr(err) && self.futureBlocks.Contains(block.ParentHash()) {
+			if err == consensus.ErrUnknownAncestor && self.futureBlocks.Contains(block.ParentHash()) {
 				self.futureBlocks.Add(block.Hash(), block)
 				stats.queued++
 				continue

core/blockchain_test.go

@@ -126,7 +126,7 @@ func testBlockChainImport(chain types.Blocks, blockchain *BlockChain) error {
 			err = blockchain.validator.ValidateBody(block)
 		}
 		if err != nil {
-			if IsKnownBlockErr(err) {
+			if err == ErrKnownBlock {
 				continue
 			}
 			return err
@@ -441,8 +441,8 @@ func testBadHashes(t *testing.T, full bool) {
 		BadHashes[headers[2].Hash()] = true
 		_, err = bc.InsertHeaderChain(headers, 1)
 	}
-	if !IsBadHashError(err) {
-		t.Errorf("error mismatch: want: BadHashError, have: %v", err)
+	if err != ErrBlacklistedHash {
+		t.Errorf("error mismatch: have: %v, want: %v", err, ErrBlacklistedHash)
 	}
 }
 

core/dao.go

@@ -18,6 +18,7 @@ package core
 
 import (
 	"bytes"
+	"fmt"
 	"math/big"
 
 	"github.com/ethereum/go-ethereum/core/state"
@@ -46,11 +47,11 @@ func ValidateDAOHeaderExtraData(config *params.ChainConfig, header *types.Header
 	// Depending whether we support or oppose the fork, validate the extra-data contents
 	if config.DAOForkSupport {
 		if !bytes.Equal(header.Extra, params.DAOForkBlockExtra) {
-			return ValidationError("DAO pro-fork bad block extra-data: 0x%x", header.Extra)
+			return fmt.Errorf("DAO pro-fork bad block extra-data: 0x%x", header.Extra)
 		}
 	} else {
 		if bytes.Equal(header.Extra, params.DAOForkBlockExtra) {
-			return ValidationError("DAO no-fork bad block extra-data: 0x%x", header.Extra)
+			return fmt.Errorf("DAO no-fork bad block extra-data: 0x%x", header.Extra)
 		}
 	}
 	// All ok, header has the same extra-data we expect