This will make writing a robust implementation of GetMinerFundWhitelist easier
as we add another path for a policy whitelist.
Also took this opportunity to use auto where appropriate.
Details
Details
- Reviewers
PiRK Fabien - Group Reviewers
Restricted Project - Commits
- rABC94fab2062c10: Return a set instead of a vector for the miner fund whitelist
ninja check-functional
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- minerfund-set
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 21915 Build 43465: Build Diff lint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang Build 43464: arc lint + arc unit
Event Timeline
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-clang) failed.
Tail of the build log:
[178/480] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/__/__/wallet/test/wallet_test_fixture.cpp.o [179/480] pow: testing aserti32d_tests [180/480] Running utility command for check-pow-aserti32d_tests [181/480] Running pow test suite PASSED: pow test suite [182/480] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/validationinterface_tests.cpp.o [183/480] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/versionbits_tests.cpp.o [184/480] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/validation_tests.cpp.o [185/480] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/message_writer_tests.cpp.o [186/480] Linking CXX executable src/seeder/test/test-seeder [187/480] seeder: testing message_writer_tests [188/480] seeder: testing p2p_messaging_tests [189/480] seeder: testing parse_name_tests [190/480] seeder: testing options_tests [191/480] Running utility command for check-seeder-message_writer_tests [192/480] Running utility command for check-seeder-p2p_messaging_tests [193/480] Running utility command for check-seeder-parse_name_tests [194/480] seeder: testing write_name_tests [195/480] Running utility command for check-seeder-options_tests [196/480] Running utility command for check-seeder-write_name_tests [197/480] Running seeder test suite PASSED: seeder test suite [198/480] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/apptests.cpp.o [199/480] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_main.cpp.o [200/480] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/rpcnestedtests.cpp.o [201/480] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o [202/480] Building CXX object src/avalanche/test/CMakeFiles/test-avalanche.dir/processor_tests.cpp.o [203/480] Linking CXX executable src/avalanche/test/test-avalanche [204/480] avalanche: testing init_tests [205/480] Running utility command for check-avalanche-init_tests [206/480] avalanche: testing delegation_tests [207/480] Running utility command for check-avalanche-delegation_tests [208/480] avalanche: testing proofcomparator_tests [209/480] Running utility command for check-avalanche-proofcomparator_tests [210/480] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/__/wallet/test/wallet_tests.cpp.o [211/480] avalanche: testing proofpool_tests [212/480] Running utility command for check-avalanche-proofpool_tests [213/480] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o [214/480] avalanche: testing proof_tests [215/480] Running utility command for check-avalanche-proof_tests [216/480] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o [217/480] avalanche: testing compactproofs_tests [218/480] Running utility command for check-avalanche-compactproofs_tests [219/480] Linking CXX executable src/qt/test/test_bitcoin-qt [220/480] bitcoin-qt: testing test_bitcoin-qt [221/480] Running bitcoin-qt test suite PASSED: bitcoin-qt test suite [222/480] avalanche: testing processor_tests [223/480] Running utility command for check-avalanche-processor_tests [224/480] avalanche: testing voterecord_tests [225/480] Running utility command for check-avalanche-voterecord_tests [226/480] secp256k1: testing secp256k1-tests [227/480] Running secp256k1 test suite PASSED: secp256k1 test suite [228/480] avalanche: testing peermanager_tests [229/480] Running utility command for check-avalanche-peermanager_tests [230/480] Running avalanche test suite PASSED: avalanche test suite ninja: build stopped: cannot make progress due to previous errors. Build build-clang failed with exit code 1
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-clang-tidy) failed.
Tail of the build log:
[170/473] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/daa_tests.cpp.o [171/473] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/options_tests.cpp.o [172/473] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/write_name_tests.cpp.o [173/473] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/parse_name_tests.cpp.o [174/473] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/eda_tests.cpp.o [175/473] Automatic MOC for target test_bitcoin-qt [176/473] Building CXX object src/pow/test/CMakeFiles/test-pow.dir/grasberg_tests.cpp.o [177/473] avalanche: testing processor_tests [178/473] Running utility command for check-avalanche-processor_tests [179/473] Linking CXX executable src/pow/test/test-pow [180/473] avalanche: testing peermanager_tests [181/473] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/p2p_messaging_tests.cpp.o [182/473] Test Bitcoin utilities... [183/473] pow: testing daa_tests [184/473] Running utility command for check-avalanche-peermanager_tests [185/473] Running utility command for check-pow-daa_tests [186/473] Running avalanche test suite PASSED: avalanche test suite [187/473] pow: testing eda_tests [188/473] Running utility command for check-pow-eda_tests [189/473] pow: testing grasberg_tests [190/473] Running utility command for check-pow-grasberg_tests [191/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/bitcoinaddressvalidatortests.cpp.o [192/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/util.cpp.o [193/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/compattests.cpp.o [194/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/guiutiltests.cpp.o [195/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_bitcoin-qt_autogen/mocs_compilation.cpp.o [196/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/uritests.cpp.o [197/473] pow: testing aserti32d_tests [198/473] Running utility command for check-pow-aserti32d_tests [199/473] Running pow test suite PASSED: pow test suite [200/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/__/__/wallet/test/wallet_test_fixture.cpp.o [201/473] Building CXX object src/seeder/test/CMakeFiles/test-seeder.dir/message_writer_tests.cpp.o [202/473] Linking CXX executable src/seeder/test/test-seeder [203/473] seeder: testing message_writer_tests [204/473] seeder: testing options_tests [205/473] seeder: testing p2p_messaging_tests [206/473] seeder: testing parse_name_tests [207/473] Running utility command for check-seeder-message_writer_tests [208/473] Running utility command for check-seeder-options_tests [209/473] seeder: testing write_name_tests [210/473] Running utility command for check-seeder-p2p_messaging_tests [211/473] Running utility command for check-seeder-parse_name_tests [212/473] Running utility command for check-seeder-write_name_tests [213/473] Running seeder test suite PASSED: seeder test suite [214/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/test_main.cpp.o [215/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/rpcnestedtests.cpp.o [216/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/apptests.cpp.o [217/473] Building CXX object src/test/CMakeFiles/test_bitcoin.dir/__/wallet/test/wallet_tests.cpp.o [218/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/paymentservertests.cpp.o [219/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/addressbooktests.cpp.o [220/473] Building CXX object src/qt/test/CMakeFiles/test_bitcoin-qt.dir/wallettests.cpp.o [221/473] Linking CXX executable src/qt/test/test_bitcoin-qt [222/473] bitcoin-qt: testing test_bitcoin-qt [223/473] Running bitcoin-qt test suite PASSED: bitcoin-qt test suite ninja: build stopped: cannot make progress due to previous errors. Build build-clang-tidy failed with exit code 1
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-without-wallet) failed.
Tail of the build log:
from ../../src/test/minerfund_tests.cpp:5:
/usr/include/c++/9/bits/stl_iterator.h:886:5: note: candidate: ‘template<class _IteratorL, class _IteratorR, class _Container> bool __gnu_cxx::operator==(const __gnu_cxx::__normal_iterator<_IteratorL, _Container>&, const __gnu_cxx::__normal_iterator<_IteratorR, _Container>&)’
886 | operator==(const __normal_iterator<_IteratorL, _Container>& __lhs,
| ^~~~~~~~
/usr/include/c++/9/bits/stl_iterator.h:886:5: note: template argument deduction/substitution failed:
In file included from /usr/include/boost/test/test_tools.hpp:45,
from /usr/include/boost/test/unit_test.hpp:18,
from ../../src/test/minerfund_tests.cpp:13:
../../src/test/minerfund_tests.cpp:46:17: note: ‘std::set<boost::variant<CNoDestination, PKHash, ScriptHash> >’ is not derived from ‘const __gnu_cxx::__normal_iterator<_IteratorL, _Container>’
46 | expectedMinerFund);
| ^~~~~~~~~~~~~~~~~
In file included from /usr/include/c++/9/bits/stl_algobase.h:67,
from /usr/include/c++/9/algorithm:61,
from ../../src/./prevector.h:8,
from ../../src/./serialize.h:10,
from ../../src/./consensus/amount.h:10,
from ../../src/./minerfund.h:8,
from ../../src/test/minerfund_tests.cpp:5:
/usr/include/c++/9/bits/stl_iterator.h:893:5: note: candidate: ‘template<class _Iterator, class _Container> bool __gnu_cxx::operator==(const __gnu_cxx::__normal_iterator<_Iterator, _Container>&, const __gnu_cxx::__normal_iterator<_Iterator, _Container>&)’
893 | operator==(const __normal_iterator<_Iterator, _Container>& __lhs,
| ^~~~~~~~
/usr/include/c++/9/bits/stl_iterator.h:893:5: note: template argument deduction/substitution failed:
In file included from /usr/include/boost/test/test_tools.hpp:45,
from /usr/include/boost/test/unit_test.hpp:18,
from ../../src/test/minerfund_tests.cpp:13:
../../src/test/minerfund_tests.cpp:46:17: note: ‘std::set<boost::variant<CNoDestination, PKHash, ScriptHash> >’ is not derived from ‘const __gnu_cxx::__normal_iterator<_Iterator, _Container>’
46 | expectedMinerFund);
| ^~~~~~~~~~~~~~~~~
In file included from /usr/include/x86_64-linux-gnu/c++/9/bits/c++allocator.h:33,
from /usr/include/c++/9/bits/allocator.h:46,
from /usr/include/c++/9/string:41,
from /usr/include/c++/9/stdexcept:39,
from /usr/include/c++/9/array:39,
from /usr/include/c++/9/tuple:39,
from /usr/include/c++/9/functional:54,
from /usr/include/c++/9/pstl/glue_algorithm_defs.h:13,
from /usr/include/c++/9/algorithm:71,
from ../../src/./prevector.h:8,
from ../../src/./serialize.h:10,
from ../../src/./consensus/amount.h:10,
from ../../src/./minerfund.h:8,
from ../../src/test/minerfund_tests.cpp:5:
/usr/include/c++/9/ext/new_allocator.h:166:2: note: candidate: ‘template<class _Up> bool __gnu_cxx::operator==(const __gnu_cxx::new_allocator<boost::variant<CNoDestination, PKHash, ScriptHash> >&, const __gnu_cxx::new_allocator<_Tp>&)’
166 | operator==(const new_allocator&, const new_allocator<_Up>&)
| ^~~~~~~~
/usr/include/c++/9/ext/new_allocator.h:166:2: note: template argument deduction/substitution failed:
In file included from /usr/include/boost/test/test_tools.hpp:45,
from /usr/include/boost/test/unit_test.hpp:18,
from ../../src/test/minerfund_tests.cpp:13:
../../src/test/minerfund_tests.cpp:46:17: note: ‘const std::vector<boost::variant<CNoDestination, PKHash, ScriptHash> >’ is not derived from ‘const __gnu_cxx::new_allocator<_Tp>’
46 | expectedMinerFund);
| ^~~~~~~~~~~~~~~~~
[198/441] Running bitcoin-qt test suite
PASSED: bitcoin-qt test suite
[203/441] Running pow test suite
PASSED: pow test suite
[208/441] Running avalanche test suite
PASSED: avalanche test suite
ninja: build stopped: cannot make progress due to previous errors.
Build build-without-wallet failed with exit code 1Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-debug) failed.
Tail of the build log:
[0m[0;32mwallet_import_with_label.py | ✓ Passed | 1 s [0m[0;32mwallet_importdescriptors.py | ✓ Passed | 7 s [0m[0;32mwallet_importmulti.py | ✓ Passed | 5 s [0m[0;32mwallet_importprunedfunds.py | ✓ Passed | 5 s [0m[0;32mwallet_importprunedfunds.py --descriptors | ✓ Passed | 4 s [0m[0;32mwallet_keypool.py | ✓ Passed | 3 s [0m[0;32mwallet_keypool_topup.py | ✓ Passed | 7 s [0m[0;32mwallet_keypool_topup.py --descriptors | ✓ Passed | 9 s [0m[0;32mwallet_labels.py | ✓ Passed | 2 s [0m[0;32mwallet_labels.py --descriptors | ✓ Passed | 2 s [0m[0;32mwallet_listreceivedby.py | ✓ Passed | 9 s [0m[0;32mwallet_listsinceblock.py | ✓ Passed | 9 s [0m[0;32mwallet_listsinceblock.py --descriptors | ✓ Passed | 11 s [0m[0;32mwallet_listtransactions.py | ✓ Passed | 6 s [0m[0;32mwallet_listtransactions.py --descriptors | ✓ Passed | 5 s [0m[0;32mwallet_multiwallet.py | ✓ Passed | 50 s [0m[0;32mwallet_multiwallet.py --usecli | ✓ Passed | 15 s [0m[0;32mwallet_reorgsrestore.py | ✓ Passed | 4 s [0m[0;32mwallet_resendwallettransactions.py | ✓ Passed | 4 s [0m[0;32mwallet_send.py | ✓ Passed | 8 s [0m[0;32mwallet_startup.py | ✓ Passed | 3 s [0m[0;32mwallet_timelock.py | ✓ Passed | 1 s [0m[0;32mwallet_txn_clone.py | ✓ Passed | 3 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 | 3 s [0m[0;32mwallet_watchonly.py | ✓ Passed | 1 s [0m[0;32mwallet_watchonly.py --usecli | ✓ Passed | 1 s [0m[1;30mchronik_serve.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 | 2044 s (accumulated) [0mRuntime: 409 s [160/481] Test Bitcoin RPC authentication... ... ---------------------------------------------------------------------- Ran 3 tests in 0.006s OK [161/481] cd /work/contrib/devtools/chainparams && /usr/bin/python3.9 ./test_make_chainparams.py ..... ---------------------------------------------------------------------- Ran 5 tests in 0.001s OK [217/481] Running seeder test suite PASSED: seeder test suite [222/481] Running pow test suite PASSED: pow test suite [225/481] Running avalanche test suite PASSED: avalanche test suite [228/481] Running bitcoin-qt test suite PASSED: bitcoin-qt test suite [230/481] Running secp256k1 test suite PASSED: secp256k1 test suite ninja: build stopped: cannot make progress due to previous errors. Build build-debug failed with exit code 1
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (build-diff) failed.
Tail of the build log:
[0m[0;32mwallet_address_types.py | ✓ Passed | 12 s [0m[0;32mwallet_address_types.py --descriptors | ✓ Passed | 7 s [0m[0;32mwallet_avoidreuse.py | ✓ Passed | 4 s [0m[0;32mwallet_avoidreuse.py --descriptors | ✓ Passed | 4 s [0m[0;32mwallet_backup.py | ✓ Passed | 17 s [0m[0;32mwallet_balance.py | ✓ Passed | 4 s [0m[0;32mwallet_balance.py --descriptors | ✓ Passed | 7 s [0m[0;32mwallet_basic.py | ✓ Passed | 14 s [0m[0;32mwallet_coinbase_category.py | ✓ Passed | 1 s [0m[0;32mwallet_create_tx.py | ✓ Passed | 5 s [0m[0;32mwallet_createwallet.py | ✓ Passed | 2 s [0m[0;32mwallet_createwallet.py --descriptors | ✓ Passed | 2 s [0m[0;32mwallet_createwallet.py --usecli | ✓ Passed | 3 s [0m[0;32mwallet_descriptor.py | ✓ Passed | 7 s [0m[0;32mwallet_disable.py | ✓ Passed | 0 s [0m[0;32mwallet_dump.py | ✓ Passed | 4 s [0m[0;32mwallet_encryption.py | ✓ Passed | 5 s [0m[0;32mwallet_encryption.py --descriptors | ✓ Passed | 5 s [0m[0;32mwallet_groups.py | ✓ Passed | 13 s [0m[0;32mwallet_hd.py | ✓ Passed | 7 s [0m[0;32mwallet_hd.py --descriptors | ✓ Passed | 6 s [0m[0;32mwallet_import_rescan.py | ✓ Passed | 7 s [0m[0;32mwallet_import_with_label.py | ✓ Passed | 1 s [0m[0;32mwallet_importdescriptors.py | ✓ Passed | 6 s [0m[0;32mwallet_importmulti.py | ✓ Passed | 3 s [0m[0;32mwallet_importprunedfunds.py | ✓ Passed | 2 s [0m[0;32mwallet_importprunedfunds.py --descriptors | ✓ Passed | 2 s [0m[0;32mwallet_keypool.py | ✓ Passed | 3 s [0m[0;32mwallet_keypool_topup.py | ✓ Passed | 4 s [0m[0;32mwallet_keypool_topup.py --descriptors | ✓ Passed | 5 s [0m[0;32mwallet_labels.py | ✓ Passed | 1 s [0m[0;32mwallet_labels.py --descriptors | ✓ Passed | 1 s [0m[0;32mwallet_listreceivedby.py | ✓ Passed | 5 s [0m[0;32mwallet_listsinceblock.py | ✓ Passed | 6 s [0m[0;32mwallet_listsinceblock.py --descriptors | ✓ Passed | 7 s [0m[0;32mwallet_listtransactions.py | ✓ Passed | 4 s [0m[0;32mwallet_listtransactions.py --descriptors | ✓ Passed | 4 s [0m[0;32mwallet_multiwallet.py | ✓ Passed | 38 s [0m[0;32mwallet_multiwallet.py --usecli | ✓ Passed | 9 s [0m[0;32mwallet_reorgsrestore.py | ✓ Passed | 3 s [0m[0;32mwallet_resendwallettransactions.py | ✓ Passed | 13 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 | 1 s [0m[0;32mwallet_txn_doublespend.py --mineblock | ✓ Passed | 3 s [0m[0;32mwallet_watchonly.py | ✓ Passed | 1 s [0m[0;32mwallet_watchonly.py --usecli | ✓ Passed | 1 s [0m[1;30mchronik_serve.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 | 1198 s (accumulated) [0mRuntime: 248 s ninja: build stopped: cannot make progress due to previous errors. Build build-diff failed with exit code 1
Comment Actions
If ordering matters then it should have a comparator and appropriated rationale/tests
Comment Actions
Ordering does not matter at this time. If it did, then the code is broken already. It appears to be designed this way so that the API is more clear if multiple addresses are introduced later.
Comment Actions
So you want an unordered_set, and an appropriated hash function to check for equality
| src/test/hasher_tests.cpp | ||
|---|---|---|
| 29 ↗ | (On Diff #37843) | The test is missing the main part: checking that 2 identical CTxDestination return the same hash (with the same hasher). |
| src/test/hasher_tests.cpp | ||
|---|---|---|
| 25–28 ↗ | (On Diff #37849) | You're not testing what you want here, see suggestion |