Merge pull request #596 from etclabscore/fix/miner-downloader-pause-alt3

Fix/miner downloader autopause

Author: meowsbits

eth/fetcher/block_fetcher.go

@@ -32,6 +32,13 @@ import (
 	"github.com/ethereum/go-ethereum/trie"
 )
 
+// InsertChainEvent is posted by the handler when a propagated block is successfully imported.
+// The block may have been propagated via announcement (hashes) or broadcast (full block, via its miner).
+// The event should not be posted if the insert function returns any error.
+type InsertChainEvent struct {
+	Blocks types.Blocks
+}
+
 const (
 	lightTimeout  = time.Millisecond       // Time allowance before an announced header is explicitly requested
 	arriveTimeout = 500 * time.Millisecond // Time allowance before an announced block/transaction is explicitly requested

eth/handler.go

@@ -293,7 +293,9 @@ func newHandler(config *handlerConfig) (*handler, error) {
 		}
 		n, err := h.chain.InsertChain(blocks)
 		if err == nil {
-			h.acceptTxs.Store(true) // Mark initial sync done on any fetcher import
+			// Mark initial sync done on any fetcher import
+			h.acceptTxs.Store(true)
+			h.eventMux.Post(fetcher.InsertChainEvent{Blocks: blocks})
 		}
 		return n, err
 	}

miner/miner.go

@@ -31,6 +31,7 @@ import (
 	"github.com/ethereum/go-ethereum/core/txpool"
 	"github.com/ethereum/go-ethereum/core/types"
 	"github.com/ethereum/go-ethereum/eth/downloader"
+	"github.com/ethereum/go-ethereum/eth/fetcher"
 	"github.com/ethereum/go-ethereum/event"
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/params/types/ctypes"
@@ -107,7 +108,7 @@ func New(eth Backend, config *Config, chainConfig ctypes.ChainConfigurator, mux
 func (miner *Miner) update() {
 	defer miner.wg.Done()
 
-	events := miner.mux.Subscribe(downloader.StartEvent{}, downloader.DoneEvent{}, downloader.FailedEvent{})
+	events := miner.mux.Subscribe(downloader.StartEvent{}, downloader.DoneEvent{}, downloader.FailedEvent{}, fetcher.InsertChainEvent{})
 	defer func() {
 		if !events.Closed() {
 			events.Unsubscribe()
@@ -116,13 +117,13 @@ func (miner *Miner) update() {
 
 	shouldStart := false
 	canStart := true
-	dlEventCh := events.Chan()
+	eventCh := events.Chan()
 	for {
 		select {
-		case ev := <-dlEventCh:
+		case ev := <-eventCh:
 			if ev == nil {
 				// Unsubscription done, stop listening
-				dlEventCh = nil
+				eventCh = nil
 				continue
 			}
 			switch ev.Data.(type) {
@@ -142,7 +143,12 @@ func (miner *Miner) update() {
 					miner.worker.start()
 				}
 				miner.worker.syncing.Store(false)
-			case downloader.DoneEvent:
+			case downloader.DoneEvent, fetcher.InsertChainEvent:
+				// InsertBlockEvents are posted by the block fetcher, which handles the fetching and importing
+				// of network head blocks.
+				// This event should be treated by the miner equivalently to the downloader.DoneEvent;
+				// both events indicate that the local node should be treated as fully synced
+				// and thus ready to mine.
 				canStart = true
 				if shouldStart {
 					miner.worker.start()

miner/miner_test.go

@@ -34,6 +34,7 @@ import (
 	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/ethereum/go-ethereum/eth/downloader"
+	"github.com/ethereum/go-ethereum/eth/fetcher"
 	"github.com/ethereum/go-ethereum/event"
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/ethereum/go-ethereum/params/types/ctypes"
@@ -182,6 +183,60 @@ func TestMinerStartStopAfterDownloaderEvents(t *testing.T) {
 	waitForMiningState(t, miner, false)
 }
 
+// TestMinerStartAfterFetcherInsertEvent tests that the miner resumes mining during a downloader sync attempt
+// if a propagated block (announce, broadcast) is inserted successfully to the local chain.
+func TestMinerStartAfterFetcherInsertEvent(t *testing.T) {
+	miner, mux, cleanup := createMiner(t)
+	defer cleanup(false)
+
+	miner.Start()
+	waitForMiningState(t, miner, true)
+	// Start the downloader
+	mux.Post(downloader.StartEvent{})
+	waitForMiningState(t, miner, false)
+
+	// During the sync, import a block via the fetcher.
+	// Miner should resume mining, inferring that we're at or near-enough (<32 block) the head of the network canonical chain.
+	mux.Post(fetcher.InsertChainEvent{})
+	waitForMiningState(t, miner, true)
+
+	mux.Post(downloader.FailedEvent{})
+	waitForMiningState(t, miner, true)
+
+	mux.Post(downloader.StartEvent{})
+	waitForMiningState(t, miner, true) // Has not autopaused mining.
+
+	// Downloader finally succeeds.
+	mux.Post(downloader.DoneEvent{})
+	waitForMiningState(t, miner, true) // Still mining.
+}
+
+// TestMinerStartAfterFetcherInsertEvent2 tests that the miner does not autopause if outside any downloader sync attempt
+// a propagated block (announce, broadcast) is inserted successfully to the local chain; that is,
+// a fetcher->import event disables the downloader event listener and autopause mechanism.
+func TestMinerStartAfterFetcherInsertEvent2(t *testing.T) {
+	miner, mux, cleanup := createMiner(t)
+	defer cleanup(false)
+
+	miner.Start()
+	waitForMiningState(t, miner, true)
+
+	// Before we've started the downloader, import a block via the fetcher.
+	// This should disable subsequent autopausing for downloader events thereafter, since
+	// we consider the local chain sufficiently synced.
+	mux.Post(fetcher.InsertChainEvent{})
+	waitForMiningState(t, miner, true)
+
+	// Start the downloader
+	mux.Post(downloader.StartEvent{})
+	// Still mining; we've inferred by a successful import of a propagated block
+	// that we're already synced, and we're no longer treating downloader events as autopause-able.
+	waitForMiningState(t, miner, true)
+
+	mux.Post(downloader.FailedEvent{})
+	waitForMiningState(t, miner, true)
+}
+
 func TestStartWhileDownload(t *testing.T) {
 	miner, mux, cleanup := createMiner(t)
 	defer cleanup(false)