Page MenuHomePhabricator

Check for overflow when calculating sum of outputs
ClosedPublic

Authored by PiRK on Jan 13 2021, 14:20.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABC4a38b361f68c: Check for overflow when calculating sum of outputs
Summary

Address a possible undefined behavior when summing outputs for a transaction, when the first outputs are valid but a following output causes an overflow.

The exact order of the if, is important, we first do !MoneyRange(tx_out.nValue) to make sure the amount is non-negative. and then std::numeric_limits<CAmount>::max() - tx_out.nValue < nValueOut checks that the addition cannot overflow (if we won't check that the amount is positive this check can also overflow! (by doing something like max - -max))
and only then we make sure that the sum is also valid !MoneyRange(nValueOut + tx_out.nValue)
if any of these conditions fail we throw.

This is a backport of Core PR18383

Test Plan
mkdir build_ubsan
cd build_ubsan

cmake -GNinja .. \
  -DCMAKE_BUILD_TYPE=Debug \
  -DENABLE_SANITIZERS=undefined

ninja check

Event Timeline

PiRK requested review of this revision.Jan 13 2021, 14:20
deadalnix requested changes to this revision.Jan 13 2021, 14:22
deadalnix added a subscriber: deadalnix.

I know there isn't int he original code, but there should really be tests cases for this.

This revision now requires changes to proceed.Jan 13 2021, 14:22

add tests for CTransaction::GetValueOut

I added a few tests, but now I 'm no longer sure that all situations described in the PR are possible. I don't know how to reasonably generate a CTransaction whose outputs would add up to an int64 overflow without any single one of the outputs being larger than MAX_MONEY.
It would require 439208192231 outputs.

deadalnix requested changes to this revision.Jan 14 2021, 02:51
In D8899#204315, @PiRK wrote:

I added a few tests, but now I 'm no longer sure that all situations described in the PR are possible. I don't know how to reasonably generate a CTransaction whose outputs would add up to an int64 overflow without any single one of the outputs being larger than MAX_MONEY.
It would require 439208192231 outputs.

It require way less outputs than this - but yes, still a significant amount. You should see how ling it takes before dismissing adding a test case.

This revision now requires changes to proceed.Jan 14 2021, 02:51

Reverse the outputs for the int64 overflow test.

The first output needs to be valid if we want to trigger the overflow when summing.
This test now fails prior to this patch, when compiled with UBSan enabled.

Test plan updated to run UBSan.

PiRK edited the test plan for this revision. (Show Details)

@bot build-ubsan

Tail of the build log:

[382/434] Running utility command for check-bitcoin-script_commitment_tests
[383/434] Running utility command for check-bitcoin-bip32_tests
[384/434] bitcoin: testing settings_tests
[385/434] Running utility command for check-bitcoin-settings_tests
[386/434] bitcoin: testing timedata_tests
[387/434] bitcoin: testing streams_tests
[388/434] Running utility command for check-bitcoin-timedata_tests
[389/434] Running utility command for check-bitcoin-streams_tests
[390/434] bitcoin: testing uint256_tests
[391/434] Running utility command for check-bitcoin-uint256_tests
[392/434] bitcoin: testing undo_tests
[393/434] Running utility command for check-bitcoin-undo_tests
[394/434] bitcoin: testing walletdb_tests
[395/434] bitcoin: testing util_threadnames_tests
[396/434] Running utility command for check-bitcoin-walletdb_tests
[397/434] bitcoin: testing txvalidationcache_tests
[398/434] Running utility command for check-bitcoin-util_threadnames_tests
[399/434] bitcoin: testing serialize_tests
[400/434] Running utility command for check-bitcoin-txvalidationcache_tests
[401/434] Running utility command for check-bitcoin-serialize_tests
[402/434] bitcoin: testing validation_chainstatemanager_tests
[403/434] Running utility command for check-bitcoin-validation_chainstatemanager_tests
[404/434] bitcoin: testing validationinterface_tests
[405/434] bitcoin: testing radix_tests
[406/434] bitcoin: testing schnorr_tests
[407/434] Running utility command for check-bitcoin-validationinterface_tests
[408/434] Running utility command for check-bitcoin-schnorr_tests
[409/434] Running utility command for check-bitcoin-radix_tests
[410/434] bitcoin: testing cashaddr_tests
[411/434] Running utility command for check-bitcoin-cashaddr_tests
[412/434] bitcoin: testing crypto_tests
[413/434] bitcoin: testing getarg_tests
[414/434] Running utility command for check-bitcoin-crypto_tests
[415/434] Running utility command for check-bitcoin-getarg_tests
[416/434] bitcoin: testing script_tests
[417/434] Running utility command for check-bitcoin-script_tests
[418/434] bitcoin: testing cuckoocache_tests
[419/434] Running utility command for check-bitcoin-cuckoocache_tests
[420/434] bitcoin: testing monolith_opcodes_tests
[421/434] bitcoin: testing util_tests
[422/434] bitcoin: testing transaction_tests
FAILED: src/test/CMakeFiles/check-bitcoin-transaction_tests 
cd /work/abc-ci-builds/build-clang/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-clang/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-clang/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-clang/test/log/bitcoin-transaction_tests.log /work/abc-ci-builds/build-clang/src/test/test_bitcoin --run_test=transaction_tests --logger=HRF,message:JUNIT,message,bitcoin-transaction_tests.xml --catch_system_errors=no
Running 9 test cases...
../../src/test/transaction_tests.cpp(907): error: in "transaction_tests/tx_getvalueout": exception std::runtime_error expected but not raised
../../src/test/transaction_tests.cpp(913): error: in "transaction_tests/tx_getvalueout": check valid_tx.GetValueOut() == 1379 * SATOSHI has failed [42 != 1379]

*** 2 failures are detected in the test module "Bitcoin ABC unit tests"
[423/434] bitcoin: testing skiplist_tests
[424/434] bitcoin: testing coinselector_tests
[425/434] Running utility command for check-bitcoin-monolith_opcodes_tests
[426/434] Running utility command for check-bitcoin-util_tests
[427/434] Running utility command for check-bitcoin-skiplist_tests
[428/434] Running utility command for check-bitcoin-coinselector_tests
[429/434] bitcoin: testing coins_tests
[430/434] Running utility command for check-bitcoin-coins_tests
[431/434] bitcoin: testing op_reversebytes_tests
[432/434] Running utility command for check-bitcoin-op_reversebytes_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang failed with exit code 1

Tail of the build log:

[346/398] Running utility command for check-bitcoin-streams_tests
[347/398] bitcoin: testing serialize_tests
[348/398] bitcoin: testing sigencoding_tests
[349/398] Running utility command for check-bitcoin-radix_tests
[350/398] Running utility command for check-bitcoin-serialize_tests
[351/398] bitcoin: testing schnorr_tests
[352/398] Running utility command for check-bitcoin-sigencoding_tests
[353/398] Running utility command for check-bitcoin-schnorr_tests
[354/398] bitcoin: testing timedata_tests
[355/398] Running utility command for check-bitcoin-timedata_tests
[356/398] bitcoin: testing util_threadnames_tests
[357/398] bitcoin: testing undo_tests
[358/398] bitcoin: testing blockcheck_tests
[359/398] Running utility command for check-bitcoin-util_threadnames_tests
[360/398] bitcoin: testing uint256_tests
[361/398] bitcoin: testing compilerbug_tests
[362/398] Running utility command for check-bitcoin-undo_tests
[363/398] bitcoin: testing validation_chainstatemanager_tests
[364/398] Running utility command for check-bitcoin-blockcheck_tests
[365/398] bitcoin: testing checkpoints_tests
[366/398] Running utility command for check-bitcoin-uint256_tests
[367/398] Running utility command for check-bitcoin-compilerbug_tests
[368/398] Running utility command for check-bitcoin-validation_chainstatemanager_tests
[369/398] Running utility command for check-bitcoin-checkpoints_tests
[370/398] bitcoin: testing validationinterface_tests
[371/398] bitcoin: testing crypto_tests
[372/398] Running utility command for check-bitcoin-validationinterface_tests
[373/398] Running utility command for check-bitcoin-crypto_tests
[374/398] bitcoin: testing cashaddr_tests
[375/398] bitcoin: testing script_standard_tests
[376/398] Running utility command for check-bitcoin-cashaddr_tests
[377/398] Running utility command for check-bitcoin-script_standard_tests
[378/398] bitcoin: testing getarg_tests
[379/398] Running utility command for check-bitcoin-getarg_tests
[380/398] bitcoin: testing skiplist_tests
[381/398] Running utility command for check-bitcoin-skiplist_tests
[382/398] bitcoin: testing validation_tests
[383/398] Running utility command for check-bitcoin-validation_tests
[384/398] bitcoin: testing cuckoocache_tests
[385/398] Running utility command for check-bitcoin-cuckoocache_tests
[386/398] bitcoin: testing monolith_opcodes_tests
[387/398] bitcoin: testing util_tests
[388/398] Running utility command for check-bitcoin-monolith_opcodes_tests
[389/398] Running utility command for check-bitcoin-util_tests
[390/398] bitcoin: testing validation_block_tests
[391/398] Running utility command for check-bitcoin-validation_block_tests
[392/398] bitcoin: testing op_reversebytes_tests
[393/398] Running utility command for check-bitcoin-op_reversebytes_tests
[394/398] bitcoin: testing transaction_tests
FAILED: src/test/CMakeFiles/check-bitcoin-transaction_tests 
cd /work/abc-ci-builds/build-without-wallet/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-without-wallet/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-without-wallet/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-without-wallet/test/log/bitcoin-transaction_tests.log /work/abc-ci-builds/build-without-wallet/src/test/test_bitcoin --run_test=transaction_tests --logger=HRF,message:JUNIT,message,bitcoin-transaction_tests.xml --catch_system_errors=no
Running 9 test cases...
../../src/test/transaction_tests.cpp(907): error: in "transaction_tests/tx_getvalueout": exception std::runtime_error expected but not raised
../../src/test/transaction_tests.cpp(913): error: in "transaction_tests/tx_getvalueout": check valid_tx.GetValueOut() == 1379 * SATOSHI has failed [42 != 1379]

*** 2 failures are detected in the test module "Bitcoin ABC unit tests"
[395/398] bitcoin: testing coins_tests
[396/398] Running utility command for check-bitcoin-coins_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1

Tail of the build log:

../../src/test/transaction_tests.cpp(913): error: in "transaction_tests/tx_getvalueout": check valid_tx.GetValueOut() == 1379 * SATOSHI has failed [42 != 1379]

*** 2 failures are detected in the test module "Bitcoin ABC unit tests"
[371/427] bitcoin: testing wallet_tests
[372/427] Running utility command for check-bitcoin-bswap_tests
[373/427] bitcoin: testing merkleblock_tests
[374/427] Running utility command for check-bitcoin-wallet_tests
[375/427] bitcoin: testing script_commitment_tests
[376/427] bitcoin: testing sighashtype_tests
[377/427] bitcoin: testing finalization_tests
[378/427] Running utility command for check-bitcoin-merkleblock_tests
[379/427] bitcoin: testing bip32_tests
[380/427] Running utility command for check-bitcoin-script_commitment_tests
[381/427] Running utility command for check-bitcoin-sighashtype_tests
[382/427] Running utility command for check-bitcoin-finalization_tests
[383/427] Running utility command for check-bitcoin-bip32_tests
[384/427] bitcoin: testing settings_tests
[385/427] Running utility command for check-bitcoin-settings_tests
[386/427] bitcoin: testing timedata_tests
[387/427] bitcoin: testing uint256_tests
[388/427] Running utility command for check-bitcoin-timedata_tests
[389/427] Running utility command for check-bitcoin-uint256_tests
[390/427] bitcoin: testing streams_tests
[391/427] Running utility command for check-bitcoin-streams_tests
[392/427] bitcoin: testing undo_tests
[393/427] bitcoin: testing walletdb_tests
[394/427] Running utility command for check-bitcoin-undo_tests
[395/427] bitcoin: testing util_threadnames_tests
[396/427] Running utility command for check-bitcoin-walletdb_tests
[397/427] Running utility command for check-bitcoin-util_threadnames_tests
[398/427] bitcoin: testing validation_chainstatemanager_tests
[399/427] Running utility command for check-bitcoin-validation_chainstatemanager_tests
[400/427] bitcoin: testing validationinterface_tests
[401/427] bitcoin: testing txvalidationcache_tests
[402/427] Running utility command for check-bitcoin-validationinterface_tests
[403/427] Running utility command for check-bitcoin-txvalidationcache_tests
[404/427] bitcoin: testing serialize_tests
[405/427] bitcoin: testing getarg_tests
[406/427] Running utility command for check-bitcoin-serialize_tests
[407/427] Running utility command for check-bitcoin-getarg_tests
[408/427] bitcoin: testing radix_tests
[409/427] Running utility command for check-bitcoin-radix_tests
[410/427] bitcoin: testing schnorr_tests
[411/427] bitcoin: testing util_tests
[412/427] Running utility command for check-bitcoin-schnorr_tests
[413/427] Running utility command for check-bitcoin-util_tests
[414/427] bitcoin: testing crypto_tests
[415/427] Running utility command for check-bitcoin-crypto_tests
[416/427] bitcoin: testing coinselector_tests
[417/427] Running utility command for check-bitcoin-coinselector_tests
[418/427] bitcoin: testing coins_tests
[419/427] bitcoin: testing script_tests
[420/427] Running utility command for check-bitcoin-coins_tests
[421/427] Running utility command for check-bitcoin-script_tests
[422/427] bitcoin: testing skiplist_tests
[423/427] Running utility command for check-bitcoin-skiplist_tests
[424/427] bitcoin: testing op_reversebytes_tests
[425/427] Running utility command for check-bitcoin-op_reversebytes_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-clang-tidy failed with exit code 1

Tail of the build log:

wallet_encryption.py                             | ✓ Passed  | 6 s
wallet_groups.py                                 | ✓ Passed  | 47 s
wallet_hd.py                                     | ✓ Passed  | 7 s
wallet_import_rescan.py                          | ✓ Passed  | 9 s
wallet_import_with_label.py                      | ✓ Passed  | 1 s
wallet_importdescriptors.py                      | ✓ Passed  | 8 s
wallet_importmulti.py                            | ✓ Passed  | 5 s
wallet_importprunedfunds.py                      | ✓ Passed  | 1 s
wallet_keypool.py                                | ✓ Passed  | 3 s
wallet_keypool_topup.py                          | ✓ Passed  | 3 s
wallet_labels.py                                 | ✓ Passed  | 2 s
wallet_listreceivedby.py                         | ✓ Passed  | 28 s
wallet_listsinceblock.py                         | ✓ Passed  | 6 s
wallet_listtransactions.py                       | ✓ Passed  | 13 s
wallet_multiwallet.py                            | ✓ Passed  | 11 s
wallet_multiwallet.py --usecli                   | ✓ Passed  | 12 s
wallet_reorgsrestore.py                          | ✓ Passed  | 3 s
wallet_resendwallettransactions.py               | ✓ Passed  | 5 s
wallet_txn_clone.py                              | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock                  | ✓ Passed  | 3 s
wallet_txn_doublespend.py                        | ✓ Passed  | 2 s
wallet_txn_doublespend.py --mineblock            | ✓ Passed  | 4 s
wallet_watchonly.py                              | ✓ Passed  | 1 s
wallet_watchonly.py --usecli                     | ✓ Passed  | 1 s
wallet_zapwallettxes.py                          | ✓ Passed  | 5 s

ALL                                              | ✓ Passed  | 1165 s (accumulated) 
Runtime: 234 s

[173/435] Running avalanche test suite
PASSED: avalanche test suite
[185/435] Running seeder test suite
PASSED: seeder test suite
[187/435] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.005s

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

OK
[381/435] bitcoin: testing transaction_tests
FAILED: src/test/CMakeFiles/check-bitcoin-transaction_tests 
cd /work/abc-ci-builds/build-debug/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-debug/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-debug/test/log && /usr/bin/cmake -E env /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-debug/test/log/bitcoin-transaction_tests.log /work/abc-ci-builds/build-debug/src/test/test_bitcoin --run_test=transaction_tests --logger=HRF,message:JUNIT,message,bitcoin-transaction_tests.xml --catch_system_errors=no
Running 9 test cases...
../../src/test/transaction_tests.cpp(907): error: in "transaction_tests/tx_getvalueout": exception std::runtime_error expected but not raised
../../src/test/transaction_tests.cpp(913): error: in "transaction_tests/tx_getvalueout": check valid_tx.GetValueOut() == 1379 * SATOSHI has failed [42 != 1379]

*** 2 failures are detected in the test module "Bitcoin ABC unit tests"
[407/435] Running pow test suite
PASSED: pow test suite
[428/435] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[432/435] Running utility command for check-bitcoin-coinselector_tests
ninja: build stopped: cannot make progress due to previous errors.
Build build-debug failed with exit code 1

Tail of the build log:

rpc_invalidateblock.py                           | ✓ Passed  | 1 s
rpc_misc.py                                      | ✓ Passed  | 1 s
rpc_named_arguments.py                           | ✓ Passed  | 1 s
rpc_net.py                                       | ✓ Passed  | 1 s
rpc_preciousblock.py                             | ✓ Passed  | 1 s
rpc_psbt.py                                      | ✓ Passed  | 18 s
rpc_rawtransaction.py                            | ✓ Passed  | 39 s
rpc_scantxoutset.py                              | ✓ Passed  | 4 s
rpc_setban.py                                    | ✓ Passed  | 2 s
rpc_signmessage.py                               | ✓ Passed  | 1 s
rpc_signrawtransaction.py                        | ✓ Passed  | 1 s
rpc_txoutproof.py                                | ✓ Passed  | 2 s
rpc_uptime.py                                    | ✓ Passed  | 1 s
rpc_users.py                                     | ✓ Passed  | 5 s
rpc_whitelist.py                                 | ✓ Passed  | 1 s
tool_wallet.py                                   | ✓ Passed  | 5 s
wallet_abandonconflict.py                        | ✓ Passed  | 4 s
wallet_address_types.py                          | ✓ Passed  | 16 s
wallet_avoidreuse.py                             | ✓ Passed  | 4 s
wallet_backup.py                                 | ✓ Passed  | 30 s
wallet_balance.py                                | ✓ Passed  | 13 s
wallet_basic.py                                  | ✓ Passed  | 28 s
wallet_coinbase_category.py                      | ✓ Passed  | 1 s
wallet_create_tx.py                              | ✓ Passed  | 5 s
wallet_createwallet.py                           | ✓ Passed  | 2 s
wallet_createwallet.py --usecli                  | ✓ Passed  | 3 s
wallet_descriptor.py                             | ✓ Passed  | 6 s
wallet_disable.py                                | ✓ Passed  | 1 s
wallet_dump.py                                   | ✓ Passed  | 4 s
wallet_encryption.py                             | ✓ Passed  | 5 s
wallet_groups.py                                 | ✓ Passed  | 45 s
wallet_hd.py                                     | ✓ Passed  | 5 s
wallet_import_rescan.py                          | ✓ Passed  | 5 s
wallet_import_with_label.py                      | ✓ Passed  | 1 s
wallet_importdescriptors.py                      | ✓ Passed  | 4 s
wallet_importmulti.py                            | ✓ Passed  | 3 s
wallet_importprunedfunds.py                      | ✓ Passed  | 2 s
wallet_keypool.py                                | ✓ Passed  | 3 s
wallet_keypool_topup.py                          | ✓ Passed  | 3 s
wallet_labels.py                                 | ✓ Passed  | 1 s
wallet_listreceivedby.py                         | ✓ Passed  | 11 s
wallet_listsinceblock.py                         | ✓ Passed  | 3 s
wallet_listtransactions.py                       | ✓ Passed  | 21 s
wallet_multiwallet.py                            | ✓ Passed  | 12 s
wallet_multiwallet.py --usecli                   | ✓ Passed  | 13 s
wallet_reorgsrestore.py                          | ✓ Passed  | 3 s
wallet_resendwallettransactions.py               | ✓ Passed  | 6 s
wallet_txn_clone.py                              | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock                  | ✓ Passed  | 3 s
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  | 931 s (accumulated) 
Runtime: 187 s

ninja: build stopped: cannot make progress due to previous errors.
Build build-diff failed with exit code 1

Tail of the build log:

wallet_encryption.py                             | ✓ Passed  | 5 s
wallet_groups.py                                 | ✓ Passed  | 36 s
wallet_hd.py                                     | ✓ Passed  | 6 s
wallet_import_rescan.py                          | ✓ Passed  | 6 s
wallet_import_with_label.py                      | ✓ Passed  | 1 s
wallet_importdescriptors.py                      | ✓ Passed  | 5 s
wallet_importmulti.py                            | ✓ Passed  | 3 s
wallet_importprunedfunds.py                      | ✓ Passed  | 2 s
wallet_keypool.py                                | ✓ Passed  | 3 s
wallet_keypool_topup.py                          | ✓ Passed  | 3 s
wallet_labels.py                                 | ✓ Passed  | 2 s
wallet_listreceivedby.py                         | ✓ Passed  | 14 s
wallet_listsinceblock.py                         | ✓ Passed  | 4 s
wallet_listtransactions.py                       | ✓ Passed  | 20 s
wallet_multiwallet.py                            | ✓ Passed  | 11 s
wallet_multiwallet.py --usecli                   | ✓ Passed  | 13 s
wallet_reorgsrestore.py                          | ✓ Passed  | 3 s
wallet_resendwallettransactions.py               | ✓ Passed  | 10 s
wallet_txn_clone.py                              | ✓ Passed  | 2 s
wallet_txn_clone.py --mineblock                  | ✓ Passed  | 3 s
wallet_txn_doublespend.py                        | ✓ Passed  | 2 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  | 6 s

ALL                                              | ✓ Passed  | 1027 s (accumulated) 
Runtime: 206 s

[174/435] Running pow test suite
PASSED: pow test suite
[180/435] Running seeder test suite
PASSED: seeder test suite
[187/435] Test Bitcoin RPC authentication...
...
----------------------------------------------------------------------
Ran 3 tests in 0.005s

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

OK
[195/435] Running secp256k1 test suite
PASSED: secp256k1 test suite
[405/435] bitcoin: testing transaction_tests
FAILED: src/test/CMakeFiles/check-bitcoin-transaction_tests 
cd /work/abc-ci-builds/build-ubsan/src/test && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-ubsan/test/junit && /usr/bin/cmake -E make_directory /work/abc-ci-builds/build-ubsan/test/log && /usr/bin/cmake -E env UBSAN_OPTIONS=suppressions=/work/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:log_path=stdout /work/cmake/utils/log-and-print-on-failure.sh /work/abc-ci-builds/build-ubsan/test/log/bitcoin-transaction_tests.log /work/abc-ci-builds/build-ubsan/src/test/test_bitcoin --run_test=transaction_tests --logger=HRF,message:JUNIT,message,bitcoin-transaction_tests.xml --catch_system_errors=no
Running 9 test cases...
../../src/test/transaction_tests.cpp(907): error: in "transaction_tests/tx_getvalueout": exception std::runtime_error expected but not raised
../../src/test/transaction_tests.cpp(913): error: in "transaction_tests/tx_getvalueout": check valid_tx.GetValueOut() == 1379 * SATOSHI has failed [42 != 1379]

*** 2 failures are detected in the test module "Bitcoin ABC unit tests"
free(): invalid size
Aborted (core dumped)
[417/435] bitcoin: testing coinselector_tests
ninja: build stopped: subcommand failed.
Build build-ubsan failed with exit code 1
PiRK edited the test plan for this revision. (Show Details)

mtx.vout.resize(2) must be before the first test using 2 outputs

I made the mistake of moving a test just before arc diff

deadalnix requested changes to this revision.Jan 16 2021, 15:55
deadalnix added inline comments.
doc/developer-notes.md
19 ↗(On Diff #26937)

revert

This revision now requires changes to proceed.Jan 16 2021, 15:55

revert accidental change to unrelated documentation file

deadalnix requested changes to this revision.Jan 22 2021, 23:06
deadalnix added inline comments.
src/test/transaction_tests.cpp
901

These test case aren't checking much. For all you know, the function could be failing somewhere else rather on the proper values and this would still be green.

You want to check that this is okay for MAX_MONEY and not okay for MAX_MONEY + 1 * SATOSHI . The a couple of value bellow/above to check things are as expected.

This revision now requires changes to proceed.Jan 22 2021, 23:06

add tests for the case of a sum of outputs equal to MAX_MONEY, which is a valid case.

Add also a comment to better explain why the overflow test has been added.

This revision is now accepted and ready to land.Mar 30 2021, 16:17