fix: improve reliability of get block template protocol in mm proxy #3141
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sdbondi
force-pushed
the
cucumber-flakey-fixes
branch
from
508bc99
to
0c78a32
Compare
sdbondi
changed the title
sdbondi
force-pushed
the
cucumber-flakey-fixes
branch
from
0c78a32
to
fc5452a
Compare
sdbondi
changed the title
sdbondi
force-pushed
the
cucumber-flakey-fixes
branch
2 times, most recently
from
0e3b322
to
e1c8114
Compare
sdbondi
changed the title
sdbondi
force-pushed
the
cucumber-flakey-fixes
branch
from
e1c8114
to
ab23095
Compare
The main change here is to check the tip height after retrieving the block template but before MMR roots are calculated. When mining quickly in cucumber tests, this race condition occurs. Previously, this would be ok, the miner would simply mine 1 behind the current tip height. But because we can no longer calculate MMRs prior to the tip, this MMR call (`get_new_block`) now returns an error. Emits handling errors to stderr to make it quicker/easier to diagnose issues without going into the logs. This refactor was performed because the protocol was gaining too much complexity to exist in a single function.
sdbondi
force-pushed
the
cucumber-flakey-fixes
branch
from
ab23095
to
62a38c2
Compare
stringhandler
approved these changes
Jul 28, 2021
|
PR queued successfully. Your position in queue is: 1 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The main change here is to check the tip height after retrieving the
block template but before MMR roots are calculated. When mining quickly
in cucumber tests, this race condition occurs. Previously, this would be
ok, the miner would simply mine 1 behind the current tip height. But
because we can no longer calculate MMRs prior to the tip, this MMR call
(
get_new_block) now returns an error, which fails cucumber.get_new_blocknow returns aFailedPreconditionGRPC error status in the eventthat the block template submitted is not for the current chain tip. This allows
clients to more easily identify this case, typically by asking for a new template.
This refactor was performed because the protocol was gaining too much
complexity to exist in a single function.
Some extra changes to get all cucumbers to pass:
--daemonflags with--non-interactiveAs a user I want to change base node for a walletscenarioMotivation and Context
The
Verify all coinbases in hybrid mining are accounted forcucumber test is flakeydue to change in semantics for get_new_block since #3109. The possibility of this occurring
with normal difficulties is usually tiny, but may be a symptom of a slow get_coinbase request.
How Has This Been Tested?
Merge mined on local machine
Verify all coinbases in hybrid mining are accounted forpassesSuggest that we run tests with monero pool and nodejs pool
Checklist:
developmentbranch.