HomeSource World

miner: don't interrupt mining after successful sync (#21701)


miner: don't interrupt mining after successful sync (#21701)

  • miner: exit loop when downloader Done or Failed

Following the logic of the comment at the method,
this fixes a regression introduced at 7cf56d6f064869cb62b1673f9ee437020c595391
, which would allow external parties to DoS with
blocks, preventing mining progress.

Signed-off-by: meows <>

  • miner: remove ineff assign (lint)

Signed-off-by: meows <>

  • miner: update test re downloader events

Signed-off-by: meows <>

  • Revert "miner: remove ineff assign (lint)"

This reverts commit eaefcd34ab4862ebc936fb8a07578aa2744bc058.

  • Revert "miner: exit loop when downloader Done or Failed"

This reverts commit 23abd34265aa246c38fc390bb72572ad6ae9fe3b.

  • miner: add test showing imprecise TestMiner

Signed-off-by: meows <>

  • miner: fix waitForMiningState precision

This helper function would return an affirmation
on the first positive match on a desired bool.

This was imprecise; it return false positives
by not waiting initially for an 'updated' value.

This fix causes TestMiner_2 to fail, which is

Signed-off-by: meows <>

  • miner: remove TestMiner_2 demonstrating broken test

This test demonstrated the imprecision of the test
helper function waitForMiningState. This function
has been fixed with 6d365c2851, and this test test
may now be removed.

Signed-off-by: meows <>

  • miner: fix test regarding downloader event/mining expectations

See comment for logic.

Signed-off-by: meows <>

  • miner: add test describing expectations for downloader/mining events

We expect that once the downloader emits a DoneEvent,
signaling a successful sync, that subsequent StartEvents
are not longer permitted to stop the miner.

This prevents a security vulnerability where forced syncs via
fake high blocks would stall mining operation.

Signed-off-by: meows <>

  • miner: use 'canStop' state to fix downloader event handling
  • Break downloader event handling into event

separating Done and Failed events. We need to
treat these cases differently since a DoneEvent
should prevent the miner from being stopped on
subsequent downloader Start events.

  • Use canStop state to handle the one-off

case when a downloader first succeeds.

Signed-off-by: meows <>

  • miner: improve comment wording

Signed-off-by: meows <>

  • miner: start mining on downloader events iff not already mining

Signed-off-by: meows <>

  • miner: refactor miner update logic w/r/t downloader events

This makes mining pause/start logic regarding downloader
events more explicit. Instead of eternally handling downloader
events after the first done event, the subscription is closed
when downloader events are no longer actionable.

Signed-off-by: meows <>

  • miner: fix handling downloader events on subcription closed

Signed-off-by: meows <>

  • miner: (lint:gosimple) use range over chan instead of for/select

Signed-off-by: meows <>

  • miner: refactor update loop to remove race condition

The go routine handling the downloader events handling
vars in parallel with the parent routine, causing a
race condition.

This change, though ugly, remove the condition while
still allowing the downloader event subscription to be
closed when the miner has no further use for it (ie DoneEvent).

  • miner: alternate fix for miner-flaw

Co-authored-by: meows <>


Martin Holst Swende <>Authored on Oct 13 2020, 12:12 PM
GitHub <>Committed on Oct 13 2020, 12:12 PM
rGETH2a9ea6be87a2: cmd/geth, cmd/utils: fixed flags name (#21700)

Event Timeline

GitHub <> committed rGETH1e10489196fd: miner: don't interrupt mining after successful sync (#21701) (authored by Martin Holst Swende <>).Oct 13 2020, 12:12 PM