test: add reset_chainstate parameter for snapshot unittests
ClosedPublic
Actions

Authored by PiRK on Oct 19 2023, 14:11.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABC26b3b6bfd4aa: test: add reset_chainstate parameter for snapshot unittests

Summary

validation: add ResetChainstates()

Necessary for the following test commit.

https://github.com/bitcoin/bitcoin/pull/25667/commits/00b357c215ed900145bd770525a341ba0ed9c027

test: add reset_chainstate parameter for snapshot unittests

This CreateAndActivateUTXOSnapshot parameter is necessary once we
perform snapshot completion within ABC, since the existing UpdateTip
test will fail because the IBD chain that has generated the snapshot
will exceed the base of the snapshot.

Being able to test snapshots being loaded into a mostly-uninitialized
datadir allows for more realistic unittest scenarios.

https://github.com/bitcoin/bitcoin/pull/25667/commits/3c361391b8f5971eb3c7b620aa7ad9b437cc515e

Backport note: this process of resetting the chainstate without also resetting the block index would cause CheckBlockIndex to fail on assert(setBlockIndexCandidates.count(pindex))); after D4717 made CheckBlockIndex run on intermediate steps in ActiveBestChain. I disabled CheckBlockIndex until after the snapshot chain is activated.

This is a partial backport of core#25667
Depends on D14652

Test Plan

ninja all check-all

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 25376
Build 50333: Build Diff	build-without-wallet · lint-circular-dependencies · build-diff · build-debug · build-clang-tidy · build-clang
Build 50332: arc lint + arc unit

Event Timeline

PiRK created this revision.Oct 19 2023, 14:11

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 19 2023, 14:11

PiRK requested review of this revision.Oct 19 2023, 14:11

teamcity edited the summary of this revision. (Show Details)Oct 19 2023, 14:11

PiRK added inline comments.Oct 19 2023, 14:13

src/test/util/chainstate.h
23–25	The first part of the docstring was missed in a previous backport when this function was moved from a test suite to this source file.

PiRK added a child revision: D14654: validation: remove snapshot datadirs upon validation failure.Oct 19 2023, 14:14

Harbormaster completed remote builds in B25376: Diff 42711.Oct 19 2023, 14:26

Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.

Tail of the build log:

.....
----------------------------------------------------------------------
Ran 5 tests in 0.002s

OK
[179/482] Automatic MOC for target test_bitcoin-qt
[180/482] Test Bitcoin utilities...
[181/482] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/grasberg_tests.cpp.o
[182/482] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/options_tests.cpp.o
[183/482] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/parse_name_tests.cpp.o
[184/482] Linking CXX executable src/pow/test/test-pow
[185/482] pow: testing daa_tests
[186/482] Running utility command for check-pow-daa_tests
[187/482] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/util_tests.cpp.o
[188/482] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/write_name_tests.cpp.o
[189/482] pow: testing eda_tests
[190/482] Running utility command for check-pow-eda_tests
[191/482] pow: testing grasberg_tests
[192/482] Running utility command for check-pow-grasberg_tests
[193/482] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/script_tests.cpp.o
[194/482] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/p2p_messaging_tests.cpp.o
[195/482] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/message_writer_tests.cpp.o
[196/482] pow: testing aserti32d_tests
[197/482] Running utility command for check-pow-aserti32d_tests
[198/482] Running pow test suite
PASSED: pow test suite
[199/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/bitcoinaddressvalidatortests.cpp.o
[200/482] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/__/wallet/test/wallet_tests.cpp.o
[201/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/util.cpp.o
[202/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/compattests.cpp.o
[203/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_bitcoin-qt_autogen/mocs_compilation.cpp.o
[204/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/guiutiltests.cpp.o
[205/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/uritests.cpp.o
[206/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/apptests.cpp.o
[207/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/rpcnestedtests.cpp.o
[208/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_main.cpp.o
[209/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/__/__/wallet/test/wallet_test_fixture.cpp.o
[210/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o
[211/482] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/fixture.cpp.o
[212/482] Linking CXX executable src/seeder/test/test-seeder
[213/482] seeder: testing options_tests
[214/482] seeder: testing message_writer_tests
[215/482] seeder: testing parse_name_tests
[216/482] seeder: testing p2p_messaging_tests
[217/482] Running utility command for check-seeder-options_tests
[218/482] Running utility command for check-seeder-message_writer_tests
[219/482] Running utility command for check-seeder-parse_name_tests
[220/482] seeder: testing write_name_tests
[221/482] Running utility command for check-seeder-p2p_messaging_tests
[222/482] Running utility command for check-seeder-write_name_tests
[223/482] Running seeder test suite
PASSED: seeder test suite
[224/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o
[225/482] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o
[226/482] Linking CXX executable src/qt/test/test_bitcoin-qt
[227/482] bitcoin-qt: testing test_bitcoin-qt
[228/482] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

PiRK planned changes to this revision.Oct 19 2023, 15:35

i got the order of the arguments wrong because I had to resort the commits. reset_chainstate must be before in_memory_chainstate at the end of the backport

Harbormaster completed remote builds in B25379: Diff 42714.Oct 19 2023, 15:58

Not actually requesting changes, but this question has to be answered before this can move forward

src/test/util/chainstate.h
73 ↗	(On Diff #42714)	Is that thread safe ?

This revision now requires changes to proceed.Oct 23 2023, 12:46

PiRK requested review of this revision.Oct 23 2023, 15:29

PiRK added inline comments.

src/test/util/chainstate.h
73 ↗	(On Diff #42714)	At the moment there is no thread-safety issue here. This global is set once in init.cpp or in the test fixture constructor, and only read in `CheckBlockIndex`which is always accessed in the same thread.