-
Notifications
You must be signed in to change notification settings - Fork 130
[NC-1384] Disable mining while catching up to chain head #125
Conversation
…ener" instead of "observer".
…nc target is cleared.
… update if the sync target is cleared.
…ver APIs publicly.
… when there are no peers with a better chain, we are in sync.
…state and the update of lastInSync happen atomically.
…mining when first enabled if we're out of sync.
…upt mining operations when we could still find an ommer.
package tech.pegasys.pantheon.ethereum.eth.sync.state; | ||
|
||
@FunctionalInterface | ||
public interface InSyncListener { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Possibly move this to be SyncState.InSyncListener like ChainState.EstimatedHeightListener, or move the ChainState one to be like this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I don't think we need to be strict about always making these interfaces nested or not - it depends on how the listener is referenced in other places but in this case it didn't wind up justifying being top level.
import tech.pegasys.pantheon.util.uint.UInt256; | ||
|
||
import java.util.Optional; | ||
|
||
public class SyncState { | ||
private static final long SYNC_TOLERANCE = 5; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future work, 5 might not be universally appropriate for all consensus mechanisms where there's no ommers blocks e.g. ibft
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only vaguely related to ommers - the point is just to avoid halting mining immediately when a peer broadcasts a block to us. That creates a lot of noise, means we might not find an ommer we could have and it may turn out that the block wasn't even valid. So it's really just a little leeway to say we're in sync while going through the usual process of block propagation.
PR description
Disable mining while our current chain is behind the best known chain head and resume once sync has caught up.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
Related Issue(s)
https://pegasys1.atlassian.net/browse/NC-1384
Types of changes