Page MenuHomePhabricator

Move replay protection mempool management in ConnectTip

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



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

Test Plan
make check

Diff Detail

rABC Bitcoin ABC
Automatic diff as part of commit; lint not applicable.
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.