HomePhabricator

Don't park blocks when there is no actual reorg

Description

Don't park blocks when there is no actual reorg

Summary:
This is a remake of D4909, without dependencies. I wouldn't salvage it as it doesn't apply cleanly anymore, so I ended up having to redo it. Previous description:

This code currently classifies a 'deep reorg' based on the length of the new
chain... this is a bit weird for a couple of reasons:

  • If there is no reorg at all, but we merely receive a block out-of-order, then our log file will be shitted up with 'Park block' messages for all the future blocks after tip+1 until we actually receive the tip+1 block and can move the tip forward.
  • Most people classify a deep reorg based on how many blocks get rewinded, not based on how many new blocks there are. If we only have to rewind a single block it shouldn't count as a deep reorg, but the current code does count it so.

The more conventional sense (a deep reorg is based on the number of blocks
that need to be rewinded) is already used by finalization code, and parking
should be similar.

Test Plan:

  1. Run make check all.
  2. Run test_runner.py --nocleanup.
  3. Go into the test directory left over from previous step (e.g., /tmp/bitcoin_test_runner_20200112_XXXXXX).
  4. Run grep -rI "Park block" to find all instances of this log message in the tests' debug.log files.
  5. Observe that with this patch, there are only a few hundred messages, coming from tests that actually do nontrivial reorgs: abc-invalid-chains, abc-parkedchain, p2p_fingerprint, rpc_invalidateblock, feature_bip68_sequence, abc-mempool-coherence-on-activations. The abc-parkedchain test makes the majority of these messages, unsurprisingly.
  6. Observe that the functional tests do pass.
  7. Repeat steps 1-6 above without this patch applied, and observe there are THOUSANDS of park block messages, mainly from tests that don't perform reorgs at all.
  8. Try the newly added test without this patch, and observe it fails.

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D5370

Details

Provenance
deadalnixAuthored on Feb 28 2020, 16:18
deadalnixPushed on Feb 28 2020, 23:07
Reviewer
Restricted Project
Differential Revision
D5370: Don't park blocks when there is no actual reorg
Parents
rSTAGING4b1d6c6905ec: [automated-commits] Add update-timings
Branches
Unknown
Tags
Unknown
References
tag: phabricator/base/16639, tag: phabricator/base/16638