Skip to content

miner: fix regression, add test for starting while download #21547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fjl merged 2 commits into from
Sep 11, 2020
Merged

miner: fix regression, add test for starting while download #21547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e38bde
miner: fix regression, add test for starting while download
MariusVanDerWijden Sep 11, 2020
ce70ee4
miner: fix regression
MariusVanDerWijden Sep 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions miner/miner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ func (miner *Miner) update() {
defer events.Unsubscribe()

shouldStart := false
canStart := true
for {
select {
case ev := <-events.Chan():
Expand All @@ -98,21 +99,27 @@ func (miner *Miner) update() {
case downloader.StartEvent:
wasMining := miner.Mining()
miner.worker.stop()
canStart = false
if wasMining {
// Resume mining after sync was finished
shouldStart = true
log.Info("Mining aborted due to sync")
}
case downloader.DoneEvent, downloader.FailedEvent:
canStart = true
if shouldStart {
miner.SetEtherbase(miner.coinbase)
miner.worker.start()
}
}
case addr := <-miner.startCh:
miner.SetEtherbase(addr)
miner.worker.start()
if canStart {
miner.SetEtherbase(addr)
miner.worker.start()
}
Comment on lines +116 to +119
Copy link
Copy Markdown
Contributor

@holiman holiman Sep 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought - what happens if user calls miner.start(xx) twice in a row? I'm assuming that doesn't cause any problems...?

Copy link
Copy Markdown
Member Author

@MariusVanDerWijden MariusVanDerWijden Sep 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it doesn't cause any problems, just checked. Should I add a test for it?

shouldStart = true
case <-miner.stopCh:
shouldStart = false
miner.worker.stop()
case <-miner.exitCh:
miner.worker.close()
Expand Down
13 changes: 13 additions & 0 deletions miner/miner_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,19 @@ func TestMiner(t *testing.T) {
waitForMiningState(t, miner, true)
}

func TestStartWhileDownload(t *testing.T) {
miner, mux := createMiner(t)
waitForMiningState(t, miner, false)
miner.Start(common.HexToAddress("0x12345"))
waitForMiningState(t, miner, true)
// Stop the downloader and wait for the update loop to run
mux.Post(downloader.StartEvent{})
waitForMiningState(t, miner, false)
// Starting the miner after the downloader should not work
miner.Start(common.HexToAddress("0x12345"))
waitForMiningState(t, miner, false)
}

func TestStartStopMiner(t *testing.T) {
miner, _ := createMiner(t)
waitForMiningState(t, miner, false)
Expand Down