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 diff --git a/CMakeLists.txt b/CMakeLists.txt index 6c3f65883db27..adb2b77e1b2e1 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) 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} "}") 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/CMakeLists.txt b/src/CMakeLists.txt index 6f5419dcd548a..2e144759c2bd1 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, 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/kernel/CMakeLists.txt b/src/kernel/CMakeLists.txt index 018bf8046ee7d..1567b9cc5542e 100644 --- a/src/kernel/CMakeLists.txt +++ b/src/kernel/CMakeLists.txt @@ -101,6 +101,40 @@ set_target_properties(groestlcoinkernel 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) + + # 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 groestlcoinkernel RUNTIME 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/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 diff --git a/src/test/CMakeLists.txt b/src/test/CMakeLists.txt index 1970c817c20a2..b3b8e2f73d694 100644 --- a/src/test/CMakeLists.txt +++ b/src/test/CMakeLists.txt @@ -132,6 +132,7 @@ add_executable(test_groestlcoin 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/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 diff --git a/src/test/fuzz/hex.cpp b/src/test/fuzz/hex.cpp index dcf91fd383fcd..3dcf1ed3d51ce 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)); } try { (void)HexToPubKey(random_hex_string); diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp index efbc2e4b9b9da..142d7a6fde24a 100644 --- a/src/test/uint256_tests.cpp +++ b/src/test/uint256_tests.cpp @@ -299,15 +299,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))); @@ -334,7 +334,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/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. 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/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 diff --git a/src/util/string.h b/src/util/string.h index 30c0a0d6c1cd6..c5183d6c80191 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. @@ -6,6 +6,7 @@ #define BITCOIN_UTIL_STRING_H #include +#include #include #include @@ -17,6 +18,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. @@ -173,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/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, diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 714298e2e88cd..3297a1069939b 100644 --- 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)) diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index 9f5e0f312ecb8..c30975fea7f7f 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -16,10 +16,7 @@ import sys FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS = [ - 'FatalErrorf,0', - 'fprintf,1', 'tfm::format,1', # Assuming tfm::::format(std::ostream&, ... - 'LogConnectFailure,1', 'LogError,0', 'LogWarning,0', 'LogInfo,0', @@ -27,14 +24,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..a32717653aaf8 100644 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -13,10 +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)"), ("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__)"),