From a43fdd469a55c5014bd1ac660aa6a7a1a44a558b Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 22 Apr 2021 12:29:20 +0200 Subject: [PATCH 01/10] Merge bitcoin/bitcoin#21564: net: Avoid calling getnameinfo when formatting IPv4 addresses in CNetAddr::ToStringIP 58580a827d10c0675c3483e2aeca1d3ab8050ae0 net: Avoid calling getnameinfo when formatting IPv4 addresses in CNetAddr::ToStringIP (practicalswift) 58580573843858068db69a378b6e9363889b1f6d net: Add IPv4ToString (we already have IPv6ToString) (practicalswift) Pull request description: Avoid calling `getnameinfo` when formatting IPv4 addresses in `CNetAddr::ToStringIP`. ACKs for top commit: naumenkogs: ACK 58580a827d10c0675c3483e2aeca1d3ab8050ae0 0xB10C: ACK 58580a827d10c0675c3483e2aeca1d3ab8050ae0 vasild: ACK 58580a827d10c0675c3483e2aeca1d3ab8050ae0 Tree-SHA512: 25e3c416acb74908d001baf1cf64c04cbc0d94ce8e7ce5a601f1343062d5d748cb406a3404e6f2b6e7e979c6300b38439e1bfd70ea90ec8c0ec2d7568f09fbcd --- src/netaddress.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 4d43550ba8..f4134df18d 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -503,6 +503,11 @@ enum Network CNetAddr::GetNetwork() const return m_net; } +static std::string IPv4ToString(Span a) +{ + return strprintf("%u.%u.%u.%u", a[0], a[1], a[2], a[3]); +} + static std::string IPv6ToString(Span a) { assert(a.size() == ADDR_IPV6_SIZE); @@ -523,6 +528,7 @@ std::string CNetAddr::ToStringIP(bool fUseGetnameinfo) const { switch (m_net) { case NET_IPV4: + return IPv4ToString(m_addr); case NET_IPV6: { if (fUseGetnameinfo) { CService serv(*this, 0); @@ -535,9 +541,6 @@ std::string CNetAddr::ToStringIP(bool fUseGetnameinfo) const return std::string(name); } } - if (m_net == NET_IPV4) { - return strprintf("%u.%u.%u.%u", m_addr[0], m_addr[1], m_addr[2], m_addr[3]); - } return IPv6ToString(m_addr); } case NET_ONION: From 5d11b57e5c1480ee6cfde8a69bd2e4309d002096 Mon Sep 17 00:00:00 2001 From: fanquake Date: Sun, 9 May 2021 11:46:05 +0800 Subject: [PATCH 02/10] Merge bitcoin/bitcoin#21869: depends: Add missing -D_LIBCPP_DEBUG=1 to debug flags fa9249aaccc3ef7a0a91a822e1cb666c4c9716ec depends: Add missing -D_LIBCPP_DEBUG=1 to debug flags (MarcoFalke) Pull request description: Commands that can be used for testing: ``` $ cat 1.cpp #include int main() { std::vector foo; foo.begin() + 7; } ``` ``` clang++ -stdlib=libc++ -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_DEBUG=1 -Wall 1.cpp -o exe && ./exe g++ -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_DEBUG=1 -Wall 1.cpp -o exe && ./exe ACKs for top commit: practicalswift: cr ACK fa9249aaccc3ef7a0a91a822e1cb666c4c9716ec: patch looks correct fanquake: ACK fa9249aaccc3ef7a0a91a822e1cb666c4c9716ec - was going to suggest adding this to the macOS CPP flags as well, however it seems doing that is less straight forward. Could be looked at by someone in a followup. Tree-SHA512: 2ffbaaf0ccb36bcc9fa1a15426566406c6115c8878ff211a4794d982c5d198672d444a20f6c7ae9f341193f6d8118c7cc50896daf98af9553834379e47ddb39e --- depends/hosts/linux.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/depends/hosts/linux.mk b/depends/hosts/linux.mk index 602206d634..99434c90d5 100644 --- a/depends/hosts/linux.mk +++ b/depends/hosts/linux.mk @@ -7,7 +7,7 @@ linux_release_CXXFLAGS=$(linux_release_CFLAGS) linux_debug_CFLAGS=-O1 linux_debug_CXXFLAGS=$(linux_debug_CFLAGS) -linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC +linux_debug_CPPFLAGS=-D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_DEBUG=1 ifeq (86,$(findstring 86,$(build_arch))) i686_linux_CC=gcc -m32 From 34c414e8dbded647bdbb3d92b589372ef4bd1610 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 10 May 2021 08:22:05 +0200 Subject: [PATCH 03/10] Merge bitcoin/bitcoin#21581: streams: Accept URef obj for VectorReader unserialize fa2204f6adef493079d1ca5148b0fdc2b55816e6 streams: Accept URef obj for VectorReader unserialize (MarcoFalke) Pull request description: Missed in commit 172f5fa738d419efda99542e2ad2a0f4db5be580. An URef may collapse into an LRef or RRef depending on context. There is no reason to forbid RRef in `VectorReader::operator>>`, so add it for consistency. ACKs for top commit: ryanofsky: Code review ACK fa2204f6adef493079d1ca5148b0fdc2b55816e6, just expanded test since last review Tree-SHA512: 09ff4e8a918e15b08cebd8c125d37e78bfb3a635c38546fc8454a97a882b2c81c55ef552243617e78744799d31127e6fbf78c4e319c030480b370aab6f38b645 --- src/streams.h | 2 +- src/test/streams_tests.cpp | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/streams.h b/src/streams.h index 1dc30901ce..866cdef762 100644 --- a/src/streams.h +++ b/src/streams.h @@ -174,7 +174,7 @@ public: } template - VectorReader& operator>>(T& obj) + VectorReader& operator>>(T&& obj) { // Unserialize from this stream ::Unserialize(*this, obj); diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 435fc1905b..34e0c587ed 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -114,6 +114,17 @@ BOOST_AUTO_TEST_CASE(streams_vector_reader) BOOST_CHECK_THROW(reader >> d, std::ios_base::failure); } +BOOST_AUTO_TEST_CASE(streams_vector_reader_rvalue) +{ + std::vector data{0x82, 0xa7, 0x31}; + VectorReader reader(SER_NETWORK, INIT_PROTO_VERSION, data, /* pos= */ 0); + uint32_t varint = 0; + // Deserialize into r-value + reader >> VARINT(varint); + BOOST_CHECK_EQUAL(varint, 54321); + BOOST_CHECK(reader.empty()); +} + BOOST_AUTO_TEST_CASE(bitstream_reader_writer) { CDataStream data(SER_NETWORK, INIT_PROTO_VERSION); From 38b3d180831914d7723e5393b34d7530aeccba03 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 11 May 2021 07:00:31 +0200 Subject: [PATCH 04/10] Merge bitcoin/bitcoin#21895: refactor: Add TSA annotations to the WorkQueue class members 34b04eec4448bd37a8dbf560e4d99c7e7ca7e9c0 refactor: Add TSA annotations to the WorkQueue class members (Hennadii Stepanov) Pull request description: Noted while reviewing #19033, and hoping this will not conflict with it :) ACKs for top commit: promag: Code review ACK 34b04eec4448bd37a8dbf560e4d99c7e7ca7e9c0. Tree-SHA512: 4c15729acd95223263c19bc0dd64b9e7960872b48edee6eee97a5d0c2b99b8838185ac3a2ccd5bee992cb3a12498633427fe9919be5a12da9949fcf69a6275a0 --- src/httpserver.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 127edc2594..2cf97280d6 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -74,12 +74,11 @@ template class WorkQueue { private: - /** Mutex protects entire object */ Mutex cs; - std::condition_variable cond; - std::deque> queue; - bool running; - size_t maxDepth; + std::condition_variable cond GUARDED_BY(cs); + std::deque> queue GUARDED_BY(cs); + bool running GUARDED_BY(cs); + const size_t maxDepth; public: explicit WorkQueue(size_t _maxDepth) : running(true), From aaf105c8b5a23cc1af794d5df304bf5657da15d6 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 12 May 2021 19:35:58 +0200 Subject: [PATCH 05/10] Merge bitcoin/bitcoin#21911: build: add configure~ to .gitignore bc4538806e3c53e7821e01d5db896f65dd3358ad build: add *~ to .gitignore (Sjors Provoost) Pull request description: The file `configure~` recently started appearing for me on macOS (11.3.1) whenever configure is (re)run. ACKs for top commit: hebasto: ACK bc4538806e3c53e7821e01d5db896f65dd3358ad, tested on Linux Mint 20.1 with different build scenarios including cross-compiling for Windows and macOS. Tree-SHA512: 830c7baf392ff6d66250a79c6ed0a98dac3daaace54a6d2e7940b9a72e3bac79ab44bbecd7642c931fde8a446654e2260d6afdecc679a1743fae6ec5eeda79f1 --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 737b076b7a..43f8eb1d72 100644 --- a/.gitignore +++ b/.gitignore @@ -58,7 +58,7 @@ libconftest.dylib* .dirstamp .libs .*.swp -*.*~* +*~ *.bak *.rej *.orig From 0d8f4fae3b97d285bd8bfd8da10498f66fbdbc77 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 19 May 2021 10:07:25 +0200 Subject: [PATCH 06/10] Merge bitcoin/bitcoin#21173: util: faster HexStr => 13% faster blockToJSON 74bf850ac47735f2ef4306059d3e664d40cac85e faster HexStr => 13% faster blockToJSON (Martin Ankerl) Pull request description: `std::string`'s push_back is rather slow because it needs to check & update the string size. For `HexStr` the output string size is already easily know, so we can initially create the string with the correct size and then just assign the data. `HexStr` is heavily usd in `blockToJSON`, so this change is a noticeable benefit. Benchmark on an i7-8700 @3.2GHz: * 71,315,461.00 ns/op master * 62,842,490.00 ns/op this commit So this little change makes `blockToJSON` about ~13% faster. ACKs for top commit: laanwj: Code review ACK 74bf850ac47735f2ef4306059d3e664d40cac85e theStack: re-ACK 74bf850ac47735f2ef4306059d3e664d40cac85e Tree-SHA512: fc99105123edc11f4e40ed77aea80cf7f32e49c53369aa364b38395dcb48575e15040b0489ed30d0fe857c032a04e225c33e9d95cdfa109a3cb5a6ec9a972415 --- src/util/strencodings.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 5dda538e41..7e0969df8f 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -605,13 +605,14 @@ bool ParseHDKeypath(const std::string& keypath_str, std::vector& keypa std::string HexStr(const Span s) { - std::string rv; + std::string rv(s.size() * 2, '\0'); static constexpr char hexmap[16] = { '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; - rv.reserve(s.size() * 2); - for (uint8_t v: s) { - rv.push_back(hexmap[v >> 4]); - rv.push_back(hexmap[v & 15]); + auto it = rv.begin(); + for (uint8_t v : s) { + *it++ = hexmap[v >> 4]; + *it++ = hexmap[v & 15]; } + assert(it == rv.end()); return rv; } From 93a2f05bb56eb378137cdf39731d8ada2503e44b Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 28 May 2021 14:14:21 +0800 Subject: [PATCH 07/10] Merge bitcoin/bitcoin#22078: Add src/qt/android/.gitignore 7d07192ddeaad28a26e3c4a54757a4f27f2a08ff Add src/qt/android/.gitignore (Hennadii Stepanov) Pull request description: This PR makes `git` ignore files created by `make apk`. ACKs for top commit: icota: ACK https://github.com/bitcoin/bitcoin/pull/22078/commits/7d07192ddeaad28a26e3c4a54757a4f27f2a08ff Tree-SHA512: 4be20bd84830217a10d8ea7634799e71ed50be73f4f60c91c56311a2c95b22ff1f28d3b7bc077f1417318bb75e446e3fc3bdbf9dbc037b4cbc8428f0875f2c77 --- src/qt/android/.gitignore | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 src/qt/android/.gitignore diff --git a/src/qt/android/.gitignore b/src/qt/android/.gitignore new file mode 100644 index 0000000000..74cf42f934 --- /dev/null +++ b/src/qt/android/.gitignore @@ -0,0 +1,9 @@ +/.gradle +/build +/gradle/wrapper +/gradlew* +/libs +/res/layout +/res/values* +/src/org/kde +/src/org/qtproject From 106e69f979437bfd338738d42758982a297c5476 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 31 May 2021 07:24:49 +0200 Subject: [PATCH 08/10] Merge bitcoin/bitcoin#22103: test: Fix IPv6 check on BSD systems 2be35725069fd4c589497b93e09e1c6db6946372 test: Fix IPv6 check on BSD systems (nthumann) Pull request description: I noticed that `test_ipv6_local()` always returns `False` on macOS or FreeBSD, even though IPv6 is working perfectly fine. This causes `test/functional/rpc_bind.py --ipv6` and `test/functional/feature_proxy.py` to skip their run. Apparently, there's a check if the port number is `0` (see [here](https://github.com/freebsd/freebsd-src/blob/64881da478071431a2d9e62613997a5772c56cdf/sys/netinet6/udp6_usrreq.c#L248) or [here](https://github.com/apple/darwin-xnu/blob/8f02f2a044b9bb1ad951987ef5bab20ec9486310/bsd/netinet6/udp6_usrreq.c#L282)), while Linux has no problem with this. This is fixed by specifying any other port number than `0`, e.g. `1`. Still, because of `SOCK_DGRAM`, no actual connection is made. ACKs for top commit: fanquake: ACK 2be35725069fd4c589497b93e09e1c6db6946372 - nice improvement. I checked that with this change ipv6 related tests in `feature_proxy.py` are being run. theStack: ACK 2be35725069fd4c589497b93e09e1c6db6946372 Tree-SHA512: 8417c2d3cf71050529f3fa409a03872040fe5d249eae4172f276e62156e505a20d375b963712a186c9ad7967d8a497b5900d327c74a9693f68c33063871d4691 --- test/functional/test_framework/netutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/test_framework/netutil.py b/test/functional/test_framework/netutil.py index 195ec1c5e8..0bac633008 100644 --- a/test/functional/test_framework/netutil.py +++ b/test/functional/test_framework/netutil.py @@ -151,7 +151,7 @@ def test_ipv6_local(): have_ipv6 = True try: s = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM) - s.connect(('::1', 0)) + s.connect(('::1', 1)) except socket.error: have_ipv6 = False return have_ipv6 From 8ebffdfa7cb47a18f86d46913f165aafc9e1db9e Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 2 Jun 2021 20:31:11 +0200 Subject: [PATCH 09/10] Merge bitcoin/bitcoin#21231: Add /opt/homebrew to path to look for boost libraries 9a0969585fce03b45be7004bba865bc15909904c build: Add /opt/homebrew to path to look for boost libraries (Fu Yong Quah) Pull request description: Following the instruction in https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md for building on the M1 Macs don't work out of the box, because homebrew now defaults to /opt/homebrew instead of /usr/local. This PR fixes that. ACKs for top commit: jonasschnelli: utACK 9a0969585fce03b45be7004bba865bc15909904c promag: Tested ACK 9a0969585fce03b45be7004bba865bc15909904c. Tree-SHA512: 472568b97fbd8623481fe6fd43b0509fa32fe7f1c1d8090321a6a6a5bdc7343d4ad4122c10dcc7c9c93068db8a3f009a73befaf1ba11e4af54a66afd2c2dbe14 --- build-aux/m4/ax_boost_base.m4 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/build-aux/m4/ax_boost_base.m4 b/build-aux/m4/ax_boost_base.m4 index 16fa69b41f..35a961a75b 100644 --- a/build-aux/m4/ax_boost_base.m4 +++ b/build-aux/m4/ax_boost_base.m4 @@ -11,7 +11,7 @@ # Test for the Boost C++ libraries of a particular version (or newer) # # If no path to the installed boost library is given the macro searchs -# under /usr, /usr/local, /opt and /opt/local and evaluates the +# under /usr, /usr/local, /opt, /opt/local and /opt/homebrew and evaluates the # $BOOST_ROOT environment variable. Further documentation is available at # . # @@ -150,7 +150,7 @@ AC_DEFUN([_AX_BOOST_BASE_RUNDETECT],[ else search_libsubdirs="$multiarch_libsubdir $libsubdirs" fi - for _AX_BOOST_BASE_boost_path_tmp in /usr /usr/local /opt /opt/local ; do + for _AX_BOOST_BASE_boost_path_tmp in /usr /usr/local /opt /opt/local /opt/homebrew/; do if test -d "$_AX_BOOST_BASE_boost_path_tmp/include/boost" && test -r "$_AX_BOOST_BASE_boost_path_tmp/include/boost" ; then for libsubdir in $search_libsubdirs ; do if ls "$_AX_BOOST_BASE_boost_path_tmp/$libsubdir/libboost_"* >/dev/null 2>&1 ; then break; fi @@ -226,7 +226,7 @@ AC_DEFUN([_AX_BOOST_BASE_RUNDETECT],[ fi else if test "x$cross_compiling" != "xyes" ; then - for _AX_BOOST_BASE_boost_path in /usr /usr/local /opt /opt/local ; do + for _AX_BOOST_BASE_boost_path in /usr /usr/local /opt /opt/local /opt/homebrew ; do if test -d "$_AX_BOOST_BASE_boost_path" && test -r "$_AX_BOOST_BASE_boost_path" ; then for i in `ls -d $_AX_BOOST_BASE_boost_path/include/boost-* 2>/dev/null`; do _version_tmp=`echo $i | sed "s#$_AX_BOOST_BASE_boost_path##" | sed 's/\/include\/boost-//' | sed 's/_/./'` From d3f9a66580613c931f6e9b272fbf823b0e7e2a35 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Mon, 21 Jun 2021 11:15:11 +0200 Subject: [PATCH 10/10] Merge bitcoin/bitcoin#19033: http: Release work queue after event base finish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 4e353cb618745cdb5d98e58e7dcd400ded01299a http: Release work queue after event base finish (João Barbosa) Pull request description: This fixes a race between `http_request_cb` and `StopHTTPServer` where the work queue is used after release. Fixes #18856. ACKs for top commit: fjahr: Code review ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a achow101: ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a LarryRuane: ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a hebasto: ACK 4e353cb618745cdb5d98e58e7dcd400ded01299a, tested (rebased on top of master 9313c4e6aa4b707c06a86b33d5d2753cd8383340) on Linux Mint 20.1 (x86_64) using MarcoFalke's [patch](https://github.com/bitcoin/bitcoin/pull/19033#issuecomment-640106647), including different `-rpcthreads`/`-rpcworkqueue` cases. The bug is fixed. The code is correct. Tree-SHA512: 185d2a9744d0d5134d782bf321ac9958ba17b11a5b3d70b4897c8243e6b146dfd3f23c57aef8e10ae9484374120b64389c1949a9cf0a21dccc47ffc934c20930 --- src/httpserver.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 2cf97280d6..60864e06dc 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -94,7 +94,7 @@ public: bool Enqueue(WorkItem* item) { LOCK(cs); - if (queue.size() >= maxDepth) { + if (!running || queue.size() >= maxDepth) { return false; } queue.emplace_back(std::unique_ptr(item)); @@ -110,7 +110,7 @@ public: WAIT_LOCK(cs, lock); while (running && queue.empty()) cond.wait(lock); - if (!running) + if (!running && queue.empty()) break; i = std::move(queue.front()); queue.pop_front(); @@ -468,8 +468,6 @@ void StopHTTPServer() thread.join(); } g_thread_http_workers.clear(); - delete workQueue; - workQueue = nullptr; } // Unlisten sockets, these are what make the event loop running, which means // that after this and all connections are closed the event loop will quit. @@ -489,6 +487,10 @@ void StopHTTPServer() event_base_free(eventBase); eventBase = nullptr; } + if (workQueue) { + delete workQueue; + workQueue = nullptr; + } LogPrint(BCLog::HTTP, "Stopped HTTP server\n"); }