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

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

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

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

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

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 28999
Build 57535: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang · build-clang-tidy
Build 57534: arc lint + arc unit

Event Timeline

PiRK created this revision.May 14 2024, 13:26

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 14 2024, 13:26

PiRK requested review of this revision.May 14 2024, 13:26

teamcity edited the summary of this revision. (Show Details)May 14 2024, 13:26

PiRK added inline comments.May 14 2024, 13:32

src/test/validation_flush_tests.cpp
34	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)

Harbormaster completed remote builds in B28999: Diff 47769.May 14 2024, 14:25

PiRK added a child revision: D16161: test: refactor: clarify the coins simulation.May 15 2024, 12:02

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

teamcity awarded a token.May 16 2024, 07:57

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

Tail of the build log:

[0m[0;32mwallet_listtransactions.py --descriptors   | ✓ Passed  | 3 s
[0m[0;32mwallet_multiwallet.py                      | ✓ Passed  | 12 s
[0m[0;32mwallet_multiwallet.py --descriptors        | ✓ Passed  | 11 s
[0m[0;32mwallet_multiwallet.py --usecli             | ✓ Passed  | 9 s
[0m[0;32mwallet_reorgsrestore.py                    | ✓ Passed  | 3 s
[0m[0;32mwallet_resendwallettransactions.py         | ✓ Passed  | 3 s
[0m[0;32mwallet_send.py                             | ✓ Passed  | 8 s
[0m[0;32mwallet_startup.py                          | ✓ Passed  | 2 s
[0m[0;32mwallet_timelock.py                         | ✓ Passed  | 1 s
[0m[0;32mwallet_txn_clone.py                        | ✓ Passed  | 1 s
[0m[0;32mwallet_txn_clone.py --mineblock            | ✓ Passed  | 3 s
[0m[0;32mwallet_txn_doublespend.py                  | ✓ Passed  | 2 s
[0m[0;32mwallet_txn_doublespend.py --mineblock      | ✓ Passed  | 2 s
[0m[0;32mwallet_watchonly.py                        | ✓ Passed  | 1 s
[0m[0;32mwallet_watchonly.py --usecli               | ✓ Passed  | 1 s
[0m[1;30mchronik_plugins_setup.py                   | ○ Skipped | 0 s
[0m[1;30mfeature_bind_port_discover.py              | ○ Skipped | 0 s
[0m[1;30mfeature_bind_port_externalip.py            | ○ Skipped | 0 s
[0m[1;30minterface_usdt_net.py                      | ○ Skipped | 0 s
[0m[1;30minterface_usdt_utxocache.py                | ○ Skipped | 0 s
[0m[1;30minterface_usdt_validation.py               | ○ Skipped | 0 s
[0m[1m
ALL                                        | ✓ Passed  | 1520 s (accumulated) 
[0mRuntime: 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

now you know :)

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

Build Bitcoin ABC Diffs / Diff Testing (build-linux-arm) failed.

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

teamcity updated a token.May 16 2024, 08:45

Harbormaster completed remote builds in B29026: Diff 47810.May 16 2024, 09:34

Fabien accepted this revision.May 16 2024, 10:27

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

Closed by commit rABC9006f52c6661: test: Create InsecureRandMoneyAmount() helper, de-duplicate add_coin methods (authored by PiRK). · Explain WhyMay 16 2024, 11:57

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABC9006f52c6661: test: Create InsecureRandMoneyAmount() helper, de-duplicate add_coin methods.