Page MenuHomePhabricator
Differential D15802

make checkpoints_tests work with the same Config as the rest of the validation code
ClosedPublic
Actions

Authored by PiRK on Mar 28 2024, 19:30.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCe4521cbbe266: make checkpoints_tests work with the same Config as the rest of the validation…
Summary

Change ChainTestingSetup and TestingSetup to allow passing a Config to be used when initializing the ChainstateManager. Leverage this in checkpoints_tests to have a consistent custom config throughout the test, instead of passing a custom config to ProcessNewBlockHeaders while the rest of the validation code uses a different global config.

To make it work, I had to change DummyConfig::GetMaxBlockSize to return a value that makes more sense.

Remove the unused DummyConfig::SetChainPairams function.

This diff will allow us to use the chainparam's config in more validation functions: AcceptBlockHeader, AcceptBlock, ProcessNewBlockHeaders, ProcessNewBlock

It is also needed to backport https://github.com/bitcoin/bitcoin/pull/24595/commits/78adef17536edef833a0bfca06b61ce28120e486, which currently causes checkpoints test to fail.

Test Plan

ninja all check-all

Diff Detail

Event Timeline

PiRK created this revision.Mar 28 2024, 19:30
Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 28 2024, 19:30
PiRK requested review of this revision.Mar 28 2024, 19:30
Harbormaster completed remote builds in B28127: Diff 46554.Mar 28 2024, 19:38
teamcity added a comment.Mar 28 2024, 19:38
Build Bitcoin ABC Diffs / Diff Testing (build-clang) failed.

Tail of the build log:

[443/499] Running utility command for check-bitcoin-walletdb_tests
[444/499] bitcoin: testing result_tests
[445/499] bitcoin: testing crypto_tests
[446/499] Running utility command for check-bitcoin-result_tests
[447/499] bitcoin: testing psbt_wallet_tests
[448/499] Running utility command for check-bitcoin-crypto_tests
[449/499] bitcoin: testing cuckoocache_tests
[450/499] Running utility command for check-bitcoin-psbt_wallet_tests
[451/499] bitcoin: testing merkle_tests
[452/499] Running utility command for check-bitcoin-cuckoocache_tests
[453/499] Running utility command for check-bitcoin-merkle_tests
[454/499] bitcoin: testing denialofservice_tests
[455/499] Running utility command for check-bitcoin-denialofservice_tests
[456/499] Running utility command for check-pow-aserti32d_tests
[457/499] bitcoin: testing init_tests
[458/499] Running pow test suite
PASSED: pow test suite
[459/499] bitcoin: testing uint256_tests
[460/499] Running utility command for check-bitcoin-init_tests
[461/499] bitcoin: testing coinstatsindex_tests
[462/499] bitcoin: testing blockmanager_tests
[463/499] Running utility command for check-bitcoin-uint256_tests
[464/499] bitcoin: testing sighash_tests
[465/499] Running utility command for check-bitcoin-coinstatsindex_tests
[466/499] Running utility command for check-bitcoin-blockmanager_tests
[467/499] Running utility command for check-bitcoin-sighash_tests
[468/499] bitcoin: testing validation_chainstate_tests
[469/499] bitcoin: testing rcu_tests
[470/499] Running utility command for check-bitcoin-validation_chainstate_tests
[471/499] Running utility command for check-bitcoin-rcu_tests
[472/499] Linking CXX executable src/qt/test/test_bitcoin-qt
[473/499] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/fixture.cpp.o
[474/499] bitcoin: testing wallet_crypto_tests
[475/499] Running utility command for check-bitcoin-wallet_crypto_tests
[476/499] Linking CXX executable src/seeder/test/test-seeder
[477/499] seeder: testing message_writer_tests
[478/499] seeder: testing parse_name_tests
[479/499] seeder: testing options_tests
[480/499] seeder: testing p2p_messaging_tests
[481/499] Running utility command for check-seeder-message_writer_tests
[482/499] Running utility command for check-seeder-options_tests
[483/499] seeder: testing write_name_tests
[484/499] Running utility command for check-seeder-parse_name_tests
[485/499] Running utility command for check-seeder-p2p_messaging_tests
[486/499] Running utility command for check-seeder-write_name_tests
[487/499] Running seeder test suite
PASSED: seeder test suite
[488/499] bitcoin-qt: testing test_bitcoin-qt
[489/499] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[490/499] bitcoin: testing wallet_tests
[491/499] Running utility command for check-bitcoin-wallet_tests
[492/499] bitcoin: testing blockcheck_tests
[493/499] Running utility command for check-bitcoin-blockcheck_tests
[494/499] bitcoin: testing coinselector_tests
[495/499] Running utility command for check-bitcoin-coinselector_tests
[496/499] bitcoin: testing coins_tests
[497/499] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1
teamcity added a comment.Mar 28 2024, 19:40
Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:

