From b0443e20a63ab9b840664ab9edfefc1710b874c3 Mon Sep 17 00:00:00 2001 From: merge-script Date: Mon, 10 Jun 2024 12:01:19 +0100 Subject: [PATCH 1/6] Merge bitcoin/bitcoin#30257: build: Remove --enable-gprof fa780e1c25e8e98253e32d93db65f78a0092433f build: Remove --enable-gprof (MarcoFalke) Pull request description: It is unclear what benefit this option has, given that: * `gprof` requires re-compilation (`perf` and other tools can instead be used on existing executables) * `gprof` requires hardening to be disabled * `gprof` doesn't work with `clang` * `perf` is documented in the dev-notes, and test notes, and embedded into the functional test framework; `gprof` isn't * Anyone really wanting to use it could pass the required flags to `./configure` * I couldn't find any mention of the use of `gprof` in the discussions in this repo, apart from the initial pull request adding it (cfaac2a60f3ac63ae8deccb03d88bd559449b78c) * Keeping it means that it needs to be maintained and ported to CMake Fix all issues by removing it. ACKs for top commit: TheCharlatan: ACK fa780e1c25e8e98253e32d93db65f78a0092433f hebasto: ACK fa780e1c25e8e98253e32d93db65f78a0092433f, I have reviewed the code and it looks OK. willcl-ark: crACK fa780e1c25e8e98253e32d93db65f78a0092433f Tree-SHA512: 0a9ff363ac2bec8b743878a4e3147f18bc16823d00c5007568432c36320bd0199b13b6d0ce828a9a83c2cc434c058afaa64eb2eccfbd93ed85b81ce10c41760c --- configure.ac | 36 ++++-------------------------------- doc/developer-notes.md | 5 ----- src/Makefile.am | 4 ++-- 3 files changed, 6 insertions(+), 39 deletions(-) diff --git a/configure.ac b/configure.ac index 215b797f544c..c168c823cc4c 100644 --- a/configure.ac +++ b/configure.ac @@ -200,9 +200,9 @@ AC_ARG_WITH([qrencode], AC_ARG_ENABLE([hardening], [AS_HELP_STRING([--disable-hardening], - [do not attempt to harden the resulting executables (default is to harden when possible)])], + [do not attempt to harden the resulting executables (default is to harden)])], [use_hardening=$enableval], - [use_hardening=auto]) + [use_hardening=yes]) AC_ARG_ENABLE([reduce-exports], [AS_HELP_STRING([--enable-reduce-exports], @@ -306,13 +306,6 @@ AC_ARG_WITH([sanitizers], [comma separated list of extra sanitizers to build with (default is none enabled)])], [use_sanitizers=$withval]) -dnl Enable gprof profiling -AC_ARG_ENABLE([gprof], - [AS_HELP_STRING([--enable-gprof], - [use gprof profiling compiler flags (default is no)])], - [enable_gprof=$enableval], - [enable_gprof=no]) - dnl Turn warnings into errors AC_ARG_ENABLE([werror], [AS_HELP_STRING([--enable-werror], @@ -1044,30 +1037,12 @@ if test "$ac_cv_sys_large_files" != "" && CORE_CPPFLAGS="$CORE_CPPFLAGS -D_LARGE_FILES=$ac_cv_sys_large_files" fi -if test "$enable_gprof" = "yes"; then - dnl -pg is incompatible with -pie. Since hardening and profiling together doesn't make sense, - dnl we simply make them mutually exclusive here. Additionally, hardened toolchains may force - dnl -pie by default, in which case it needs to be turned off with -no-pie. - - if test "$use_hardening" = "yes"; then - AC_MSG_ERROR([gprof profiling is not compatible with hardening. Reconfigure with --disable-hardening or --disable-gprof]) - fi - use_hardening=no - AX_CHECK_COMPILE_FLAG([-pg],[GPROF_CXXFLAGS="-pg"], - [AC_MSG_ERROR([gprof profiling requested but not available])], [$CXXFLAG_WERROR]) - - AX_CHECK_LINK_FLAG([-no-pie], [GPROF_LDFLAGS="-no-pie"]) - AX_CHECK_LINK_FLAG([-pg], [GPROF_LDFLAGS="$GPROF_LDFLAGS -pg"], - [AC_MSG_ERROR([gprof profiling requested but not available])], [$GPROF_LDFLAGS]) -fi - if test "$TARGET_OS" != "windows"; then dnl All windows code is PIC, forcing it on just adds useless compile warnings AX_CHECK_COMPILE_FLAG([-fPIC], [PIC_FLAGS="-fPIC"]) fi if test "$use_hardening" != "no"; then - use_hardening=yes AX_CHECK_COMPILE_FLAG([-Wstack-protector], [HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -Wstack-protector"]) AX_CHECK_COMPILE_FLAG([-fstack-protector-all], [HARDENED_CXXFLAGS="$HARDENED_CXXFLAGS -fstack-protector-all"]) @@ -2009,8 +1984,6 @@ AC_SUBST(DEBUG_CXXFLAGS) AC_SUBST(WARN_CXXFLAGS) AC_SUBST(NOWARN_CXXFLAGS) AC_SUBST(ERROR_CXXFLAGS) -AC_SUBST(GPROF_CXXFLAGS) -AC_SUBST(GPROF_LDFLAGS) AC_SUBST(HARDENED_CXXFLAGS) AC_SUBST(HARDENED_CPPFLAGS) AC_SUBST(HARDENED_LDFLAGS) @@ -2131,7 +2104,6 @@ echo " debug enabled = $enable_debug" echo " stacktraces = $enable_stacktraces" echo " crash hooks = $enable_crashhooks" echo " miner enabled = $enable_miner" -echo " gprof enabled = $enable_gprof" echo " werror = $enable_werror" echo echo " target os = $host_os" @@ -2141,8 +2113,8 @@ echo " CC = $CC" echo " CFLAGS = $DEBUG_CFLAGS $PTHREAD_CFLAGS $BACKTRACE_FLAGS $CFLAGS" echo " CPPFLAGS = $DEBUG_CPPFLAGS $HARDENED_CPPFLAGS $CORE_CPPFLAGS $CPPFLAGS" echo " CXX = $CXX" -echo " CXXFLAGS = $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $GPROF_CXXFLAGS $CORE_CXXFLAGS $BACKTRACE_FLAGS $CXXFLAGS" -echo " LDFLAGS = $PTHREAD_LIBS $HARDENED_LDFLAGS $GPROF_LDFLAGS $CORE_LDFLAGS $BACKTRACE_LDFLAGS $LDFLAGS" +echo " CXXFLAGS = $DEBUG_CXXFLAGS $HARDENED_CXXFLAGS $WARN_CXXFLAGS $NOWARN_CXXFLAGS $ERROR_CXXFLAGS $CORE_CXXFLAGS $BACKTRACE_FLAGS $CXXFLAGS" +echo " LDFLAGS = $PTHREAD_LIBS $HARDENED_LDFLAGS $CORE_LDFLAGS $BACKTRACE_LDFLAGS $LDFLAGS" echo " AR = $AR" echo " ARFLAGS = $ARFLAGS" echo diff --git a/doc/developer-notes.md b/doc/developer-notes.md index c343f3a7e007..dd19b91c1f28 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -13,7 +13,6 @@ Developer Notes - [Development tips and tricks](#development-tips-and-tricks) - [Compiling for debugging](#compiling-for-debugging) - [Show sources in debugging](#show-sources-in-debugging) - - [Compiling for gprof profiling](#compiling-for-gprof-profiling) - [`debug.log`](#debuglog) - [Devnet, testnet, and regtest modes](#devnet-testnet-and-regtest-modes) - [DEBUG_LOCKORDER](#debug_lockorder) @@ -385,10 +384,6 @@ ln -s /path/to/project/root/src src 3. Use `debugedit` to modify debug information in the binary. -### Compiling for gprof profiling - -Run configure with the `--enable-gprof` option, then make. - ### `debug.log` If the code is behaving strangely, take a look in the `debug.log` file in the data directory; diff --git a/src/Makefile.am b/src/Makefile.am index a0eca537b48d..cb741f9a35cd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -9,9 +9,9 @@ print-%: FORCE DIST_SUBDIRS = secp256k1 -AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) $(CORE_LDFLAGS) $(BACKTRACE_LDFLAGS) +AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(SANITIZER_LDFLAGS) $(CORE_LDFLAGS) $(BACKTRACE_LDFLAGS) AM_CFLAGS = $(DEBUG_CFLAGS) $(BACKTRACE_FLAGS) -AM_CXXFLAGS = $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(CORE_CXXFLAGS) $(BACKTRACE_FLAGS) +AM_CXXFLAGS = $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(CORE_CXXFLAGS) $(BACKTRACE_FLAGS) AM_OBJCXXFLAGS = $(AM_CXXFLAGS) AM_CPPFLAGS = $(DEBUG_CPPFLAGS) $(HARDENED_CPPFLAGS) $(CORE_CPPFLAGS) AM_LIBTOOLFLAGS = --preserve-dup-deps From 9b4a17e8620b129871bd62506080739f8d500a56 Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 11 Jun 2024 15:29:18 +0100 Subject: [PATCH 2/6] Merge bitcoin/bitcoin#30264: test: add coverage for errors for `combinerawtransaction` ab98e6fd03970d6b5a593674c84e762a47b90ea6 test: add coverage for errors for `combinerawtransaction` RPC (brunoerg) Pull request description: This PR adds test coverage for the following errors for the `combinerawtransaction` RPC: * Tx decode failed * Missing transactions * Input not found or already spent For reference: https://maflcko.github.io/b-c-cov/total.coverage/src/rpc/rawtransaction.cpp.gcov.html ACKs for top commit: maflcko: lgtm ACK ab98e6fd03970d6b5a593674c84e762a47b90ea6 tdb3: ACK ab98e6fd03970d6b5a593674c84e762a47b90ea6 Tree-SHA512: 8a133c25dad2e1b236e0278a88796f60f763e3fd6fbbc080f926bb23f9dcc55599aa242d6e0c4ec15a179d9ded10a1f17ee5b6063719107ea84e6099f10416b2 --- test/functional/rpc_rawtransaction.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index ad0c55ba82e3..9458ba35fd50 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -436,6 +436,8 @@ def raw_multisig_transaction_legacy_tests(self): rawTxPartialSigned2 = self.nodes[2].signrawtransactionwithwallet(rawTx2, inputs) self.log.debug(rawTxPartialSigned2) assert_equal(rawTxPartialSigned2['complete'], False) # node2 only has one key, can't comp. sign the tx + assert_raises_rpc_error(-22, "TX decode failed", self.nodes[0].combinerawtransaction, [rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex'] + "00"]) + assert_raises_rpc_error(-22, "Missing transactions", self.nodes[0].combinerawtransaction, []) rawTxComb = self.nodes[2].combinerawtransaction([rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']]) self.log.debug(rawTxComb) self.nodes[2].sendrawtransaction(rawTxComb) @@ -443,6 +445,7 @@ def raw_multisig_transaction_legacy_tests(self): self.sync_all() self.generate(self.nodes[0], 1) assert_equal(self.nodes[0].getbalance(), bal + Decimal('500.00000000') + Decimal('2.19000000')) # block reward + tx + assert_raises_rpc_error(-25, "Input not found or already spent", self.nodes[0].combinerawtransaction, [rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']]) if __name__ == '__main__': From 869a317526977542d57840aa300875f4c467a8a9 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 13 Jun 2024 12:18:49 -0400 Subject: [PATCH 3/6] Merge bitcoin/bitcoin#29607: refactor: Reduce memory copying operations in bech32 encoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 07f64177a49f1b6b4d486d10cf67fddfa3c995eb Reduce memory copying operations in bech32 encode (Lőrinc) d5ece3c4b5e109f65f5d3315c43239dd87bb2c81 Reserve hrp memory in Decode and LocateErrors (Lőrinc) Pull request description: Started optimizing the base conversions in [TryParseHex](https://github.com/bitcoin/bitcoin/pull/29458), [Base58](https://github.com/bitcoin/bitcoin/pull/29473) and [IsSpace](https://github.com/bitcoin/bitcoin/pull/29602) - this is the next step. Part of this change was already merged in https://github.com/bitcoin/bitcoin/pull/30047, which made decoding `~26%` faster. Here I've reduced the memory reallocations and copying operations in bech32 encode, making it `~15%` faster. > make && ./src/bench/bench_bitcoin --filter='Bech32Encode' --min-time=1000 Before: ``` | ns/byte | byte/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 19.97 | 50,074,562.72 | 0.1% | 1.06 | `Bech32Encode` ``` After: ``` | ns/byte | byte/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 17.33 | 57,687,668.20 | 0.1% | 1.10 | `Bech32Encode` ``` ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/pull/29607/commits/07f64177a49f1b6b4d486d10cf67fddfa3c995eb sipa: utACK 07f64177a49f1b6b4d486d10cf67fddfa3c995eb achow101: ACK 07f64177a49f1b6b4d486d10cf67fddfa3c995eb Tree-SHA512: 511885217d044ad7ef2bdf9203b8e0b94eec8b279bc193bb7e63e29ab868df6d21e9e4c7a24390358e1f9c131447ee42039df72edcf1e2b11e1856eb2b3e10dd --- src/bech32.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/bech32.cpp b/src/bech32.cpp index 9da2488ef2af..c9239aa3eb1a 100644 --- a/src/bech32.cpp +++ b/src/bech32.cpp @@ -184,13 +184,13 @@ std::string Encode(Encoding encoding, const std::string& hrp, const data& values // to return a lowercase Bech32/Bech32m string, but if given an uppercase HRP, the // result will always be invalid. for (const char& c : hrp) assert(c < 'A' || c > 'Z'); - data checksum = CreateChecksum(encoding, hrp, values); - data combined = Cat(values, checksum); - std::string ret = hrp + '1'; - ret.reserve(ret.size() + combined.size()); - for (const auto c : combined) { - ret += CHARSET[c]; - } + + std::string ret; + ret.reserve(hrp.size() + 1 + values.size() + CHECKSUM_SIZE); + ret += hrp; + ret += '1'; + for (const uint8_t& i : values) ret += CHARSET[i]; + for (const uint8_t& i : CreateChecksum(encoding, hrp, values)) ret += CHARSET[i]; return ret; } @@ -219,6 +219,7 @@ DecodeResult Decode(const std::string& str) { values[i] = rev; } std::string hrp; + hrp.reserve(pos); for (size_t i = 0; i < pos; ++i) { hrp += LowerCase(str[i]); } From df412b456f63c0ce6c3e6f98318814dee5d256b2 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 4 Jun 2024 20:32:25 -0400 Subject: [PATCH 4/6] Merge bitcoin/bitcoin#30047: refactor: Model the bech32 charlimit as an Enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd refactor: replace hardcoded numbers (Lőrinc) 5676aec1e1a6d2c6fd3099e120e263a0a7def089 refactor: Model the bech32 charlimit as an Enum (josibake) Pull request description: Broken out from #28122 --- Bech32(m) was defined with a 90 character limit so that certain guarantees for error detection could be made for segwit addresses (see https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#checksum-design). However, there is nothing about the encoding scheme itself that requires a limit of 90 and in practice bech32(m) is being used without the 90 char limit (e.g. lightning invoices, silent payments). Further, increasing the character limit doesn't do away with error detection, it simply changes the guarantee. The primary motivation for this change is for being able to parse BIP352 v0 silent payment addresses (see https://github.com/bitcoin/bitcoin/pull/28122/commits/622c7a98b9f08177a3cfb601306daabb101af1fd), which require up to 118 characters. In addition to BIP352, modeling the character limit as an enum allows us to easily support new address types that use bech32m and specify their own character limit. ACKs for top commit: paplorinc: re-ACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd achow101: ACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd theuni: utACK 7f3f6c6dc80247e6dfb0d406dc53bc8198f029fd Tree-SHA512: 9c793d657448c1f795093b9f7d4d6dfa431598f48d54e1c899a69fb2f43aeb68b40ca2ff08864eefeeb6627d4171877234b5df0056ff2a2b84415bc3558bd280 --- src/bech32.cpp | 41 +++++++++++++++++++++++------------------ src/bech32.h | 10 +++++++++- 2 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/bech32.cpp b/src/bech32.cpp index c9239aa3eb1a..c74e7dfbbc43 100644 --- a/src/bech32.cpp +++ b/src/bech32.cpp @@ -15,6 +15,9 @@ namespace typedef std::vector data; +/** The Bech32 and Bech32m checksum size */ +constexpr size_t CHECKSUM_SIZE = 6; + /** The Bech32 and Bech32m character set for encoding. */ const char* CHARSET = "qpzry9x8gf2tvdw0s3jn54khce6mua7l"; @@ -133,18 +136,18 @@ inline unsigned char LowerCase(unsigned char c) return (c >= 'A' && c <= 'Z') ? (c - 'A') + 'a' : c; } -/** Expand a HRP for use in checksum computation. */ -data ExpandHRP(const std::string& hrp) +std::vector PreparePolynomialCoefficients(const std::string& hrp, const data& values) { data ret; - ret.reserve(hrp.size() + 90); - ret.resize(hrp.size() * 2 + 1); - for (size_t i = 0; i < hrp.size(); ++i) { - unsigned char c = hrp[i]; - ret[i] = c >> 5; - ret[i + hrp.size() + 1] = c & 0x1f; - } - ret[hrp.size()] = 0; + ret.reserve(hrp.size() + 1 + hrp.size() + values.size() + CHECKSUM_SIZE); + + /** Expand a HRP for use in checksum computation. */ + for (size_t i = 0; i < hrp.size(); ++i) ret.push_back(hrp[i] >> 5); + ret.push_back(0); + for (size_t i = 0; i < hrp.size(); ++i) ret.push_back(hrp[i] & 0x1f); + + ret.insert(ret.end(), values.begin(), values.end()); + return ret; } @@ -156,7 +159,8 @@ Encoding VerifyChecksum(const std::string& hrp, const data& values) // list of values would result in a new valid list. For that reason, Bech32 requires the // resulting checksum to be 1 instead. In Bech32m, this constant was amended. See // https://gist.github.com/sipa/14c248c288c3880a3b191f978a34508e for details. - const uint32_t check = PolyMod(Cat(ExpandHRP(hrp), values)); + auto enc = PreparePolynomialCoefficients(hrp, values); + const uint32_t check = PolyMod(enc); if (check == EncodingConstant(Encoding::BECH32)) return Encoding::BECH32; if (check == EncodingConstant(Encoding::BECH32M)) return Encoding::BECH32M; return Encoding::INVALID; @@ -165,11 +169,11 @@ Encoding VerifyChecksum(const std::string& hrp, const data& values) /** Create a checksum. */ data CreateChecksum(Encoding encoding, const std::string& hrp, const data& values) { - data enc = Cat(ExpandHRP(hrp), values); - enc.resize(enc.size() + 6); // Append 6 zeroes + auto enc = PreparePolynomialCoefficients(hrp, values); + enc.insert(enc.end(), CHECKSUM_SIZE, 0x00); uint32_t mod = PolyMod(enc) ^ EncodingConstant(encoding); // Determine what to XOR into those 6 zeroes. - data ret(6); - for (size_t i = 0; i < 6; ++i) { + data ret(CHECKSUM_SIZE); + for (size_t i = 0; i < CHECKSUM_SIZE; ++i) { // Convert the 5-bit groups in mod to checksum values. ret[i] = (mod >> (5 * (5 - i))) & 31; } @@ -195,7 +199,7 @@ std::string Encode(Encoding encoding, const std::string& hrp, const data& values } /** Decode a Bech32 or Bech32m string. */ -DecodeResult Decode(const std::string& str) { +DecodeResult Decode(const std::string& str, CharLimit limit) { bool lower = false, upper = false; for (size_t i = 0; i < str.size(); ++i) { unsigned char c = str[i]; @@ -205,7 +209,8 @@ DecodeResult Decode(const std::string& str) { } if (lower && upper) return {}; size_t pos = str.rfind('1'); - if (str.size() > 90 || pos == str.npos || pos == 0 || pos + 7 > str.size()) { + if (str.size() > limit) return {}; + if (pos == str.npos || pos == 0 || pos + CHECKSUM_SIZE >= str.size()) { return {}; } data values(str.size() - 1 - pos); @@ -225,7 +230,7 @@ DecodeResult Decode(const std::string& str) { } Encoding result = VerifyChecksum(hrp, values); if (result == Encoding::INVALID) return {}; - return {result, std::move(hrp), data(values.begin(), values.end() - 6)}; + return {result, std::move(hrp), data(values.begin(), values.end() - CHECKSUM_SIZE)}; } } // namespace bech32 diff --git a/src/bech32.h b/src/bech32.h index e9450ccc2b35..d4ef4813d9d4 100644 --- a/src/bech32.h +++ b/src/bech32.h @@ -27,6 +27,14 @@ enum class Encoding { BECH32M, //!< Bech32m encoding as defined in BIP350 }; +/** Character limits for Bech32(m) encoded strings. Character limits are how we provide error location guarantees. + * These values should never exceed 2^31 - 1 (max value for a 32-bit int), since there are places where we may need to + * convert the CharLimit::VALUE to an int. In practice, this should never happen since this CharLimit applies to an address encoding + * and we would never encode an address with such a massive value */ +enum CharLimit : size_t { + BECH32 = 90, //!< BIP173/350 imposed character limit for Bech32(m) encoded addresses. This guarantees finding up to 4 errors. +}; + /** Encode a Bech32 or Bech32m string. If hrp contains uppercase characters, this will cause an * assertion error. Encoding must be one of BECH32 or BECH32M. */ std::string Encode(Encoding encoding, const std::string& hrp, const std::vector& values); @@ -42,7 +50,7 @@ struct DecodeResult }; /** Decode a Bech32 or Bech32m string. */ -DecodeResult Decode(const std::string& str); +DecodeResult Decode(const std::string& str, CharLimit limit = CharLimit::BECH32); } // namespace bech32 From 793c781a61f163fb319272917a80a987bb051b5e Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 9 May 2024 18:31:03 -0400 Subject: [PATCH 5/6] Merge bitcoin/bitcoin#30006: test: use sleepy wait-for-log in reindex readonly fd6a7d3a13d89d74e161095b0e9bd3570210a40c test: use sleepy wait-for-log in reindex readonly (Matthew Zipkin) Pull request description: Also rename the busy wait-for-log method to prevent recurrence. See https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1532578152 ACKs for top commit: maflcko: utACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c achow101: ACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c tdb3: ACK for fd6a7d3a13d89d74e161095b0e9bd3570210a40c rkrux: ACK [fd6a7d3](https://github.com/bitcoin/bitcoin/pull/30006/commits/fd6a7d3a13d89d74e161095b0e9bd3570210a40c) Tree-SHA512: 7ff0574833df1ec843159b35ee88b8bb345a513ac13ed0b72abd1bf330c454a3f9df4d927871b9e3d37bfcc07542b06ef63acef8e822cd18499adae8cbb0cda8 --- test/functional/feature_init.py | 2 +- test/functional/test_framework/test_node.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py index ce12319933de..d468ceb775ae 100755 --- a/test/functional/feature_init.py +++ b/test/functional/feature_init.py @@ -87,7 +87,7 @@ def check_clean_start(): for terminate_line in lines_to_terminate_after: self.log.info(f"Starting node and will exit after line {terminate_line}") - with node.wait_for_debug_log([terminate_line]): + with node.busy_wait_for_debug_log([terminate_line]): node.start(extra_args=['-txindex=1', '-blockfilterindex=1', '-coinstatsindex=1']) self.log.debug("Terminating node after terminate line was found") sigterm_node() diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index cbe42cd1de88..cb9da3959df5 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -490,7 +490,7 @@ def assert_debug_log(self, expected_msgs, unexpected_msgs=None, timeout=2): self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log)) @contextlib.contextmanager - def wait_for_debug_log(self, expected_msgs, timeout=60): + def busy_wait_for_debug_log(self, expected_msgs, timeout=60): """ Block until we see a particular debug log message fragment or until we exceed the timeout. Return: From b8aa61dc81a4bed0dad713901f992573e3a92d0a Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 21 Jun 2023 13:27:54 +0100 Subject: [PATCH 6/6] Merge bitcoin/bitcoin#27905: validation: add missing insert to m_dirty_blockindex e639364495a26bd67dd08998fc7ec400747f9a15 validation: add missing insert to m_dirty_blockindex (Martin Zumsande) Pull request description: When the status of a block index is changed, we must add it to `m_dirty_blockindex` or the change might not get persisted to disk. This is missing from one spot in `FindMostWorkChain()`, where `BLOCK_FAILED_CHILD` is set. Since we have [code](https://github.com/bitcoin/bitcoin/blob/f0758d8a6696657269d9c057e7aa079ffa9e1c16/src/node/blockstorage.cpp#L284-L287) that later sets missing `BLOCK_FAILED_CHILD` during the next startup, I don't think that this can lead to bad block indexes in practice, but I still think it's worth fixing. ACKs for top commit: TheCharlatan: ACK e639364495a26bd67dd08998fc7ec400747f9a15 stickies-v: ACK e639364495a26bd67dd08998fc7ec400747f9a15 Tree-SHA512: a97af9c173e31b90b677a1f95de822e08078d78013de5fa5fe4c3bec06f45d6e1823b7694cdacb887d031329e4b4afc6a2003916e0ae131279dee71f43e1f478 --- src/validation.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/validation.cpp b/src/validation.cpp index 89dfdaa75166..00c0c2f105a6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3288,6 +3288,7 @@ CBlockIndex* CChainState::FindMostWorkChain() while (pindexTest != pindexFailed) { if (fFailedChain) { pindexFailed->nStatus |= BLOCK_FAILED_CHILD; + m_blockman.m_dirty_blockindex.insert(pindexFailed); } else if (fConflictingChain) { // We don't need data for conflciting blocks pindexFailed->nStatus |= BLOCK_CONFLICT_CHAINLOCK;