From 5ce92ca9ea19b70f65532ac53415eea8d2470dbe Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 3 Apr 2024 10:43:14 +0100 Subject: [PATCH 01/17] Merge bitcoin/bitcoin#29527: depends: add -g to DEBUG=1 flags 84fbf9b2841a9ba1ebd1421b9ff9fe444bb1abd9 depends: remove -g from sqlite debug flags (fanquake) eef51afc6a29c693a68400930ef8011be41b7401 depends: add -g to DEBUG=1 flags (fanquake) Pull request description: Add `-g` to the base DEBUG=1 flags in depends. Avoids the need to specify it per-package. More alignment with `--enable-debug` behaviour in configure. We also want to align the optimization flags, currently -O1 vs -O0, however that can be it's own PR. ACKs for top commit: theuni: ACK 84fbf9b2841a9ba1ebd1421b9ff9fe444bb1abd9 Tree-SHA512: 1ee98ba0c13e4b80bb87632658b4f53ce49c73e0e7712990c30da60deca4a349a744232f2d78f243dee9a07f5b9b70f9c2c4ae34082c34ae51b37b054fac61fd --- depends/hosts/darwin.mk | 2 +- depends/hosts/linux.mk | 2 +- depends/hosts/mingw32.mk | 2 +- depends/packages/sqlite.mk | 1 - 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/depends/hosts/darwin.mk b/depends/hosts/darwin.mk index 0ee08ea8dd..c8fe2c949c 100644 --- a/depends/hosts/darwin.mk +++ b/depends/hosts/darwin.mk @@ -123,7 +123,7 @@ darwin_CXXFLAGS=$(darwin_CFLAGS) darwin_release_CFLAGS=-O2 darwin_release_CXXFLAGS=$(darwin_release_CFLAGS) -darwin_debug_CFLAGS=-O1 +darwin_debug_CFLAGS=-O1 -g darwin_debug_CXXFLAGS=$(darwin_debug_CFLAGS) darwin_cmake_system=Darwin diff --git a/depends/hosts/linux.mk b/depends/hosts/linux.mk index c9669c9219..c06d64f452 100644 --- a/depends/hosts/linux.mk +++ b/depends/hosts/linux.mk @@ -10,7 +10,7 @@ linux_CXXFLAGS=$(linux_CFLAGS) linux_release_CFLAGS=-O2 linux_release_CXXFLAGS=$(linux_release_CFLAGS) -linux_debug_CFLAGS=-O1 +linux_debug_CFLAGS=-O1 -g linux_debug_CXXFLAGS=$(linux_debug_CFLAGS) linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_ENABLE_DEBUG_MODE=1 diff --git a/depends/hosts/mingw32.mk b/depends/hosts/mingw32.mk index 979280b5cb..c781a1541b 100644 --- a/depends/hosts/mingw32.mk +++ b/depends/hosts/mingw32.mk @@ -14,7 +14,7 @@ mingw32_CXXFLAGS=$(mingw32_CFLAGS) mingw32_release_CFLAGS=-O2 mingw32_release_CXXFLAGS=$(mingw32_release_CFLAGS) -mingw32_debug_CFLAGS=-O1 +mingw32_debug_CFLAGS=-O1 -g mingw32_debug_CXXFLAGS=$(mingw32_debug_CFLAGS) mingw32_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC diff --git a/depends/packages/sqlite.mk b/depends/packages/sqlite.mk index 7d175ec4bb..15bc0f4d7a 100644 --- a/depends/packages/sqlite.mk +++ b/depends/packages/sqlite.mk @@ -8,7 +8,6 @@ define $(package)_set_vars $(package)_config_opts=--disable-shared --disable-readline --disable-dynamic-extensions --enable-option-checking $(package)_config_opts+= --disable-rtree --disable-fts4 --disable-fts5 # We avoid using `--enable-debug` because it overrides CFLAGS, a behavior we want to prevent. -$(package)_cflags_debug += -g $(package)_cppflags_debug += -DSQLITE_DEBUG $(package)_cppflags+=-DSQLITE_DQS=0 -DSQLITE_DEFAULT_MEMSTATUS=0 -DSQLITE_OMIT_DEPRECATED $(package)_cppflags+=-DSQLITE_OMIT_SHARED_CACHE -DSQLITE_OMIT_JSON -DSQLITE_LIKE_DOESNT_MATCH_BLOBS From 81ca71c266f553578ed24c3403f1b4908c276486 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 9 Apr 2024 09:30:31 +0200 Subject: [PATCH 02/17] Merge bitcoin/bitcoin#29781: depends: add new LLVM debug macro 5efebc0edbb479d2041b3fb2d43be3a77e817b3e depends: add the new LLVM debug macro (fanquake) Pull request description: `LIBCXX_HARDENING_MODE` is the new macro, the previous one was removed in LLVM 18. See https://libcxx.llvm.org/Hardening.html. Required before https://github.com/google/oss-fuzz/pull/11725 will do anything (with the bump to 18.x). Seems reasonable to do now that almost all our test infra is using LLVM 18. ACKs for top commit: theuni: ACK 5efebc0edbb479d2041b3fb2d43be3a77e817b3e Tree-SHA512: 43078eeb5940c55ef4f95c72682f8a372dcd3eb97956b3114149c16d9f59b067a999b2aab7f34ffb57eab191524514408e2bba154ff4a6ea0cd6ec4d119c5d18 --- depends/hosts/linux.mk | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/depends/hosts/linux.mk b/depends/hosts/linux.mk index c06d64f452..4ed565c6f8 100644 --- a/depends/hosts/linux.mk +++ b/depends/hosts/linux.mk @@ -13,7 +13,11 @@ linux_release_CXXFLAGS=$(linux_release_CFLAGS) linux_debug_CFLAGS=-O1 -g linux_debug_CXXFLAGS=$(linux_debug_CFLAGS) -linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_ENABLE_DEBUG_MODE=1 +# https://gcc.gnu.org/onlinedocs/libstdc++/manual/debug_mode.html +linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC + +# https://libcxx.llvm.org/Hardening.html +linux_debug_CPPFLAGS+=-D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG ifeq (86,$(findstring 86,$(build_arch))) i686_linux_CC=gcc -m32 From 1a8e805aab8e3ffbc5edbd963ccdbb04b6c51c09 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 9 Apr 2024 09:41:44 +0200 Subject: [PATCH 03/17] Merge bitcoin/bitcoin#29498: test: Update --tmpdir doc string to say directory must not exist d4e36ae80d4b3f03647fd9057461edf7ecd794a3 test: Update --tmpdir doc string to say directory must not exist (kevkevin) Pull request description: The error message given if passing an existing dir to --tmpdir is confusing so this makes it clear that the directory must not already exist This change is motivated by this comment https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-1960913020 ACKs for top commit: maflcko: lgtm ACK d4e36ae80d4b3f03647fd9057461edf7ecd794a3 davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/29498/commits/d4e36ae80d4b3f03647fd9057461edf7ecd794a3 Tree-SHA512: fb31fd079767abbf94076615817943f35f5c9262fc97e65c631a18d33b3a343fe6a2d151613256e632d2b372ab2de0435f4712309b4a77ed3c663fd93a7dcdd1 --- test/functional/test_framework/test_framework.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index f2ef7f0e03..024ab330ef 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -192,7 +192,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): help="Don't stop dashds after the test execution") parser.add_argument("--cachedir", dest="cachedir", default=os.path.abspath(os.path.dirname(os.path.realpath(__file__)) + "/../../cache"), help="Directory for caching pregenerated datadirs (default: %(default)s)") - parser.add_argument("--tmpdir", dest="tmpdir", help="Root directory for datadirs") + parser.add_argument("--tmpdir", dest="tmpdir", help="Root directory for datadirs (must not exist)") parser.add_argument("-l", "--loglevel", dest="loglevel", default="INFO", help="log events at this level and higher to the console. Can be set to DEBUG, INFO, WARNING, ERROR or CRITICAL. Passing --loglevel DEBUG will output all logs to console. Note that logs at all levels are always written to the test_framework.log file in the temporary test directory.") parser.add_argument("--tracerpc", dest="trace_rpc", default=False, action="store_true", From 4ecb76104e01b828bd08d821c7b3c1dfa52ed061 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 9 Apr 2024 10:17:58 +0200 Subject: [PATCH 04/17] Merge bitcoin/bitcoin#29786: Drop Windows Socket dependency for `randomenv.cpp` 03b87a3e64305ba651e22a730e35271dea8fea64 Drop Windows Socket dependency for `randomenv.cpp` (Hennadii Stepanov) Pull request description: This change drops a dependency on the ws2_32 library for our libbitcoinkernel by switching to [`GetComputerName`](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getcomputernamew) function. ACKs for top commit: sipsorcery: utACK 03b87a3e64305ba651e22a730e35271dea8fea64. laanwj: Code review ACK 03b87a3e64305ba651e22a730e35271dea8fea64. fanquake: ACK 03b87a3e64305ba651e22a730e35271dea8fea64 Tree-SHA512: a4abd5499176634d5f3fbf4e794a7504c40232fb73bd7f41955fbfb2cc7c44bc7ea4518c5203836e52f552c30414c6c3e1b24f0922641dbf1c8377981c0ffaf0 --- src/randomenv.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/randomenv.cpp b/src/randomenv.cpp index ddb8983c6c..9f12def2df 100644 --- a/src/randomenv.cpp +++ b/src/randomenv.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include // for GetTime() #ifdef WIN32 @@ -358,10 +359,19 @@ void RandAddStaticEnv(CSHA512& hasher) hasher << &hasher << &RandAddStaticEnv << &malloc << &errno << &environ; // Hostname +#ifdef WIN32 + constexpr DWORD max_size = MAX_COMPUTERNAME_LENGTH + 1; + char hname[max_size]; + DWORD size = max_size; + if (GetComputerNameA(hname, &size) != 0) { + hasher.Write(UCharCast(hname), size); + } +#else char hname[256]; if (gethostname(hname, 256) == 0) { hasher.Write((const unsigned char*)hname, strnlen(hname, 256)); } +#endif #if HAVE_DECL_GETIFADDRS && HAVE_DECL_FREEIFADDRS // Network interfaces From bb4102c5908dd08bd9d37a7249aef573dce57a4e Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 17 Apr 2024 11:24:24 +0100 Subject: [PATCH 05/17] Merge bitcoin/bitcoin#29893: test: fix intermittent failure in p2p_compactblocks_hb.py 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197 test: fix intermittent failure in p2p_compactblocks_hb.py (Martin Zumsande) Pull request description: Fixes #29860 As a result of node1 receiving a block, it sends out SENDCMPCT messages to some of its peers to update the high-bandwidth status. We need to wait until those are received and processed by the peers to avoid intermittent failures. Before, we'd only wait until all peers have synced with the new block (within `generate`) which is not sufficient. I could reproduce the failure by adding a `std::this_thread::sleep_for(std::chrono::milliseconds(1000));` sleep to the [net_processing code](https://github.com/bitcoin/bitcoin/blob/c7567d9223a927a88173ff04eeb4f54a5c02b43d/src/net_processing.cpp#L3763) that processes `NetMsgType::SENDCMPCT`. ACKs for top commit: instagibbs: ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197 alfonsoromanz: Tested ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197 glozow: ACK 1ae5b208d339fa984d9caf4fab89b0b2ba9cc197 Tree-SHA512: 47c29616e73a5e0ff966fc231e4f672c1a6892511e5c10a3905b30ad6b2a3d1267fa0a88bd8f64b523fe580199d22a43545c84e361879e5096483152065c4b9a --- test/functional/p2p_compactblocks_hb.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/test/functional/p2p_compactblocks_hb.py b/test/functional/p2p_compactblocks_hb.py index 72b3897b4f..747ce22dfe 100755 --- a/test/functional/p2p_compactblocks_hb.py +++ b/test/functional/p2p_compactblocks_hb.py @@ -33,10 +33,15 @@ class CompactBlocksConnectionTest(BitcoinTestFramework): self.generate(self.nodes[0], 1) self.sync_blocks() self.disconnect_nodes(peer, 0) - status_to = [self.peer_info(1, i)['bip152_hb_to'] for i in range(2, 6)] - status_from = [self.peer_info(i, 1)['bip152_hb_from'] for i in range(2, 6)] - assert_equal(status_to, status_from) - return status_to + + def status_to(): + return [self.peer_info(1, i)['bip152_hb_to'] for i in range(2, 6)] + + def status_from(): + return [self.peer_info(i, 1)['bip152_hb_from'] for i in range(2, 6)] + + self.wait_until(lambda: status_to() == status_from()) + return status_to() def run_test(self): self.log.info("Testing reserved high-bandwidth mode slot for outbound peer...") From 51bc8bdcd6e819c811a3dfb3e95ff2e84d83237b Mon Sep 17 00:00:00 2001 From: merge-script Date: Wed, 17 Apr 2024 14:00:40 +0100 Subject: [PATCH 06/17] Merge bitcoin/bitcoin#29859: build: Fix false positive `CHECK_ATOMIC` test dd3e0fa12534c9e782dc9c24d2e30b70a0d73176 build: Fix false positive `CHECK_ATOMIC` test for clang-15 (Hennadii Stepanov) Pull request description: On the master branch @ 0de63b8b46eff5cda85b4950062703324ba65a80, a building `bitcoind` with clang-15 for `i686-pc-linux-gnu` fails to link: ``` CXXLD bitcoind /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `std::remove_volatile::type std::__atomic_impl::load(double const*, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:948: undefined reference to `__atomic_load' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store(double*, std::remove_volatile::type, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store(double*, std::remove_volatile::type, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `std::remove_volatile::type std::__atomic_impl::load(double const*, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:948: undefined reference to `__atomic_load' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): in function `void std::__atomic_impl::store(double*, std::remove_volatile::type, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-backup.o): in function `void std::__atomic_impl::store(double*, std::remove_volatile::type, std::memory_order)': /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: undefined reference to `__atomic_store' /usr/bin/ld: libbitcoin_wallet.a(libbitcoin_wallet_a-backup.o):/usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/atomic_base.h:940: more undefined references to `__atomic_store' follow clang: error: linker command failed with exit code 1 (use -v to see invocation) ``` due to false positive `CHECK_ATOMIC` test in the `configure` script. This PR fixes this test. ACKs for top commit: maflcko: review ACK dd3e0fa12534c9e782dc9c24d2e30b70a0d73176 fanquake: ACK dd3e0fa12534c9e782dc9c24d2e30b70a0d73176 Tree-SHA512: b60acf8d83fc84cc3280d95028395d341ed9ed2fcf38ae0a101d50aa19cc35540e9763aa36668c4dc1e1bc7e1f33dbda0e662df39c9e414a284ef91d7fc55fba --- build-aux/m4/l_atomic.m4 | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/build-aux/m4/l_atomic.m4 b/build-aux/m4/l_atomic.m4 index aa00168fce..859ddaabbb 100644 --- a/build-aux/m4/l_atomic.m4 +++ b/build-aux/m4/l_atomic.m4 @@ -7,7 +7,7 @@ dnl warranty. # Clang, when building for 32-bit, # and linking against libstdc++, requires linking with # -latomic if using the C++ atomic library. -# Can be tested with: clang++ test.cpp -m32 +# Can be tested with: clang++ -std=c++20 test.cpp -m32 # # Sourced from http://bugs.debian.org/797228 @@ -27,8 +27,11 @@ m4_define([_CHECK_ATOMIC_testbody], [[ auto t1 = t.load(); t.compare_exchange_strong(t1, 3s); - std::atomic a{}; + std::atomic d{}; + d.store(3.14); + auto d1 = d.load(); + std::atomic a{}; int64_t v = 5; int64_t r = a.fetch_add(v); return static_cast(r); From acfdf9e4389859402e81f1e457b75d984b8c3c2b Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 17 Apr 2024 11:28:48 -0400 Subject: [PATCH 07/17] Merge bitcoin/bitcoin#28373: doc: Add example of mixing private and public keys in descriptors 24b67fa9f602cdeac0e9736256f77d048f616c48 doc: Add example of mixing private and public keys in descriptors (Anton A) Pull request description: closes: #27414 ACKs for top commit: achow101: ACK 24b67fa9f602cdeac0e9736256f77d048f616c48 alfonsoromanz: Re ACK 24b67fa9f602cdeac0e9736256f77d048f616c48 Tree-SHA512: 8c063f23199ac0ff35909f786a5b0de1b4a9b15d1e93bdcdac10cb4bd2002c12e99b6fb1c2e56d16971e7622b67d910b79088429df92c48279be2d7797049911 --- doc/descriptors.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/doc/descriptors.md b/doc/descriptors.md index c9ce37ac46..c90463657a 100644 --- a/doc/descriptors.md +++ b/doc/descriptors.md @@ -166,7 +166,18 @@ Often it is useful to communicate a description of scripts along with the necessary private keys. For this reason, anywhere a public key or xpub is supported, a private key in WIF format or xprv may be provided instead. This is useful when private keys are necessary for hardened derivation -steps, or for dumping wallet descriptors including private key material. +steps, for signing transactions, or for dumping wallet descriptors +including private key material. + +For example, after importing the following 2-of-3 multisig descriptor +into a wallet, one could use `signrawtransactionwithwallet` +to sign a transaction with the first key: +``` +sh(multi(2,xprv.../84'/0'/0'/0/0,xpub1...,xpub2...)) +``` +Note how the first key is an xprv private key with a specific derivation path, +while the other two are public keys. + ### Compatibility with old wallets From c4a147cfea557e3f32637ea0749ebf02004033fd Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 17 Apr 2024 12:59:37 -0400 Subject: [PATCH 08/17] Merge bitcoin/bitcoin#28340: security: restrict abis in bitcoind.service 0244416aacbad03e4ebe8f2c95c7861a318916ea security: restrict abis in bitcoind.service (Charlie) Pull request description: [As noted here](https://www.freedesktop.org/software/systemd/man/systemd.exec.html#MemoryDenyWriteExecute=), it's a good idea to pair `MemoryDenyWriteExecute=true` with `SystemCallArchitectures=native` because `MemoryDenyWriteExecute` can be circumvented in some operating systems which support multiple ABIs like x86/x86-64. This helps restrict the possible application binary interfaces (ABIs) that can be used when running bitcoind through systemd, reducing the attack surface area. ACKs for top commit: laanwj: ACK 0244416aacbad03e4ebe8f2c95c7861a318916ea . This is a sensible security feature. 0xB10C: ACK 0244416aacbad03e4ebe8f2c95c7861a318916ea Tree-SHA512: 77a35b0674d8d67d857cd20ae1b8cd011f82d6f5ed21bc106cbe45bfa937e786ddc1bf7261e3bdb8c289df1224e91658760905d2c8f37cc4c6506ef8037ad158 --- contrib/init/dashd.service | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/init/dashd.service b/contrib/init/dashd.service index 223c05a875..60140bdf7a 100644 --- a/contrib/init/dashd.service +++ b/contrib/init/dashd.service @@ -78,5 +78,8 @@ PrivateDevices=true # Deny the creation of writable and executable memory mappings. MemoryDenyWriteExecute=true +# Restrict ABIs to help ensure MemoryDenyWriteExecute is enforced +SystemCallArchitectures=native + [Install] WantedBy=multi-user.target From b53b85409f21af756105b6cbbaf7f81f30527059 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 22 Apr 2024 12:09:17 -0400 Subject: [PATCH 09/17] Merge bitcoin/bitcoin#29850: net: Decrease nMaxIPs when learning from DNS seeds f2e3662e57eca1330962faf38ff428a564d50a11 net: Decrease nMaxIPs when learning from DNS seeds (laanwj) Pull request description: Limit number of IPs learned from a single DNS seed to 32, to prevent the results from one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is bounded to 33 already, but it is possible for it to use TCP where a larger number of results can be returned. Closes #16070. ACKs for top commit: Sjors: utACK f2e3662e57eca1330962faf38ff428a564d50a11 achow101: ACK f2e3662e57eca1330962faf38ff428a564d50a11 1440000bytes: utACK https://github.com/bitcoin/bitcoin/pull/29850/commits/f2e3662e57eca1330962faf38ff428a564d50a11 mzumsande: utACK f2e3662e57eca1330962faf38ff428a564d50a11 Tree-SHA512: 3f108c2baba7adfedb8019daaf60aa00e628b38d3942e1319c7183a4683670be01929ced9e6372c8e983c902e8633f81fbef12d7cdcaadd7f77ed729c1019942 --- src/net.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index 351c114d60..be0656f227 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -3062,7 +3062,11 @@ void CConnman::ThreadDNSAddressSeed() if (!resolveSource.SetInternal(host)) { continue; } - unsigned int nMaxIPs = 256; // Limits number of IPs learned from a DNS seed + // Limit number of IPs learned from a single DNS seed. This limit exists to prevent the results from + // one DNS seed from dominating AddrMan. Note that the number of results from a UDP DNS query is + // bounded to 33 already, but it is possible for it to use TCP where a larger number of results can be + // returned. + unsigned int nMaxIPs = 32; const auto addresses{LookupHost(host, nMaxIPs, true)}; if (!addresses.empty()) { for (const CNetAddr& ip : addresses) { From 3df1ca102b05d83bfa88f23ba116d04c9fa4ccde Mon Sep 17 00:00:00 2001 From: merge-script Date: Thu, 25 Apr 2024 21:22:45 +0800 Subject: [PATCH 10/17] Merge bitcoin/bitcoin#29953: doc: Bash is needed in gen_id and is not installed on FreeBSD by default 9381052194a78024b3994cc6ad906858c477b88f doc: Bash is needed in gen_id and is not installed on FreeBSD by default (Hennadii Stepanov) Pull request description: On FreeBSD 14.0, in the `depends` directory: - without `bash`: ``` $ gmake print-bdb_build_id_long env: bash: No such file or directory env: bash: No such file or directory bdb_build_id_long=bdb-4.8.30-4b0c6f8e95251b9c6731844fc34111c04b75fd9f15c671d6e34f2a4d014ec1be-release $ gmake print-final_build_id env: bash: No such file or directory env: bash: No such file or directory final_build_id=722b2d3e264 ``` - with `bash`: ``` $ gmake print-bdb_build_id_long bdb_build_id_long=bdb-4.8.30-4b0c6f8e95251b9c6731844fc34111c04b75fd9f15c671d6e34f2a4d014ec1be-release 1ed47cefe468014c79dedb275cf921f44ab28d91dd56bf94712409b81326d765 $ gmake print-final_build_id final_build_id=7b4f9aaa683 ``` ACKs for top commit: vasild: ACK 9381052194a78024b3994cc6ad906858c477b88f kristapsk: ACK 9381052194a78024b3994cc6ad906858c477b88f alfonsoromanz: ACK 9381052194a78024b3994cc6ad906858c477b88f Tree-SHA512: da3f3469ac416518180194f09fb054fb352a2793848fb9a7982439de08244ff6149a7f449ad21fcdf0e9bd79b6949a91751f9cc35833953d2b6a35cea5c6ae21 --- depends/README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/depends/README.md b/depends/README.md index 988c2f16f4..308be7814a 100644 --- a/depends/README.md +++ b/depends/README.md @@ -87,6 +87,10 @@ For linux S390X cross compilation: sudo apt-get install g++-s390x-linux-gnu binutils-s390x-linux-gnu +### Install the required dependencies: FreeBSD + + pkg install bash + ### Install the required dependencies: OpenBSD pkg_add bash gtar From a7daee71da6b00ff8d9a1679bcfd08643ad2e03d Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 25 Apr 2024 12:34:32 -0400 Subject: [PATCH 11/17] Merge bitcoin/bitcoin#29689: lint: scripted-diff verification also requires GNU grep 3bf4f8db669e1e274ce2633cf84add2938b9914b lint: scripted-diff verification also requires GNU grep (Sjors Provoost) Pull request description: I noticed while trying to verify all historical `scripted-diff:` commits on macOS that some scripts require GNU sed. For example 0d6d2b650d1017691f48c9109a6cd020ab46aa73 uses `git grep --perl-regexp`. ACKs for top commit: hernanmarino: cr ACK 3bf4f8db669e1e274ce2633cf84add2938b9914b maflcko: utACK 3bf4f8db669e1e274ce2633cf84add2938b9914b achow101: ACK 3bf4f8db669e1e274ce2633cf84add2938b9914b alfonsoromanz: Tested ACK 3bf4f8db669e1e274ce2633cf84add2938b9914b kristapsk: cr utACK 3bf4f8db669e1e274ce2633cf84add2938b9914b Tree-SHA512: 09a060ab1bafad03df60d0f20c3dd1451850868dbd66ea38b18178b6230c1f06cf48622db82d9c51422d5689962ee0cd7aae0a31f84bd6d878215e6d73c1d47e --- test/lint/commit-script-check.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/lint/commit-script-check.sh b/test/lint/commit-script-check.sh index ecf8039839..37251f8be9 100755 --- a/test/lint/commit-script-check.sh +++ b/test/lint/commit-script-check.sh @@ -22,6 +22,11 @@ if ! sed --help 2>&1 | grep -q 'GNU'; then exit 1; fi +if ! grep --help 2>&1 | grep -q 'GNU'; then + echo "Error: the installed grep package is not compatible. Please make sure you have GNU grep installed in your system."; + exit 1; +fi + RET=0 PREV_BRANCH=$(git name-rev --name-only HEAD) PREV_HEAD=$(git rev-parse HEAD) From 23f25a94fa17ce115fd54aff2722df6656ed3ec6 Mon Sep 17 00:00:00 2001 From: merge-script Date: Mon, 29 Apr 2024 21:37:49 +0800 Subject: [PATCH 12/17] Merge bitcoin/bitcoin#29872: test: Add missing Assert(mock_time_in >= 0s) to SetMockTime fae0db555c12dca75fb09e5fa7bbabdf39b8c1df refactor: Use chrono type for g_mock_time (MarcoFalke) fa382d3dd0592f3cbd6e1de791449f49e06dae86 test: Add missing Assert(mock_time_in >= 0s) to SetMockTime (MarcoFalke) Pull request description: Seems odd to have the assert in the *deprecated* function, but not in the other. Fix this by adding it to the other, and by inlining the deprecated one. Also, use chrono type for the global mocktime variable. ACKs for top commit: davidgumberg: crACK https://github.com/bitcoin/bitcoin/pull/29872/commits/fae0db555c12dca75fb09e5fa7bbabdf39b8c1df stickies-v: ACK fae0db555c12dca75fb09e5fa7bbabdf39b8c1df Tree-SHA512: 630c2917422ff2a7fa307114f95f22ad3c205429ffe36e67f0b2650733e40c876289c1aecebe882a9123d3106db7606bd6eff067ed6e2ecb95765984d3fe8612 --- src/util/time.cpp | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/util/time.cpp b/src/util/time.cpp index 14183d540d..fa0ad5b5a4 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -21,7 +21,7 @@ void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread::sleep_for(n); } -static std::atomic nMockTime(0); //!< For testing +static std::atomic g_mock_time{}; //!< For testing bool ChronoSanityCheck() { @@ -69,7 +69,7 @@ bool ChronoSanityCheck() template T GetTime() { - const std::chrono::seconds mocktime{nMockTime.load(std::memory_order_relaxed)}; + const auto mocktime{g_mock_time.load(std::memory_order_relaxed)}; const auto ret{ mocktime.count() ? mocktime : @@ -89,20 +89,16 @@ static T GetSystemTime() return now; } -void SetMockTime(int64_t nMockTimeIn) -{ - Assert(nMockTimeIn >= 0); - nMockTime.store(nMockTimeIn, std::memory_order_relaxed); -} - +void SetMockTime(int64_t nMockTimeIn) { SetMockTime(std::chrono::seconds{nMockTimeIn}); } void SetMockTime(std::chrono::seconds mock_time_in) { - nMockTime.store(mock_time_in.count(), std::memory_order_relaxed); + Assert(mock_time_in >= 0s); + g_mock_time.store(mock_time_in, std::memory_order_relaxed); } std::chrono::seconds GetMockTime() { - return std::chrono::seconds(nMockTime.load(std::memory_order_relaxed)); + return g_mock_time.load(std::memory_order_relaxed); } int64_t GetTimeMillis() From 61a5832a6ac45638a3e21d7d447bccd004bca84f Mon Sep 17 00:00:00 2001 From: merge-script Date: Sat, 4 May 2024 09:07:44 +0800 Subject: [PATCH 13/17] Merge bitcoin/bitcoin#29907: test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos (Hennadii Stepanov) Pull request description: On systems where `int8_t` is defined as `char`, the `{S,Uns}erialize(Stream&, signed char)` functions become undefined. This PR resolves the issue by testing `{S,Uns}erialize(Stream&, int8_t)` instead. No behavior change on systems where `int8_t` is defined as `signed char`, which is the case for most other systems. Fixes https://github.com/bitcoin/bitcoin/issues/29884. An alternative approach is mentioned in https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2058434577 as well. ACKs for top commit: maflcko: lgtm ACK 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b theuni: ACK 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b. Nice to have the serialization concept actually tested :) Tree-SHA512: 1033863e584fa8e99a281b236fa01fc919f610a024bcec792116762e28c1c16ee481bd01325c3a0ca9dd9d753176aa63bd9ac7e08a9bbce772db2949d06f6e61 --- src/test/streams_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 2f8b76d592..9ef9574859 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -85,8 +85,8 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader) BOOST_CHECK_EQUAL(reader.size(), 5U); BOOST_CHECK(!reader.empty()); - // Read a single byte as a signed char. - signed char b; + // Read a single byte as a int8_t. + int8_t b; reader >> b; BOOST_CHECK_EQUAL(b, -1); BOOST_CHECK_EQUAL(reader.size(), 4U); From daa6eeed5fe75c4ad304b9358ca0c29e7068e47f Mon Sep 17 00:00:00 2001 From: merge-script Date: Mon, 6 May 2024 09:47:29 +0800 Subject: [PATCH 14/17] Merge bitcoin/bitcoin#29960: depends: pass verbose through to cmake based makefiles 7c69baf227252511455bc06e315f6a3c7fc5a398 depends: pass verbose through to cmake based make (Max Edwards) Pull request description: While testing https://github.com/bitcoin/bitcoin/pull/29708 I was not able to enable verbose output to check which flags were being given to the compiler. With this PR, running depends with V=1 will enable verbose output from makefiles generated by cmake. How to test: ```shell make -C depends libnatpmp V=1 ``` ACKs for top commit: hebasto: ACK 7c69baf227252511455bc06e315f6a3c7fc5a398. Tested using the folowing command: fanquake: ACK 7c69baf227252511455bc06e315f6a3c7fc5a398 Tree-SHA512: 81cd1326e940c5f14cbde96735fd02b03c1150881ed88d1e8dfa9385dfa12284bfa2cdfe097ce5f43a726c1718afb76ae16f71552ab68c207d74fdc1f7bb46ae --- depends/funcs.mk | 1 + 1 file changed, 1 insertion(+) diff --git a/depends/funcs.mk b/depends/funcs.mk index 76b205ae6d..6a024d5ef4 100644 --- a/depends/funcs.mk +++ b/depends/funcs.mk @@ -183,6 +183,7 @@ $(1)_cmake=env CC="$$($(1)_cc)" \ cmake -DCMAKE_INSTALL_PREFIX:PATH="$$($($(1)_type)_prefix)" \ -DCMAKE_INSTALL_LIBDIR=lib/ \ -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ + -DCMAKE_VERBOSE_MAKEFILE:BOOL=$(V) \ $$($(1)_config_opts) ifeq ($($(1)_type),build) $(1)_cmake += -DCMAKE_INSTALL_RPATH:PATH="$$($($(1)_type)_prefix)/lib" From f1907ea997ec654f1aa25bf1e5513d674a4cbbb4 Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 7 May 2024 10:28:58 +0800 Subject: [PATCH 15/17] Merge bitcoin/bitcoin#29984: net: Replace ifname check with IFF_LOOPBACK in Discover a68fed111be393ddbbcd7451f78bc63601253ee0 net: Fix misleading comment for Discover (laanwj) 7766dd280d9a4a7ffdfcec58224d0985cfd4169b net: Replace ifname check with IFF_LOOPBACK in Discover (laanwj) Pull request description: Checking the interface name is kind of brittle. In the age of network namespaces and containers, there is no reason a loopback interface can't be called differently. Check for the `IFF_LOOPBACK` flag to detect loopback interface instead. Also remove a misleading comment in Discover's doc comment. ACKs for top commit: sipa: utACK a68fed111be393ddbbcd7451f78bc63601253ee0 willcl-ark: utACK a68fed111be393ddbbcd7451f78bc63601253ee0 theuni: utACK a68fed111be393ddbbcd7451f78bc63601253ee0. Satoshi-era brittleness :) Tree-SHA512: e2d7fc541f40f6a6af08286e7bcb0873ff55debdcd8b38b03f274897b673a6fb51d84d6c7241a02a9567ddf2645f50231d91bb1f55307ba7c6e68196c29b0edf --- src/net.cpp | 3 +-- src/net.h | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index be0656f227..8e62536bcc 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -4090,8 +4090,7 @@ void Discover() { if (ifa->ifa_addr == nullptr) continue; if ((ifa->ifa_flags & IFF_UP) == 0) continue; - if (strcmp(ifa->ifa_name, "lo") == 0) continue; - if (strcmp(ifa->ifa_name, "lo0") == 0) continue; + if ((ifa->ifa_flags & IFF_LOOPBACK) != 0) continue; if (ifa->ifa_addr->sa_family == AF_INET) { struct sockaddr_in* s4 = (struct sockaddr_in*)(ifa->ifa_addr); diff --git a/src/net.h b/src/net.h index 1691f6f8d5..83ac6a1095 100644 --- a/src/net.h +++ b/src/net.h @@ -169,8 +169,7 @@ struct CSerializedNetMsg { /** * Look up IP addresses from all interfaces on the machine and add them to the * list of local addresses to self-advertise. - * The loopback interface is skipped and only the first address from each - * interface is used. + * The loopback interface is skipped. */ void Discover(); From a0cd305a7cc362983d7b82720538946e4250678c Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Fri, 10 May 2024 12:44:42 -0400 Subject: [PATCH 16/17] Merge bitcoin/bitcoin#29948: test: add missing comparison of node1's mempool in MempoolPackagesTest e912717ff63f111d8f1cd7ed1fcf054e28f36409 test: add missing comparison of node1's mempool in MempoolPackagesTest (umiumi) Pull request description: #29941 Recreated a pull request because there was a conflict. Trying to resolve the conflict but the old one automatically closed. Add missing comparison for TODO comments in `mempool_packages.py` Also, notice that the ancestor size limits and descendant size limits actually implemented in #21800 , so I removed the todo for those two size limits. ACKs for top commit: kevkevinpal: ACK [e912717](https://github.com/bitcoin/bitcoin/pull/29948/commits/e912717ff63f111d8f1cd7ed1fcf054e28f36409) achow101: ACK e912717ff63f111d8f1cd7ed1fcf054e28f36409 alfonsoromanz: Tested ACK e912717ff63f111d8f1cd7ed1fcf054e28f36409. The code looks good to me and the test execution is successful. rkrux: tACK [e912717](https://github.com/bitcoin/bitcoin/pull/29948/commits/e912717ff63f111d8f1cd7ed1fcf054e28f36409) Tree-SHA512: 8cb51746b0547369344c9ceef59599bfe9c91d424687af5e24dc6641f9e99fb433515d79c724e71fd3d5e02994f0cef623d3674367b8296b05c3c6fcdde282ef --- test/functional/mempool_packages.py | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index 6b9096bbc5..35416aaa22 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -201,13 +201,13 @@ class MempoolPackagesTest(BitcoinTestFramework): assert set(mempool1).issubset(set(mempool0)) for tx in chain[:MAX_ANCESTORS_CUSTOM]: assert tx in mempool1 - # TODO: more detailed check of node1's mempool (fees etc.) - # check transaction unbroadcast info (should be false if in both mempools) - mempool = self.nodes[0].getrawmempool(True) - for tx in mempool: - assert_equal(mempool[tx]['unbroadcast'], False) - - # TODO: test ancestor size limits + entry0 = self.nodes[0].getmempoolentry(tx) + entry1 = self.nodes[1].getmempoolentry(tx) + assert not entry0['unbroadcast'] + assert not entry1['unbroadcast'] + assert_equal(entry1['fees']['base'], entry0['fees']['base']) + assert_equal(entry1['vsize'], entry0['vsize']) + assert_equal(entry1['depends'], entry0['depends']) # Now test descendant chain limits txid = utxo[1]['txid'] @@ -258,10 +258,14 @@ class MempoolPackagesTest(BitcoinTestFramework): assert tx in mempool1 for tx in chain[MAX_DESCENDANTS_CUSTOM:]: assert tx not in mempool1 - # TODO: more detailed check of node1's mempool (fees etc.) - - # TODO: test descendant size limits - + for tx in mempool1: + entry0 = self.nodes[0].getmempoolentry(tx) + entry1 = self.nodes[1].getmempoolentry(tx) + assert not entry0['unbroadcast'] + assert not entry1['unbroadcast'] + assert_equal(entry1['fees']['base'], entry0['fees']['base']) + assert_equal(entry1['vsize'], entry0['vsize']) + assert_equal(entry1['depends'], entry0['depends']) # Test reorg handling # First, the basics: self.generate(self.nodes[0], 1) From 700b8c5ac5d88af75649a5ab33f447915c9d68d5 Mon Sep 17 00:00:00 2001 From: merge-script <32963518+hebasto@users.noreply.github.com> Date: Sun, 12 May 2024 14:21:21 +0100 Subject: [PATCH 17/17] Merge bitcoin/bitcoin#29658: Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one d1ed09a7643b567e021b2ecb756bc925c48fc708 Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one (Luke Dashjr) Pull request description: Reviewing #29585, I noticed that `bitcoin-qt` adds an extra newline for `-help` and `-version` beyond the other binaries'. ACKs for top commit: hebasto: ACK d1ed09a7643b567e021b2ecb756bc925c48fc708, tested on Ubuntu 24.04. Tree-SHA512: 15ee9d1060c2492bb3b04a0ac4cb53f7b959bbe32bce415793da0c221f1c963c8f2bb3996ea07d1a7c192bfc2e23be2cd7d4e5649c592eb3fc03906c2763f1aa --- src/qt/utilitydialog.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/utilitydialog.cpp b/src/qt/utilitydialog.cpp index eaa52946f7..83743878b3 100644 --- a/src/qt/utilitydialog.cpp +++ b/src/qt/utilitydialog.cpp @@ -158,7 +158,7 @@ HelpMessageDialog::~HelpMessageDialog() void HelpMessageDialog::printToConsole() { // On other operating systems, the expected action is to print the message to the console. - tfm::format(std::cout, "%s\n", qPrintable(text)); + tfm::format(std::cout, "%s", qPrintable(text)); } void HelpMessageDialog::showOrPrint()