[436/492] Running utility command for check-bitcoin-walletdb_tests
[437/492] Running utility command for check-bitcoin-multisig_tests
[438/492] bitcoin: testing coinstatsindex_tests
[439/492] Running utility command for check-bitcoin-coinstatsindex_tests
[440/492] bitcoin: testing result_tests
[441/492] bitcoin: testing merkle_tests
[442/492] Running utility command for check-bitcoin-result_tests
[443/492] bitcoin: testing psbt_wallet_tests
[444/492] Running utility command for check-bitcoin-merkle_tests
[445/492] Running utility command for check-bitcoin-psbt_wallet_tests
[446/492] bitcoin: testing denialofservice_tests
[447/492] bitcoin: testing init_tests
[448/492] Running utility command for check-bitcoin-denialofservice_tests
[449/492] bitcoin: testing uint256_tests
[450/492] Running utility command for check-bitcoin-init_tests
[451/492] Running utility command for check-bitcoin-uint256_tests
[452/492] Running utility command for check-pow-grasberg_tests
[453/492] Running utility command for check-pow-aserti32d_tests
[454/492] bitcoin: testing sighash_tests
[455/492] Running pow test suite
PASSED: pow test suite
[456/492] bitcoin: testing rcu_tests
[457/492] Running utility command for check-bitcoin-sighash_tests
[458/492] bitcoin: testing blockmanager_tests
[459/492] Running utility command for check-bitcoin-rcu_tests
[460/492] Running utility command for check-bitcoin-blockmanager_tests
[461/492] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/fixture.cpp.o
[462/492] bitcoin: testing validation_chainstate_tests
[463/492] Running utility command for check-bitcoin-validation_chainstate_tests
[464/492] bitcoin: testing wallet_crypto_tests
[465/492] Running utility command for check-bitcoin-wallet_crypto_tests
[466/492] Linking CXX executable src/seeder/test/test-seeder
[467/492] seeder: testing message_writer_tests
[468/492] seeder: testing options_tests
[469/492] seeder: testing parse_name_tests
[470/492] seeder: testing p2p_messaging_tests
[471/492] seeder: testing write_name_tests
[472/492] Running utility command for check-seeder-message_writer_tests
[473/492] Running utility command for check-seeder-parse_name_tests
[474/492] Running utility command for check-seeder-options_tests
[475/492] Running utility command for check-seeder-p2p_messaging_tests
[476/492] Running utility command for check-seeder-write_name_tests
[477/492] Running seeder test suite
PASSED: seeder test suite
[478/492] bitcoin: testing blockcheck_tests
[479/492] Running utility command for check-bitcoin-blockcheck_tests
[480/492] bitcoin: testing coinselector_tests
[481/492] Running utility command for check-bitcoin-coinselector_tests
[482/492] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o
[483/492] bitcoin: testing wallet_tests
[484/492] Running utility command for check-bitcoin-wallet_tests
[485/492] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[486/492] Linking CXX executable src/qt/test/test_bitcoin-qt
[487/492] bitcoin-qt: testing test_bitcoin-qt
[488/492] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[489/492] bitcoin: testing coins_tests
[490/492] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1
teamcity added a comment.Mar 28 2024, 19:45
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.

Tail of the build log:

chronik_resync.py                          | ○ Skipped | 0 s
chronik_script_confirmed_txs.py            | ○ Skipped | 0 s
chronik_script_history.py                  | ○ Skipped | 0 s
chronik_script_unconfirmed_txs.py          | ○ Skipped | 0 s
chronik_script_utxos.py                    | ○ Skipped | 0 s
chronik_serve.py                           | ○ Skipped | 0 s
chronik_spent_by.py                        | ○ Skipped | 0 s
chronik_token_alp.py                       | ○ Skipped | 0 s
chronik_token_broadcast_txs.py             | ○ Skipped | 0 s
chronik_token_burn.py                      | ○ Skipped | 0 s
chronik_token_id_group.py                  | ○ Skipped | 0 s
chronik_token_parse_failure.py             | ○ Skipped | 0 s
chronik_token_script_group.py              | ○ Skipped | 0 s
chronik_token_slp_fungible.py              | ○ Skipped | 0 s
chronik_token_slp_mint_vault.py            | ○ Skipped | 0 s
chronik_token_slp_nft1.py                  | ○ Skipped | 0 s
chronik_tx.py                              | ○ Skipped | 0 s
chronik_tx_removal_order.py                | ○ Skipped | 0 s
chronik_ws.py                              | ○ Skipped | 0 s
chronik_ws_ordering.py                     | ○ Skipped | 0 s
chronik_ws_ping.py                         | ○ Skipped | 0 s
chronik_ws_script.py                       | ○ Skipped | 0 s
feature_bind_port_discover.py              | ○ Skipped | 0 s
feature_bind_port_externalip.py            | ○ Skipped | 0 s
interface_usdt_net.py                      | ○ Skipped | 0 s
interface_usdt_utxocache.py                | ○ Skipped | 0 s
interface_usdt_validation.py               | ○ Skipped | 0 s

