ethash, core, eth, miner, mobile: fix review requests

consensus/ethash/consensus.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"math/big"
 	"runtime"
+	"sync/atomic"
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -40,6 +41,7 @@ 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")
@@ -100,9 +102,15 @@ func (ethash *Ethash) VerifyHeaders(chain consensus.ChainReader, headers []*type
 	inputs := make(chan int, workers)
 	outputs := make(chan result, len(headers))
 
+	var badblock uint64
 	for i := 0; i < workers; i++ {
 		go func() {
 			for index := range inputs {
+				// If we've found a bad block already before this, stop validating
+				if bad := atomic.LoadUint64(&badblock); bad != 0 && bad <= headers[index].Number.Uint64() {
+					outputs <- result{index: index, err: ErrInvalidChain}
+					continue
+				}
 				// We need to look up the first parent
 				var parent *types.Header
 				if index == 0 {
@@ -111,13 +119,29 @@ func (ethash *Ethash) VerifyHeaders(chain consensus.ChainReader, headers []*type
 					parent = headers[index-1]
 				}
 				// Ensure the validation is useful and execute it
+				var failure error
 				switch {
 				case chain.GetHeader(headers[index].Hash(), headers[index].Number.Uint64()-1) != nil:
 					outputs <- result{index: index, err: nil}
 				case parent == nil:
-					outputs <- result{index: index, err: ErrParentUnknown}
+					failure = ErrParentUnknown
+					outputs <- result{index: index, err: failure}
 				default:
-					outputs <- result{index: index, err: ethash.verifyHeader(chain, headers[index], parent, false, seals[index])}
+					failure = ethash.verifyHeader(chain, headers[index], parent, false, seals[index])
+					outputs <- result{index: index, err: failure}
+				}
+				// If a validation failure occurred, mark subsequent blocks invalid
+				if failure != nil {
+					number := headers[index].Number.Uint64()
+					if prev := atomic.LoadUint64(&badblock); prev == 0 || prev > number {
+						// This two step atomic op isn't thread-safe in that `badblock` might end
+						// up slightly higher than the block number of the first failure (if many
+						// workers try to write at the same time), but it's fine as we're mostly
+						// interested to avoid large useless work, we don't care about 1-2 extra
+						// runs. Doing "full thread safety" would involve mutexes, which would be
+						// a noticeable sync overhead on the fast spinning worker routines.
+						atomic.StoreUint64(&badblock, number)
+					}
 				}
 			}
 		}()

consensus/ethash/ethash.go

@@ -333,6 +333,7 @@ type Ethash struct {
 	fdataset *dataset            // Pre-generated dataset for the estimated future epoch
 
 	// Mining related fields
+	rand     *rand.Rand    // Properly seeded random source for nonces
 	threads  int           // Number of threads to mine on if mining
 	update   chan struct{} // Notification channel to update mining parameters
 	hashrate metrics.Meter // Meter tracking the average hashrate
@@ -409,7 +410,7 @@ func NewFakeDelayer(delay time.Duration) *Ethash {
 }
 
 // NewFullFaker creates a ethash consensus engine with a full fake scheme that
-// accepts all blocks as valid, without checking any consenss rules whatsoever.
+// accepts all blocks as valid, without checking any consensus rules whatsoever.
 func NewFullFaker() *Ethash {
 	return &Ethash{fakeMode: true, fakeFull: true}
 }
@@ -544,7 +545,7 @@ func (ethash *Ethash) dataset(block uint64) []uint32 {
 }
 
 // Threads returns the number of mining threads currently enabled. This doesn't
-// necesarilly mean that mining is running!
+// necessarily mean that mining is running!
 func (ethash *Ethash) Threads() int {
 	ethash.lock.Lock()
 	defer ethash.lock.Unlock()

consensus/ethash/sealer.go

@@ -17,6 +17,8 @@
 package ethash
 
 import (
+	crand "crypto/rand"
+	"math"
 	"math/big"
 	"math/rand"
 	"runtime"
@@ -47,6 +49,14 @@ func (ethash *Ethash) Seal(chain consensus.ChainReader, block *types.Block, stop
 
 	ethash.lock.Lock()
 	threads := ethash.threads
+	if ethash.rand == nil {
+		seed, err := crand.Int(crand.Reader, big.NewInt(math.MaxInt64))
+		if err != nil {
+			ethash.lock.Unlock()
+			return nil, err
+		}
+		ethash.rand = rand.New(rand.NewSource(seed.Int64()))
+	}
 	ethash.lock.Unlock()
 	if threads == 0 {
 		threads = runtime.NumCPU()
@@ -54,10 +64,10 @@ func (ethash *Ethash) Seal(chain consensus.ChainReader, block *types.Block, stop
 	var pend sync.WaitGroup
 	for i := 0; i < threads; i++ {
 		pend.Add(1)
-		go func(id int) {
+		go func(id int, nonce uint64) {
 			defer pend.Done()
-			ethash.mine(block, id, uint64(rand.Int63()), abort, found)
+			ethash.mine(block, id, nonce, abort, found)
-		}(i)
+		}(i, uint64(ethash.rand.Int63()))
 	}
 	// Wait until sealing is terminated or a nonce is found
 	var result *types.Block
@@ -80,7 +90,7 @@ func (ethash *Ethash) Seal(chain consensus.ChainReader, block *types.Block, stop
 }
 
 // mine is the actual proof-of-work miner that searches for a nonce starting from
-// seed that results in corrent final block difficulty.
+// seed that results in correct final block difficulty.
 func (ethash *Ethash) mine(block *types.Block, id int, seed uint64, abort chan struct{}, found chan *types.Block) {
 	// Extract some data from the header
 	var (

consensus/utils.go

@@ -1,78 +0,0 @@
-// 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/>.
-
-// Contains some utility methods to allow creating hooked consensus structures
-// mostly for test code where only a few methods are needed.
-
-package consensus
-
-import (
-	"github.com/ethereum/go-ethereum/common"
-	"github.com/ethereum/go-ethereum/core/types"
-	"github.com/ethereum/go-ethereum/params"
-)
-
-type CurrentHeaderFn func() *types.Header
-type HeaderRetrievalFn func(hash common.Hash, number uint64) *types.Header
-type HeaderByNumberRetrievalFn func(number uint64) *types.Header
-type BlockRetrievalFn func(hash common.Hash, number uint64) *types.Block
-
-// hookedChainReader is a tiny implementation of ChainReader based on callbacks
-// and a preset chain configuration. It's useful to avoid defining various custom
-// types where a cain reader isn't readily available.
-type hookedChainReader struct {
-	config            *params.ChainConfig
-	currentHeader     CurrentHeaderFn
-	getHeader         HeaderRetrievalFn
-	getHeaderByNumber HeaderByNumberRetrievalFn
-	getBlock          BlockRetrievalFn
-}
-
-// MakeChainReader creates a callback based chain reader.
-func MakeChainReader(config *params.ChainConfig, currentHeader CurrentHeaderFn, getHeader HeaderRetrievalFn, getHeaderByNumber HeaderByNumberRetrievalFn, getBlock BlockRetrievalFn) ChainReader {
-	return &hookedChainReader{
-		config:            config,
-		currentHeader:     currentHeader,
-		getHeader:         getHeader,
-		getHeaderByNumber: getHeaderByNumber,
-		getBlock:          getBlock,
-	}
-}
-
-// Config implements ChainReader, retrieving the blockchain's chain configuration.
-func (hcr *hookedChainReader) Config() *params.ChainConfig { return hcr.config }
-
-// CurrentHeader implements ChainReader, retrieving the current header.
-func (hcr *hookedChainReader) CurrentHeader() *types.Header {
-	return hcr.currentHeader()
-}
-
-// GetHeader implements ChainReader, retrieving a block header from the database
-// by hash and number.
-func (hcr *hookedChainReader) GetHeader(hash common.Hash, number uint64) *types.Header {
-	return hcr.getHeader(hash, number)
-}
-
-// GetHeaderByNumber implements ChainReader, retrieving a block header from the
-// database by number.
-func (hcr *hookedChainReader) GetHeaderByNumber(number uint64) *types.Header {
-	return hcr.getHeaderByNumber(number)
-}
-
-// GetBlock implements ChainReader, retrieving a block from the database by hash and number.
-func (hcr *hookedChainReader) GetBlock(hash common.Hash, number uint64) *types.Block {
-	return hcr.getBlock(hash, number)
-}

core/block_validator.go

@@ -51,7 +51,7 @@ func NewBlockValidator(config *params.ChainConfig, blockchain *BlockChain, engin
 // header's transaction and uncle roots. The headers are assumed to be already
 // validated at this point.
 func (v *BlockValidator) ValidateBody(block *types.Block) error {
-	// Check whether the blokc's known, and if not, that it's linkable
+	// 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()}
@@ -70,7 +70,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error {
 		return err
 	}
 	if hash := types.CalcUncleHash(block.Uncles()); hash != header.UncleHash {
-		return fmt.Errorf("uncle root hsh mismatch: have %x, want %x", hash, header.UncleHash)
+		return fmt.Errorf("uncle root hash mismatch: have %x, want %x", hash, header.UncleHash)
 	}
 	if hash := types.DeriveSha(block.Transactions()); hash != header.TxHash {
 		return fmt.Errorf("transaction root hash mismatch: have %x, want %x", hash, header.TxHash)

core/block_validator_test.go

@@ -101,7 +101,7 @@ func testHeaderConcurrentVerification(t *testing.T, threads int) {
 	defer runtime.GOMAXPROCS(old)
 
 	// Run the header checker for the entire block chain at once both for a valid and
-	// also an invalid chain (enough if one is invalid, last but one (arbitrary)).
+	// also an invalid chain (enough if one arbitrary block is invalid).
 	for i, valid := range []bool{true, false} {
 		var results <-chan error
 
@@ -125,10 +125,16 @@ func testHeaderConcurrentVerification(t *testing.T, threads int) {
 		}
 		// Check nonce check validity
 		for j := 0; j < len(blocks); j++ {
-			want := valid || (j != len(blocks)-2) // We chose the last but one nonce in the chain to fail
+			want := valid || (j < len(blocks)-2) // We chose the last-but-one nonce in the chain to fail
 			if (checks[j] == nil) != want {
 				t.Errorf("test %d.%d: validity mismatch: have %v, want %v", i, j, checks[j], want)
 			}
+			if !want {
+				// A few blocks after the first error may pass verification due to concurrent
+				// workers. We don't care about those in this test, just that the correct block
+				// errors out.
+				break
+			}
 		}
 		// Make sure no more data is returned
 		select {

eth/api.go

@@ -133,7 +133,7 @@ func NewPrivateMinerAPI(e *Ethereum) *PrivateMinerAPI {
 }
 
 // Start the miner with the given number of threads. If threads is nil the number
-// of workers started is equal to the number of logical CPU'api that are usable by
+// of workers started is equal to the number of logical CPUs that are usable by
 // this process. If mining is already running, this method adjust the number of
 // threads allowed to use.
 func (api *PrivateMinerAPI) Start(threads *int) error {

miner/remote_agent.go

@@ -131,8 +131,8 @@ func (a *RemoteAgent) GetWork() ([3]string, error) {
 	return res, errors.New("No work available yet, don't panic.")
 }
 
-// SubmitWork tries to inject a pow solution tinto the remote agent, returning
+// SubmitWork tries to inject a pow solution into the remote agent, returning
-// whether the solution was acceted or not (not can be both a bad pow as well as
+// whether the solution was accepted or not (not can be both a bad pow as well as
 // any other error, like no work pending).
 func (a *RemoteAgent) SubmitWork(nonce types.BlockNonce, mixDigest, hash common.Hash) bool {
 	a.mu.Lock()

mobile/params.go

@@ -37,7 +37,6 @@ func MainnetChainConfig() *ChainConfig {
 		EIP150Hash:     Hash{params.MainNetHomesteadGasRepriceHash},
 		EIP155Block:    params.MainNetSpuriousDragon.Int64(),
 		EIP158Block:    params.MainNetSpuriousDragon.Int64(),
-		PoWConfig:      new(PoWConfig),
 	}
 }
 
@@ -58,7 +57,6 @@ func TestnetChainConfig() *ChainConfig {
 		EIP150Hash:     Hash{params.TestNetHomesteadGasRepriceHash},
 		EIP155Block:    params.TestNetSpuriousDragon.Int64(),
 		EIP158Block:    params.TestNetSpuriousDragon.Int64(),
-		PoWConfig:      new(PoWConfig),
 	}
 }
 
@@ -73,25 +71,14 @@ func TestnetGenesis() string {
 
 // ChainConfig is the core config which determines the blockchain settings.
 type ChainConfig struct {
-	ChainID        int64      // Chain ID for replay protection
+	ChainID        int64 // Chain ID for replay protection
-	HomesteadBlock int64      // Homestead switch block
+	HomesteadBlock int64 // Homestead switch block
-	DAOForkBlock   int64      // TheDAO hard-fork switch block
+	DAOForkBlock   int64 // TheDAO hard-fork switch block
-	DAOForkSupport bool       // Whether the nodes supports or opposes the DAO hard-fork
+	DAOForkSupport bool  // Whether the nodes supports or opposes the DAO hard-fork
-	EIP150Block    int64      // Homestead gas reprice switch block
+	EIP150Block    int64 // Homestead gas reprice switch block
-	EIP150Hash     Hash       // Homestead gas reprice switch block hash
+	EIP150Hash     Hash  // Homestead gas reprice switch block hash
-	EIP155Block    int64      // Replay protection switch block
+	EIP155Block    int64 // Replay protection switch block
-	EIP158Block    int64      // Empty account pruning switch block
+	EIP158Block    int64 // Empty account pruning switch block
-	PoWConfig      *PoWConfig // Consensus rules for the proof-of-work engine
-	PoAConfig      *PoAConfig // Consensus rules for the proof-of-authority engine
-}
-
-// PoWConfig is the consensus engine configs for proof-of-work based sealing.
-type PoWConfig struct{}
-
-// PoAConfig is the consensus engine configs for proof-of-authorization based
-// sealing.
-type PoAConfig struct {
-	Signers Addresses
 }
 
 // NewChainConfig creates a new chain configuration that transitions immediately