Skip to content
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.

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

eth/blockwatch: BackfillStartBlock option #1157

Merged
merged 2 commits into from
Nov 13, 2019

Conversation

kyriediculous
Copy link
Contributor

What does this pull request do? Explain your changes. (required)
This PR updates the blockwatch package with a config option to start backfilling missed events from a certain block height, whereas previously backfilling would happen from the last retained block on the stack (in the DB in our case), if present.

  1. If there is a last retained block on the blockwatch stack, start backfilling from there. This should lead to an up to date view for the node
  2. If there is no last retained block, start backfilling from the hardcoded blockwatchBackfillStartBlock defined in livepeer.go this is currently set to 0 and should lead to an up to date view for the node
  3. If neither is defined return zero value and no error

The one caveat is that this strategy makes some of the bash tests that use mainnet/rinkeby take quite a while since it starts backfilling from genesis...

Specific updates (required)

  • Added a BackFillStartBlock *big.Int field to blockwatch.Config
  • Changed watcher.getMissedEventsToBackfill() to start backfilling from watcher.backFillStartBlock if defined, otherwise continue operating as before the changes this PR makes
  • Added a unit test for backfilling when config.BackfillStartBlock is defined
  • Moved around SenderWatcher to not process events when backfilling because it will just lead to no-ops anyway (mapping is empty when starting the node)

How did you test each of these updates (required)
Ran unit tests

Does this pull request close any open issues?
Fixes #1149

Checklist:

  • README and other documentation updated
  • Node runs in OSX and devenv
  • All tests in ./test.sh pass

@kyriediculous
Copy link
Contributor Author

7b53fe4 adds a unit test for when there is no last retained block and no defined value for backfillStartBlock

cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
eth/blockwatch/block_watcher.go Outdated Show resolved Hide resolved
eth/blockwatch/block_watcher.go Show resolved Hide resolved
eth/blockwatch/block_watcher.go Outdated Show resolved Hide resolved
eth/blockwatch/block_watcher_test.go Outdated Show resolved Hide resolved
eth/blockwatch/block_watcher_test.go Show resolved Hide resolved
eth/blockwatch/block_watcher_test.go Outdated Show resolved Hide resolved
@kyriediculous
Copy link
Contributor Author

5cba1fe adds support for determining the blockWatchBackfillStartBlock based on the current round start block vs the last seen block by the node.

  1. Fetch the node's lastRetainedBlock and store it in a temp variable originalLastRetainedBlock since the value will change after backfilling completes
  2. Let backfillStartBlock be the current round start block
  3. Start backfilling - see the options below for what block to start from
  4. If lastRetainedBlock < backfillStartBlock, start from backfillStartBlock
  5. If lastRetainedBlock >= backfillStartBlock, start from lastRetainedBlock
  6. Backfilling completes

cmd/livepeer/livepeer.go Outdated Show resolved Hide resolved
eth/blockwatch/block_watcher.go Outdated Show resolved Hide resolved
eth/blockwatch/block_watcher.go Outdated Show resolved Hide resolved
eth/blockwatch/block_watcher.go Show resolved Hide resolved
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

Changes look good - let's rebase

@kyriediculous
Copy link
Contributor Author

kyriediculous commented Nov 13, 2019 via email

Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

@kyriediculous kyriediculous merged commit f60935d into master Nov 13, 2019
@kyriediculous kyriediculous deleted the nv/blockwatch-startdepth branch November 13, 2019 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

StartBackfillBlockDepth config param for blockwatch package
2 participants