ALL                                        | ✓ Passed  | 2508 s (accumulated) 
Runtime: 510 s

[165/500] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.005s

OK
[167/500] cd /work/contrib/devtools/chainparams && /usr/bin/python3.9 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[324/500] Running secp256k1 test suite
PASSED: secp256k1 test suite
[415/500] bitcoin: testing checkpoints_tests
FAILED: src/test/CMakeFiles/check-bitcoin-checkpoints_tests 
cd /work/abc-ci-builds/build-debug/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-debug/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-debug/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-debug/test/log/bitcoin-checkpoints_tests.log /work/abc-ci-builds/build-debug/src/test/test_bitcoin --run_test=checkpoints_tests --logger=HRF,message:JUNIT,message,bitcoin-checkpoints_tests.xml --catch_system_errors=no
Segmentation fault (core dumped)
[468/500] Running seeder test suite
PASSED: seeder test suite
[480/500] Running avalanche test suite
PASSED: avalanche test suite
[486/500] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[491/500] Running pow test suite
PASSED: pow test suite
[497/500] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1
Fabien added a subscriber: Fabien.EditedMar 28 2024, 19:56

Back to your queue

Fabien requested changes to this revision.Mar 28 2024, 19:56
This revision now requires changes to proceed.Mar 28 2024, 19:56
PiRK added a comment.Mar 28 2024, 22:33

@bot build-tsan

teamcity awarded a token.Mar 28 2024, 22:33
teamcity added a comment.Mar 28 2024, 22:51
Build Bitcoin ABC Diffs / Diff Testing (build-tsan) failed.

Tail of the build log:

chronik_ws_ordering.py                     | ○ Skipped | 0 s
chronik_ws_ping.py                         | ○ Skipped | 0 s
chronik_ws_script.py                       | ○ Skipped | 0 s
feature_bind_port_discover.py              | ○ Skipped | 0 s
feature_bind_port_externalip.py            | ○ Skipped | 0 s
interface_usdt_net.py                      | ○ Skipped | 0 s
interface_usdt_utxocache.py                | ○ Skipped | 0 s
interface_usdt_validation.py               | ○ Skipped | 0 s

ALL                                        | ✓ Passed  | 3039 s (accumulated) 
Runtime: 609 s

[176/493] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.018s

