diff --git a/miner/miner.go b/miner/miner.go index 8cbd70b42..35c036ba7 100644 --- a/miner/miner.go +++ b/miner/miner.go @@ -85,15 +85,22 @@ func New(eth Backend, config *Config, chainConfig *params.ChainConfig, mux *even // and halt your mining operation for as long as the DOS continues. func (miner *Miner) update() { events := miner.mux.Subscribe(downloader.StartEvent{}, downloader.DoneEvent{}, downloader.FailedEvent{}) - defer events.Unsubscribe() + defer func() { + if !events.Closed() { + events.Unsubscribe() + } + }() shouldStart := false canStart := true + dlEventCh := events.Chan() for { select { - case ev := <-events.Chan(): + case ev := <-dlEventCh: if ev == nil { - return + // Unsubscription done, stop listening + dlEventCh = nil + continue } switch ev.Data.(type) { case downloader.StartEvent: @@ -105,12 +112,20 @@ func (miner *Miner) update() { shouldStart = true log.Info("Mining aborted due to sync") } - case downloader.DoneEvent, downloader.FailedEvent: + case downloader.FailedEvent: canStart = true if shouldStart { miner.SetEtherbase(miner.coinbase) miner.worker.start() } + case downloader.DoneEvent: + canStart = true + if shouldStart { + miner.SetEtherbase(miner.coinbase) + miner.worker.start() + } + // Stop reacting to downloader events + events.Unsubscribe() } case addr := <-miner.startCh: if canStart { diff --git a/miner/miner_test.go b/miner/miner_test.go index 2ed03a239..20bf2534c 100644 --- a/miner/miner_test.go +++ b/miner/miner_test.go @@ -89,12 +89,75 @@ func TestMiner(t *testing.T) { // Stop the downloader and wait for the update loop to run mux.Post(downloader.DoneEvent{}) waitForMiningState(t, miner, true) - // Start the downloader and wait for the update loop to run + + // Subsequent downloader events after a successful DoneEvent should not cause the + // miner to start or stop. This prevents a security vulnerability + // that would allow entities to present fake high blocks that would + // stop mining operations by causing a downloader sync + // until it was discovered they were invalid, whereon mining would resume. + mux.Post(downloader.StartEvent{}) + waitForMiningState(t, miner, true) + + mux.Post(downloader.FailedEvent{}) + waitForMiningState(t, miner, true) +} + +// TestMinerDownloaderFirstFails tests that mining is only +// permitted to run indefinitely once the downloader sees a DoneEvent (success). +// An initial FailedEvent should allow mining to stop on a subsequent +// downloader StartEvent. +func TestMinerDownloaderFirstFails(t *testing.T) { + miner, mux := createMiner(t) + miner.Start(common.HexToAddress("0x12345")) + waitForMiningState(t, miner, true) + // Start the downloader mux.Post(downloader.StartEvent{}) waitForMiningState(t, miner, false) + // Stop the downloader and wait for the update loop to run mux.Post(downloader.FailedEvent{}) waitForMiningState(t, miner, true) + + // Since the downloader hasn't yet emitted a successful DoneEvent, + // we expect the miner to stop on next StartEvent. + mux.Post(downloader.StartEvent{}) + waitForMiningState(t, miner, false) + + // Downloader finally succeeds. + mux.Post(downloader.DoneEvent{}) + waitForMiningState(t, miner, true) + + // Downloader starts again. + // Since it has achieved a DoneEvent once, we expect miner + // state to be unchanged. + mux.Post(downloader.StartEvent{}) + waitForMiningState(t, miner, true) + + mux.Post(downloader.FailedEvent{}) + waitForMiningState(t, miner, true) +} + +func TestMinerStartStopAfterDownloaderEvents(t *testing.T) { + miner, mux := createMiner(t) + + miner.Start(common.HexToAddress("0x12345")) + waitForMiningState(t, miner, true) + // Start the downloader + mux.Post(downloader.StartEvent{}) + waitForMiningState(t, miner, false) + + // Downloader finally succeeds. + mux.Post(downloader.DoneEvent{}) + waitForMiningState(t, miner, true) + + miner.Stop() + waitForMiningState(t, miner, false) + + miner.Start(common.HexToAddress("0x678910")) + waitForMiningState(t, miner, true) + + miner.Stop() + waitForMiningState(t, miner, false) } func TestStartWhileDownload(t *testing.T) { @@ -137,10 +200,10 @@ func waitForMiningState(t *testing.T, m *Miner, mining bool) { var state bool for i := 0; i < 100; i++ { + time.Sleep(10 * time.Millisecond) if state = m.Mining(); state == mining { return } - time.Sleep(10 * time.Millisecond) } t.Fatalf("Mining() == %t, want %t", state, mining) }