Page MenuHomePhabricator
Differential D8938

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

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

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC7680ec0096b1: [backport#16945#18181] refactor: introduce CChainState::GetCoinsCacheSizeState…
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

Event Timeline

majcosta created this revision.Jan 17 2021, 10:26
Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 17 2021, 10:26
majcosta requested review of this revision.Jan 17 2021, 10:26
teamcity edited the summary of this revision. (Show Details)Jan 17 2021, 10:26
Harbormaster completed remote builds in B14812: Diff 27026.Jan 17 2021, 10:37
majcosta edited the summary of this revision. (Show Details)Jan 17 2021, 12:11
Fabien added a subscriber: Fabien.Jan 18 2021, 08:38

@bot build-linux32

teamcity awarded a token.Jan 18 2021, 08:38
Fabien requested changes to this revision.Jan 18 2021, 08:45
Fabien added inline comments.
src/test/validation_flush_tests.cpp
41

remove

This revision now requires changes to proceed.Jan 18 2021, 08:45
teamcity added a comment.Jan 18 2021, 08:49
Build Bitcoin ABC Diffs / Diff Testing (build-linux32) failed.

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
Fabien added a comment.Jan 18 2021, 08:54

@bot build-native-osx

teamcity updated a token.Jan 18 2021, 08:54
majcosta updated this revision to Diff 27033.Jan 18 2021, 11:12

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

Harbormaster completed remote builds in B14817: Diff 27033.Jan 18 2021, 11:25
Fabien added a comment.Jan 18 2021, 11:31

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

teamcity updated a token.Jan 18 2021, 11:32
Fabien accepted this revision.Jan 18 2021, 12:30
This revision is now accepted and ready to land.Jan 18 2021, 12:30
Closed by commit rABC7680ec0096b1: [backport#16945#18181] refactor: introduce CChainState::GetCoinsCacheSizeState… (authored by Wladimir J. van der Laan <laanwj@protonmail.com>, committed by majcosta). · Explain WhyJan 18 2021, 13:06
This revision was automatically updated to reflect the committed changes.
majcosta added a commit: rABC7680ec0096b1: [backport#16945#18181] refactor: introduce CChainState::GetCoinsCacheSizeState….

Revision Contents
Changeset List

PathSize
src/
test/
1 line
180 lines
2 lines
18 lines
53 lines

Diff 27026

View Options

src/test/CMakeLists.txt

Loading...
View Options

src/test/validation_flush_tests.cpp

Loading...
View Options

src/txdb.h

Loading...
View Options

src/validation.h

Loading...
View Options

src/validation.cpp

Loading...