Page MenuHomePhabricator

Move replay protection mempool management in ConnectTip
ClosedPublic

Authored by deadalnix on Feb 7 2019, 23:23.

Details

Summary

Various method that do not actually connect a block call ConnectBlock. ConnectTip is a much better place.

Test Plan
make check
./test/functional/test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Feb 7 2019, 23:23
Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 7 2019, 23:23
Herald added a subscriber: schancel. · View Herald Transcript
markblundeberg accepted this revision.Feb 7 2019, 23:46
markblundeberg added a subscriber: markblundeberg.

Change in the right direction with good tests.

Note that ConnectBlock has the following callsites besides ConnectTip:

  • TestBlockValidity, used during mining. Previously it was incorrectly clearing during CreateNewBlock at the activation point, now fixed .
  • VerifyDB, used during initialization. Clearing during VerifyDB is benign: the VerifyDB is used during step 4 of init, whereas the mempool is loaded from disk in step 10 (ThreadImport).

So, the other call sites did not rely on the mempool clearing for maintaining consensus correctness, and in fact it was harming CreateNewBlock.

(In principle this doesn't have to happen in ConnectTip which has only one callsite (ActivateBestChainStep). Sometimes the ConnectTip result gets dropped due to invalid blocks happening later on, in which case it wasn't necessary to clear the mempool. Still it's an improvement over the previous. 👍)

This revision is now accepted and ready to land.Feb 7 2019, 23:46
This revision was automatically updated to reflect the committed changes.