Page MenuHomePhabricator

[backport#16945#18181] refactor: introduce CChainState::GetCoinsCacheSizeState [plus fix]
ClosedPublic

Authored by majcosta on Jan 17 2021, 10:26.

Details

Summary

Core PR16945

02b9511d6bace5711e454d2b685b2fee0d65e341 tests: add tests for GetCoinsCacheSizeState (James O'Beirne)
b17e91d842724d2888a179a73585cc4c2ef1dc21 refactoring: introduce CChainState::GetCoinsCacheSizeState (James O'Beirne)

Pull request description:

This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):

Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal

This pulls out the routine for detection of how full the coins cache is from
FlushStateToDisk. We use this logic independently when deciding when to flush
the coins cache during UTXO snapshot activation ([see here](https://github.com/bitcoin/bitcoin/pull/15606/commits/231fb5f17e3bb8ce3323c559ef6f9b3aaa066543#diff-24efdb00bfbe56b140fb006b562cc70bR5275)).

Core PR18181

faca8eff39876cc8c0ee609a89bdcebc21976d47 test: Remove incorrect assumptions in validation_flush_tests (MarcoFalke)
fa31eebfe9107e14dc1d6b588f5b4c878d1010de test: Tabs to spaces in all tests (MarcoFalke)

Pull request description:

The tests assume standard library internals that may not hold on all supported archs or when the code is instrumented for sanitizer or debug use cases

Fixes #18111

Backport of Core PR16945 and PR18181 (the fix in PR18181 is contained to
test/validation_flush_tests.cpp)

Test Plan
ninja all check check-functional

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

Fabien requested changes to this revision.Jan 18 2021, 08:45
Fabien added inline comments.
src/test/validation_flush_tests.cpp
41 ↗(On Diff #27026)

remove

This revision now requires changes to proceed.Jan 18 2021, 08:45

Tail of the build log:

wallet_txn_doublespend.py                        | ✓ Passed  | 1 s
wallet_txn_doublespend.py --mineblock            | ✓ Passed  | 3 s
wallet_watchonly.py                              | ✓ Passed  | 1 s
wallet_watchonly.py --usecli                     | ✓ Passed  | 1 s
wallet_zapwallettxes.py                          | ✓ Passed  | 5 s

ALL                                              | ✓ Passed  | 794 s (accumulated) 
Runtime: 159 s

[164/438] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.005s

OK
[165/438] cd /work/contrib/devtools/chainparams && /usr/bin/python3.7 ./test_make_chainparams.py
.....
----------------------------------------------------------------------
Ran 5 tests in 0.001s

OK
[177/438] Running pow test suite
PASSED: pow test suite
[190/438] Running seeder test suite
PASSED: seeder test suite
[193/438] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/script_tests.cpp.o
In file included from ../../depends/i686-pc-linux-gnu/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) {
                      ^~~~~~~~~~~~
[403/438] bitcoin: testing validation_flush_tests
FAILED: src/test/CMakeFiles/check-bitcoin-validation_flush_tests 
cd /work/abc-ci-builds/build-linux32/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-linux32/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-linux32/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-linux32/test/log/bitcoin-validation_flush_tests.log /work/abc-ci-builds/build-linux32/src/test/test_bitcoin --run_test=validation_flush_tests --logger=HRF,message:JUNIT,message,bitcoin-validation_flush_tests.xml --catch_system_errors=no
Running 1 test case...
CCoinsViewCache memory usage: 16
CCoinsViewCache memory usage: 192
../../src/test/validation_flush_tests.cpp(101): error: in "validation_flush_tests/getcoinscachesizestate": check view.AccessCoin(res).DynamicMemoryUsage() == COIN_SIZE has failed [72 != 64]
CCoinsViewCache memory usage: 360
../../src/test/validation_flush_tests.cpp(101): error: in "validation_flush_tests/getcoinscachesizestate": check view.AccessCoin(res).DynamicMemoryUsage() == COIN_SIZE has failed [72 != 64]
CCoinsViewCache memory usage: 544
../../src/test/validation_flush_tests.cpp(101): error: in "validation_flush_tests/getcoinscachesizestate": check view.AccessCoin(res).DynamicMemoryUsage() == COIN_SIZE has failed [72 != 64]
CCoinsViewCache memory usage: 712
CCoinsViewCache memory usage: 880
CCoinsViewCache memory usage: 1048
CCoinsViewCache memory usage: 1256
CCoinsViewCache memory usage: 1424
CCoinsViewCache memory usage: 1592
CCoinsViewCache memory usage: 1760
CCoinsViewCache memory usage: 5984

*** 3 failures are detected in the test module "Bitcoin ABC unit tests"
[429/438] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[433/438] Running secp256k1 test suite
PASSED: secp256k1 test suite
[435/438] Running utility command for check-bitcoin-transaction_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-linux32 failed with exit code 1

removed commented leftover debugging line and adapted the test for Coin::DynamicMemoryUsage() on 32-bit

@bot build-linux32 build-linux-arm build-linux-aarch64

This revision is now accepted and ready to land.Jan 18 2021, 12:30