Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

[NC-1384] Disable mining while catching up to chain head #125

Merged
merged 27 commits into from
Oct 24, 2018
Merged

[NC-1384] Disable mining while catching up to chain head #125

merged 27 commits into from
Oct 24, 2018

Conversation

ajsutton
Copy link
Contributor

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

  • [] Docs change / refactoring / dependency upgrade
  • [] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • [] Breaking change (fix or feature that would cause existing functionality to change)

Errorific and others added 25 commits October 22, 2018 12:08
… 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.
@ajsutton ajsutton requested a review from rain-on October 23, 2018 05:58
@Errorific Errorific self-requested a review October 23, 2018 23:42
package tech.pegasys.pantheon.ethereum.eth.sync.state;

@FunctionalInterface
public interface InSyncListener {
Copy link
Contributor

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?

Copy link
Contributor Author

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;
Copy link
Contributor

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

Copy link
Contributor Author

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.

@ajsutton ajsutton merged commit d2583e3 into PegaSysEng:master Oct 24, 2018
@ajsutton ajsutton deleted the NC-1384 branch October 24, 2018 03:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants