Page MenuHomePhabricator

test: Create InsecureRandMoneyAmount() helper, de-duplicate add_coin methods
ClosedPublic

Authored by PiRK on May 14 2024, 13:26.

Details

Summary

Generate semi-random Amounts up to MAX_MONEY rather than only uint32, and use it in the unit tests.

This concludes backport of core#26490
https://github.com/bitcoin/bitcoin/pull/26940/commits/9d92c3d7f42c18939a9a6aa1ee185f1c958360a0
https://github.com/bitcoin/bitcoin/pull/26940/commits/4275195606e6f42466d9a8ef766b3035833df4d5

Depends on D16155

Test Plan

ninja check

Event Timeline

PiRK requested review of this revision.May 14 2024, 13:26
src/test/validation_flush_tests.cpp
34 ↗(On Diff #47769)

I'm not sure why this ended up using 62 rather than 56 in D8938
The tests work either way. validation_chainstate_tests.cpp used 56 (like Core does for both tests)

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

Tail of the build log:

wallet_listtransactions.py --descriptors   | ✓ Passed  | 3 s
wallet_multiwallet.py                      | ✓ Passed  | 12 s
wallet_multiwallet.py --descriptors        | ✓ Passed  | 11 s
wallet_multiwallet.py --usecli             | ✓ Passed  | 9 s
wallet_reorgsrestore.py                    | ✓ Passed  | 3 s
wallet_resendwallettransactions.py         | ✓ Passed  | 3 s
wallet_send.py                             | ✓ Passed  | 8 s
wallet_startup.py                          | ✓ Passed  | 2 s
wallet_timelock.py                         | ✓ Passed  | 1 s
wallet_txn_clone.py                        | ✓ Passed  | 1 s
wallet_txn_clone.py --mineblock            | ✓ Passed  | 3 s
wallet_txn_doublespend.py                  | ✓ Passed  | 2 s
wallet_txn_doublespend.py --mineblock      | ✓ Passed  | 2 s
wallet_watchonly.py                        | ✓ Passed  | 1 s
wallet_watchonly.py --usecli               | ✓ Passed  | 1 s
chronik_plugins_setup.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  | 1520 s (accumulated) 
Runtime: 304 s

[198/514] Running secp256k1 test suite
PASSED: secp256k1 test suite
[219/514] Running avalanche test suite
PASSED: avalanche test suite
[241/514] Running seeder test suite
PASSED: seeder test suite
[244/514] Running pow test suite
PASSED: pow test suite
[247/514] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[291/514] 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: 224
../../src/test/validation_flush_tests.cpp(80): error: in "validation_flush_tests/getcoinscachesizestate": check view.AccessCoin(res).DynamicMemoryUsage() == COIN_SIZE has failed [64 != 72]
CCoinsViewCache memory usage: 384
../../src/test/validation_flush_tests.cpp(80): error: in "validation_flush_tests/getcoinscachesizestate": check view.AccessCoin(res).DynamicMemoryUsage() == COIN_SIZE has failed [64 != 72]
CCoinsViewCache memory usage: 544
../../src/test/validation_flush_tests.cpp(80): error: in "validation_flush_tests/getcoinscachesizestate": check view.AccessCoin(res).DynamicMemoryUsage() == COIN_SIZE has failed [64 != 72]
CCoinsViewCache memory usage: 704
CCoinsViewCache memory usage: 864
CCoinsViewCache memory usage: 1024
CCoinsViewCache memory usage: 1184
CCoinsViewCache memory usage: 1344
CCoinsViewCache memory usage: 1504
CCoinsViewCache memory usage: 1664
CCoinsViewCache memory usage: 1824
CCoinsViewCache memory usage: 4448

*** 3 failures are detected in the test module "Bitcoin ABC unit tests"
[511/514] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-linux32 failed with exit code 1
Fabien requested changes to this revision.May 16 2024, 08:19

now you know :)

This revision now requires changes to proceed.May 16 2024, 08:19

Tail of the build log:

../../chronik/test/chronikbridge_tests.cpp:131:10: warning: missing initializer for member ‘chronik_bridge::Tx::outputs’ [-Wmissing-field-initializers]
../../chronik/test/chronikbridge_tests.cpp:131:10: warning: missing initializer for member ‘chronik_bridge::Tx::locktime’ [-Wmissing-field-initializers]
In file included from ../../chronik/chronik-cpp/chronik_bridge.h:9,
                 from cargo/build/arm-unknown-linux-gnueabihf/cxxbridge/chronik-bridge/src/ffi.rs.h:3,
                 from ../../chronik/test/chronikbridge_tests.cpp:6:
cargo/build/arm-unknown-linux-gnueabihf/cxxbridge/rust/cxx.h: In instantiation of ‘T& rust::cxxbridge1::Vec<T>::operator[](std::size_t) [with T = chronik_bridge::OutPoint; std::size_t = unsigned int]’:
../../chronik/test/chronikbridge_tests.cpp:140:5:   required from here
cargo/build/arm-unknown-linux-gnueabihf/cxxbridge/rust/cxx.h:911:11: warning: cast from ‘char*’ to ‘chronik_bridge::OutPoint*’ increases required alignment of target type [-Wcast-align]
  911 |   return *reinterpret_cast<T *>(data + n * size_of<T>());
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cargo/build/arm-unknown-linux-gnueabihf/cxxbridge/rust/cxx.h: In instantiation of ‘T& rust::cxxbridge1::Vec<T>::operator[](std::size_t) [with T = chronik_bridge::TxInput; std::size_t = unsigned int]’:
../../chronik/test/chronikbridge_tests.cpp:140:5:   required from here
cargo/build/arm-unknown-linux-gnueabihf/cxxbridge/rust/cxx.h:911:11: warning: cast from ‘char*’ to ‘chronik_bridge::TxInput*’ increases required alignment of target type [-Wcast-align]
cargo/build/arm-unknown-linux-gnueabihf/cxxbridge/rust/cxx.h: In instantiation of ‘void rust::cxxbridge1::Vec<T>::emplace_back(Args&& ...) [with Args = {const chronik_bridge::TxInput&}; T = chronik_bridge::TxInput]’:
cargo/build/arm-unknown-linux-gnueabihf/cxxbridge/rust/cxx.h:941:3:   required from ‘void rust::cxxbridge1::Vec<T>::push_back(const T&) [with T = chronik_bridge::TxInput]’
/usr/arm-linux-gnueabihf/include/c++/9/bits/stl_iterator.h:515:2:   required from ‘std::back_insert_iterator<_Container>& std::back_insert_iterator<_Container>::operator=(const typename _Container::value_type&) [with _Container = rust::cxxbridge1::Vec<chronik_bridge::TxInput>; typename _Container::value_type = chronik_bridge::TxInput]’
/usr/arm-linux-gnueabihf/include/c++/9/bits/stl_algobase.h:361:18:   required from ‘static _OI std::__copy_move<true, false, std::random_access_iterator_tag>::__copy_m(_II, _II, _OI) [with _II = const chronik_bridge::TxInput*; _OI = std::back_insert_iterator<rust::cxxbridge1::Vec<chronik_bridge::TxInput> >]’
/usr/arm-linux-gnueabihf/include/c++/9/bits/stl_algobase.h:404:30:   required from ‘_OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove = true; _II = const chronik_bridge::TxInput*; _OI = std::back_insert_iterator<rust::cxxbridge1::Vec<chronik_bridge::TxInput> >]’
/usr/arm-linux-gnueabihf/include/c++/9/bits/stl_algobase.h:441:30:   required from ‘_OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove = true; _II = const chronik_bridge::TxInput*; _OI = std::back_insert_iterator<rust::cxxbridge1::Vec<chronik_bridge::TxInput> >]’
/usr/arm-linux-gnueabihf/include/c++/9/bits/stl_algobase.h:505:39:   required from ‘_OI std::move(_II, _II, _OI) [with _II = const chronik_bridge::TxInput*; _OI = std::back_insert_iterator<rust::cxxbridge1::Vec<chronik_bridge::TxInput> >]’
cargo/build/arm-unknown-linux-gnueabihf/cxxbridge/rust/cxx.h:834:12:   required from ‘rust::cxxbridge1::Vec<T>::Vec(std::initializer_list<_Tp>) [with T = chronik_bridge::TxInput]’
../../chronik/test/chronikbridge_tests.cpp:131:10:   required from here
cargo/build/arm-unknown-linux-gnueabihf/cxxbridge/rust/cxx.h:954:10: warning: cast from ‘char*’ to ‘chronik_bridge::TxInput*’ increases required alignment of target type [-Wcast-align]
  954 |   ::new (reinterpret_cast<T *>(reinterpret_cast<char *>(this->data()) +
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  955 |                                size * size_of<T>()))
      |                                ~~~~~~~~~~~~~~~~~~~~
[207/514] Running seeder test suite
PASSED: seeder test suite
[281/514] bitcoin: testing validation_flush_tests
FAILED: src/test/CMakeFiles/check-bitcoin-validation_flush_tests 
cd /work/abc-ci-builds/build-linux-arm/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-linux-arm/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-linux-arm/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-linux-arm/test/log/bitcoin-validation_flush_tests.log /usr/bin/qemu-arm-static /work/abc-ci-builds/build-linux-arm/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: 240
../../src/test/validation_flush_tests.cpp(80): error: in "validation_flush_tests/getcoinscachesizestate": check view.AccessCoin(res).DynamicMemoryUsage() == COIN_SIZE has failed [64 != 72]
CCoinsViewCache memory usage: 416
../../src/test/validation_flush_tests.cpp(80): error: in "validation_flush_tests/getcoinscachesizestate": check view.AccessCoin(res).DynamicMemoryUsage() == COIN_SIZE has failed [64 != 72]
CCoinsViewCache memory usage: 592
../../src/test/validation_flush_tests.cpp(80): error: in "validation_flush_tests/getcoinscachesizestate": check view.AccessCoin(res).DynamicMemoryUsage() == COIN_SIZE has failed [64 != 72]
CCoinsViewCache memory usage: 768
CCoinsViewCache memory usage: 944
CCoinsViewCache memory usage: 1120
CCoinsViewCache memory usage: 1296
CCoinsViewCache memory usage: 1472
CCoinsViewCache memory usage: 1648
CCoinsViewCache memory usage: 1824
CCoinsViewCache memory usage: 4448

*** 3 failures are detected in the test module "Bitcoin ABC unit tests"
[483/514] Running avalanche test suite
PASSED: avalanche test suite
[485/514] Running pow test suite
PASSED: pow test suite
[499/514] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[511/514] Running secp256k1 test suite
PASSED: secp256k1 test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-linux-arm failed with exit code 1

test keeping 56 bytes CScript but adjusting COINS_SIZE to use the same value as Core

@bot build-linux32 build-linux-arm

This revision is now accepted and ready to land.May 16 2024, 10:27