From e16f420547fc72a5a2902927aa7138e43c0fb7c8 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Fri, 29 Sep 2023 23:23:36 +0200 Subject: [PATCH 01/37] net: Optionally include terrible addresses in GetAddr results --- src/addrman.cpp | 12 ++++++------ src/addrman.h | 3 ++- src/addrman_impl.h | 4 ++-- src/net.cpp | 4 ++-- src/net.h | 3 ++- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 6ce9c81c63..93f596cf00 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -800,7 +800,7 @@ int AddrManImpl::GetEntry(bool use_tried, size_t bucket, size_t position) const return -1; } -std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional network) const +std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const { AssertLockHeld(cs); @@ -830,7 +830,7 @@ std::vector AddrManImpl::GetAddr_(size_t max_addresses, size_t max_pct if (network != std::nullopt && ai.GetNetClass() != network) continue; // Filter for quality - if (ai.IsTerrible(now)) continue; + if (ai.IsTerrible(now) && filtered) continue; addresses.push_back(ai); } @@ -1214,11 +1214,11 @@ std::pair AddrManImpl::Select(bool new_only, std::optiona return addrRet; } -std::vector AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const +std::vector AddrManImpl::GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const { LOCK(cs); Check(); - auto addresses = GetAddr_(max_addresses, max_pct, network); + auto addresses = GetAddr_(max_addresses, max_pct, network, filtered); Check(); return addresses; } @@ -1317,9 +1317,9 @@ std::pair AddrMan::Select(bool new_only, std::optionalSelect(new_only, network); } -std::vector AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const +std::vector AddrMan::GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const { - return m_impl->GetAddr(max_addresses, max_pct, network); + return m_impl->GetAddr(max_addresses, max_pct, network, filtered); } std::vector> AddrMan::GetEntries(bool use_tried) const diff --git a/src/addrman.h b/src/addrman.h index 4d44c943ac..6318fbef97 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -164,10 +164,11 @@ class AddrMan * @param[in] max_addresses Maximum number of addresses to return (0 = all). * @param[in] max_pct Maximum percentage of addresses to return (0 = all). * @param[in] network Select only addresses of this network (nullopt = all). + * @param[in] filtered Select only addresses that are considered good quality (false = all). * * @return A vector of randomly selected addresses from vRandom. */ - std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const; + std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const; /** * Returns an information-location pair for all addresses in the selected addrman table. diff --git a/src/addrman_impl.h b/src/addrman_impl.h index 512f085a21..867c894d01 100644 --- a/src/addrman_impl.h +++ b/src/addrman_impl.h @@ -129,7 +129,7 @@ class AddrManImpl std::pair Select(bool new_only, std::optional network) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network) const + std::vector GetAddr(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(!cs); std::vector> GetEntries(bool from_tried) const @@ -261,7 +261,7 @@ class AddrManImpl * */ int GetEntry(bool use_tried, size_t bucket, size_t position) const EXCLUSIVE_LOCKS_REQUIRED(cs); - std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network) const EXCLUSIVE_LOCKS_REQUIRED(cs); + std::vector GetAddr_(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const EXCLUSIVE_LOCKS_REQUIRED(cs); std::vector> GetEntries_(bool from_tried) const EXCLUSIVE_LOCKS_REQUIRED(cs); diff --git a/src/net.cpp b/src/net.cpp index 13f4430424..8c39eaab2b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3410,9 +3410,9 @@ CConnman::~CConnman() Stop(); } -std::vector CConnman::GetAddresses(size_t max_addresses, size_t max_pct, std::optional network) const +std::vector CConnman::GetAddresses(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered) const { - std::vector addresses = addrman.GetAddr(max_addresses, max_pct, network); + std::vector addresses = addrman.GetAddr(max_addresses, max_pct, network, filtered); if (m_banman) { addresses.erase(std::remove_if(addresses.begin(), addresses.end(), [this](const CAddress& addr){return m_banman->IsDiscouraged(addr) || m_banman->IsBanned(addr);}), diff --git a/src/net.h b/src/net.h index 2f7b832fba..5a80896db4 100644 --- a/src/net.h +++ b/src/net.h @@ -1172,8 +1172,9 @@ class CConnman * @param[in] max_addresses Maximum number of addresses to return (0 = all). * @param[in] max_pct Maximum percentage of addresses to return (0 = all). * @param[in] network Select only addresses of this network (nullopt = all). + * @param[in] filtered Select only addresses that are considered high quality (false = all). */ - std::vector GetAddresses(size_t max_addresses, size_t max_pct, std::optional network) const; + std::vector GetAddresses(size_t max_addresses, size_t max_pct, std::optional network, const bool filtered = true) const; /** * Cache is used to minimize topology leaks, so it should * be used for all non-trusted calls, for example, p2p. From b8843d37aed1276ff8527328c956c70c6e02ee13 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Fri, 29 Sep 2023 23:27:48 +0200 Subject: [PATCH 02/37] fuzz: Let fuzzers use filter options in GetAddr/GetAddresses --- src/test/fuzz/addrman.cpp | 3 ++- src/test/fuzz/connman.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/test/fuzz/addrman.cpp b/src/test/fuzz/addrman.cpp index 1b11ff6fdf..ece396aadf 100644 --- a/src/test/fuzz/addrman.cpp +++ b/src/test/fuzz/addrman.cpp @@ -286,7 +286,8 @@ FUZZ_TARGET(addrman, .init = initialize_addrman) (void)const_addr_man.GetAddr( /*max_addresses=*/fuzzed_data_provider.ConsumeIntegralInRange(0, 4096), /*max_pct=*/fuzzed_data_provider.ConsumeIntegralInRange(0, 4096), - network); + network, + /*filtered=*/fuzzed_data_provider.ConsumeBool()); (void)const_addr_man.Select(fuzzed_data_provider.ConsumeBool(), network); std::optional in_new; if (fuzzed_data_provider.ConsumeBool()) { diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp index 0dab2a2e97..a9a4d57560 100644 --- a/src/test/fuzz/connman.cpp +++ b/src/test/fuzz/connman.cpp @@ -88,7 +88,8 @@ FUZZ_TARGET(connman, .init = initialize_connman) (void)connman.GetAddresses( /*max_addresses=*/fuzzed_data_provider.ConsumeIntegral(), /*max_pct=*/fuzzed_data_provider.ConsumeIntegral(), - /*network=*/std::nullopt); + /*network=*/std::nullopt, + /*filtered=*/fuzzed_data_provider.ConsumeBool()); }, [&] { (void)connman.GetAddresses( From 28d7e55dff826a69f3f8e58139dbffb611cc5947 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Fri, 29 Sep 2023 23:42:28 +0200 Subject: [PATCH 03/37] test: Add tests for unfiltered GetAddr usage --- src/test/addrman_tests.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index b01ba81c5f..1a8475cd25 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -429,6 +429,24 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) BOOST_CHECK_EQUAL(addrman->Size(), 2006U); } +BOOST_AUTO_TEST_CASE(getaddr_unfiltered) +{ + auto addrman = std::make_unique(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)); + + // Set time on this addr so isTerrible = false + CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE); + addr1.nTime = Now(); + // Not setting time so this addr should be isTerrible = true + CAddress addr2 = CAddress(ResolveService("250.251.2.2", 9999), NODE_NONE); + + CNetAddr source = ResolveIP("250.1.2.1"); + BOOST_CHECK(addrman->Add({addr1, addr2}, source)); + + // Filtered GetAddr should only return addr1 + BOOST_CHECK_EQUAL(addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt).size(), 1U); + // Unfiltered GetAddr should return addr1 and addr2 + BOOST_CHECK_EQUAL(addrman->GetAddr(/*max_addresses=*/0, /*max_pct=*/0, /*network=*/std::nullopt, /*filtered=*/false).size(), 2U); +} BOOST_AUTO_TEST_CASE(caddrinfo_get_tried_bucket_legacy) { From 11b7269d83a56f919f9dddb7f7c72a96d428627f Mon Sep 17 00:00:00 2001 From: pablomartin4btc Date: Fri, 13 Oct 2023 00:53:15 -0300 Subject: [PATCH 04/37] script: Enhance validations in utxo_snapshot.sh - Ensure that the snapshot height is higher than the pruned block height when the node is pruned. - Validate the correctness of the file path and check if the file already exists. - Make network activity disablement optional for the user. - Ensure the reconsiderblock command is triggered on exit, even in the case of user interruption (Ctrl-C). Co-authored-by: Chris Heyes <22148308+hazeycode@users.noreply.github.com> Co-authored-by: Sjors Provoost --- contrib/devtools/utxo_snapshot.sh | 58 ++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/contrib/devtools/utxo_snapshot.sh b/contrib/devtools/utxo_snapshot.sh index dee25ff67b..80336954a9 100755 --- a/contrib/devtools/utxo_snapshot.sh +++ b/contrib/devtools/utxo_snapshot.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # -# Copyright (c) 2019 The Bitcoin Core developers +# Copyright (c) 2019-2023 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. # @@ -8,6 +8,8 @@ export LC_ALL=C set -ueo pipefail +NETWORK_DISABLED=false + if (( $# < 3 )); then echo 'Usage: utxo_snapshot.sh ' echo @@ -26,9 +28,60 @@ OUTPUT_PATH="${1}"; shift; # Most of the calls we make take a while to run, so pad with a lengthy timeout. BITCOIN_CLI_CALL="${*} -rpcclienttimeout=9999999" +# Check if the node is pruned and get the pruned block height +PRUNED=$( ${BITCOIN_CLI_CALL} getblockchaininfo | awk '/pruneheight/ {print $2}' | tr -d ',' ) + +if (( GENERATE_AT_HEIGHT < PRUNED )); then + echo "Error: The requested snapshot height (${GENERATE_AT_HEIGHT}) should be greater than the pruned block height (${PRUNED})." + exit 1 +fi + +# Early exit if file at OUTPUT_PATH already exists +if [[ -e "$OUTPUT_PATH" ]]; then + (>&2 echo "Error: $OUTPUT_PATH already exists or is not a valid path.") + exit 1 +fi + +# Validate that the path is correct +if [[ "${OUTPUT_PATH}" != "-" && ! -d "$(dirname "${OUTPUT_PATH}")" ]]; then + (>&2 echo "Error: The directory $(dirname "${OUTPUT_PATH}") does not exist.") + exit 1 +fi + +function cleanup { + (>&2 echo "Restoring chain to original height; this may take a while") + ${BITCOIN_CLI_CALL} reconsiderblock "${PIVOT_BLOCKHASH}" + + if $NETWORK_DISABLED; then + (>&2 echo "Restoring network activity") + ${BITCOIN_CLI_CALL} setnetworkactive true + fi +} + +function early_exit { + (>&2 echo "Exiting due to Ctrl-C") + cleanup + exit 1 +} + +# Prompt the user to disable network activity +read -p "Do you want to disable network activity (setnetworkactive false) before running invalidateblock? (Y/n): " -r +if [[ "$REPLY" =~ ^[Yy]*$ || -z "$REPLY" ]]; then + # User input is "Y", "y", or Enter key, proceed with the action + NETWORK_DISABLED=true + (>&2 echo "Disabling network activity") + ${BITCOIN_CLI_CALL} setnetworkactive false +else + (>&2 echo "Network activity remains enabled") +fi + # Block we'll invalidate/reconsider to rewind/fast-forward the chain. PIVOT_BLOCKHASH=$($BITCOIN_CLI_CALL getblockhash $(( GENERATE_AT_HEIGHT + 1 )) ) +# Trap for normal exit and Ctrl-C +trap cleanup EXIT +trap early_exit INT + (>&2 echo "Rewinding chain back to height ${GENERATE_AT_HEIGHT} (by invalidating ${PIVOT_BLOCKHASH}); this may take a while") ${BITCOIN_CLI_CALL} invalidateblock "${PIVOT_BLOCKHASH}" @@ -39,6 +92,3 @@ else (>&2 echo "Generating UTXO snapshot...") ${BITCOIN_CLI_CALL} dumptxoutset "${OUTPUT_PATH}" fi - -(>&2 echo "Restoring chain to original height; this may take a while") -${BITCOIN_CLI_CALL} reconsiderblock "${PIVOT_BLOCKHASH}" From bb4554c81e0d819d74996f89cbb9c00476aedf8c Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 16 Nov 2023 11:15:08 -0300 Subject: [PATCH 05/37] bench: add benchmark for wallet creation procedure --- src/Makefile.bench.include | 1 + src/bench/wallet_create.cpp | 55 +++++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 src/bench/wallet_create.cpp diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 28b779a5a8..217f123b16 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -84,6 +84,7 @@ endif if ENABLE_WALLET bench_bench_bitcoin_SOURCES += bench/coin_selection.cpp bench_bench_bitcoin_SOURCES += bench/wallet_balance.cpp +bench_bench_bitcoin_SOURCES += bench/wallet_create.cpp bench_bench_bitcoin_SOURCES += bench/wallet_loading.cpp bench_bench_bitcoin_SOURCES += bench/wallet_create_tx.cpp bench_bench_bitcoin_LDADD += $(BDB_LIBS) $(SQLITE_LIBS) diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp new file mode 100644 index 0000000000..ba3c25d25e --- /dev/null +++ b/src/bench/wallet_create.cpp @@ -0,0 +1,55 @@ +// Copyright (c) 2023-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include + +namespace wallet { +static void WalletCreate(benchmark::Bench& bench, bool encrypted) +{ + auto test_setup = MakeNoLogFileContext(); + FastRandomContext random; + + WalletContext context; + context.args = &test_setup->m_args; + context.chain = test_setup->m_node.chain.get(); + + DatabaseOptions options; + options.require_format = DatabaseFormat::SQLITE; + options.require_create = true; + options.create_flags = WALLET_FLAG_DESCRIPTORS; + + if (encrypted) { + options.create_passphrase = random.rand256().ToString(); + } + + DatabaseStatus status; + bilingual_str error_string; + std::vector warnings; + + fs::path wallet_path = test_setup->m_path_root / strprintf("test_wallet_%d", random.rand32()).c_str(); + bench.run([&] { + auto wallet = CreateWallet(context, wallet_path.u8string(), /*load_on_start=*/std::nullopt, options, status, error_string, warnings); + assert(status == DatabaseStatus::SUCCESS); + assert(wallet != nullptr); + + // Cleanup + wallet.reset(); + fs::remove_all(wallet_path); + }); +} + +static void WalletCreatePlain(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/false); } +static void WalletCreateEncrypted(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/true); } + +#ifdef USE_SQLITE +BENCHMARK(WalletCreatePlain, benchmark::PriorityLevel::LOW); +BENCHMARK(WalletCreateEncrypted, benchmark::PriorityLevel::LOW); +#endif + +} // namespace wallet From facf629ce8ff1d1f6d254dde4e89b5018f8df60e Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 21 Nov 2023 17:06:22 +0100 Subject: [PATCH 06/37] refactor: Remove unused and fragile string interface from arith_uint256 --- src/arith_uint256.cpp | 25 +------------------ src/arith_uint256.h | 7 +----- src/test/arith_uint256_tests.cpp | 42 +++++++++++++++++--------------- src/test/pow_tests.cpp | 2 +- src/test/uint256_tests.cpp | 4 +-- 5 files changed, 28 insertions(+), 52 deletions(-) diff --git a/src/arith_uint256.cpp b/src/arith_uint256.cpp index 3776cfb6de..0d5b3d5b0e 100644 --- a/src/arith_uint256.cpp +++ b/src/arith_uint256.cpp @@ -8,14 +8,7 @@ #include #include - -template -base_uint::base_uint(const std::string& str) -{ - static_assert(BITS/32 > 0 && BITS%32 == 0, "Template parameter BITS must be a positive multiple of 32."); - - SetHex(str); -} +#include template base_uint& base_uint::operator<<=(unsigned int shift) @@ -153,22 +146,6 @@ std::string base_uint::GetHex() const return b.GetHex(); } -template -void base_uint::SetHex(const char* psz) -{ - base_blob b; - b.SetHex(psz); - for (int x = 0; x < this->WIDTH; ++x) { - this->pn[x] = ReadLE32(b.begin() + x*4); - } -} - -template -void base_uint::SetHex(const std::string& str) -{ - SetHex(str.c_str()); -} - template std::string base_uint::ToString() const { diff --git a/src/arith_uint256.h b/src/arith_uint256.h index c710fe9471..ba36cebbdc 100644 --- a/src/arith_uint256.h +++ b/src/arith_uint256.h @@ -6,10 +6,10 @@ #ifndef BITCOIN_ARITH_UINT256_H #define BITCOIN_ARITH_UINT256_H +#include #include #include #include -#include #include class uint256; @@ -56,8 +56,6 @@ class base_uint pn[i] = 0; } - explicit base_uint(const std::string& str); - base_uint operator~() const { base_uint ret; @@ -219,8 +217,6 @@ class base_uint friend inline bool operator!=(const base_uint& a, uint64_t b) { return !a.EqualTo(b); } std::string GetHex() const; - void SetHex(const char* psz); - void SetHex(const std::string& str); std::string ToString() const; unsigned int size() const @@ -247,7 +243,6 @@ class arith_uint256 : public base_uint<256> { arith_uint256() {} arith_uint256(const base_uint<256>& b) : base_uint<256>(b) {} arith_uint256(uint64_t b) : base_uint<256>(b) {} - explicit arith_uint256(const std::string& str) : base_uint<256>(str) {} /** * The "compact" format is a representation of a whole diff --git a/src/test/arith_uint256_tests.cpp b/src/test/arith_uint256_tests.cpp index 6a37b7d83b..10028c7c93 100644 --- a/src/test/arith_uint256_tests.cpp +++ b/src/test/arith_uint256_tests.cpp @@ -22,6 +22,7 @@ static inline arith_uint256 arith_uint256V(const std::vector& vch { return UintToArith256(uint256(vch)); } +static inline arith_uint256 arith_uint256S(const std::string& str) { return UintToArith256(uint256S(str)); } const unsigned char R1Array[] = "\x9c\x52\x4a\xdb\xcf\x56\x11\x12\x2b\x29\x12\x5e\x5d\x35\xd2\xd2" @@ -95,25 +96,25 @@ BOOST_AUTO_TEST_CASE( basics ) // constructors, equality, inequality BOOST_CHECK(ZeroL == (OneL << 256)); // String Constructor and Copy Constructor - BOOST_CHECK(arith_uint256("0x"+R1L.ToString()) == R1L); - BOOST_CHECK(arith_uint256("0x"+R2L.ToString()) == R2L); - BOOST_CHECK(arith_uint256("0x"+ZeroL.ToString()) == ZeroL); - BOOST_CHECK(arith_uint256("0x"+OneL.ToString()) == OneL); - BOOST_CHECK(arith_uint256("0x"+MaxL.ToString()) == MaxL); - BOOST_CHECK(arith_uint256(R1L.ToString()) == R1L); - BOOST_CHECK(arith_uint256(" 0x"+R1L.ToString()+" ") == R1L); - BOOST_CHECK(arith_uint256("") == ZeroL); - BOOST_CHECK(R1L == arith_uint256(R1ArrayHex)); + BOOST_CHECK(arith_uint256S("0x" + R1L.ToString()) == R1L); + BOOST_CHECK(arith_uint256S("0x" + R2L.ToString()) == R2L); + BOOST_CHECK(arith_uint256S("0x" + ZeroL.ToString()) == ZeroL); + BOOST_CHECK(arith_uint256S("0x" + OneL.ToString()) == OneL); + BOOST_CHECK(arith_uint256S("0x" + MaxL.ToString()) == MaxL); + BOOST_CHECK(arith_uint256S(R1L.ToString()) == R1L); + BOOST_CHECK(arith_uint256S(" 0x" + R1L.ToString() + " ") == R1L); + BOOST_CHECK(arith_uint256S("") == ZeroL); + BOOST_CHECK(R1L == arith_uint256S(R1ArrayHex)); BOOST_CHECK(arith_uint256(R1L) == R1L); BOOST_CHECK((arith_uint256(R1L^R2L)^R2L) == R1L); BOOST_CHECK(arith_uint256(ZeroL) == ZeroL); BOOST_CHECK(arith_uint256(OneL) == OneL); // uint64_t constructor - BOOST_CHECK( (R1L & arith_uint256("0xffffffffffffffff")) == arith_uint256(R1LLow64)); + BOOST_CHECK((R1L & arith_uint256S("0xffffffffffffffff")) == arith_uint256(R1LLow64)); BOOST_CHECK(ZeroL == arith_uint256(0)); BOOST_CHECK(OneL == arith_uint256(1)); - BOOST_CHECK(arith_uint256("0xffffffffffffffff") == arith_uint256(0xffffffffffffffffULL)); + BOOST_CHECK(arith_uint256S("0xffffffffffffffff") == arith_uint256(0xffffffffffffffffULL)); // Assignment (from base_uint) arith_uint256 tmpL = ~ZeroL; BOOST_CHECK(tmpL == ~ZeroL); @@ -282,7 +283,7 @@ BOOST_AUTO_TEST_CASE( comparison ) // <= >= < > BOOST_AUTO_TEST_CASE( plusMinus ) { arith_uint256 TmpL = 0; - BOOST_CHECK(R1L+R2L == arith_uint256(R1LplusR2L)); + BOOST_CHECK(R1L + R2L == arith_uint256S(R1LplusR2L)); TmpL += R1L; BOOST_CHECK(TmpL == R1L); TmpL += R2L; @@ -346,8 +347,8 @@ BOOST_AUTO_TEST_CASE( multiply ) BOOST_AUTO_TEST_CASE( divide ) { - arith_uint256 D1L("AD7133AC1977FA2B7"); - arith_uint256 D2L("ECD751716"); + arith_uint256 D1L{arith_uint256S("AD7133AC1977FA2B7")}; + arith_uint256 D2L{arith_uint256S("ECD751716")}; BOOST_CHECK((R1L / D1L).ToString() == "00000000000000000b8ac01106981635d9ed112290f8895545a7654dde28fb3a"); BOOST_CHECK((R1L / D2L).ToString() == "000000000873ce8efec5b67150bad3aa8c5fcb70e947586153bf2cec7c37c57a"); BOOST_CHECK(R1L / OneL == R1L); @@ -368,7 +369,7 @@ static bool almostEqual(double d1, double d2) return fabs(d1-d2) <= 4*fabs(d1)*std::numeric_limits::epsilon(); } -BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHex size() GetLow64 GetSerializeSize, Serialize, Unserialize +BOOST_AUTO_TEST_CASE(methods) // GetHex operator= size() GetLow64 GetSerializeSize, Serialize, Unserialize { BOOST_CHECK(R1L.GetHex() == R1L.ToString()); BOOST_CHECK(R2L.GetHex() == R2L.ToString()); @@ -376,11 +377,14 @@ BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHex size() GetLow64 GetSerializeSiz BOOST_CHECK(MaxL.GetHex() == MaxL.ToString()); arith_uint256 TmpL(R1L); BOOST_CHECK(TmpL == R1L); - TmpL.SetHex(R2L.ToString()); BOOST_CHECK(TmpL == R2L); - TmpL.SetHex(ZeroL.ToString()); BOOST_CHECK(TmpL == 0); - TmpL.SetHex(HalfL.ToString()); BOOST_CHECK(TmpL == HalfL); + TmpL = R2L; + BOOST_CHECK(TmpL == R2L); + TmpL = ZeroL; + BOOST_CHECK(TmpL == 0); + TmpL = HalfL; + BOOST_CHECK(TmpL == HalfL); - TmpL.SetHex(R1L.ToString()); + TmpL = R1L; BOOST_CHECK(R1L.size() == 32); BOOST_CHECK(R2L.size() == 32); BOOST_CHECK(ZeroL.size() == 32); diff --git a/src/test/pow_tests.cpp b/src/test/pow_tests.cpp index 5bd14f22c6..3a44d1da49 100644 --- a/src/test/pow_tests.cpp +++ b/src/test/pow_tests.cpp @@ -177,7 +177,7 @@ void sanity_check_chainparams(const ArgsManager& args, ChainType chain_type) // check max target * 4*nPowTargetTimespan doesn't overflow -- see pow.cpp:CalculateNextWorkRequired() if (!consensus.fPowNoRetargeting) { - arith_uint256 targ_max("0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"); + arith_uint256 targ_max{UintToArith256(uint256S("0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"))}; targ_max /= consensus.nPowTargetTimespan*4; BOOST_CHECK(UintToArith256(consensus.powLimit) < targ_max); } diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp index 933988dd7a..8aa540e761 100644 --- a/src/test/uint256_tests.cpp +++ b/src/test/uint256_tests.cpp @@ -260,8 +260,8 @@ BOOST_AUTO_TEST_CASE( conversion ) BOOST_CHECK(UintToArith256(OneL) == 1); BOOST_CHECK(ArithToUint256(0) == ZeroL); BOOST_CHECK(ArithToUint256(1) == OneL); - BOOST_CHECK(arith_uint256(R1L.GetHex()) == UintToArith256(R1L)); - BOOST_CHECK(arith_uint256(R2L.GetHex()) == UintToArith256(R2L)); + BOOST_CHECK(arith_uint256(UintToArith256(uint256S(R1L.GetHex()))) == UintToArith256(R1L)); + BOOST_CHECK(arith_uint256(UintToArith256(uint256S(R2L.GetHex()))) == UintToArith256(R2L)); BOOST_CHECK(R1L.GetHex() == UintToArith256(R1L).GetHex()); BOOST_CHECK(R2L.GetHex() == UintToArith256(R2L).GetHex()); } From fa63f16018d9468e1751d2107b5102184ac2d5ae Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 21 Nov 2023 17:04:21 +0100 Subject: [PATCH 07/37] test: Add uint256 string parse tests --- src/test/uint256_tests.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/uint256_tests.cpp b/src/test/uint256_tests.cpp index 8aa540e761..b363b4c04d 100644 --- a/src/test/uint256_tests.cpp +++ b/src/test/uint256_tests.cpp @@ -279,6 +279,34 @@ BOOST_AUTO_TEST_CASE( operator_with_self ) BOOST_CHECK(v == UintToArith256(uint256S("0"))); } +BOOST_AUTO_TEST_CASE(parse) +{ + { + std::string s_12{"0000000000000000000000000000000000000000000000000000000000000012"}; + BOOST_CHECK_EQUAL(uint256S("12\0").GetHex(), s_12); + BOOST_CHECK_EQUAL(uint256S(std::string{"12\0", 3}).GetHex(), s_12); + BOOST_CHECK_EQUAL(uint256S("0x12").GetHex(), s_12); + BOOST_CHECK_EQUAL(uint256S(" 0x12").GetHex(), s_12); + BOOST_CHECK_EQUAL(uint256S(" 12").GetHex(), s_12); + } + { + std::string s_1{uint256::ONE.GetHex()}; + BOOST_CHECK_EQUAL(uint256S("1\0").GetHex(), s_1); + BOOST_CHECK_EQUAL(uint256S(std::string{"1\0", 2}).GetHex(), s_1); + BOOST_CHECK_EQUAL(uint256S("0x1").GetHex(), s_1); + BOOST_CHECK_EQUAL(uint256S(" 0x1").GetHex(), s_1); + BOOST_CHECK_EQUAL(uint256S(" 1").GetHex(), s_1); + } + { + std::string s_0{uint256::ZERO.GetHex()}; + BOOST_CHECK_EQUAL(uint256S("\0").GetHex(), s_0); + BOOST_CHECK_EQUAL(uint256S(std::string{"\0", 1}).GetHex(), s_0); + BOOST_CHECK_EQUAL(uint256S("0x").GetHex(), s_0); + BOOST_CHECK_EQUAL(uint256S(" 0x").GetHex(), s_0); + BOOST_CHECK_EQUAL(uint256S(" ").GetHex(), s_0); + } +} + BOOST_AUTO_TEST_CASE( check_ONE ) { uint256 one = uint256S("0000000000000000000000000000000000000000000000000000000000000001"); From 075aa44ceba41fa82bb3ce2295e2962e5fd0508e Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 20 Jul 2023 18:20:00 -0300 Subject: [PATCH 08/37] wallet: batch descriptor spkm TopUp Instead of performing multiple atomic write operations per descriptor setup call, batch them all within a single atomic db txn. --- src/wallet/scriptpubkeyman.cpp | 16 ++++++++++++++-- src/wallet/scriptpubkeyman.h | 5 ++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index d2b2801aa8..997348d371 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -2135,6 +2135,15 @@ std::map DescriptorScriptPubKeyMan::GetKeys() const } bool DescriptorScriptPubKeyMan::TopUp(unsigned int size) +{ + WalletBatch batch(m_storage.GetDatabase()); + if (!batch.TxnBegin()) return false; + bool res = TopUpWithDB(batch, size); + if (!batch.TxnCommit()) throw std::runtime_error(strprintf("Error during descriptors keypool top up. Cannot commit changes for wallet %s", m_storage.GetDisplayName())); + return res; +} + +bool DescriptorScriptPubKeyMan::TopUpWithDB(WalletBatch& batch, unsigned int size) { LOCK(cs_desc_man); unsigned int target_size; @@ -2157,7 +2166,6 @@ bool DescriptorScriptPubKeyMan::TopUp(unsigned int size) FlatSigningProvider provider; provider.keys = GetKeys(); - WalletBatch batch(m_storage.GetDatabase()); uint256 id = GetID(); for (int32_t i = m_max_cached_index + 1; i < new_range_end; ++i) { FlatSigningProvider out_keys; @@ -2326,6 +2334,8 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ // Store the master private key, and descriptor WalletBatch batch(m_storage.GetDatabase()); + if (!batch.TxnBegin()) throw std::runtime_error(std::string(__func__) + ": cannot start db transaction"); + if (!AddDescriptorKeyWithDB(batch, master_key.key, master_key.key.GetPubKey())) { throw std::runtime_error(std::string(__func__) + ": writing descriptor master private key failed"); } @@ -2334,9 +2344,11 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ } // TopUp - TopUp(); + TopUpWithDB(batch); m_storage.UnsetBlankWalletFlag(batch); + + if (!batch.TxnCommit()) throw std::runtime_error(std::string(__func__) + ": error committing db transaction"); return true; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 7c0eca1475..7bdfbf0d34 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -583,7 +583,10 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan std::unique_ptr GetSigningProvider(int32_t index, bool include_private = false) const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); protected: - WalletDescriptor m_wallet_descriptor GUARDED_BY(cs_desc_man); + WalletDescriptor m_wallet_descriptor GUARDED_BY(cs_desc_man); + + //! Same as 'TopUp' but designed for use within a batch transaction context + bool TopUpWithDB(WalletBatch& batch, unsigned int size = 0); public: DescriptorScriptPubKeyMan(WalletStorage& storage, WalletDescriptor& descriptor, int64_t keypool_size) From 3eb769f15013873755e482707cad341bc1ce8a8c Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 20 Jul 2023 18:10:41 -0300 Subject: [PATCH 09/37] wallet: batch legacy spkm TopUp Instead of performing multiple atomic write operations per legacy spkm setup call, batch them all within a single atomic db txn. --- src/wallet/scriptpubkeyman.cpp | 13 ++++++++----- src/wallet/scriptpubkeyman.h | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 997348d371..ce757a1c6b 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -333,7 +333,8 @@ bool LegacyScriptPubKeyMan::TopUpInactiveHDChain(const CKeyID seed_id, int64_t i chain.m_next_external_index = std::max(chain.m_next_external_index, index + 1); } - TopUpChain(chain, 0); + WalletBatch batch(m_storage.GetDatabase()); + TopUpChain(batch, chain, 0); return true; } @@ -1274,19 +1275,22 @@ bool LegacyScriptPubKeyMan::TopUp(unsigned int kpSize) return false; } - if (!TopUpChain(m_hd_chain, kpSize)) { + WalletBatch batch(m_storage.GetDatabase()); + if (!batch.TxnBegin()) return false; + if (!TopUpChain(batch, m_hd_chain, kpSize)) { return false; } for (auto& [chain_id, chain] : m_inactive_hd_chains) { - if (!TopUpChain(chain, kpSize)) { + if (!TopUpChain(batch, chain, kpSize)) { return false; } } + if (!batch.TxnCommit()) throw std::runtime_error(strprintf("Error during keypool top up. Cannot commit changes for wallet %s", m_storage.GetDisplayName())); NotifyCanGetAddressesChanged(); return true; } -bool LegacyScriptPubKeyMan::TopUpChain(CHDChain& chain, unsigned int kpSize) +bool LegacyScriptPubKeyMan::TopUpChain(WalletBatch& batch, CHDChain& chain, unsigned int kpSize) { LOCK(cs_KeyStore); @@ -1318,7 +1322,6 @@ bool LegacyScriptPubKeyMan::TopUpChain(CHDChain& chain, unsigned int kpSize) missingInternal = 0; } bool internal = false; - WalletBatch batch(m_storage.GetDatabase()); for (int64_t i = missingInternal + missingExternal; i--;) { if (i < missingInternal) { internal = true; diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 7bdfbf0d34..a0e9dbb6c2 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -366,7 +366,7 @@ class LegacyScriptPubKeyMan : public ScriptPubKeyMan, public FillableSigningProv */ bool TopUpInactiveHDChain(const CKeyID seed_id, int64_t index, bool internal); - bool TopUpChain(CHDChain& chain, unsigned int size); + bool TopUpChain(WalletBatch& batch, CHDChain& chain, unsigned int size); public: LegacyScriptPubKeyMan(WalletStorage& storage, int64_t keypool_size) : ScriptPubKeyMan(storage), m_keypool_size(keypool_size) {} From 1f65241b733cd1e962c88909ae66816bc6451fd1 Mon Sep 17 00:00:00 2001 From: furszy Date: Fri, 29 Sep 2023 15:20:25 -0300 Subject: [PATCH 10/37] wallet: descriptors setup, batch db operations Instead of doing one db transaction per descriptor setup, batch all descriptors' setup writes in a single db txn. Speeding up the process and preventing the wallet from entering an inconsistent state if any of the intermediate transactions fail. --- src/wallet/scriptpubkeyman.cpp | 7 +------ src/wallet/scriptpubkeyman.h | 2 +- src/wallet/wallet.cpp | 18 +++++++++++++++--- src/wallet/wallet.h | 3 +++ 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index ce757a1c6b..0b4800b848 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -2275,7 +2275,7 @@ bool DescriptorScriptPubKeyMan::AddDescriptorKeyWithDB(WalletBatch& batch, const } } -bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type, bool internal) +bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(WalletBatch& batch, const CExtKey& master_key, OutputType addr_type, bool internal) { LOCK(cs_desc_man); assert(m_storage.IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); @@ -2336,9 +2336,6 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ m_wallet_descriptor = w_desc; // Store the master private key, and descriptor - WalletBatch batch(m_storage.GetDatabase()); - if (!batch.TxnBegin()) throw std::runtime_error(std::string(__func__) + ": cannot start db transaction"); - if (!AddDescriptorKeyWithDB(batch, master_key.key, master_key.key.GetPubKey())) { throw std::runtime_error(std::string(__func__) + ": writing descriptor master private key failed"); } @@ -2350,8 +2347,6 @@ bool DescriptorScriptPubKeyMan::SetupDescriptorGeneration(const CExtKey& master_ TopUpWithDB(batch); m_storage.UnsetBlankWalletFlag(batch); - - if (!batch.TxnCommit()) throw std::runtime_error(std::string(__func__) + ": error committing db transaction"); return true; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index a0e9dbb6c2..936c76c223 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -621,7 +621,7 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan bool IsHDEnabled() const override; //! Setup descriptors based on the given CExtkey - bool SetupDescriptorGeneration(const CExtKey& master_key, OutputType addr_type, bool internal); + bool SetupDescriptorGeneration(WalletBatch& batch, const CExtKey& master_key, OutputType addr_type, bool internal); /** Provide a descriptor at setup time * Returns false if already setup or setup fails, true if setup is successful diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ecf18fbe78..4df34923d3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3535,6 +3535,10 @@ void CWallet::SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) { AssertLockHeld(cs_wallet); + // Create single batch txn + WalletBatch batch(GetDatabase()); + if (!batch.TxnBegin()) throw std::runtime_error("Error: cannot create db transaction for descriptors setup"); + for (bool internal : {false, true}) { for (OutputType t : OUTPUT_TYPES) { auto spk_manager = std::unique_ptr(new DescriptorScriptPubKeyMan(*this, m_keypool_size)); @@ -3542,16 +3546,19 @@ void CWallet::SetupDescriptorScriptPubKeyMans(const CExtKey& master_key) if (IsLocked()) { throw std::runtime_error(std::string(__func__) + ": Wallet is locked, cannot setup new descriptors"); } - if (!spk_manager->CheckDecryptionKey(vMasterKey) && !spk_manager->Encrypt(vMasterKey, nullptr)) { + if (!spk_manager->CheckDecryptionKey(vMasterKey) && !spk_manager->Encrypt(vMasterKey, &batch)) { throw std::runtime_error(std::string(__func__) + ": Could not encrypt new descriptors"); } } - spk_manager->SetupDescriptorGeneration(master_key, t, internal); + spk_manager->SetupDescriptorGeneration(batch, master_key, t, internal); uint256 id = spk_manager->GetID(); AddScriptPubKeyMan(id, std::move(spk_manager)); - AddActiveScriptPubKeyMan(id, t, internal); + AddActiveScriptPubKeyManWithDb(batch, id, t, internal); } } + + // Ensure information is committed to disk + if (!batch.TxnCommit()) throw std::runtime_error("Error: cannot commit db transaction for descriptors setup"); } void CWallet::SetupDescriptorScriptPubKeyMans() @@ -3606,6 +3613,11 @@ void CWallet::SetupDescriptorScriptPubKeyMans() void CWallet::AddActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal) { WalletBatch batch(GetDatabase()); + return AddActiveScriptPubKeyManWithDb(batch, id, type, internal); +} + +void CWallet::AddActiveScriptPubKeyManWithDb(WalletBatch& batch, uint256 id, OutputType type, bool internal) +{ if (!batch.WriteActiveScriptPubKeyMan(static_cast(type), id, internal)) { throw std::runtime_error(std::string(__func__) + ": writing active ScriptPubKeyMan id failed"); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bc45010200..11b7964316 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -419,6 +419,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati // Must be the only method adding data to it. void AddScriptPubKeyMan(const uint256& id, std::unique_ptr spkm_man); + // Same as 'AddActiveScriptPubKeyMan' but designed for use within a batch transaction context + void AddActiveScriptPubKeyManWithDb(WalletBatch& batch, uint256 id, OutputType type, bool internal); + /** * Catch wallet up to current chain, scanning new blocks, updating the best * block locator and m_last_block_processed, and registering for From f05302427386fe63f4929a7198652cb1e4ab3bcc Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 21 Nov 2023 23:07:00 -0300 Subject: [PATCH 11/37] wallet: batch external signer descriptor import Co-authored-by: furszy --- src/wallet/external_signer_scriptpubkeyman.cpp | 5 ++--- src/wallet/external_signer_scriptpubkeyman.h | 2 +- src/wallet/scriptpubkeyman.h | 5 ----- src/wallet/wallet.cpp | 11 +++++++++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/wallet/external_signer_scriptpubkeyman.cpp b/src/wallet/external_signer_scriptpubkeyman.cpp index 6d026d02c1..a71f8f9fbc 100644 --- a/src/wallet/external_signer_scriptpubkeyman.cpp +++ b/src/wallet/external_signer_scriptpubkeyman.cpp @@ -16,7 +16,7 @@ #include namespace wallet { -bool ExternalSignerScriptPubKeyMan::SetupDescriptor(std::unique_ptr desc) +bool ExternalSignerScriptPubKeyMan::SetupDescriptor(WalletBatch& batch, std::unique_ptr desc) { LOCK(cs_desc_man); assert(m_storage.IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); @@ -29,13 +29,12 @@ bool ExternalSignerScriptPubKeyMan::SetupDescriptor(std::unique_ptr m_wallet_descriptor = w_desc; // Store the descriptor - WalletBatch batch(m_storage.GetDatabase()); if (!batch.WriteDescriptor(GetID(), m_wallet_descriptor)) { throw std::runtime_error(std::string(__func__) + ": writing descriptor failed"); } // TopUp - TopUp(); + TopUpWithDB(batch); m_storage.UnsetBlankWalletFlag(batch); return true; diff --git a/src/wallet/external_signer_scriptpubkeyman.h b/src/wallet/external_signer_scriptpubkeyman.h index 01dc80b1ca..c052ce6129 100644 --- a/src/wallet/external_signer_scriptpubkeyman.h +++ b/src/wallet/external_signer_scriptpubkeyman.h @@ -23,7 +23,7 @@ class ExternalSignerScriptPubKeyMan : public DescriptorScriptPubKeyMan /** Provide a descriptor at setup time * Returns false if already setup or setup fails, true if setup is successful */ - bool SetupDescriptor(std::unique_ptrdesc); + bool SetupDescriptor(WalletBatch& batch, std::unique_ptrdesc); static ExternalSigner GetExternalSigner(); diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index 936c76c223..dccbf3ced6 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -623,11 +623,6 @@ class DescriptorScriptPubKeyMan : public ScriptPubKeyMan //! Setup descriptors based on the given CExtkey bool SetupDescriptorGeneration(WalletBatch& batch, const CExtKey& master_key, OutputType addr_type, bool internal); - /** Provide a descriptor at setup time - * Returns false if already setup or setup fails, true if setup is successful - */ - bool SetupDescriptor(std::unique_ptrdesc); - bool HavePrivateKeys() const override; std::optional GetOldestKeyPoolTime() const override; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4df34923d3..4ca1c75abf 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3585,6 +3585,10 @@ void CWallet::SetupDescriptorScriptPubKeyMans() UniValue signer_res = signer.GetDescriptors(account); if (!signer_res.isObject()) throw std::runtime_error(std::string(__func__) + ": Unexpected result"); + + WalletBatch batch(GetDatabase()); + if (!batch.TxnBegin()) throw std::runtime_error("Error: cannot create db transaction for descriptors import"); + for (bool internal : {false, true}) { const UniValue& descriptor_vals = signer_res.find_value(internal ? "internal" : "receive"); if (!descriptor_vals.isArray()) throw std::runtime_error(std::string(__func__) + ": Unexpected result"); @@ -3601,12 +3605,15 @@ void CWallet::SetupDescriptorScriptPubKeyMans() } OutputType t = *desc->GetOutputType(); auto spk_manager = std::unique_ptr(new ExternalSignerScriptPubKeyMan(*this, m_keypool_size)); - spk_manager->SetupDescriptor(std::move(desc)); + spk_manager->SetupDescriptor(batch, std::move(desc)); uint256 id = spk_manager->GetID(); AddScriptPubKeyMan(id, std::move(spk_manager)); - AddActiveScriptPubKeyMan(id, t, internal); + AddActiveScriptPubKeyManWithDb(batch, id, t, internal); } } + + // Ensure imported descriptors are committed to disk + if (!batch.TxnCommit()) throw std::runtime_error("Error: cannot commit db transaction for descriptors import"); } } From 3ea54e5db7d53da5afa321e1800c29aa269dd3b3 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Fri, 5 May 2023 11:14:51 +0200 Subject: [PATCH 12/37] net: Add continuous ASMap health check logging --- src/net.cpp | 19 +++++++++++++++++++ src/net.h | 3 +++ src/netgroup.cpp | 21 +++++++++++++++++++++ src/netgroup.h | 10 ++++++++++ test/functional/feature_asmap.py | 9 +++++++++ 5 files changed, 62 insertions(+) diff --git a/src/net.cpp b/src/net.cpp index 8c39eaab2b..7ca33efb8d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3305,6 +3305,12 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions) // Dump network addresses scheduler.scheduleEvery([this] { DumpAddresses(); }, DUMP_PEERS_INTERVAL); + // Run the ASMap Health check once and then schedule it to run every 24h. + if (m_netgroupman.UsingASMap()) { + ASMapHealthCheck(); + scheduler.scheduleEvery([this] { ASMapHealthCheck(); }, ASMAP_HEALTH_CHECK_INTERVAL); + } + return true; } @@ -3853,6 +3859,19 @@ void CConnman::PerformReconnections() } } +void CConnman::ASMapHealthCheck() +{ + const std::vector v4_addrs{GetAddresses(/*max_addresses=*/ 0, /*max_pct=*/ 0, Network::NET_IPV4, /*filtered=*/ false)}; + const std::vector v6_addrs{GetAddresses(/*max_addresses=*/ 0, /*max_pct=*/ 0, Network::NET_IPV6, /*filtered=*/ false)}; + std::vector clearnet_addrs; + clearnet_addrs.reserve(v4_addrs.size() + v6_addrs.size()); + std::transform(v4_addrs.begin(), v4_addrs.end(), std::back_inserter(clearnet_addrs), + [](const CAddress& addr) { return static_cast(addr); }); + std::transform(v6_addrs.begin(), v6_addrs.end(), std::back_inserter(clearnet_addrs), + [](const CAddress& addr) { return static_cast(addr); }); + m_netgroupman.ASMapHealthCheck(clearnet_addrs); +} + // Dump binary message to file, with timestamp. static void CaptureMessageToFile(const CAddress& addr, const std::string& msg_type, diff --git a/src/net.h b/src/net.h index 5a80896db4..8fcb9a0072 100644 --- a/src/net.h +++ b/src/net.h @@ -87,6 +87,8 @@ static const bool DEFAULT_BLOCKSONLY = false; static const int64_t DEFAULT_PEER_CONNECT_TIMEOUT = 60; /** Number of file descriptors required for message capture **/ static const int NUM_FDS_MESSAGE_CAPTURE = 1; +/** Interval for ASMap Health Check **/ +static constexpr std::chrono::hours ASMAP_HEALTH_CHECK_INTERVAL{24}; static constexpr bool DEFAULT_FORCEDNSSEED{false}; static constexpr bool DEFAULT_DNSSEED{true}; @@ -1138,6 +1140,7 @@ class CConnman void SetNetworkActive(bool active); void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant&& grant_outbound, const char* strDest, ConnectionType conn_type, bool use_v2transport) EXCLUSIVE_LOCKS_REQUIRED(!m_unused_i2p_sessions_mutex); bool CheckIncomingNonce(uint64_t nonce); + void ASMapHealthCheck(); // alias for thread safety annotations only, not defined RecursiveMutex& GetNodesMutex() const LOCK_RETURNED(m_nodes_mutex); diff --git a/src/netgroup.cpp b/src/netgroup.cpp index 2d4d231f2b..96e2b09746 100644 --- a/src/netgroup.cpp +++ b/src/netgroup.cpp @@ -5,6 +5,7 @@ #include #include +#include #include uint256 NetGroupManager::GetAsmapChecksum() const @@ -109,3 +110,23 @@ uint32_t NetGroupManager::GetMappedAS(const CNetAddr& address) const uint32_t mapped_as = Interpret(m_asmap, ip_bits); return mapped_as; } + +void NetGroupManager::ASMapHealthCheck(const std::vector& clearnet_addrs) const { + std::set clearnet_asns{}; + int unmapped_count{0}; + + for (const auto& addr : clearnet_addrs) { + uint32_t asn = GetMappedAS(addr); + if (asn == 0) { + ++unmapped_count; + continue; + } + clearnet_asns.insert(asn); + } + + LogPrintf("ASMap Health Check: %i clearnet peers are mapped to %i ASNs with %i peers being unmapped\n", clearnet_addrs.size(), clearnet_asns.size(), unmapped_count); +} + +bool NetGroupManager::UsingASMap() const { + return m_asmap.size() > 0; +} diff --git a/src/netgroup.h b/src/netgroup.h index 2dd63ec66b..5aa6ef7742 100644 --- a/src/netgroup.h +++ b/src/netgroup.h @@ -41,6 +41,16 @@ class NetGroupManager { */ uint32_t GetMappedAS(const CNetAddr& address) const; + /** + * Analyze and log current health of ASMap based buckets. + */ + void ASMapHealthCheck(const std::vector& clearnet_addrs) const; + + /** + * Indicates whether ASMap is being used for clearnet bucketing. + */ + bool UsingASMap() const; + private: /** Compressed IP->ASN mapping, loaded from a file when a node starts. * diff --git a/test/functional/feature_asmap.py b/test/functional/feature_asmap.py index 9cff8042a8..ae483fe449 100755 --- a/test/functional/feature_asmap.py +++ b/test/functional/feature_asmap.py @@ -111,6 +111,14 @@ def test_empty_asmap(self): self.node.assert_start_raises_init_error(extra_args=['-asmap'], expected_msg=msg) os.remove(self.default_asmap) + def test_asmap_health_check(self): + self.log.info('Test bitcoind -asmap logs ASMap Health Check with basic stats') + shutil.copyfile(self.asmap_raw, self.default_asmap) + msg = "ASMap Health Check: 2 clearnet peers are mapped to 1 ASNs with 0 peers being unmapped" + with self.node.assert_debug_log(expected_msgs=[msg]): + self.start_node(0, extra_args=['-asmap']) + os.remove(self.default_asmap) + def run_test(self): self.node = self.nodes[0] self.datadir = self.node.chain_path @@ -124,6 +132,7 @@ def run_test(self): self.test_asmap_interaction_with_addrman_containing_entries() self.test_default_asmap_with_missing_file() self.test_empty_asmap() + self.test_asmap_health_check() if __name__ == '__main__': From 55e3dc3e03510e97caba1547a82e3e022b0bbd42 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Sun, 3 Dec 2023 16:04:20 +0000 Subject: [PATCH 13/37] test: Fix test by checking the actual exception instance The BOOST_REQUIRE_THROW passes even if the command raises an exception in the underlying subprocess implementation, which might have a type derived from std::runtime_error. --- src/test/system_tests.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 740f461548..6a96b60db0 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -90,7 +90,13 @@ BOOST_AUTO_TEST_CASE(run_command) }); } { - BOOST_REQUIRE_THROW(RunCommandParseJSON("echo \"{\""), std::runtime_error); // Unable to parse JSON + // Unable to parse JSON +#ifdef WIN32 + const std::string command{"cmd.exe /c echo {"}; +#else + const std::string command{"echo {"}; +#endif + BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, HasReason("Unable to parse JSON: {")); } // Test std::in, except for Windows #ifndef WIN32 From fa83b65ef8934b44fbac02da8dbc27fc0bc230e6 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 4 Dec 2023 09:44:21 +0100 Subject: [PATCH 14/37] ci: Use Ubuntu 24.04 Noble for tsan,tidy,fuzz --- ci/test/00_setup_env_native_fuzz.sh | 2 +- ci/test/00_setup_env_native_tidy.sh | 2 +- ci/test/00_setup_env_native_tsan.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ci/test/00_setup_env_native_fuzz.sh b/ci/test/00_setup_env_native_fuzz.sh index 3585b2b417..abee3c1541 100755 --- a/ci/test/00_setup_env_native_fuzz.sh +++ b/ci/test/00_setup_env_native_fuzz.sh @@ -6,7 +6,7 @@ export LC_ALL=C.UTF-8 -export CI_IMAGE_NAME_TAG="docker.io/ubuntu:23.10" # This version will reach EOL in Jul 2024, and can be replaced by "ubuntu:24.04" (or anything else that ships the wanted clang version). +export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" export CONTAINER_NAME=ci_native_fuzz export PACKAGES="clang-17 llvm-17 libclang-rt-17-dev libevent-dev libboost-dev libsqlite3-dev" export NO_DEPENDS=1 diff --git a/ci/test/00_setup_env_native_tidy.sh b/ci/test/00_setup_env_native_tidy.sh index 6b0c708f19..c03392af06 100755 --- a/ci/test/00_setup_env_native_tidy.sh +++ b/ci/test/00_setup_env_native_tidy.sh @@ -6,7 +6,7 @@ export LC_ALL=C.UTF-8 -export CI_IMAGE_NAME_TAG="docker.io/ubuntu:23.10" # This version will reach EOL in Jul 2024, and can be replaced by "ubuntu:24.04" (or anything else that ships the wanted clang version). +export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" export CONTAINER_NAME=ci_native_tidy export TIDY_LLVM_V="17" export PACKAGES="clang-${TIDY_LLVM_V} libclang-${TIDY_LLVM_V}-dev llvm-${TIDY_LLVM_V}-dev libomp-${TIDY_LLVM_V}-dev clang-tidy-${TIDY_LLVM_V} jq bear cmake libevent-dev libboost-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev systemtap-sdt-dev libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools libqrencode-dev libsqlite3-dev libdb++-dev" diff --git a/ci/test/00_setup_env_native_tsan.sh b/ci/test/00_setup_env_native_tsan.sh index 67c29d4bc8..aa23bad809 100755 --- a/ci/test/00_setup_env_native_tsan.sh +++ b/ci/test/00_setup_env_native_tsan.sh @@ -7,7 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_tsan -export CI_IMAGE_NAME_TAG="docker.io/ubuntu:23.10" # This version will reach EOL in Jul 2024, and can be replaced by "ubuntu:24.04" (or anything else that ships the wanted clang version). +export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" export PACKAGES="clang-17 llvm-17 libclang-rt-17-dev libc++abi-17-dev libc++-17-dev python3-zmq" export DEP_OPTS="CC=clang-17 CXX='clang++-17 -stdlib=libc++'" export GOAL="install" From fad2392c5861a88a87cb8a03d2fc9773e178feb8 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 4 Dec 2023 12:35:04 +0100 Subject: [PATCH 15/37] ci: Use Ubuntu 24.04 Noble for asan --- .cirrus.yml | 4 ++-- ci/test/00_setup_env_native_asan.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 5c5942e2d5..eb2414dc0a 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -43,7 +43,7 @@ env: # Global defaults # The following specific types should exist, with the following requirements: # - small: For an x86_64 machine, recommended to have 2 CPUs and 8 GB of memory. # - medium: For an x86_64 machine, recommended to have 4 CPUs and 16 GB of memory. -# - mantic: For a machine running the Linux kernel shipped with exaclty Ubuntu Mantic 23.10. The machine is recommended to have 4 CPUs and 16 GB of memory. +# - noble: For a machine running the Linux kernel shipped with exaclty Ubuntu Noble 24.04. The machine is recommended to have 4 CPUs and 16 GB of memory. # - arm64: For an aarch64 machine, recommended to have 2 CPUs and 8 GB of memory. # https://cirrus-ci.org/guide/tips-and-tricks/#sharing-configuration-between-tasks @@ -168,7 +168,7 @@ task: << : *GLOBAL_TASK_TEMPLATE persistent_worker: labels: - type: mantic # Must use this specific worker (needed for USDT functional tests) + type: noble # Must use this specific worker (needed for USDT functional tests) env: FILE_ENV: "./ci/test/00_setup_env_native_asan.sh" diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh index 93d84bf17e..60486f8f61 100755 --- a/ci/test/00_setup_env_native_asan.sh +++ b/ci/test/00_setup_env_native_asan.sh @@ -6,6 +6,7 @@ export LC_ALL=C.UTF-8 +export CI_IMAGE_NAME_TAG="docker.io/ubuntu:24.04" # Only install BCC tracing packages in Cirrus CI. if [[ "${CIRRUS_CI}" == "true" ]]; then BPFCC_PACKAGE="bpfcc-tools linux-headers-$(uname --kernel-release)" @@ -17,7 +18,6 @@ fi export CONTAINER_NAME=ci_native_asan export PACKAGES="systemtap-sdt-dev clang-17 llvm-17 libclang-rt-17-dev python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev libboost-dev libdb5.3++-dev libminiupnpc-dev libnatpmp-dev libzmq3-dev libqrencode-dev libsqlite3-dev ${BPFCC_PACKAGE}" -export CI_IMAGE_NAME_TAG="docker.io/ubuntu:23.10" # This version will reach EOL in Jul 2024, and can be replaced by "ubuntu:24.04" (or anything else that ships the wanted clang version). export NO_DEPENDS=1 export GOAL="install" export BITCOIN_CONFIG="--enable-c++20 --enable-usdt --enable-zmq --with-incompatible-bdb --with-gui=qt5 \ From 8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b Mon Sep 17 00:00:00 2001 From: willcl-ark Date: Mon, 27 Nov 2023 10:29:02 +0000 Subject: [PATCH 16/37] init: don't delete PID file if it was not generated Previously, starting a second bitcoind using the same datadir would correctly fail to init and shutdown. However during shutdown the PID file belonging to the first instance would be erroneously removed by the second process shutting down. Fix this to only delete the PID file if we created it. --- src/init.cpp | 25 ++++++++++++++++++------- test/functional/feature_filelock.py | 3 +++ 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 526a8d6271..ac52b34fc5 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -155,6 +155,11 @@ static const char* DEFAULT_ASMAP_FILENAME="ip_asn.map"; * The PID file facilities. */ static const char* BITCOIN_PID_FILENAME = "bitcoind.pid"; +/** + * True if this process has created a PID file. + * Used to determine whether we should remove the PID file on shutdown. + */ +static bool g_generated_pid{false}; static fs::path GetPidFile(const ArgsManager& args) { @@ -170,12 +175,24 @@ static fs::path GetPidFile(const ArgsManager& args) #else tfm::format(file, "%d\n", getpid()); #endif + g_generated_pid = true; return true; } else { return InitError(strprintf(_("Unable to create the PID file '%s': %s"), fs::PathToString(GetPidFile(args)), SysErrorString(errno))); } } +static void RemovePidFile(const ArgsManager& args) +{ + if (!g_generated_pid) return; + const auto pid_path{GetPidFile(args)}; + if (std::error_code error; !fs::remove(pid_path, error)) { + std::string msg{error ? error.message() : "File does not exist"}; + LogPrintf("Unable to remove PID file (%s): %s\n", fs::PathToString(pid_path), msg); + } +} + + ////////////////////////////////////////////////////////////////////////////// // // Shutdown @@ -352,13 +369,7 @@ void Shutdown(NodeContext& node) node.scheduler.reset(); node.kernel.reset(); - try { - if (!fs::remove(GetPidFile(*node.args))) { - LogPrintf("%s: Unable to remove PID file: File does not exist\n", __func__); - } - } catch (const fs::filesystem_error& e) { - LogPrintf("%s: Unable to remove PID file: %s\n", __func__, fsbridge::get_filesystem_error_message(e)); - } + RemovePidFile(*node.args); LogPrintf("%s: done\n", __func__); } diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py index 0f4be54d0e..567207915e 100755 --- a/test/functional/feature_filelock.py +++ b/test/functional/feature_filelock.py @@ -30,8 +30,11 @@ def run_test(self): expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['PACKAGE_NAME']} is probably already running." self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir_path}', '-noserver'], expected_msg=expected_msg) + self.log.info("Check that cookie and PID file are not deleted when attempting to start a second bitcoind using the same datadir") cookie_file = datadir / ".cookie" assert cookie_file.exists() # should not be deleted during the second bitcoind instance shutdown + pid_file = datadir / "bitcoind.pid" + assert pid_file.exists() if self.is_wallet_compiled(): def check_wallet_filelock(descriptors): From 90389c95e9edf3d705fb9376388c83f07d1a570e Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 4 Dec 2023 14:39:58 +0000 Subject: [PATCH 17/37] depends: Build `capnp` package with CMake This change fixes the `capnp` package cross-compiling for the `x86_64-w64-mingw32` and `arm64-apple-darwin` platforms. --- depends/packages/capnp.mk | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/depends/packages/capnp.mk b/depends/packages/capnp.mk index 47df202771..1c1a77eb16 100644 --- a/depends/packages/capnp.mk +++ b/depends/packages/capnp.mk @@ -4,18 +4,15 @@ $(package)_download_path=$(native_$(package)_download_path) $(package)_download_file=$(native_$(package)_download_file) $(package)_file_name=$(native_$(package)_file_name) $(package)_sha256_hash=$(native_$(package)_sha256_hash) -$(package)_dependencies=native_$(package) define $(package)_set_vars := -$(package)_config_opts := --with-external-capnp -$(package)_config_opts += --without-openssl -$(package)_config_opts += CAPNP="$$(native_capnp_prefixbin)/capnp" -$(package)_config_opts += CAPNP_CXX="$$(native_capnp_prefixbin)/capnp-c++" -$(package)_config_opts_android := --disable-shared +$(package)_config_opts := -DBUILD_TESTING=OFF +$(package)_config_opts += -DWITH_OPENSSL=OFF +$(package)_config_opts += -DWITH_ZLIB=OFF endef define $(package)_config_cmds - $($(package)_autoconf) + $($(package)_cmake) . endef define $(package)_build_cmds @@ -25,3 +22,7 @@ endef define $(package)_stage_cmds $(MAKE) DESTDIR=$($(package)_staging_dir) install endef + +define $(package)_postprocess_cmds + rm -rf lib/pkgconfig +endef From 11d797e3a078b8f5f0039a1073047d3f0a8c6cdc Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 13 Nov 2023 14:35:52 +0000 Subject: [PATCH 18/37] depends: Build `native_capnp` package with CMake --- depends/packages/native_capnp.mk | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/depends/packages/native_capnp.mk b/depends/packages/native_capnp.mk index ad87eed354..484e78d5d9 100644 --- a/depends/packages/native_capnp.mk +++ b/depends/packages/native_capnp.mk @@ -6,11 +6,13 @@ $(package)_file_name=capnproto-cxx-$($(package)_version).tar.gz $(package)_sha256_hash=0f7f4b8a76a2cdb284fddef20de8306450df6dd031a47a15ac95bc43c3358e09 define $(package)_set_vars - $(package)_config_opts = --without-openssl + $(package)_config_opts := -DBUILD_TESTING=OFF + $(package)_config_opts += -DWITH_OPENSSL=OFF + $(package)_config_opts += -DWITH_ZLIB=OFF endef define $(package)_config_cmds - $($(package)_autoconf) + $($(package)_cmake) . endef define $(package)_build_cmds @@ -20,3 +22,7 @@ endef define $(package)_stage_cmds $(MAKE) DESTDIR=$($(package)_staging_dir) install endef + +define $(package)_postprocess_cmds + rm -rf lib/pkgconfig +endef From 38816ff64ed90a55e4879e9b440cdc876302f750 Mon Sep 17 00:00:00 2001 From: Greg Sanders Date: Mon, 4 Dec 2023 14:42:13 -0500 Subject: [PATCH 19/37] fuzz: txorphan check wtxids using GenTxid::Wtxid not GenTxid::Txid --- src/test/fuzz/txorphan.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp index e9ceb299fe..714b321bce 100644 --- a/src/test/fuzz/txorphan.cpp +++ b/src/test/fuzz/txorphan.cpp @@ -91,13 +91,13 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) { CTransactionRef ref = orphanage.GetTxToReconsider(peer_id); if (ref) { - bool have_tx = orphanage.HaveTx(GenTxid::Txid(ref->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(ref->GetHash())); + bool have_tx = orphanage.HaveTx(GenTxid::Txid(ref->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(ref->GetWitnessHash())); Assert(have_tx); } } }, [&] { - bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); + bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash())); // AddTx should return false if tx is too big or already have it // tx weight is unknown, we only check when tx is already in orphanage { @@ -105,7 +105,7 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) // have_tx == true -> add_tx == false Assert(!have_tx || !add_tx); } - have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); + have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash())); { bool add_tx = orphanage.AddTx(tx, peer_id); // if have_tx is still false, it must be too big @@ -114,12 +114,12 @@ FUZZ_TARGET(txorphan, .init = initialize_orphanage) } }, [&] { - bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); + bool have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash())); // EraseTx should return 0 if m_orphans doesn't have the tx { Assert(have_tx == orphanage.EraseTx(tx->GetHash())); } - have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetHash())); + have_tx = orphanage.HaveTx(GenTxid::Txid(tx->GetHash())) || orphanage.HaveTx(GenTxid::Wtxid(tx->GetWitnessHash())); // have_tx should be false and EraseTx should fail { Assert(!have_tx && !orphanage.EraseTx(tx->GetHash())); From 423949a13b39a193dff8b2758d23d6691d11dbc3 Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 15 Nov 2023 11:22:20 +0000 Subject: [PATCH 20/37] depends: add -platform_version to macOS build flags ```bash -platform_version platform min_version sdk_version This is set to indicate the platform, oldest supported version of that platform that output is to be used on, and the SDK that the output was built against. ``` --- depends/hosts/darwin.mk | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk index ecd45540cf..e21af00899 100644 --- a/depends/hosts/darwin.mk +++ b/depends/hosts/darwin.mk @@ -71,6 +71,10 @@ $(foreach TOOL,$(cctools_TOOLS),$(eval darwin_$(TOOL) = $$(build_prefix)/bin/$$( # # Adds the desired paths from the SDK # +# -platform_version +# +# Indicate to the linker the platform, the oldest supported version, +# and the SDK used. darwin_CC=env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH \ -u OBJC_INCLUDE_PATH -u OBJCPLUS_INCLUDE_PATH -u CPATH \ @@ -91,6 +95,7 @@ darwin_CXX=env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH \ darwin_CFLAGS=-pipe -std=$(C_STANDARD) darwin_CXXFLAGS=-pipe -std=$(CXX_STANDARD) +darwin_LDFLAGS=-Wl,-platform_version,macos,$(OSX_MIN_VERSION),$(OSX_SDK_VERSION) ifneq ($(LTO),) darwin_CFLAGS += -flto From 51c97ffb6989a4cf56ad966d360a9fa0426e174c Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 5 Dec 2023 09:54:55 +0000 Subject: [PATCH 21/37] build: patch boost process for macOS 14 SDK --- depends/packages/boost.mk | 5 ++++ depends/patches/boost/process_macos_sdk.patch | 27 +++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 depends/patches/boost/process_macos_sdk.patch diff --git a/depends/packages/boost.mk b/depends/packages/boost.mk index ebc097d686..ab43764b38 100644 --- a/depends/packages/boost.mk +++ b/depends/packages/boost.mk @@ -3,6 +3,11 @@ $(package)_version=1.81.0 $(package)_download_path=https://boostorg.jfrog.io/artifactory/main/release/$($(package)_version)/source/ $(package)_file_name=boost_$(subst .,_,$($(package)_version)).tar.bz2 $(package)_sha256_hash=71feeed900fbccca04a3b4f2f84a7c217186f28a940ed8b7ed4725986baf99fa +$(package)_patches=process_macos_sdk.patch + +define $(package)_preprocess_cmds + patch -p1 < $($(package)_patch_dir)/process_macos_sdk.patch +endef define $(package)_stage_cmds mkdir -p $($(package)_staging_prefix_dir)/include && \ diff --git a/depends/patches/boost/process_macos_sdk.patch b/depends/patches/boost/process_macos_sdk.patch new file mode 100644 index 0000000000..ebc556d972 --- /dev/null +++ b/depends/patches/boost/process_macos_sdk.patch @@ -0,0 +1,27 @@ +Fix Boost Process compilation with macOS 14 SDK. +Can be dropped with Boost 1.84.0. +https://github.com/boostorg/process/pull/343. +https://github.com/boostorg/process/issues/342. + +diff --git a/boost/process/detail/posix/handles.hpp b/boost/process/detail/posix/handles.hpp +index cd9e1ce5a..304e77b1c 100644 +--- a/boost/process/detail/posix/handles.hpp ++++ b/boost/process/detail/posix/handles.hpp +@@ -33,7 +33,7 @@ inline std::vector get_handles(std::error_code & ec) + else + ec.clear(); + +- auto my_fd = ::dirfd(dir.get()); ++ auto my_fd = dirfd(dir.get()); + + struct ::dirent * ent_p; + +@@ -117,7 +117,7 @@ struct limit_handles_ : handler_base_ext + return; + } + +- auto my_fd = ::dirfd(dir); ++ auto my_fd = dirfd(dir); + struct ::dirent * ent_p; + + while ((ent_p = readdir(dir)) != nullptr) From 8ea45e626e5a0482ee11d4376f961d8126ce7c7b Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 9 Oct 2023 12:21:08 +0100 Subject: [PATCH 22/37] build: use macOS 14 SDK (Xcode 15.0) --- ci/test/00_setup_env_mac.sh | 4 ++-- contrib/devtools/symbol-check.py | 2 +- contrib/macdeploy/README.md | 34 +++++++++++++------------------- contrib/macdeploy/gen-sdk | 7 +------ depends/hosts/darwin.mk | 6 +++--- 5 files changed, 21 insertions(+), 32 deletions(-) diff --git a/ci/test/00_setup_env_mac.sh b/ci/test/00_setup_env_mac.sh index d97a292778..1651c5ec26 100755 --- a/ci/test/00_setup_env_mac.sh +++ b/ci/test/00_setup_env_mac.sh @@ -12,8 +12,8 @@ export CONTAINER_NAME=ci_macos_cross export CI_IMAGE_NAME_TAG="docker.io/ubuntu:22.04" export HOST=x86_64-apple-darwin export PACKAGES="cmake zip" -export XCODE_VERSION=12.2 -export XCODE_BUILD_ID=12B45b +export XCODE_VERSION=15.0 +export XCODE_BUILD_ID=15A240d export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false export GOAL="deploy" diff --git a/contrib/devtools/symbol-check.py b/contrib/devtools/symbol-check.py index e3e8e398c2..ff1678116d 100755 --- a/contrib/devtools/symbol-check.py +++ b/contrib/devtools/symbol-check.py @@ -235,7 +235,7 @@ def check_MACHO_min_os(binary) -> bool: return False def check_MACHO_sdk(binary) -> bool: - if binary.build_version.sdk == [11, 0, 0]: + if binary.build_version.sdk == [14, 0, 0]: return True return False diff --git a/contrib/macdeploy/README.md b/contrib/macdeploy/README.md index 16fb0dad21..ea599df3d8 100644 --- a/contrib/macdeploy/README.md +++ b/contrib/macdeploy/README.md @@ -14,51 +14,45 @@ When complete, it will have produced `Bitcoin-Core.zip`. A free Apple Developer Account is required to proceed. -Our current macOS SDK -(`Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers.tar.gz`) -can be extracted from -[Xcode_12.2.xip](https://download.developer.apple.com/Developer_Tools/Xcode_12.2/Xcode_12.2.xip). +Our macOS SDK can be extracted from +[Xcode_15.xip](https://download.developer.apple.com/Developer_Tools/Xcode_15/Xcode_15.xip). Alternatively, after logging in to your account go to 'Downloads', then 'More' -and search for [`Xcode 12.2`](https://developer.apple.com/download/all/?q=Xcode%2012.2). +and search for [`Xcode 15`](https://developer.apple.com/download/all/?q=Xcode%2015). An Apple ID and cookies enabled for the hostname are needed to download this. -The `sha256sum` of the downloaded XIP archive should be `28d352f8c14a43d9b8a082ac6338dc173cb153f964c6e8fb6ba389e5be528bd0`. +The `sha256sum` of the downloaded XIP archive should be `4daaed2ef2253c9661779fa40bfff50655dc7ec45801aba5a39653e7bcdde48e`. -After Xcode version 7.x, Apple started shipping the `Xcode.app` in a `.xip` -archive. This makes the SDK less-trivial to extract on non-macOS machines. One -approach (tested on Debian Buster) is outlined below: +To extract the `.xip` on Linux: ```bash # Install/clone tools needed for extracting Xcode.app apt install cpio git clone https://github.com/bitcoin-core/apple-sdk-tools.git -# Unpack Xcode_12.2.xip and place the resulting Xcode.app in your current +# Unpack the .xip and place the resulting Xcode.app in your current # working directory -python3 apple-sdk-tools/extract_xcode.py -f Xcode_12.2.xip | cpio -d -i +python3 apple-sdk-tools/extract_xcode.py -f Xcode_15.xip | cpio -d -i ``` -On macOS the process is more straightforward: +On macOS: ```bash -xip -x Xcode_12.2.xip +xip -x Xcode_15.xip ``` -### Step 2: Generating `Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers.tar.gz` from `Xcode.app` +### Step 2: Generating the SDK tarball from `Xcode.app` -To generate `Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers.tar.gz`, run -the script [`gen-sdk`](./gen-sdk) with the path to `Xcode.app` (extracted in the -previous stage) as the first argument. +To generate the SDK, run the script [`gen-sdk`](./gen-sdk) with the +path to `Xcode.app` (extracted in the previous stage) as the first argument. ```bash -# Generate a Xcode-12.2-12B45b-extracted-SDK-with-libcxx-headers.tar.gz from -# the supplied Xcode.app ./contrib/macdeploy/gen-sdk '/path/to/Xcode.app' ``` -The `sha256sum` of the generated TAR.GZ archive should be `df75d30ecafc429e905134333aeae56ac65fac67cb4182622398fd717df77619`. +The generated archive should be: `Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers.tar.gz`. +The `sha256sum` should be `c0c2e7bb92c1fee0c4e9f3a485e4530786732d6c6dd9e9f418c282aa6892f55d`. ## Deterministic macOS App Notes diff --git a/contrib/macdeploy/gen-sdk b/contrib/macdeploy/gen-sdk index 6efaaccb8e..b73f5cba14 100755 --- a/contrib/macdeploy/gen-sdk +++ b/contrib/macdeploy/gen-sdk @@ -62,9 +62,6 @@ def run(): out_name = "Xcode-{xcode_version}-{xcode_build_id}-extracted-SDK-with-libcxx-headers".format(xcode_version=xcode_version, xcode_build_id=xcode_build_id) - xcode_libcxx_dir = xcode_app.joinpath("Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/include/c++/v1") - assert xcode_libcxx_dir.is_dir() - if args.out_sdktgz: out_sdktgz_path = pathlib.Path(args.out_sdktgz_path) else: @@ -72,7 +69,7 @@ def run(): out_sdktgz_path = pathlib.Path("./{}.tar.gz".format(out_name)) def tarfp_add_with_base_change(tarfp, dir_to_add, alt_base_dir): - """Add all files in dir_to_add to tarfp, but prepent MEMBERPREFIX to the files' + """Add all files in dir_to_add to tarfp, but prepent alt_base_dir to the files' names e.g. if the only file under /root/bazdir is /root/bazdir/qux, invoking: @@ -107,8 +104,6 @@ def run(): with tarfile.open(mode="w", fileobj=gzf, format=tarfile.GNU_FORMAT) as tarfp: print("Adding MacOSX SDK {} files...".format(sdk_version)) tarfp_add_with_base_change(tarfp, sdk_dir, out_name) - print("Adding libc++ headers...") - tarfp_add_with_base_change(tarfp, xcode_libcxx_dir, "{}/usr/include/c++/v1".format(out_name)) print("Done! Find the resulting gzipped tarball at:") print(out_sdktgz_path.resolve()) diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk index e21af00899..a89c82e408 100644 --- a/depends/hosts/darwin.mk +++ b/depends/hosts/darwin.mk @@ -1,7 +1,7 @@ OSX_MIN_VERSION=11.0 -OSX_SDK_VERSION=11.0 -XCODE_VERSION=12.2 -XCODE_BUILD_ID=12B45b +OSX_SDK_VERSION=14.0 +XCODE_VERSION=15.0 +XCODE_BUILD_ID=15A240d LD64_VERSION=711 OSX_SDK=$(SDK_PATH)/Xcode-$(XCODE_VERSION)-$(XCODE_BUILD_ID)-extracted-SDK-with-libcxx-headers From 494a926d05df44b60b3bc1145ad2a64acf96f61b Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 5 Dec 2023 13:07:39 -0500 Subject: [PATCH 23/37] rpc: fix getrawtransaction segfault The crash would happen when querying a mempool transaction with verbosity=2, while pruning. --- src/rpc/rawtransaction.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 2df2999523..5f68e7832c 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -396,7 +396,7 @@ static RPCHelpMan getrawtransaction() LOCK(cs_main); blockindex = chainman.m_blockman.LookupBlockIndex(hash_block); } - if (verbosity == 1) { + if (verbosity == 1 || !blockindex) { TxToJSON(*tx, hash_block, result, chainman.ActiveChainstate()); return result; } @@ -405,8 +405,7 @@ static RPCHelpMan getrawtransaction() CBlock block; const bool is_block_pruned{WITH_LOCK(cs_main, return chainman.m_blockman.IsBlockPruned(blockindex))}; - if (tx->IsCoinBase() || - !blockindex || is_block_pruned || + if (tx->IsCoinBase() || is_block_pruned || !(chainman.m_blockman.UndoReadFromDisk(blockUndo, *blockindex) && chainman.m_blockman.ReadBlockFromDisk(block, *blockindex))) { TxToJSON(*tx, hash_block, result, chainman.ActiveChainstate()); return result; From ca09415e630f0f7de9160cab234bd5ba6968ff2d Mon Sep 17 00:00:00 2001 From: furszy Date: Thu, 30 Nov 2023 21:29:58 -0300 Subject: [PATCH 24/37] rpc, doc: encryptwallet, mention HD seed rotation and new backup Better to notify users about the HD seed rotation and the new backup requirement before executing the encryption process. Ensuring they are prepared to update previous backups and securely safeguard the updated wallet file. Co-authored-by: jonatack --- src/wallet/rpc/encrypt.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpc/encrypt.cpp b/src/wallet/rpc/encrypt.cpp index 0226d15698..e237286603 100644 --- a/src/wallet/rpc/encrypt.cpp +++ b/src/wallet/rpc/encrypt.cpp @@ -220,7 +220,11 @@ RPCHelpMan encryptwallet() "After this, any calls that interact with private keys such as sending or signing \n" "will require the passphrase to be set prior the making these calls.\n" "Use the walletpassphrase call for this, and then walletlock call.\n" - "If the wallet is already encrypted, use the walletpassphrasechange call.\n", + "If the wallet is already encrypted, use the walletpassphrasechange call.\n" + "** IMPORTANT **\n" + "For security reasons, the encryption process will generate a new HD seed, resulting\n" + "in the creation of a fresh set of active descriptors. Therefore, it is crucial to\n" + "securely back up the newly generated wallet file using the backupwallet RPC.\n", { {"passphrase", RPCArg::Type::STR, RPCArg::Optional::NO, "The pass phrase to encrypt the wallet with. It must be at least 1 character, but should be long."}, }, @@ -268,7 +272,7 @@ RPCHelpMan encryptwallet() throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Error: Failed to encrypt the wallet."); } - return "wallet encrypted; The keypool has been flushed and a new HD seed was generated (if you are using HD). You need to make a new backup."; + return "wallet encrypted; The keypool has been flushed and a new HD seed was generated. You need to make a new backup with the backupwallet RPC."; }, }; } From 9075a446461ccbc446d21af778aac50b604f39b3 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Tue, 5 Dec 2023 16:36:43 -0500 Subject: [PATCH 25/37] test: add regression test for the getrawtransaction segfault This fails on master without the previous commit. --- test/functional/rpc_rawtransaction.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 2395935620..c12865b5e3 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -32,6 +32,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, ) from test_framework.wallet import ( @@ -70,7 +71,7 @@ def set_test_params(self): self.extra_args = [ ["-txindex"], ["-txindex"], - [], + ["-fastprune", "-prune=1"], ] # whitelist all peers to speed up tx relay / mempool sync for args in self.extra_args: @@ -85,7 +86,6 @@ def run_test(self): self.wallet = MiniWallet(self.nodes[0]) self.getrawtransaction_tests() - self.getrawtransaction_verbosity_tests() self.createrawtransaction_tests() self.sendrawtransaction_tests() self.sendrawtransaction_testmempoolaccept_tests() @@ -94,6 +94,8 @@ def run_test(self): if self.is_specified_wallet_compiled() and not self.options.descriptors: self.import_deterministic_coinbase_privkeys() self.raw_multisig_transaction_legacy_tests() + self.getrawtransaction_verbosity_tests() + def getrawtransaction_tests(self): tx = self.wallet.send_self_transfer(from_node=self.nodes[0]) @@ -243,6 +245,13 @@ def getrawtransaction_verbosity_tests(self): coin_base = self.nodes[1].getblock(block1)['tx'][0] gottx = self.nodes[1].getrawtransaction(txid=coin_base, verbosity=2, blockhash=block1) assert 'fee' not in gottx + # check that verbosity 2 for a mempool tx will fallback to verbosity 1 + # Do this with a pruned chain, as a regression test for https://github.com/bitcoin/bitcoin/pull/29003 + self.generate(self.nodes[2], 400) + assert_greater_than(self.nodes[2].pruneblockchain(250), 0) + mempool_tx = self.wallet.send_self_transfer(from_node=self.nodes[2])['txid'] + gottx = self.nodes[2].getrawtransaction(txid=mempool_tx, verbosity=2) + assert 'fee' not in gottx def createrawtransaction_tests(self): self.log.info("Test createrawtransaction") From 00e0658e77f66103ebdeb29def99dc9f937c049d Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Wed, 6 Dec 2023 00:15:57 +0100 Subject: [PATCH 26/37] test: fix v2 transport intermittent test failure (#29002) Only relying on the number of peers for detecting a new connection suffers from race conditions, as unrelated previous peers could disconnect at anytime in-between. Use the more robust approach of watching for an increased highest peer id instead (again using the `getpeerinfo` RPC call), with a newly introduced context manager method `TestNode.wait_for_new_peer()`. Fixes #29009. --- test/functional/p2p_v2_transport.py | 16 ++++++++-------- test/functional/test_framework/test_node.py | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/test/functional/p2p_v2_transport.py b/test/functional/p2p_v2_transport.py index 1a3b4a6d0a..72d22cb77f 100755 --- a/test/functional/p2p_v2_transport.py +++ b/test/functional/p2p_v2_transport.py @@ -133,9 +133,8 @@ def run_test(self): V1_PREFIX = MAGIC_BYTES["regtest"] + b"version\x00\x00\x00\x00\x00" assert_equal(len(V1_PREFIX), 16) with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - num_peers = len(self.nodes[0].getpeerinfo()) - s.connect(("127.0.0.1", p2p_port(0))) - self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers + 1) + with self.nodes[0].wait_for_new_peer(): + s.connect(("127.0.0.1", p2p_port(0))) s.sendall(V1_PREFIX[:-1]) assert_equal(self.nodes[0].getpeerinfo()[-1]["transport_protocol_type"], "detecting") s.sendall(bytes([V1_PREFIX[-1]])) # send out last prefix byte @@ -144,22 +143,23 @@ def run_test(self): # Check wrong network prefix detection (hits if the next 12 bytes correspond to a v1 version message) wrong_network_magic_prefix = MAGIC_BYTES["signet"] + V1_PREFIX[4:] with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - s.connect(("127.0.0.1", p2p_port(0))) + with self.nodes[0].wait_for_new_peer(): + s.connect(("127.0.0.1", p2p_port(0))) with self.nodes[0].assert_debug_log(["V2 transport error: V1 peer with wrong MessageStart"]): s.sendall(wrong_network_magic_prefix + b"somepayload") # Check detection of missing garbage terminator (hits after fixed amount of data if terminator never matches garbage) MAX_KEY_GARB_AND_GARBTERM_LEN = 64 + 4095 + 16 with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: - num_peers = len(self.nodes[0].getpeerinfo()) - s.connect(("127.0.0.1", p2p_port(0))) - self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers + 1) + with self.nodes[0].wait_for_new_peer(): + s.connect(("127.0.0.1", p2p_port(0))) s.sendall(b'\x00' * (MAX_KEY_GARB_AND_GARBTERM_LEN - 1)) self.wait_until(lambda: self.nodes[0].getpeerinfo()[-1]["bytesrecv"] == MAX_KEY_GARB_AND_GARBTERM_LEN - 1) with self.nodes[0].assert_debug_log(["V2 transport error: missing garbage terminator"]): + peer_id = self.nodes[0].getpeerinfo()[-1]["id"] s.sendall(b'\x00') # send out last byte # should disconnect immediately - self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers) + self.wait_until(lambda: not peer_id in [p["id"] for p in self.nodes[0].getpeerinfo()]) if __name__ == '__main__': diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 850aa20db2..90c1213deb 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -519,6 +519,24 @@ def wait_for_debug_log(self, expected_msgs, timeout=60): 'Expected messages "{}" does not partially match log:\n\n{}\n\n'.format( str(expected_msgs), print_log)) + @contextlib.contextmanager + def wait_for_new_peer(self, timeout=5): + """ + Wait until the node is connected to at least one new peer. We detect this + by watching for an increased highest peer id, using the `getpeerinfo` RPC call. + Note that the simpler approach of only accounting for the number of peers + suffers from race conditions, as disconnects from unrelated previous peers + could happen anytime in-between. + """ + def get_highest_peer_id(): + peer_info = self.getpeerinfo() + return peer_info[-1]["id"] if peer_info else -1 + + initial_peer_id = get_highest_peer_id() + yield + wait_until_helper_internal(lambda: get_highest_peer_id() > initial_peer_id, + timeout=timeout, timeout_factor=self.timeout_factor) + @contextlib.contextmanager def profile_with_perf(self, profile_name: str): """ From fad1903b8a85506378101c1f857ba47b4a058fb4 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Wed, 6 Dec 2023 15:47:39 +0100 Subject: [PATCH 27/37] fuzz: Avoid timeout in bitdeque --- src/test/fuzz/bitdeque.cpp | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/test/fuzz/bitdeque.cpp b/src/test/fuzz/bitdeque.cpp index 65f5cb3fd0..d5cc9cfd34 100644 --- a/src/test/fuzz/bitdeque.cpp +++ b/src/test/fuzz/bitdeque.cpp @@ -53,21 +53,11 @@ FUZZ_TARGET(bitdeque, .init = InitRandData) --initlen; } - LIMITED_WHILE(provider.remaining_bytes() > 0, 900) + const auto iter_limit{maxlen > 6000 ? 90U : 900U}; + LIMITED_WHILE(provider.remaining_bytes() > 0, iter_limit) { - { - assert(deq.size() == bitdeq.size()); - auto it = deq.begin(); - auto bitit = bitdeq.begin(); - auto itend = deq.end(); - while (it != itend) { - assert(*it == *bitit); - ++it; - ++bitit; - } - } - - CallOneOf(provider, + CallOneOf( + provider, [&] { // constructor() deq = std::deque{}; @@ -535,7 +525,17 @@ FUZZ_TARGET(bitdeque, .init = InitRandData) assert(it == deq.begin() + before); assert(bitit == bitdeq.begin() + before); } - } - ); + }); + } + { + assert(deq.size() == bitdeq.size()); + auto it = deq.begin(); + auto bitit = bitdeq.begin(); + auto itend = deq.end(); + while (it != itend) { + assert(*it == *bitit); + ++it; + ++bitit; + } } } From 97c0dfa8942c7fd62c920deee03b4d0c59aba641 Mon Sep 17 00:00:00 2001 From: Sergi Delgado Segura Date: Thu, 14 Sep 2023 16:05:51 -0400 Subject: [PATCH 28/37] test: Extends MEMPOOL msg functional test Currently, p2p_filter.py::test_msg_mempool is not testing much. This extends the tests so the interaction between sending MEMPOOL messages with a filter that does not include all transactions in the mempool reacts, plus how it interacts with INV messages --- test/functional/p2p_filter.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/test/functional/p2p_filter.py b/test/functional/p2p_filter.py index 665f57365f..62d55cc101 100755 --- a/test/functional/p2p_filter.py +++ b/test/functional/p2p_filter.py @@ -11,6 +11,7 @@ COIN, MAX_BLOOM_FILTER_SIZE, MAX_BLOOM_HASH_FUNCS, + MSG_WTX, MSG_BLOCK, MSG_FILTERED_BLOCK, msg_filteradd, @@ -135,14 +136,22 @@ def test_msg_mempool(self): self.log.info("Check that a node with bloom filters enabled services p2p mempool messages") filter_peer = P2PBloomFilter() - self.log.debug("Create a tx relevant to the peer before connecting") - txid = self.wallet.send_to(from_node=self.nodes[0], scriptPubKey=filter_peer.watch_script_pubkey, amount=9 * COIN)["txid"] + self.log.info("Create two tx before connecting, one relevant to the node another that is not") + rel_txid = self.wallet.send_to(from_node=self.nodes[0], scriptPubKey=filter_peer.watch_script_pubkey, amount=1 * COIN)["txid"] + irr_result = self.wallet.send_to(from_node=self.nodes[0], scriptPubKey=getnewdestination()[1], amount=2 * COIN) + irr_txid = irr_result["txid"] + irr_wtxid = irr_result["wtxid"] - self.log.debug("Send a mempool msg after connecting and check that the tx is received") + self.log.info("Send a mempool msg after connecting and check that the relevant tx is announced") self.nodes[0].add_p2p_connection(filter_peer) filter_peer.send_and_ping(filter_peer.watch_filter_init) filter_peer.send_message(msg_mempool()) - filter_peer.wait_for_tx(txid) + filter_peer.wait_for_tx(rel_txid) + + self.log.info("Request the irrelevant transaction even though it was not announced") + filter_peer.send_message(msg_getdata([CInv(t=MSG_WTX, h=int(irr_wtxid, 16))])) + self.log.info("We should get it anyway because it was in the mempool on connection to peer") + filter_peer.wait_for_tx(irr_txid) def test_frelay_false(self, filter_peer): self.log.info("Check that a node with fRelay set to false does not receive invs until the filter is set") From fa67f096bdea9db59dd20c470c9e32f3dac5be94 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Sun, 27 Aug 2023 10:45:39 +0200 Subject: [PATCH 29/37] build: Require C++20 compiler --- .github/workflows/ci.yml | 4 ++-- build-aux/m4/ax_cxx_compile_stdcxx.m4 | 2 +- configure.ac | 12 +----------- depends/Makefile | 2 +- depends/README.md | 2 +- depends/packages/qt.mk | 2 +- src/.clang-format | 2 +- 7 files changed, 8 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cdf8e8e6c5..1b43eca672 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,12 +62,12 @@ jobs: echo "TEST_BASE=$(git rev-list -n$((${{ env.MAX_COUNT }} + 1)) --reverse HEAD ^$(git rev-list -n1 --merges HEAD)^@ | head -1)" >> "$GITHUB_ENV" - run: | sudo apt-get update - sudo apt-get install clang ccache build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y + sudo apt-get install clang-15 ccache build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq libevent-dev libboost-dev libsqlite3-dev libdb++-dev systemtap-sdt-dev libminiupnpc-dev libnatpmp-dev libqt5gui5 libqt5core5a libqt5dbus5 qttools5-dev qttools5-dev-tools qtwayland5 libqrencode-dev -y - name: Compile and run tests run: | # Run tests on commits after the last merge commit and before the PR head commit # Use clang++, because it is a bit faster and uses less memory than g++ - git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && ./autogen.sh && CC=clang CXX=clang++ ./configure && make clean && make -j $(nproc) check && ./test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }} + git rebase --exec "echo Running test-one-commit on \$( git log -1 ) && ./autogen.sh && CC=clang-15 CXX=clang++-15 ./configure && make clean && make -j $(nproc) check && ./test/functional/test_runner.py -j $(( $(nproc) * 2 ))" ${{ env.TEST_BASE }} macos-native-x86_64: name: 'macOS 13 native, x86_64, no depends, sqlite only, gui' diff --git a/build-aux/m4/ax_cxx_compile_stdcxx.m4 b/build-aux/m4/ax_cxx_compile_stdcxx.m4 index 51a35054d0..8a2df5627f 100644 --- a/build-aux/m4/ax_cxx_compile_stdcxx.m4 +++ b/build-aux/m4/ax_cxx_compile_stdcxx.m4 @@ -983,7 +983,7 @@ m4_define([_AX_CXX_COMPILE_STDCXX_testbody_new_in_20], [[ #error "This is not a C++ compiler" -#elif __cplusplus < 202002L +#elif __cplusplus < 201709L // Temporary patch on top of upstream to allow g++-10 #error "This is not a C++20 compiler" diff --git a/configure.ac b/configure.ac index 1b5dc32b04..9b39159a32 100644 --- a/configure.ac +++ b/configure.ac @@ -96,18 +96,8 @@ case $host in ;; esac -AC_ARG_ENABLE([c++20], - [AS_HELP_STRING([--enable-c++20], - [enable compilation in c++20 mode (disabled by default)])], - [use_cxx20=$enableval], - [use_cxx20=no]) - -dnl Require C++17 compiler (no GNU extensions) -if test "$use_cxx20" = "no"; then -AX_CXX_COMPILE_STDCXX([17], [noext], [mandatory]) -else +dnl Require C++20 compiler (no GNU extensions) AX_CXX_COMPILE_STDCXX([20], [noext], [mandatory]) -fi dnl Unless the user specified OBJCXX, force it to be the same as CXX. This ensures dnl that we get the same -std flags for both. diff --git a/depends/Makefile b/depends/Makefile index 3169117633..319c3498df 100644 --- a/depends/Makefile +++ b/depends/Makefile @@ -49,7 +49,7 @@ NO_HARDEN ?= FALLBACK_DOWNLOAD_PATH ?= https://bitcoincore.org/depends-sources C_STANDARD ?= c11 -CXX_STANDARD ?= c++17 +CXX_STANDARD ?= c++20 BUILD = $(shell ./config.guess) HOST ?= $(BUILD) diff --git a/depends/README.md b/depends/README.md index a2e05ab1e8..8af5e36bfd 100644 --- a/depends/README.md +++ b/depends/README.md @@ -98,7 +98,7 @@ The following can be set when running make: `make FOO=bar` - `SDK_PATH`: Path where SDKs can be found (used by macOS) - `FALLBACK_DOWNLOAD_PATH`: If a source file can't be fetched, try here before giving up - `C_STANDARD`: Set the C standard version used. Defaults to `c11`. -- `CXX_STANDARD`: Set the C++ standard version used. Defaults to `c++17`. +- `CXX_STANDARD`: Set the C++ standard version used. Defaults to `c++20`. - `NO_BOOST`: Don't download/build/cache Boost - `NO_LIBEVENT`: Don't download/build/cache Libevent - `NO_QT`: Don't download/build/cache Qt and its dependencies diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk index 3e3b78987a..ecf3334aa5 100644 --- a/depends/packages/qt.mk +++ b/depends/packages/qt.mk @@ -41,7 +41,7 @@ $(package)_config_opts_release += -silent $(package)_config_opts_debug = -debug $(package)_config_opts_debug += -optimized-tools $(package)_config_opts += -bindir $(build_prefix)/bin -$(package)_config_opts += -c++std c++17 +$(package)_config_opts += -c++std c++2a $(package)_config_opts += -confirm-license $(package)_config_opts += -hostprefix $(build_prefix) $(package)_config_opts += -no-compile-examples diff --git a/src/.clang-format b/src/.clang-format index 791b3b8f9f..2e5d5c6449 100644 --- a/src/.clang-format +++ b/src/.clang-format @@ -43,5 +43,5 @@ SpacesInAngles: false SpacesInContainerLiterals: true SpacesInCStyleCastParentheses: false SpacesInParentheses: false -Standard: c++17 +Standard: c++20 UseTab: Never From fa02fc0a86c410f907de4fee91dd045547ea4b6e Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 31 Oct 2023 20:41:44 +0100 Subject: [PATCH 30/37] refactor: modernize-use-default-member-init for bit-fields (C++20) --- src/txrequest.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/txrequest.cpp b/src/txrequest.cpp index b9a41f26ff..ce5fbd9a7f 100644 --- a/src/txrequest.cpp +++ b/src/txrequest.cpp @@ -73,7 +73,7 @@ struct Announcement { const bool m_is_wtxid : 1; /** What state this announcement is in. */ - State m_state : 3; + State m_state : 3 {State::CANDIDATE_DELAYED}; State GetState() const { return m_state; } void SetState(State state) { m_state = state; } @@ -97,9 +97,9 @@ struct Announcement { /** Construct a new announcement from scratch, initially in CANDIDATE_DELAYED state. */ Announcement(const GenTxid& gtxid, NodeId peer, bool preferred, std::chrono::microseconds reqtime, - SequenceNumber sequence) : - m_txhash(gtxid.GetHash()), m_time(reqtime), m_peer(peer), m_sequence(sequence), m_preferred(preferred), - m_is_wtxid{gtxid.IsWtxid()}, m_state{State::CANDIDATE_DELAYED} {} + SequenceNumber sequence) + : m_txhash(gtxid.GetHash()), m_time(reqtime), m_peer(peer), m_sequence(sequence), m_preferred(preferred), + m_is_wtxid{gtxid.IsWtxid()} {} }; //! Type alias for priorities. From fae3b77a87f4d799aca5907335a9dcbab3a51db6 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Sun, 27 Aug 2023 10:45:01 +0200 Subject: [PATCH 31/37] refactor: Drop unused _Pragma to ignore -Wgnu-zero-variadic-macro-arguments --- src/test/fuzz/fuzz.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/test/fuzz/fuzz.h b/src/test/fuzz/fuzz.h index 1f0fa5527a..ca74d53de7 100644 --- a/src/test/fuzz/fuzz.h +++ b/src/test/fuzz/fuzz.h @@ -33,11 +33,7 @@ struct FuzzTargetOptions { void FuzzFrameworkRegisterTarget(std::string_view name, TypeTestOneInput target, FuzzTargetOptions opts); -#if defined(__clang__) -#define FUZZ_TARGET(...) _Pragma("clang diagnostic push") _Pragma("clang diagnostic ignored \"-Wgnu-zero-variadic-macro-arguments\"") DETAIL_FUZZ(__VA_ARGS__) _Pragma("clang diagnostic pop") -#else #define FUZZ_TARGET(...) DETAIL_FUZZ(__VA_ARGS__) -#endif #define DETAIL_FUZZ(name, ...) \ void name##_fuzz_target(FuzzBufferType); \ From faa48388bca06df1ca7ab92461b76a6720481e45 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Sun, 27 Aug 2023 10:39:02 +0200 Subject: [PATCH 32/37] Revert "tracepoints: Disables `-Wgnu-zero-variadic-macro-arguments` to compile without warnings" This reverts commit 5197660e947435e510ef3ef72be8be8dee3ffa41. --- src/util/trace.h | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/util/trace.h b/src/util/trace.h index 1fe743f043..b7c275f19b 100644 --- a/src/util/trace.h +++ b/src/util/trace.h @@ -13,28 +13,19 @@ #include -// Disabling this warning can be removed once we switch to C++20 -#if defined(__clang__) && __cplusplus < 202002L -#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH _Pragma("clang diagnostic push") _Pragma("clang diagnostic ignored \"-Wgnu-zero-variadic-macro-arguments\"") -#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP _Pragma("clang diagnostic pop") -#else -#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH -#define BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#endif - -#define TRACE(context, event) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE(context, event) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE1(context, event, a) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE1(context, event, a) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE2(context, event, a, b) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE2(context, event, a, b) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE3(context, event, a, b, c) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE3(context, event, a, b, c) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE4(context, event, a, b, c, d) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE4(context, event, a, b, c, d) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE5(context, event, a, b, c, d, e) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE5(context, event, a, b, c, d, e) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE6(context, event, a, b, c, d, e, f) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE6(context, event, a, b, c, d, e, f) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE7(context, event, a, b, c, d, e, f, g) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE7(context, event, a, b, c, d, e, f, g) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE8(context, event, a, b, c, d, e, f, g, h) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE8(context, event, a, b, c, d, e, f, g, h) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE9(context, event, a, b, c, d, e, f, g, h, i) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE9(context, event, a, b, c, d, e, f, g, h, i) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE10(context, event, a, b, c, d, e, f, g, h, i, j) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE10(context, event, a, b, c, d, e, f, g, h, i, j) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE11(context, event, a, b, c, d, e, f, g, h, i, j, k) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE11(context, event, a, b, c, d, e, f, g, h, i, j, k) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP -#define TRACE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_PUSH DTRACE_PROBE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) BITCOIN_DISABLE_WARN_ZERO_VARIADIC_POP +#define TRACE(context, event) DTRACE_PROBE(context, event) +#define TRACE1(context, event, a) DTRACE_PROBE1(context, event, a) +#define TRACE2(context, event, a, b) DTRACE_PROBE2(context, event, a, b) +#define TRACE3(context, event, a, b, c) DTRACE_PROBE3(context, event, a, b, c) +#define TRACE4(context, event, a, b, c, d) DTRACE_PROBE4(context, event, a, b, c, d) +#define TRACE5(context, event, a, b, c, d, e) DTRACE_PROBE5(context, event, a, b, c, d, e) +#define TRACE6(context, event, a, b, c, d, e, f) DTRACE_PROBE6(context, event, a, b, c, d, e, f) +#define TRACE7(context, event, a, b, c, d, e, f, g) DTRACE_PROBE7(context, event, a, b, c, d, e, f, g) +#define TRACE8(context, event, a, b, c, d, e, f, g, h) DTRACE_PROBE8(context, event, a, b, c, d, e, f, g, h) +#define TRACE9(context, event, a, b, c, d, e, f, g, h, i) DTRACE_PROBE9(context, event, a, b, c, d, e, f, g, h, i) +#define TRACE10(context, event, a, b, c, d, e, f, g, h, i, j) DTRACE_PROBE10(context, event, a, b, c, d, e, f, g, h, i, j) +#define TRACE11(context, event, a, b, c, d, e, f, g, h, i, j, k) DTRACE_PROBE11(context, event, a, b, c, d, e, f, g, h, i, j, k) +#define TRACE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) DTRACE_PROBE12(context, event, a, b, c, d, e, f, g, h, i, j, k, l) #else From fa6e50d6c79633e22ad4cfc75f56aaa40112ecbb Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Tue, 3 Oct 2023 16:26:30 +0200 Subject: [PATCH 33/37] fuzz: Use C++20 starts_with in rpc.cpp --- src/test/fuzz/rpc.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 2189dc0067..2325bf0941 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -380,9 +380,7 @@ FUZZ_TARGET(rpc, .init = initialize_rpc) rpc_testing_setup->CallRPC(rpc_command, arguments); } catch (const UniValue& json_rpc_error) { const std::string error_msg{json_rpc_error.find_value("message").get_str()}; - // Once c++20 is allowed, starts_with can be used. - // if (error_msg.starts_with("Internal bug detected")) { - if (0 == error_msg.rfind("Internal bug detected", 0)) { + if (error_msg.starts_with("Internal bug detected")) { // Only allow the intentional internal bug assert(error_msg.find("trigger_internal_bug") != std::string::npos); } From 8df4aaabbe3da5036436b96b5839acb1d7cd45f1 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 7 Dec 2023 13:34:28 +0000 Subject: [PATCH 34/37] doc: add minimum required Linux Kernel to release-notes --- doc/release-notes-empty-template.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/release-notes-empty-template.md b/doc/release-notes-empty-template.md index 887104548b..96e28c3763 100644 --- a/doc/release-notes-empty-template.md +++ b/doc/release-notes-empty-template.md @@ -36,9 +36,9 @@ Compatibility ============== Bitcoin Core is supported and extensively tested on operating systems -using the Linux kernel, macOS 11.0+, and Windows 7 and newer. Bitcoin +using the Linux Kernel 3.17+, macOS 11.0+, and Windows 7 and newer. Bitcoin Core should also work on most other Unix-like systems but is not as -frequently tested on them. It is not recommended to use Bitcoin Core on +frequently tested on them. It is not recommended to use Bitcoin Core on unsupported systems. Notable changes From 7d4e47d1849ba00c5b45995a96f2c747f1a97c71 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 7 Dec 2023 11:39:01 +0000 Subject: [PATCH 35/37] doc: add historical release notes for 26.0 --- doc/release-notes/release-notes-26.0.md | 335 ++++++++++++++++++++++++ 1 file changed, 335 insertions(+) create mode 100644 doc/release-notes/release-notes-26.0.md diff --git a/doc/release-notes/release-notes-26.0.md b/doc/release-notes/release-notes-26.0.md new file mode 100644 index 0000000000..b30dadf62b --- /dev/null +++ b/doc/release-notes/release-notes-26.0.md @@ -0,0 +1,335 @@ +26.0 Release Notes +================== + +Bitcoin Core version 26.0 is now available from: + + + +This release includes new features, various bug fixes and performance +improvements, as well as updated translations. + +Please report bugs using the issue tracker at GitHub: + + + +To receive security and update notifications, please subscribe to: + + + +How to Upgrade +============== + +If you are running an older version, shut it down. Wait until it has completely +shut down (which might take a few minutes in some cases), then run the +installer (on Windows) or just copy over `/Applications/Bitcoin-Qt` (on macOS) +or `bitcoind`/`bitcoin-qt` (on Linux). + +Upgrading directly from a version of Bitcoin Core that has reached its EOL is +possible, but it might take some time if the data directory needs to be migrated. Old +wallet versions of Bitcoin Core are generally supported. + +Compatibility +============== + +Bitcoin Core is supported and extensively tested on operating systems +using the Linux kernel, macOS 11.0+, and Windows 7 and newer. Bitcoin +Core should also work on most other Unix-like systems but is not as +frequently tested on them. It is not recommended to use Bitcoin Core on +unsupported systems. + +Notable changes +=============== + +P2P and network changes +----------------------- + +- Experimental support for the v2 transport protocol defined in + [BIP324](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki) was added. + It is off by default, but when enabled using `-v2transport` it will be negotiated + on a per-connection basis with other peers that support it too. The existing + v1 transport protocol remains fully supported. + +- Nodes with multiple reachable networks will actively try to have at least one + outbound connection to each network. This improves individual resistance to + eclipse attacks and network level resistance to partition attacks. Users no + longer need to perform active measures to ensure being connected to multiple + enabled networks. (#27213) + +Pruning +------- + +- When using assumeutxo with `-prune`, the prune budget may be exceeded if it is set + lower than 1100MB (i.e. `MIN_DISK_SPACE_FOR_BLOCK_FILES * 2`). Prune budget is normally + split evenly across each chainstate, unless the resulting prune budget per chainstate + is beneath `MIN_DISK_SPACE_FOR_BLOCK_FILES` in which case that value will be used. (#27596) + +Updated RPCs +------------ + +- Setting `-rpcserialversion=0` is deprecated and will be removed in + a future release. It can currently still be used by also adding + the `-deprecatedrpc=serialversion` option. (#28448) + +- The `hash_serialized_2` value has been removed from `gettxoutsetinfo` since the value it + calculated contained a bug and did not take all data into account. It is superseded by + `hash_serialized_3` which provides the same functionality but serves the correctly calculated hash. (#28685) + +- New fields `transport_protocol_type` and `session_id` were added to the `getpeerinfo` RPC to indicate + whether the v2 transport protocol is in use, and if so, what the session id is. + +- A new argument `v2transport` was added to the `addnode` RPC to indicate whether a v2 transaction connection + is to be attempted with the peer. + +- [Miniscript](https://bitcoin.sipa.be/miniscript/) expressions can now be used in Taproot descriptors for all RPCs working with descriptors. (#27255) + +- `finalizepsbt` is now able to finalize a PSBT with inputs spending [Miniscript](https://bitcoin.sipa.be/miniscript/)-compatible Taproot leaves. (#27255) + +Changes to wallet related RPCs can be found in the Wallet section below. + +New RPCs +-------- + +- `loadtxoutset` has been added, which allows loading a UTXO snapshot of the format + generated by `dumptxoutset`. Once this snapshot is loaded, its contents will be + deserialized into a second chainstate data structure, which is then used to sync to + the network's tip. + + Meanwhile, the original chainstate will complete the initial block download process in + the background, eventually validating up to the block that the snapshot is based upon. + + The result is a usable bitcoind instance that is current with the network tip in a + matter of minutes rather than hours. UTXO snapshot are typically obtained via + third-party sources (HTTP, torrent, etc.) which is reasonable since their contents + are always checked by hash. + + You can find more information on this process in the `assumeutxo` design + document (). + + `getchainstates` has been added to aid in monitoring the assumeutxo sync process. + +- A new `getprioritisedtransactions` RPC has been added. It returns a map of all fee deltas created by the + user with prioritisetransaction, indexed by txid. The map also indicates whether each transaction is + present in the mempool. (#27501) + +- A new RPC, `submitpackage`, has been added. It can be used to submit a list of raw hex +transactions to the mempool to be evaluated as a package using consensus and mempool policy rules. +These policies include package CPFP, allowing a child with high fees to bump a parent below the +mempool minimum feerate (but not minimum relay feerate). (#27609) + + - Warning: successful submission does not mean the transactions will propagate throughout the + network, as package relay is not supported. + + - Not all features are available. The package is limited to a child with all of its + unconfirmed parents, and no parent may spend the output of another parent. Also, package + RBF is not supported. Refer to doc/policy/packages.md for more details on package policies + and limitations. + + - This RPC is experimental. Its interface may change. + +- A new RPC `getaddrmaninfo` has been added to view the distribution of addresses in the new and tried table of the + node's address manager across different networks(ipv4, ipv6, onion, i2p, cjdns). The RPC returns count of addresses + in new and tried table as well as their sum for all networks. (#27511) + +- A new `importmempool` RPC has been added. It loads a valid `mempool.dat` file and attempts to + add its contents to the mempool. This can be useful to import mempool data from another node + without having to modify the datadir contents and without having to restart the node. (#27460) + - Warning: Importing untrusted files is dangerous, especially if metadata from the file is taken over. + - If you want to apply fee deltas, it is recommended to use the `getprioritisedtransactions` and + `prioritisetransaction` RPCs instead of the `apply_fee_delta_priority` option to avoid + double-prioritising any already-prioritised transactions in the mempool. + +Updated settings +---------------- + +- `bitcoind` and `bitcoin-qt` will now raise an error on startup + if a datadir that is being used contains a bitcoin.conf file that + will be ignored, which can happen when a datadir= line is used in + a bitcoin.conf file. The error message is just a diagnostic intended + to prevent accidental misconfiguration, and it can be disabled to + restore the previous behavior of using the datadir while ignoring + the bitcoin.conf contained in it. (#27302) + +- Passing an invalid `-debug`, `-debugexclude`, or `-loglevel` logging configuration + option now raises an error, rather than logging an easily missed warning. (#27632) + +Changes to GUI or wallet related settings can be found in the GUI or Wallet section below. + +New settings +------------ + +Tools and Utilities +------------------- + +- A new `bitcoinconsensus_verify_script_with_spent_outputs` function is available in libconsensus which optionally accepts the spent outputs of the transaction being verified. +- A new `bitcoinconsensus_SCRIPT_FLAGS_VERIFY_TAPROOT` flag is available in libconsensus that will verify scripts with the Taproot spending rules. + +Wallet +------ + +- Wallet loading has changed in this release. Wallets with some corrupted records that could be + previously loaded (with warnings) may no longer load. For example, wallets with corrupted + address book entries may no longer load. If this happens, it is recommended + load the wallet in a previous version of Bitcoin Core and import the data into a new wallet. + Please also report an issue to help improve the software and make wallet loading more robust + in these cases. (#24914) + +- The `gettransaction`, `listtransactions`, `listsinceblock` RPCs now return + the `abandoned` field for all transactions. Previously, the "abandoned" field + was only returned for sent transactions. (#25158) + +- The `listdescriptors`, `decodepsbt` and similar RPC methods now show `h` rather than apostrophe (`'`) to indicate + hardened derivation. This does not apply when using the `private` parameter, which + matches the marker used when descriptor was generated or imported. Newly created + wallets use `h`. This change makes it easier to handle descriptor strings manually. + E.g. the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`. + With this change `listdescriptors` will use `h`, so you can copy-paste the result, + without having to add escape characters or switch `'` to 'h' manually. + Note that this changes the descriptor checksum. + For legacy wallets the `hdkeypath` field in `getaddressinfo` is unchanged, + nor is the serialization format of wallet dumps. (#26076) + +- The `getbalances` RPC now returns a `lastprocessedblock` JSON object which contains the wallet's last processed block + hash and height at the time the balances were calculated. This result shouldn't be cached because importing new keys could invalidate it. (#26094) + +- The `gettransaction` RPC now returns a `lastprocessedblock` JSON object which contains the wallet's last processed block + hash and height at the time the transaction information was generated. (#26094) + +- The `getwalletinfo` RPC now returns a `lastprocessedblock` JSON object which contains the wallet's last processed block + hash and height at the time the wallet information was generated. (#26094) + +- Coin selection and transaction building now accounts for unconfirmed low-feerate ancestor transactions. When it is necessary to spend unconfirmed outputs, the wallet will add fees to ensure that the new transaction with its ancestors will achieve a mining score equal to the feerate requested by the user. (#26152) + +- For RPC methods which accept `options` parameters ((`importmulti`, `listunspent`, + `fundrawtransaction`, `bumpfee`, `send`, `sendall`, `walletcreatefundedpsbt`, + `simulaterawtransaction`), it is now possible to pass the options as named + parameters without the need for a nested object. (#26485) + +This means it is possible make calls like: + +```sh +src/bitcoin-cli -named bumpfee txid fee_rate=100 +``` + +instead of + +```sh +src/bitcoin-cli -named bumpfee txid options='{"fee_rate": 100}' +``` + +- The `deprecatedrpc=walletwarningfield` configuration option has been removed. + The `createwallet`, `loadwallet`, `restorewallet` and `unloadwallet` RPCs no + longer return the "warning" string field. The same information is provided + through the "warnings" field added in v25.0, which returns a JSON array of + strings. The "warning" string field was deprecated also in v25.0. (#27757) + +- The `signrawtransactionwithkey`, `signrawtransactionwithwallet`, + `walletprocesspsbt` and `descriptorprocesspsbt` calls now return the more + specific RPC_INVALID_PARAMETER error instead of RPC_MISC_ERROR if their + sighashtype argument is malformed. (#28113) + +- RPC `walletprocesspsbt`, and `descriptorprocesspsbt` return + object now includes field `hex` (if the transaction + is complete) containing the serialized transaction + suitable for RPC `sendrawtransaction`. (#28414) + +- It's now possible to use [Miniscript](https://bitcoin.sipa.be/miniscript/) inside Taproot leaves for descriptor wallets. (#27255) + +GUI changes +----------- + +- The transaction list in the GUI no longer provides a special category for "payment to yourself". Now transactions that have both inputs and outputs that affect the wallet are displayed on separate lines for spending and receiving. (gui#119) + +- A new menu option allows migrating a legacy wallet based on keys and implied output script types stored in BerkeleyDB (BDB) to a modern wallet that uses descriptors stored in SQLite. (gui#738) + +- The PSBT operations dialog marks outputs paying your own wallet with "own address". (gui#740) + +- The ability to create legacy wallets is being removed. (gui#764) + +Low-level changes +================= + +Tests +----- + +- Non-standard transactions are now disabled by default on testnet + for relay and mempool acceptance. The previous behaviour can be + re-enabled by setting `-acceptnonstdtxn=1`. (#28354) + +Credits +======= + +Thanks to everyone who directly contributed to this release: + +- 0xb10c +- Amiti Uttarwar +- Andrew Chow +- Andrew Toth +- Anthony Towns +- Antoine Poinsot +- Antoine Riard +- Ari +- Aurèle Oulès +- Ayush Singh +- Ben Woosley +- Brandon Odiwuor +- Brotcrunsher +- brunoerg +- Bufo +- Carl Dong +- Casey Carter +- Cory Fields +- David Álvarez Rosa +- dergoegge +- dhruv +- dimitaracev +- Erik Arvstedt +- Erik McKelvey +- Fabian Jahr +- furszy +- glozow +- Greg Sanders +- Harris +- Hennadii Stepanov +- Hernan Marino +- ishaanam +- ismaelsadeeq +- Jake Rawsthorne +- James O'Beirne +- John Moffett +- Jon Atack +- josibake +- kevkevin +- Kiminuo +- Larry Ruane +- Luke Dashjr +- MarcoFalke +- Marnix +- Martin Leitner-Ankerl +- Martin Zumsande +- Matthew Zipkin +- Michael Ford +- Michael Tidwell +- mruddy +- Murch +- ns-xvrn +- pablomartin4btc +- Pieter Wuille +- Reese Russell +- Rhythm Garg +- Ryan Ofsky +- Sebastian Falbesoner +- Sjors Provoost +- stickies-v +- stratospher +- Suhas Daftuar +- TheCharlatan +- Tim Neubauer +- Tim Ruffing +- Vasil Dimov +- virtu +- vuittont60 +- willcl-ark +- Yusuf Sahin HAMZA + +As well as to everyone that helped with translations on +[Transifex](https://www.transifex.com/bitcoin/bitcoin/). From fa88953d6fb54fdb47485981279632c693534108 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 7 Dec 2023 15:58:18 +0100 Subject: [PATCH 36/37] doc: Add link to needs-release-notes label --- doc/release-process.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/release-process.md b/doc/release-process.md index c70b0194ab..95ef08a26f 100644 --- a/doc/release-process.md +++ b/doc/release-process.md @@ -74,7 +74,9 @@ Release Process #### Before final release - Merge the release notes from [the wiki](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/) into the branch. -- Ensure the "Needs release note" label is removed from all relevant pull requests and issues. +- Ensure the "Needs release note" label is removed from all relevant pull + requests and issues: + https://github.com/bitcoin/bitcoin/issues?q=label%3A%22Needs+release+note%22 #### Tagging a release (candidate) From ca5937553b4b4dde53995d0b66e30150401023eb Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 7 Dec 2023 15:35:44 +0000 Subject: [PATCH 37/37] doc: Missing additions to 26.0 release notes --- doc/release-notes/release-notes-26.0.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/doc/release-notes/release-notes-26.0.md b/doc/release-notes/release-notes-26.0.md index b30dadf62b..b7c7c35f65 100644 --- a/doc/release-notes/release-notes-26.0.md +++ b/doc/release-notes/release-notes-26.0.md @@ -173,6 +173,11 @@ Wallet Please also report an issue to help improve the software and make wallet loading more robust in these cases. (#24914) +- The `createwallet` RPC will no longer create legacy (BDB) wallets when + setting `descriptors=false` without also providing the + `-deprecatedrpc=create_bdb` option. This is because the legacy wallet is + being deprecated in a future release. (#28597) + - The `gettransaction`, `listtransactions`, `listsinceblock` RPCs now return the `abandoned` field for all transactions. Previously, the "abandoned" field was only returned for sent transactions. (#25158) @@ -234,6 +239,14 @@ src/bitcoin-cli -named bumpfee txid options='{"fee_rate": 100}' - It's now possible to use [Miniscript](https://bitcoin.sipa.be/miniscript/) inside Taproot leaves for descriptor wallets. (#27255) +Descriptors +----------- + +- The usage of hybrid public keys in output descriptors has been removed. Hybrid + public keys are an exotic public key encoding not supported by output descriptors + (as specified in BIP380 and documented in doc/descriptors.md). Bitcoin Core would + previously incorrectly accept descriptors containing such hybrid keys. (#28587) + GUI changes ----------- @@ -245,6 +258,15 @@ GUI changes - The ability to create legacy wallets is being removed. (gui#764) +Contrib +------- + +- Bash completion files have been renamed from `bitcoin*.bash-completion` to + `bitcoin*.bash`. This means completions can be automatically loaded on demand + based on invoked commands' names when they are put into the completion + directory (found with `pkg-config --variable=completionsdir + bash-completion`) without requiring renaming. (#28507) + Low-level changes =================