From 45be32f8384398ad8d590137d05a6373aa827b75 Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 4 Sep 2024 21:46:31 +0200 Subject: [PATCH 01/17] build: Produce a usable static kernel library Since the move to cmake, the kernel static library that is installed after a cmake --install build is unusable. It lacks symbols for the internal libraries, besides those defined in the kernel library target. This is because cmake, unlike the libtool archiver, does not combine multiple static libraries into a single static library on installation. This is likely an intentional design choice, since there were a bunch of caveats to the way libtool calculated these libraries. Fix this problem by installing all the required libraries. The user must then link all of them along with the bitcoin kernel library. --- src/kernel/CMakeLists.txt | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/kernel/CMakeLists.txt b/src/kernel/CMakeLists.txt index ffb1a857ac17e..a7475feb7f051 100644 --- a/src/kernel/CMakeLists.txt +++ b/src/kernel/CMakeLists.txt @@ -98,6 +98,32 @@ set_target_properties(bitcoinkernel PROPERTIES CXX_VISIBILITY_PRESET default ) +# When building the static library, install all static libraries the +# bitcoinkernel depends on. +if(NOT BUILD_SHARED_LIBS) + # Recursively get all the static libraries a target depends on and put them in libs_out + function(get_target_static_link_libs target libs_out) + get_target_property(linked_libraries ${target} LINK_LIBRARIES) + foreach(dep ${linked_libraries}) + if(TARGET ${dep}) + get_target_property(dep_type ${dep} TYPE) + if(dep_type STREQUAL "STATIC_LIBRARY") + list(APPEND ${libs_out} ${dep}) + get_target_static_link_libs(${dep} ${libs_out}) + endif() + endif() + endforeach() + set(${libs_out} ${${libs_out}} PARENT_SCOPE) + endfunction() + + set(all_kernel_static_link_libs "") + get_target_static_link_libs(bitcoinkernel all_kernel_static_link_libs) + + foreach(lib ${all_kernel_static_link_libs}) + install(TARGETS ${lib} ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}) + endforeach() +endif() + include(GNUInstallDirs) install(TARGETS bitcoinkernel RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} From 0dd16d7118f10ac291a45644769121cbdfd2352f Mon Sep 17 00:00:00 2001 From: TheCharlatan Date: Wed, 4 Sep 2024 22:02:34 +0200 Subject: [PATCH 02/17] build: Add a pkg-config file for libbitcoinkernel --- libbitcoinkernel.pc.in | 11 +++++++++++ src/kernel/CMakeLists.txt | 8 ++++++++ 2 files changed, 19 insertions(+) create mode 100644 libbitcoinkernel.pc.in diff --git a/libbitcoinkernel.pc.in b/libbitcoinkernel.pc.in new file mode 100644 index 0000000000000..a2cb7d3692e97 --- /dev/null +++ b/libbitcoinkernel.pc.in @@ -0,0 +1,11 @@ +prefix=@CMAKE_INSTALL_PREFIX@ +exec_prefix=${prefix} +libdir=${prefix}/@CMAKE_INSTALL_LIBDIR@ +includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@ + +Name: @PACKAGE_NAME@ kernel library +Description: Experimental library for the Bitcoin Core validation engine. +Version: @PACKAGE_VERSION@ +Libs: -L${libdir} -lbitcoinkernel +Libs.private: -L${libdir} @LIBS_PRIVATE@ +Cflags: -I${includedir} diff --git a/src/kernel/CMakeLists.txt b/src/kernel/CMakeLists.txt index a7475feb7f051..5b62bba1c013d 100644 --- a/src/kernel/CMakeLists.txt +++ b/src/kernel/CMakeLists.txt @@ -119,11 +119,19 @@ if(NOT BUILD_SHARED_LIBS) set(all_kernel_static_link_libs "") get_target_static_link_libs(bitcoinkernel all_kernel_static_link_libs) + # LIBS_PRIVATE is substituted in the pkg-config file. + set(LIBS_PRIVATE "") foreach(lib ${all_kernel_static_link_libs}) install(TARGETS ${lib} ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}) + string(APPEND LIBS_PRIVATE " -l${lib}") endforeach() + + string(STRIP "${LIBS_PRIVATE}" LIBS_PRIVATE) endif() +configure_file(${PROJECT_SOURCE_DIR}/libbitcoinkernel.pc.in ${PROJECT_BINARY_DIR}/libbitcoinkernel.pc @ONLY) +install(FILES ${PROJECT_BINARY_DIR}/libbitcoinkernel.pc DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig") + include(GNUInstallDirs) install(TARGETS bitcoinkernel RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} From 743ac30e349e181c26a2d2af0bcb93b0835ce521 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 1 Sep 2024 15:09:42 +0200 Subject: [PATCH 03/17] Add std::optional support to Boost's equality check Also moved the operators to the bottom of the file since they're less important and to group them together. Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com> Co-authored-by: stickies-v --- src/test/util/setup_common.cpp | 33 +++++++++++++++------------------ src/test/util/setup_common.h | 31 +++++++++++++++++++++---------- 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 10363f2247312..fa89ceb332af2 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -78,24 +78,6 @@ constexpr inline auto TEST_DIR_PATH_ELEMENT{"test_common bitcoin"}; // Includes /** Random context to get unique temp data dirs. Separate from m_rng, which can be seeded from a const env var */ static FastRandomContext g_rng_temp_path; -std::ostream& operator<<(std::ostream& os, const arith_uint256& num) -{ - os << num.ToString(); - return os; -} - -std::ostream& operator<<(std::ostream& os, const uint160& num) -{ - os << num.ToString(); - return os; -} - -std::ostream& operator<<(std::ostream& os, const uint256& num) -{ - os << num.ToString(); - return os; -} - struct NetworkSetup { NetworkSetup() @@ -606,3 +588,18 @@ CBlock getBlock13b8a() stream >> TX_WITH_WITNESS(block); return block; } + +std::ostream& operator<<(std::ostream& os, const arith_uint256& num) +{ + return os << num.ToString(); +} + +std::ostream& operator<<(std::ostream& os, const uint160& num) +{ + return os << num.ToString(); +} + +std::ostream& operator<<(std::ostream& os, const uint256& num) +{ + return os << num.ToString(); +} diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index a9a890b1a56d1..30d4280fa53b8 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -10,6 +10,8 @@ #include #include #include // IWYU pragma: export +#include +#include #include #include #include @@ -29,6 +31,8 @@ class arith_uint256; class CFeeRate; class Chainstate; class FastRandomContext; +class uint160; +class uint256; /** This is connected to the logger. Can be used to redirect logs to any other log */ extern const std::function G_TEST_LOG_FUN; @@ -39,15 +43,6 @@ extern const std::function()> G_TEST_COMMAND_LINE_ARGUM /** Retrieve the unit test name. */ extern const std::function G_TEST_GET_FULL_NAME; -// Enable BOOST_CHECK_EQUAL for enum class types -namespace std { -template -std::ostream& operator<<(typename std::enable_if::value, std::ostream>::type& stream, const T& e) -{ - return stream << static_cast::type>(e); -} -} // namespace std - static constexpr CAmount CENT{1000000}; struct TestOpts { @@ -250,10 +245,26 @@ std::unique_ptr MakeNoLogFileContext(const ChainType chain_type = ChainType:: CBlock getBlock13b8a(); -// Make types usable in BOOST_CHECK_* +// Make types usable in BOOST_CHECK_* @{ +namespace std { +template requires std::is_enum_v +inline std::ostream& operator<<(std::ostream& os, const T& e) +{ + return os << static_cast>(e); +} + +template +inline std::ostream& operator<<(std::ostream& os, const std::optional& v) +{ + return v ? os << *v + : os << "std::nullopt"; +} +} // namespace std + std::ostream& operator<<(std::ostream& os, const arith_uint256& num); std::ostream& operator<<(std::ostream& os, const uint160& num); std::ostream& operator<<(std::ostream& os, const uint256& num); +// @} /** * BOOST_CHECK_EXCEPTION predicates to check the specific validation error. From 19947863e16425e6a119e7a4819867292b1235ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 1 Sep 2024 15:09:48 +0200 Subject: [PATCH 04/17] Use BOOST_CHECK_EQUAL for optional, arith_uint256, uint256, uint160 Example error before: > unknown location:0: fatal error: in "validation_chainstatemanager_tests/chainstatemanager_args": std::bad_optional_access: bad_optional_access test/validation_chainstatemanager_tests.cpp:781: last checkpoint after: > test/validation_chainstatemanager_tests.cpp:801: error: in "validation_chainstatemanager_tests/chainstatemanager_args": check set_opts({"-assumevalid=0"}).assumed_valid_block == uint256::ZERO has failed [std::nullopt != 0000000000000000000000000000000000000000000000000000000000000000] Also added extra minimum_chainwork test to make it symmetric with assumevalid Co-authored-by: Ryan Ofsky Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com> --- src/test/uint256_tests.cpp | 20 +++++++------- .../validation_chainstatemanager_tests.cpp | 26 +++++++++++-------- 2 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp index 8b76e0865a153..ef534b1b78f7c 100644 --- a/src/test/uint256_tests.cpp +++ b/src/test/uint256_tests.cpp @@ -395,15 +395,15 @@ BOOST_AUTO_TEST_CASE(from_hex) BOOST_AUTO_TEST_CASE(from_user_hex) { - BOOST_CHECK_EQUAL(uint256::FromUserHex("").value(), uint256::ZERO); - BOOST_CHECK_EQUAL(uint256::FromUserHex("0x").value(), uint256::ZERO); - BOOST_CHECK_EQUAL(uint256::FromUserHex("0").value(), uint256::ZERO); - BOOST_CHECK_EQUAL(uint256::FromUserHex("00").value(), uint256::ZERO); - BOOST_CHECK_EQUAL(uint256::FromUserHex("1").value(), uint256::ONE); - BOOST_CHECK_EQUAL(uint256::FromUserHex("0x10").value(), uint256{0x10}); - BOOST_CHECK_EQUAL(uint256::FromUserHex("10").value(), uint256{0x10}); - BOOST_CHECK_EQUAL(uint256::FromUserHex("0xFf").value(), uint256{0xff}); - BOOST_CHECK_EQUAL(uint256::FromUserHex("Ff").value(), uint256{0xff}); + BOOST_CHECK_EQUAL(uint256::FromUserHex(""), uint256::ZERO); + BOOST_CHECK_EQUAL(uint256::FromUserHex("0x"), uint256::ZERO); + BOOST_CHECK_EQUAL(uint256::FromUserHex("0"), uint256::ZERO); + BOOST_CHECK_EQUAL(uint256::FromUserHex("00"), uint256::ZERO); + BOOST_CHECK_EQUAL(uint256::FromUserHex("1"), uint256::ONE); + BOOST_CHECK_EQUAL(uint256::FromUserHex("0x10"), uint256{0x10}); + BOOST_CHECK_EQUAL(uint256::FromUserHex("10"), uint256{0x10}); + BOOST_CHECK_EQUAL(uint256::FromUserHex("0xFf"), uint256{0xff}); + BOOST_CHECK_EQUAL(uint256::FromUserHex("Ff"), uint256{0xff}); const std::string valid_hex_64{"0x0123456789abcdef0123456789abcdef0123456789ABDCEF0123456789ABCDEF"}; BOOST_REQUIRE_EQUAL(valid_hex_64.size(), 2 + 64); // 0x prefix and 64 hex digits BOOST_CHECK_EQUAL(uint256::FromUserHex(valid_hex_64.substr(2)).value().ToString(), ToLower(valid_hex_64.substr(2))); @@ -430,7 +430,7 @@ BOOST_AUTO_TEST_CASE( check_ONE ) BOOST_AUTO_TEST_CASE(FromHex_vs_uint256) { - auto runtime_uint{uint256::FromHex("4A5E1E4BAAB89F3A32518A88C31BC87F618f76673e2cc77ab2127b7afdeda33b").value()}; + auto runtime_uint{uint256::FromHex("4A5E1E4BAAB89F3A32518A88C31BC87F618f76673e2cc77ab2127b7afdeda33b")}; constexpr uint256 consteval_uint{ "4a5e1e4baab89f3a32518a88c31bc87f618f76673e2cc77ab2127b7afdeda33b"}; BOOST_CHECK_EQUAL(consteval_uint, runtime_uint); } diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index b07c70e1070c6..b4fcfbd85379c 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -806,22 +806,26 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_args, BasicTestingSetup) }; // test -assumevalid - BOOST_CHECK(!get_valid_opts({}).assumed_valid_block.has_value()); - BOOST_CHECK(get_valid_opts({"-assumevalid="}).assumed_valid_block.value().IsNull()); - BOOST_CHECK(get_valid_opts({"-assumevalid=0"}).assumed_valid_block.value().IsNull()); - BOOST_CHECK(get_valid_opts({"-noassumevalid"}).assumed_valid_block.value().IsNull()); - BOOST_CHECK_EQUAL(get_valid_opts({"-assumevalid=0x1234"}).assumed_valid_block.value().ToString(), std::string(60, '0') + "1234"); - const std::string cmd{"-assumevalid=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"}; - BOOST_CHECK_EQUAL(get_valid_opts({cmd.c_str()}).assumed_valid_block.value().ToString(), cmd.substr(13, cmd.size())); + BOOST_CHECK(!get_valid_opts({}).assumed_valid_block); + BOOST_CHECK_EQUAL(get_valid_opts({"-assumevalid="}).assumed_valid_block, uint256::ZERO); + BOOST_CHECK_EQUAL(get_valid_opts({"-assumevalid=0"}).assumed_valid_block, uint256::ZERO); + BOOST_CHECK_EQUAL(get_valid_opts({"-noassumevalid"}).assumed_valid_block, uint256::ZERO); + BOOST_CHECK_EQUAL(get_valid_opts({"-assumevalid=0x12"}).assumed_valid_block, uint256{0x12}); + + std::string assume_valid{"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"}; + BOOST_CHECK_EQUAL(get_valid_opts({("-assumevalid=" + assume_valid).c_str()}).assumed_valid_block, uint256::FromHex(assume_valid)); BOOST_CHECK(!get_opts({"-assumevalid=xyz"})); // invalid hex characters BOOST_CHECK(!get_opts({"-assumevalid=01234567890123456789012345678901234567890123456789012345678901234"})); // > 64 hex chars // test -minimumchainwork - BOOST_CHECK(!get_valid_opts({}).minimum_chain_work.has_value()); - BOOST_CHECK_EQUAL(get_valid_opts({"-minimumchainwork=0"}).minimum_chain_work.value().GetCompact(), 0U); - BOOST_CHECK_EQUAL(get_valid_opts({"-nominimumchainwork"}).minimum_chain_work.value().GetCompact(), 0U); - BOOST_CHECK_EQUAL(get_valid_opts({"-minimumchainwork=0x1234"}).minimum_chain_work.value().GetCompact(), 0x02123400U); + BOOST_CHECK(!get_valid_opts({}).minimum_chain_work); + BOOST_CHECK_EQUAL(get_valid_opts({"-minimumchainwork=0"}).minimum_chain_work, arith_uint256()); + BOOST_CHECK_EQUAL(get_valid_opts({"-nominimumchainwork"}).minimum_chain_work, arith_uint256()); + BOOST_CHECK_EQUAL(get_valid_opts({"-minimumchainwork=0x1234"}).minimum_chain_work, arith_uint256{0x1234}); + + std::string minimum_chainwork{"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"}; + BOOST_CHECK_EQUAL(get_valid_opts({("-minimumchainwork=" + minimum_chainwork).c_str()}).minimum_chain_work, UintToArith256(uint256::FromHex(minimum_chainwork).value())); BOOST_CHECK(!get_opts({"-minimumchainwork=xyz"})); // invalid hex characters BOOST_CHECK(!get_opts({"-minimumchainwork=01234567890123456789012345678901234567890123456789012345678901234"})); // > 64 hex chars From 1eac96a503b6bac3eaf5d0eb3d23ffde3bfbf9aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Wed, 28 Aug 2024 23:19:21 +0200 Subject: [PATCH 05/17] Compare FromUserHex result against other hex validators and parsers --- src/test/fuzz/hex.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/test/fuzz/hex.cpp b/src/test/fuzz/hex.cpp index 964e30cc7e8d0..70c5690d8e433 100644 --- a/src/test/fuzz/hex.cpp +++ b/src/test/fuzz/hex.cpp @@ -35,7 +35,13 @@ FUZZ_TARGET(hex) assert(uint256::FromUserHex(random_hex_string)); } if (const auto result{uint256::FromUserHex(random_hex_string)}) { - assert(uint256::FromHex(result->ToString())); + const auto result_string{result->ToString()}; // ToString() returns a fixed-length string without "0x" prefix + assert(result_string.length() == 64); + assert(IsHex(result_string)); + assert(TryParseHex(result_string)); + assert(Txid::FromHex(result_string)); + assert(Wtxid::FromHex(result_string)); + assert(uint256::FromHex(result_string)); } (void)uint256S(random_hex_string); try { From fae7b83eb58d22ed83878561603991131372cdd7 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 10 Sep 2024 21:55:23 +0200 Subject: [PATCH 06/17] lint: Remove forbidden functions from lint-format-strings.py Given that all of them are forbidden by the test/lint/lint-locale-dependence.py check, they can be removed. --- test/lint/lint-format-strings.py | 8 -------- test/lint/run-lint-format-strings.py | 1 - 2 files changed, 9 deletions(-) diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index 9f5e0f312ecb8..9c9fbb2e392fd 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -17,7 +17,6 @@ FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS = [ 'FatalErrorf,0', - 'fprintf,1', 'tfm::format,1', # Assuming tfm::::format(std::ostream&, ... 'LogConnectFailure,1', 'LogError,0', @@ -27,14 +26,7 @@ 'LogTrace,1', 'LogPrintf,0', 'LogPrintLevel,2', - 'printf,0', - 'snprintf,2', - 'sprintf,1', 'strprintf,0', - 'vfprintf,1', - 'vprintf,1', - 'vsnprintf,1', - 'vsprintf,1', 'WalletLogPrintf,0', ] RUN_LINT_FILE = 'test/lint/run-lint-format-strings.py' diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index 09a2503452e5e..9a7386ee64ad1 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -13,7 +13,6 @@ import sys FALSE_POSITIVES = [ - ("src/dbwrapper.cpp", "vsnprintf(p, limit - p, format, backup_ap)"), ("src/index/base.cpp", "FatalErrorf(const char* fmt, const Args&... args)"), ("src/index/base.h", "FatalErrorf(const char* fmt, const Args&... args)"), ("src/netbase.cpp", "LogConnectFailure(bool manual_connection, const char* fmt, const Args&... args)"), From 72b46f28bf8d37a166961b5aa2a22302b932b756 Mon Sep 17 00:00:00 2001 From: Max Edwards Date: Wed, 11 Sep 2024 18:27:49 +0100 Subject: [PATCH 07/17] test: fix exclude parsing for functional runner This restores previous behaviour of being able to exclude a test by name without having to specify .py extension. --- test/functional/test_runner.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index bee962270937a..aad2af69df2cf 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -522,23 +522,24 @@ def main(): test_list += BASE_SCRIPTS # Remove the test cases that the user has explicitly asked to exclude. + # The user can specify a test case with or without the .py extension. if args.exclude: def print_warning_missing_test(test_name): print("{}WARNING!{} Test '{}' not found in current test list.".format(BOLD[1], BOLD[0], test_name)) + def remove_tests(exclude_list): + if not exclude_list: + print_warning_missing_test(exclude_test) + for exclude_item in exclude_list: + test_list.remove(exclude_item) + exclude_tests = [test.strip() for test in args.exclude.split(",")] for exclude_test in exclude_tests: - if exclude_test.endswith('.py'): - # Remove .py and .py --arg from the test list - exclude_list = [test for test in test_list if test.split('.py')[0] == exclude_test.split('.py')[0]] - if not exclude_list: - print_warning_missing_test(exclude_test) - for exclude_item in exclude_list: - test_list.remove(exclude_item) + # A space in the name indicates it has arguments such as "wallet_basic.py --descriptors" + if ' ' in exclude_test: + remove_tests([test for test in test_list if test.replace('.py', '') == exclude_test.replace('.py', '')]) else: - try: - test_list.remove(exclude_test) - except ValueError: - print_warning_missing_test(exclude_test) + # Exclude all variants of a test + remove_tests([test for test in test_list if test.split('.py')[0] == exclude_test.split('.py')[0]]) if args.filter: test_list = list(filter(re.compile(args.filter).search, test_list)) From faa62c0112f2b7ab69c80a5178f5b79217bed0a6 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 30 Jul 2024 13:23:37 +0200 Subject: [PATCH 08/17] util: Add ConstevalFormatString The type is used to wrap a format string once it has been compile-time checked to contain the right number of format specifiers. --- src/test/CMakeLists.txt | 1 + src/test/util_string_tests.cpp | 86 ++++++++++++++++++++++++++++++++++ src/util/string.h | 63 ++++++++++++++++++++++++- 3 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 src/test/util_string_tests.cpp diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 92b39f049765b..afa4addb4ece9 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -132,6 +132,7 @@ add_executable(test_bitcoin txvalidation_tests.cpp txvalidationcache_tests.cpp uint256_tests.cpp + util_string_tests.cpp util_tests.cpp util_threadnames_tests.cpp validation_block_tests.cpp diff --git a/src/test/util_string_tests.cpp b/src/test/util_string_tests.cpp new file mode 100644 index 0000000000000..8b974ffe6fb08 --- /dev/null +++ b/src/test/util_string_tests.cpp @@ -0,0 +1,86 @@ +// Copyright (c) 2024-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include + +using namespace util; + +BOOST_AUTO_TEST_SUITE(util_string_tests) + +// Helper to allow compile-time sanity checks while providing the number of +// args directly. Normally PassFmt would be used. +template +inline void PassFmt(util::ConstevalFormatString fmt) +{ + // This was already executed at compile-time, but is executed again at run-time to avoid -Wunused. + decltype(fmt)::Detail_CheckNumFormatSpecifiers(fmt.fmt); +} +template +inline void FailFmtWithError(std::string_view wrong_fmt, std::string_view error) +{ + using ErrType = const char*; + auto check_throw{[error](const ErrType& str) { return str == error; }}; + BOOST_CHECK_EXCEPTION(util::ConstevalFormatString::Detail_CheckNumFormatSpecifiers(wrong_fmt), ErrType, check_throw); +} + +BOOST_AUTO_TEST_CASE(ConstevalFormatString_NumSpec) +{ + PassFmt<0>(""); + PassFmt<0>("%%"); + PassFmt<1>("%s"); + PassFmt<0>("%%s"); + PassFmt<0>("s%%"); + PassFmt<1>("%%%s"); + PassFmt<1>("%s%%"); + PassFmt<0>(" 1$s"); + PassFmt<1>("%1$s"); + PassFmt<1>("%1$s%1$s"); + PassFmt<2>("%2$s"); + PassFmt<2>("%2$s 4$s %2$s"); + PassFmt<129>("%129$s 999$s %2$s"); + PassFmt<1>("%02d"); + PassFmt<1>("%+2s"); + PassFmt<1>("%.6i"); + PassFmt<1>("%5.2f"); + PassFmt<1>("%#x"); + PassFmt<1>("%1$5i"); + PassFmt<1>("%1$-5i"); + PassFmt<1>("%1$.5i"); + // tinyformat accepts almost any "type" spec, even '%', or '_', or '\n'. + PassFmt<1>("%123%"); + PassFmt<1>("%123%s"); + PassFmt<1>("%_"); + PassFmt<1>("%\n"); + + // The `*` specifier behavior is unsupported and can lead to runtime + // errors when used in a ConstevalFormatString. Please refer to the + // note in the ConstevalFormatString docs. + PassFmt<1>("%*c"); + PassFmt<2>("%2$*3$d"); + PassFmt<1>("%.*f"); + + auto err_mix{"Format specifiers must be all positional or all non-positional!"}; + FailFmtWithError<1>("%s%1$s", err_mix); + + auto err_num{"Format specifier count must match the argument count!"}; + FailFmtWithError<1>("", err_num); + FailFmtWithError<0>("%s", err_num); + FailFmtWithError<2>("%s", err_num); + FailFmtWithError<0>("%1$s", err_num); + FailFmtWithError<2>("%1$s", err_num); + + auto err_0_pos{"Positional format specifier must have position of at least 1"}; + FailFmtWithError<1>("%$s", err_0_pos); + FailFmtWithError<1>("%$", err_0_pos); + FailFmtWithError<0>("%0$", err_0_pos); + FailFmtWithError<0>("%0$s", err_0_pos); + + auto err_term{"Format specifier incorrectly terminated by end of string"}; + FailFmtWithError<1>("%", err_term); + FailFmtWithError<1>("%1$", err_term); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/string.h b/src/util/string.h index 30c0a0d6c1cd6..bdf074a9a602f 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -1,4 +1,4 @@ -// Copyright (c) 2019-2022 The Bitcoin Core developers +// Copyright (c) 2019-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -17,6 +17,67 @@ #include namespace util { +/** + * @brief A wrapper for a compile-time partially validated format string + * + * This struct can be used to enforce partial compile-time validation of format + * strings, to reduce the likelihood of tinyformat throwing exceptions at + * run-time. Validation is partial to try and prevent the most common errors + * while avoiding re-implementing the entire parsing logic. + * + * @note Counting of `*` dynamic width and precision fields (such as `%*c`, + * `%2$*3$d`, `%.*f`) is not implemented to minimize code complexity as long as + * they are not used in the codebase. Usage of these fields is not counted and + * can lead to run-time exceptions. Code wanting to use the `*` specifier can + * side-step this struct and call tinyformat directly. + */ +template +struct ConstevalFormatString { + const char* const fmt; + consteval ConstevalFormatString(const char* str) : fmt{str} { Detail_CheckNumFormatSpecifiers(fmt); } + constexpr static void Detail_CheckNumFormatSpecifiers(std::string_view str) + { + unsigned count_normal{0}; // Number of "normal" specifiers, like %s + unsigned count_pos{0}; // Max number in positional specifier, like %8$s + for (auto it{str.begin()}; it < str.end();) { + if (*it != '%') { + ++it; + continue; + } + + if (++it >= str.end()) throw "Format specifier incorrectly terminated by end of string"; + if (*it == '%') { + // Percent escape: %% + ++it; + continue; + } + + unsigned maybe_num{0}; + while ('0' <= *it && *it <= '9') { + maybe_num *= 10; + maybe_num += *it - '0'; + ++it; + }; + + if (*it == '$') { + // Positional specifier, like %8$s + if (maybe_num == 0) throw "Positional format specifier must have position of at least 1"; + count_pos = std::max(count_pos, maybe_num); + if (++it >= str.end()) throw "Format specifier incorrectly terminated by end of string"; + } else { + // Non-positional specifier, like %s + ++count_normal; + ++it; + } + // The remainder "[flags][width][.precision][length]type" of the + // specifier is not checked. Parsing continues with the next '%'. + } + if (count_normal && count_pos) throw "Format specifiers must be all positional or all non-positional!"; + unsigned count{count_normal | count_pos}; + if (num_params != count) throw "Format specifier count must match the argument count!"; + } +}; + void ReplaceAll(std::string& in_out, const std::string& search, const std::string& substitute); /** Split a string on any char found in separators, returning a vector. From fa7087b896c0150c29d7a27c53e0533831a2bf3b Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 30 Jul 2024 14:30:16 +0200 Subject: [PATCH 09/17] util: Use compile-time check for FatalErrorf --- src/index/base.cpp | 5 +++-- src/index/base.h | 5 +++-- src/util/string.h | 9 +++++++++ test/lint/lint-format-strings.py | 1 - test/lint/run-lint-format-strings.py | 2 -- 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/index/base.cpp b/src/index/base.cpp index 955d7b67c9e6b..6f9860415f371 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022 The Bitcoin Core developers +// Copyright (c) 2017-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include // For g_chainman @@ -27,7 +28,7 @@ constexpr auto SYNC_LOG_INTERVAL{30s}; constexpr auto SYNC_LOCATOR_WRITE_INTERVAL{30s}; template -void BaseIndex::FatalErrorf(const char* fmt, const Args&... args) +void BaseIndex::FatalErrorf(util::ConstevalFormatString fmt, const Args&... args) { auto message = tfm::format(fmt, args...); node::AbortNode(m_chain->context()->shutdown, m_chain->context()->exit_status, Untranslated(message), m_chain->context()->warnings.get()); diff --git a/src/index/base.h b/src/index/base.h index 0eb1d9ca3b229..beb1575ab214b 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022 The Bitcoin Core developers +// Copyright (c) 2017-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -94,7 +95,7 @@ class BaseIndex : public CValidationInterface virtual bool AllowPrune() const = 0; template - void FatalErrorf(const char* fmt, const Args&... args); + void FatalErrorf(util::ConstevalFormatString fmt, const Args&... args); protected: std::unique_ptr m_chain; diff --git a/src/util/string.h b/src/util/string.h index bdf074a9a602f..c5183d6c80191 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -6,6 +6,7 @@ #define BITCOIN_UTIL_STRING_H #include +#include #include #include @@ -234,4 +235,12 @@ template } } // namespace util +namespace tinyformat { +template +std::string format(util::ConstevalFormatString fmt, const Args&... args) +{ + return format(fmt.fmt, args...); +} +} // namespace tinyformat + #endif // BITCOIN_UTIL_STRING_H diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index 9c9fbb2e392fd..5dbad3f452f72 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -16,7 +16,6 @@ import sys FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS = [ - 'FatalErrorf,0', 'tfm::format,1', # Assuming tfm::::format(std::ostream&, ... 'LogConnectFailure,1', 'LogError,0', diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index 9a7386ee64ad1..f78f356a34917 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -13,8 +13,6 @@ import sys FALSE_POSITIVES = [ - ("src/index/base.cpp", "FatalErrorf(const char* fmt, const Args&... args)"), - ("src/index/base.h", "FatalErrorf(const char* fmt, const Args&... args)"), ("src/netbase.cpp", "LogConnectFailure(bool manual_connection, const char* fmt, const Args&... args)"), ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"), ("src/test/translation_tests.cpp", "strprintf(format, arg)"), From fa5bc450d5d4c1d2daf7b205f1678402c3c21678 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 10 Sep 2024 22:55:29 +0200 Subject: [PATCH 10/17] util: Use compile-time check for LogConnectFailure --- src/netbase.cpp | 3 ++- test/lint/lint-format-strings.py | 1 - test/lint/run-lint-format-strings.py | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/netbase.cpp b/src/netbase.cpp index 1a96443d4a72e..a6de72090e65c 100644 --- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -557,7 +557,8 @@ std::unique_ptr CreateSockOS(int domain, int type, int protocol) std::function(int, int, int)> CreateSock = CreateSockOS; template -static void LogConnectFailure(bool manual_connection, const char* fmt, const Args&... args) { +static void LogConnectFailure(bool manual_connection, util::ConstevalFormatString fmt, const Args&... args) +{ std::string error_message = tfm::format(fmt, args...); if (manual_connection) { LogPrintf("%s\n", error_message); diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index 5dbad3f452f72..c30975fea7f7f 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -17,7 +17,6 @@ FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS = [ 'tfm::format,1', # Assuming tfm::::format(std::ostream&, ... - 'LogConnectFailure,1', 'LogError,0', 'LogWarning,0', 'LogInfo,0', diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index f78f356a34917..a32717653aaf8 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -13,7 +13,6 @@ import sys FALSE_POSITIVES = [ - ("src/netbase.cpp", "LogConnectFailure(bool manual_connection, const char* fmt, const Args&... args)"), ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"), ("src/test/translation_tests.cpp", "strprintf(format, arg)"), ("src/validationinterface.cpp", "LogDebug(BCLog::VALIDATION, fmt \"\\n\", __VA_ARGS__)"), From 23eedc5d1e24b3d31da7902ece5182b9b24672d8 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 12 Sep 2024 14:24:26 +0100 Subject: [PATCH 11/17] build: Skip secp256k1 ctime tests when tests are not being built Co-authored-by: fanquake --- src/CMakeLists.txt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 9a76fe1eec43d..895c17541f601 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -49,6 +49,11 @@ set(SECP256K1_ENABLE_MODULE_RECOVERY ON CACHE BOOL "" FORCE) set(SECP256K1_BUILD_BENCHMARK OFF CACHE BOOL "" FORCE) set(SECP256K1_BUILD_TESTS ${BUILD_TESTS} CACHE BOOL "" FORCE) set(SECP256K1_BUILD_EXHAUSTIVE_TESTS ${BUILD_TESTS} CACHE BOOL "" FORCE) +if(NOT BUILD_TESTS) + # Always skip the ctime tests, if we are building no other tests. + # Otherwise, they are built if Valgrind is available. See SECP256K1_VALGRIND. + set(SECP256K1_BUILD_CTIME_TESTS ${BUILD_TESTS} CACHE BOOL "" FORCE) +endif() set(SECP256K1_BUILD_EXAMPLES OFF CACHE BOOL "" FORCE) include(GetTargetInterface) # -fsanitize and related flags apply to both C++ and C, From 19f4a7c95a99162122068d4badffeea240967a65 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Thu, 12 Sep 2024 16:16:54 +0200 Subject: [PATCH 12/17] test: Wait for local services to update in feature_assumeutxo --- test/functional/feature_assumeutxo.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 57f34264ec2fb..c91f254f113f2 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -314,9 +314,9 @@ def test_sync_from_assumeutxo_node(self, snapshot): self.sync_blocks(nodes=(miner, snapshot_node)) # Check the base snapshot block was stored and ensure node signals full-node service support self.wait_until(lambda: not try_rpc(-1, "Block not found", snapshot_node.getblock, snapshot_block_hash)) - assert 'NETWORK' in snapshot_node.getnetworkinfo()['localservicesnames'] + self.wait_until(lambda: 'NETWORK' in snapshot_node.getnetworkinfo()['localservicesnames']) - # Now the snapshot_node is sync, verify the ibd_node can sync from it + # Now that the snapshot_node is synced, verify the ibd_node can sync from it self.connect_nodes(snapshot_node.index, ibd_node.index) assert 'NETWORK' in ibd_node.getpeerinfo()[0]['servicesnames'] self.sync_blocks(nodes=(ibd_node, snapshot_node)) @@ -698,7 +698,7 @@ def check_tx_counts(final: bool) -> None: self.wait_until(lambda: len(n2.getchainstates()['chainstates']) == 1) # Once background chain sync completes, the full node must start offering historical blocks again. - assert {'NETWORK', 'NETWORK_LIMITED'}.issubset(n2.getnetworkinfo()['localservicesnames']) + self.wait_until(lambda: {'NETWORK', 'NETWORK_LIMITED'}.issubset(n2.getnetworkinfo()['localservicesnames'])) completed_idx_state = { 'basic block filter index': COMPLETE_IDX, From fdeb717e78fd55fbfda199c87002358bdc5895af Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Thu, 12 Sep 2024 16:34:57 +0100 Subject: [PATCH 13/17] Revert "build: Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`" This reverts commit b07fe666f27e2b2515d2cb5a0339512045e64761. --- cmake/script/GenerateHeaderFromJson.cmake | 22 ++++++++++----------- cmake/script/GenerateHeaderFromRaw.cmake | 24 +++++++++++------------ 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/cmake/script/GenerateHeaderFromJson.cmake b/cmake/script/GenerateHeaderFromJson.cmake index c0e895e9b3318..53d1165272831 100644 --- a/cmake/script/GenerateHeaderFromJson.cmake +++ b/cmake/script/GenerateHeaderFromJson.cmake @@ -5,24 +5,22 @@ file(READ ${JSON_SOURCE_PATH} hex_content HEX) string(REGEX MATCHALL "([A-Za-z0-9][A-Za-z0-9])" bytes "${hex_content}") -set(header_content "#include \n") -string(APPEND header_content "namespace json_tests{\n") +file(WRITE ${HEADER_PATH} "#include \n") +file(APPEND ${HEADER_PATH} "namespace json_tests{\n") get_filename_component(json_source_basename ${JSON_SOURCE_PATH} NAME_WE) -string(APPEND header_content "inline constexpr char detail_${json_source_basename}_bytes[]{\n") +file(APPEND ${HEADER_PATH} "inline constexpr char detail_${json_source_basename}_bytes[]{\n") set(i 0) foreach(byte ${bytes}) math(EXPR i "${i} + 1") - if(i EQUAL 8) - set(i 0) - string(APPEND header_content "0x${byte},\n") + math(EXPR remainder "${i} % 8") + if(remainder EQUAL 0) + file(APPEND ${HEADER_PATH} "0x${byte},\n") else() - string(APPEND header_content "0x${byte}, ") + file(APPEND ${HEADER_PATH} "0x${byte}, ") endif() endforeach() -string(APPEND header_content "\n};\n") -string(APPEND header_content "inline constexpr std::string_view ${json_source_basename}{std::begin(detail_${json_source_basename}_bytes), std::end(detail_${json_source_basename}_bytes)};") -string(APPEND header_content "\n}") - -file(WRITE ${HEADER_PATH} "${header_content}") +file(APPEND ${HEADER_PATH} "\n};\n") +file(APPEND ${HEADER_PATH} "inline constexpr std::string_view ${json_source_basename}{std::begin(detail_${json_source_basename}_bytes), std::end(detail_${json_source_basename}_bytes)};") +file(APPEND ${HEADER_PATH} "\n}") diff --git a/cmake/script/GenerateHeaderFromRaw.cmake b/cmake/script/GenerateHeaderFromRaw.cmake index 2a5140b9d69d1..18a6c2b407852 100644 --- a/cmake/script/GenerateHeaderFromRaw.cmake +++ b/cmake/script/GenerateHeaderFromRaw.cmake @@ -5,25 +5,23 @@ file(READ ${RAW_SOURCE_PATH} hex_content HEX) string(REGEX MATCHALL "([A-Za-z0-9][A-Za-z0-9])" bytes "${hex_content}") -set(header_content "#include \n") -string(APPEND header_content "#include \n") -string(APPEND header_content "namespace ${RAW_NAMESPACE} {\n") +file(WRITE ${HEADER_PATH} "#include \n") +file(APPEND ${HEADER_PATH} "#include \n") +file(APPEND ${HEADER_PATH} "namespace ${RAW_NAMESPACE} {\n") get_filename_component(raw_source_basename ${RAW_SOURCE_PATH} NAME_WE) -string(APPEND header_content "inline constexpr std::byte detail_${raw_source_basename}_raw[]{\n") +file(APPEND ${HEADER_PATH} "inline constexpr std::byte detail_${raw_source_basename}_raw[]{\n") set(i 0) foreach(byte ${bytes}) math(EXPR i "${i} + 1") - if(i EQUAL 8) - set(i 0) - string(APPEND header_content "std::byte{0x${byte}},\n") + math(EXPR remainder "${i} % 8") + if(remainder EQUAL 0) + file(APPEND ${HEADER_PATH} "std::byte{0x${byte}},\n") else() - string(APPEND header_content "std::byte{0x${byte}}, ") + file(APPEND ${HEADER_PATH} "std::byte{0x${byte}}, ") endif() endforeach() -string(APPEND header_content "\n};\n") -string(APPEND header_content "inline constexpr std::span ${raw_source_basename}{detail_${raw_source_basename}_raw};\n") -string(APPEND header_content "}") - -file(WRITE ${HEADER_PATH} "${header_content}") +file(APPEND ${HEADER_PATH} "\n};\n") +file(APPEND ${HEADER_PATH} "inline constexpr std::span ${raw_source_basename}{detail_${raw_source_basename}_raw};\n") +file(APPEND ${HEADER_PATH} "}") From a965f2bc07a3588f8c2b8d6a542961562e3f5d0e Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 12 Sep 2024 18:50:43 -0300 Subject: [PATCH 14/17] gui: fix crash when closing wallet The crash occurs because 'WalletController::removeAndDeleteWallet' is called twice for the same wallet model: first in the GUI's button connected function 'WalletController::closeWallet', and then again when the backend emits the 'WalletModel::unload' signal. This causes the issue because 'removeAndDeleteWallet' inlines an erase(std::remove()). So, if 'std::remove' returns an iterator to the end (indicating the element wasn't found because it was already erased), the subsequent call to 'erase' leads to an undefined behavior. --- src/qt/walletcontroller.cpp | 18 ++++++++++-------- src/qt/walletcontroller.h | 3 +++ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 512ea8a1dc116..dd093e984a37b 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -79,6 +79,14 @@ std::map> WalletController::listWallet return wallets; } +void WalletController::removeWallet(WalletModel* wallet_model) +{ + // Once the wallet is successfully removed from the node, the model will emit the 'WalletModel::unload' signal. + // This signal is already connected and will complete the removal of the view from the GUI. + // Look at 'WalletController::getOrCreateWallet' for the signal connection. + wallet_model->wallet().remove(); +} + void WalletController::closeWallet(WalletModel* wallet_model, QWidget* parent) { QMessageBox box(parent); @@ -89,10 +97,7 @@ void WalletController::closeWallet(WalletModel* wallet_model, QWidget* parent) box.setDefaultButton(QMessageBox::Yes); if (box.exec() != QMessageBox::Yes) return; - // First remove wallet from node. - wallet_model->wallet().remove(); - // Now release the model. - removeAndDeleteWallet(wallet_model); + removeWallet(wallet_model); } void WalletController::closeAllWallets(QWidget* parent) @@ -105,11 +110,8 @@ void WalletController::closeAllWallets(QWidget* parent) QMutexLocker locker(&m_mutex); for (WalletModel* wallet_model : m_wallets) { - wallet_model->wallet().remove(); - Q_EMIT walletRemoved(wallet_model); - delete wallet_model; + removeWallet(wallet_model); } - m_wallets.clear(); } WalletModel* WalletController::getOrCreateWallet(std::unique_ptr wallet) diff --git a/src/qt/walletcontroller.h b/src/qt/walletcontroller.h index 7902c3b23544a..4d2ba43539231 100644 --- a/src/qt/walletcontroller.h +++ b/src/qt/walletcontroller.h @@ -85,6 +85,9 @@ class WalletController : public QObject friend class WalletControllerActivity; friend class MigrateWalletActivity; + + //! Starts the wallet closure procedure + void removeWallet(WalletModel* wallet_model); }; class WalletControllerActivity : public QObject From 001b1cf010453adbb1316a6fa8911398953afe61 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 20 Mar 2024 14:39:41 +0000 Subject: [PATCH 15/17] build: use standard branch-protection for aarch64-linux --- CMakeLists.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 555e5b99b76f1..b8fdeefbdae8b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -506,7 +506,11 @@ if(ENABLE_HARDENING) endif() if(CMAKE_SYSTEM_PROCESSOR STREQUAL "aarch64" OR CMAKE_SYSTEM_PROCESSOR STREQUAL "arm64") - try_append_cxx_flags("-mbranch-protection=bti" TARGET hardening_interface SKIP_LINK) + if(CMAKE_SYSTEM_NAME STREQUAL "Darwin") + try_append_cxx_flags("-mbranch-protection=bti" TARGET hardening_interface SKIP_LINK) + else() + try_append_cxx_flags("-mbranch-protection=standard" TARGET hardening_interface SKIP_LINK) + endif() endif() try_append_linker_flag("-Wl,--enable-reloc-section" TARGET hardening_interface) From 282f0e92559da23e356504a564a0322b9888e50b Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Thu, 12 Sep 2024 12:22:36 -0600 Subject: [PATCH 16/17] Unit test runner documentation fix and improvements - Running `test_bitcoin --help` prints the list of arguments that may be passed, not the list of tests, so fix that. - Improve the content and order of the unit test documentation. --- src/test/README.md | 48 ++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/test/README.md b/src/test/README.md index f71e0771bf20d..7e0f245ee8e2b 100644 --- a/src/test/README.md +++ b/src/test/README.md @@ -10,14 +10,19 @@ The build system is set up to compile an executable called `test_bitcoin` that runs all of the unit tests. The main source file for the test library is found in `util/setup_common.cpp`. +The examples in this document assume the build directory is named +`build`. You'll need to adapt them if you named it differently. + ### Compiling/running unit tests Unit tests will be automatically compiled if dependencies were met during the generation of the Bitcoin Core build system and tests weren't explicitly disabled. -Assuming the build directory is named `build`, the unit tests can be run -with `ctest --test-dir build`, which includes unit tests from subtrees. +The unit tests can be run with `ctest --test-dir build`, which includes unit +tests from subtrees. + +Run `test_bitcoin --list_content` for the full list of tests. To run the unit tests manually, launch `build/src/test/test_bitcoin`. To recompile after a test file was modified, run `cmake --build build` and then run the test again. If you @@ -35,34 +40,45 @@ the `src/qt/test/test_main.cpp` file. ### Running individual tests -`test_bitcoin` accepts the command line arguments from the boost framework. -For example, to run just the `getarg_tests` suite of tests: +The `test_bitcoin` runner accepts command line arguments from the Boost +framework. To see the list of arguments that may be passed, run: + +``` +test_bitcoin --help +``` + +For example, to run only the tests in the `getarg_tests` file, with full logging: ```bash build/src/test/test_bitcoin --log_level=all --run_test=getarg_tests ``` -`log_level` controls the verbosity of the test framework, which logs when a -test case is entered, for example. +or -`test_bitcoin` also accepts some of the command line arguments accepted by -`bitcoind`. Use `--` to separate these sets of arguments: +```bash +build/src/test/test_bitcoin -l all -t getarg_tests +``` + +or to run only the doubledash test in `getarg_tests` ```bash -build/src/test/test_bitcoin --log_level=all --run_test=getarg_tests -- -printtoconsole=1 +build/src/test/test_bitcoin --run_test=getarg_tests/doubledash ``` -The `-printtoconsole=1` after the two dashes sends debug logging, which -normally goes only to `debug.log` within the data directory, also to the -standard terminal output. +The `--log_level=` (or `-l`) argument controls the verbosity of the test output. -... or to run just the doubledash test: +The `test_bitcoin` runner also accepts some of the command line arguments accepted by +`bitcoind`. Use `--` to separate these sets of arguments: ```bash -build/src/test/test_bitcoin --run_test=getarg_tests/doubledash +build/src/test/test_bitcoin --log_level=all --run_test=getarg_tests -- -printtoconsole=1 ``` -`test_bitcoin` creates a temporary working (data) directory with a randomly +The `-printtoconsole=1` after the two dashes sends debug logging, which +normally goes only to `debug.log` within the data directory, to the +standard terminal output as well. + +Running `test_bitcoin` creates a temporary working (data) directory with a randomly generated pathname within `test_common bitcoin/`, which in turn is within the system's temporary directory (see [`temp_directory_path`](https://en.cppreference.com/w/cpp/filesystem/temp_directory_path)). @@ -97,8 +113,6 @@ If you run an entire test suite, such as `--run_test=getarg_tests`, or all the t (by not specifying `--run_test`), a separate directory will be created for each individual test. -Run `test_bitcoin --help` for the full list of tests. - ### Adding test cases To add a new unit test file to our test suite, you need From 95560616fbab3ddca9d85980b7f73c8a816bc99e Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 12 Sep 2024 14:39:41 +0200 Subject: [PATCH 17/17] code style: update .editorconfig file Updates the .editorconfig file, first introduced in 2021 (see PR #21123, commit 7a135d57) w.r.t. following changes: - consider Rust .rs files (relevant since #28076, commit bbbbdb0c) - reflect build system change to CMake (#30454, #30664) - add setting for the bare Makefile still used for depends builds Can be tested e.g. by using the editorconfig-vim plugin (https://github.com/editorconfig/editorconfig-vim). --- .editorconfig | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.editorconfig b/.editorconfig index ae7e92d1c8a82..c5f3028c50366 100644 --- a/.editorconfig +++ b/.editorconfig @@ -10,17 +10,17 @@ insert_final_newline = true trim_trailing_whitespace = true # Source code files -[*.{h,cpp,py,sh}] +[*.{h,cpp,rs,py,sh}] indent_size = 4 -# .cirrus.yml, .fuzzbuzz.yml, etc. +# .cirrus.yml, etc. [*.yml] indent_size = 2 -# Makefiles -[{*.am,Makefile.*.include}] +# Makefiles (only relevant for depends build) +[Makefile] indent_style = tab -# Autoconf scripts -[configure.ac] +# CMake files +[{CMakeLists.txt,*.cmake,*.cmake.in}] indent_size = 2