Page MenuHomePhabricator
Differential D16156

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

Authored by PiRK on Tue, May 14, 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 Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK created this revision.Tue, May 14, 13:26
Herald added a reviewer: Restricted Project. · View Herald TranscriptTue, May 14, 13:26
PiRK requested review of this revision.Tue, May 14, 13:26
teamcity edited the summary of this revision. (Show Details)Tue, May 14, 13:26
PiRK added inline comments.Tue, May 14, 13:32
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)

Harbormaster completed remote builds in B28999: Diff 47769.Tue, May 14, 14:25
PiRK added a child revision: D16161: test: refactor: clarify the coins simulation.Wed, May 15, 12:02
Fabien added a subscriber: Fabien.Thu, May 16, 07:57

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

teamcity awarded a token.Thu, May 16, 07:57
teamcity added a comment.Thu, May 16, 08:15
Build Bitcoin ABC Diffs / Diff Testing (build-linux32) failed.

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.Thu, May 16, 08:19

now you know :)

This revision now requires changes to proceed.Thu, May 16, 08:19
teamcity added a comment.Thu, May 16, 08:21
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
PiRK updated this revision to Diff 47810.Thu, May 16, 08:44

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

PiRK added a comment.Thu, May 16, 08:45

@bot build-linux32 build-linux-arm

teamcity updated a token.Thu, May 16, 08:45
Harbormaster completed remote builds in B29026: Diff 47810.Thu, May 16, 09:34
Fabien accepted this revision.Thu, May 16, 10:27
This revision is now accepted and ready to land.Thu, May 16, 10:27
Closed by commit rABC9006f52c6661: test: Create InsecureRandMoneyAmount() helper, de-duplicate add_coin methods (authored by PiRK). · Explain WhyThu, May 16, 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.

Revision Contents
Changeset List

Diff 47814

View Options

src/test/CMakeLists.txt

Loading...
View Options

src/test/coins_tests.cpp

Loading...
View Options

src/test/sighash_tests.cpp

Loading...
View Options

src/test/util/coins.h

Loading...
View Options

src/test/util/coins.cpp

Loading...
View Options

src/test/util/random.h

Loading...
View Options

src/test/validation_chainstate_tests.cpp

Loading...
View Options

src/test/validation_flush_tests.cpp

Loading...