Page MenuHomePhabricator

[refactor] pass CTxMempool& to DisconnectedBlockTransactions methods

Authored by majcosta on Sep 14 2020, 21:49.



This was originally feedback given to me on D6539. Since then I've decided to pass CTxMemPool to the addForBlock method too. This avoids having to declare an extern CTxMemPool g_mempool symbol on txmempool.h, and is nicer in general.

Alternatively, I've considered adding a CTxMemPool& m_pool to DisconnectedBlockTransactions plus an explicit constructor on the assumption that such objects can only ever have one mempool they work with, but went with this instead for simplicity.

The above changes are unrelated to PR14193 however, so I'm putting them on a separate diff not to overcomplicate review of the latter.

Test Plan
ninja check check-functional

Diff Detail

rABC Bitcoin ABC
Lint OK
No Unit Test Coverage
Build Status
Buildable 12728
Build 25540: Build Diffbuild-without-wallet · build-clang-10 · build-diff · build-clang-tidy
Build 25539: arc lint + arc unit

Event Timeline

[Bot Message]
One or more PR numbers were detected in the summary.
Links to those PRs have been inserted into the summary for reference.

Snippet of first build failure:                   | ○ Skipped | 0 s --usecli          | ○ Skipped | 0 s                 | ○ Skipped | 0 s      | ○ Skipped | 0 s                     | ○ Skipped | 0 s --mineblock         | ○ Skipped | 0 s               | ○ Skipped | 0 s --mineblock   | ○ Skipped | 0 s                     | ○ Skipped | 0 s --usecli            | ○ Skipped | 0 s                 | ○ Skipped | 0 s                         | ✖ Failed  | 10 s

ALL                                     | ✖ Failed  | 403 s (accumulated) 
Runtime: 81 s

[16/373] Test Bitcoin RPC authentication...
Ran 3 tests in 0.005s

[18/373] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./
Ran 5 tests in 0.001s

[97/373] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/script_tests.cpp.o
In file included from /usr/include/boost/test/unit_test.hpp:19,
                 from ../../src/test/script_tests.cpp:30:
../../src/test/script_tests.cpp: In member function ‘void script_tests::script_build::test_method()’:
../../src/test/script_tests.cpp:541:22: note: variable tracking size limit exceeded with -fvar-tracking-assignments, retrying without
 BOOST_AUTO_TEST_CASE(script_build) {
[147/373] Running seeder test suite
PASSED: seeder test suite
[152/373] Running avalanche test suite
PASSED: avalanche test suite
[160/373] Running pow test suite
PASSED: pow test suite
[167/373] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[361/373] Running utility command for check-bitcoin-transaction_tests
FAILED: test/CMakeFiles/check-functional 
cd /work/abc-ci-builds/build-without-wallet/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-without-wallet/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-without-wallet/test/log && /usr/bin/cmake -E env /usr/bin/python3.7 ./functional/ "--testsuitename=Bitcoin ABC functional tests" --junitoutput=/work/abc-ci-builds/build-without-wallet/test/junit/functional_tests.xml
[363/373] bitcoin: testing validation_block_tests
[364/373] Running utility command for check-bitcoin-validation_block_tests
[365/373] bitcoin: testing coins_tests
[366/373] Running utility command for check-bitcoin-coins_tests
[367/373] bitcoin: testing util_tests
[368/373] Running utility command for check-bitcoin-util_tests
[369/373] bitcoin: testing op_reversebytes_tests
[370/373] Running utility command for check-bitcoin-op_reversebytes_tests
[371/373] bitcoin: testing cuckoocache_tests
[372/373] Running utility command for check-bitcoin-cuckoocache_tests
[373/373] Running bitcoin test suite
PASSED: bitcoin test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1

Each failure log is accessible here:
Bitcoin ABC functional tests:

This revision is now accepted and ready to land.Sep 15 2020, 22:26