OK
[178/493] cd /work/contrib/devtools/chainparams && /usr/bin/python3.9 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[217/493] Running seeder test suite
PASSED: seeder test suite
[433/493] bitcoin: testing checkpoints_tests
FAILED: src/test/CMakeFiles/check-bitcoin-checkpoints_tests 
cd /work/abc-ci-builds/build-tsan/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-tsan/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-tsan/test/log && /usr/bin/cmake -E env TSAN_OPTIONS=second_deadlock_stack=1:log_path=stdout TSAN_OPTIONS=suppressions=/work/test/sanitizer_suppressions/tsan:log_path=stdout /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-tsan/test/log/bitcoin-checkpoints_tests.log /work/abc-ci-builds/build-tsan/src/test/test_bitcoin --run_test=checkpoints_tests --logger=HRF,message:JUNIT,message,bitcoin-checkpoints_tests.xml --catch_system_errors=no
ThreadSanitizer:DEADLYSIGNAL
==10660==ERROR: ThreadSanitizer: SEGV on unknown address (pc 0x55668ab38191 bp 0x7fb8b66cd718 sp 0x7ffe89f058a0 T10660)
==10660==The signal is caused by a READ memory access.
==10660==Hint: this fault was caused by a dereference of a high value address (see register values below).  Disassemble the provided pc to learn which register was used.
    #0 checkpoints_tests::ban_fork_prior_to_and_at_checkpoints::test_method() /work/abc-ci-builds/build-tsan/../../src/test/checkpoints_tests.cpp:96:35 (test_bitcoin+0x7d2191)
    #1 checkpoints_tests::ban_fork_prior_to_and_at_checkpoints_invoker() /work/abc-ci-builds/build-tsan/../../src/test/checkpoints_tests.cpp:90:1 (test_bitcoin+0x7d19ca)
    #2 boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /usr/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x63bfbd)
    #3 boost::function0<void>::operator()() const /usr/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x616907)
    #4 boost::detail::forward::operator()() /usr/include/boost/test/impl/execution_monitor.ipp:1388:32 (test_bitcoin+0x616907)
    #5 boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /usr/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x616907)
    #6 boost::function0<int>::operator()() const /usr/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x59d45b)
    #7 int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> >(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:301:30 (test_bitcoin+0x59d45b)
    #8 boost::execution_monitor::catch_signals(boost::function<int ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:903:16 (test_bitcoin+0x59d45b)
    #9 boost::execution_monitor::execute(boost::function<int ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:1301:16 (test_bitcoin+0x59d764)
    #10 boost::execution_monitor::vexecute(boost::function<void ()> const&) /usr/include/boost/test/impl/execution_monitor.ipp:1397:5 (test_bitcoin+0x597c50)
    #11 boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /usr/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x59ad1c)
    #12 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /usr/include/boost/test/impl/framework.ipp:815:44 (test_bitcoin+0x5cd6ce)
    #13 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /usr/include/boost/test/impl/framework.ipp:740:54 (test_bitcoin+0x5cd832)
    #14 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /usr/include/boost/test/impl/framework.ipp:740:54 (test_bitcoin+0x5cd832)
    #15 boost::unit_test::framework::run(unsigned long, bool) /usr/include/boost/test/impl/framework.ipp:1721:29 (test_bitcoin+0x59a2e4)
    #16 boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /usr/include/boost/test/impl/unit_test_main.ipp:250:9 (test_bitcoin+0x5b478e)
    #17 main /usr/include/boost/test/impl/unit_test_main.ipp:306:12 (test_bitcoin+0x5b4eb6)
    #18 __libc_start_main <null> (libc.so.6+0x23d09)
    #19 _start <null> (test_bitcoin+0x4bd029)

ThreadSanitizer can not provide additional info.
SUMMARY: ThreadSanitizer: SEGV /work/abc-ci-builds/build-tsan/../../src/test/checkpoints_tests.cpp:96:35 in checkpoints_tests::ban_fork_prior_to_and_at_checkpoints::test_method()
==10660==ABORTING
[445/493] bitcoin: testing coins_tests
ninja: build stopped: subcommand failed.
Build build-tsan failed with exit code 1
PiRK updated this revision to Diff 46578.Mar 29 2024, 07:25

the config needs to be a global, or else we get a nullptr deref when the fixture is destructed.

Harbormaster completed remote builds in B28148: Diff 46578.Mar 29 2024, 07:45
PiRK added a child revision: D15807: get config via chainstatemanager when possible (4/n).Mar 29 2024, 07:58
Fabien requested changes to this revision.Mar 29 2024, 14:50
Fabien added inline comments.
src/config.h
53 ↗(On Diff #46578)

a bit pessimistic

src/test/checkpoints_tests.cpp
73 ↗(On Diff #46578)

static

This revision now requires changes to proceed.Mar 29 2024, 14:50
PiRK updated this revision to Diff 46621.Mar 30 2024, 09:14

address feedback

Harbormaster completed remote builds in B28187: Diff 46621.Mar 30 2024, 09:32
Fabien accepted this revision.Mar 30 2024, 15:26
This revision is now accepted and ready to land.Mar 30 2024, 15:26
Closed by commit rABCe4521cbbe266: make checkpoints_tests work with the same Config as the rest of the validation… (authored by PiRK). · Explain WhyMar 30 2024, 17:23
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABCe4521cbbe266: make checkpoints_tests work with the same Config as the rest of the validation….
PiRK mentioned this in D15825: [test] allow passing test checkpoints to ContextualCheckBlockHeader.Mar 30 2024, 20:58
PiRK mentioned this in rABCa51d89123b68: [test] allow passing test checkpoints to ContextualCheckBlockHeader.Mar 31 2024, 10:13

Revision Contents
Changeset List

PathSize
src/
3 lines
4 lines
test/
20 lines
util/
7 lines
12 lines

Diff 46621

View Options

src/config.h

Loading...
View Options

src/config.cpp

Loading...
View Options

src/test/checkpoints_tests.cpp

Loading...
View Options

src/test/util/setup_common.h

Loading...
View Options

src/test/util/setup_common.cpp

Loading...