From 44341d436ee8271273e1781d83539542164c8757 Mon Sep 17 00:00:00 2001 From: merge-script Date: Sat, 24 Aug 2024 16:24:23 +0100 Subject: [PATCH 01/10] Merge bitcoin/bitcoin#30703: test: Avoid duplicate curl call in get_previous_releases.py fa5aeab3cb18405ecf8a1401d89539b924a618f6 test: Avoid duplicate curl call in get_previous_releases.py (MarcoFalke) Pull request description: Seems odd having to translate `404` to "Binary tag was not found". Also, it seems odd to write a for-loop over a list with one item. Fix both issues by just using a single call to `curl --fail ...`. Can be tested with: `test/get_previous_releases.py -b v99.99.99` Before: ``` Releases directory: releases Fetching: https://bitcoincore.org/bin/bitcoin-core-99.99.99/bitcoin-99.99.99-x86_64-linux-gnu.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 286k 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0 Binary tag was not found ``` After: ``` Releases directory: releases Fetching: https://bitcoincore.org/bin/bitcoin-core-99.99.99/bitcoin-99.99.99-x86_64-linux-gnu.tar.gz % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 286k 0 0 0 0 0 0 --:--:-- 0:00:01 --:--:-- 0 curl: (22) The requested URL returned error: 404 ACKs for top commit: fanquake: ACK fa5aeab3cb18405ecf8a1401d89539b924a618f6 brunoerg: utACK fa5aeab3cb18405ecf8a1401d89539b924a618f6 tdb3: tested ACK fa5aeab3cb18405ecf8a1401d89539b924a618f6 Tree-SHA512: d5d31e0bccdd9de9b4a8ecf2e69348f4e8cee773050c8259b61db1ce5de73f6fbfffbe8c4d2571f7bef2de29cb42fd244573deebfbec614e487e76ef41681b9c --- test/get_previous_releases.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/test/get_previous_releases.py b/test/get_previous_releases.py index d56648bf4d43..f70ad7b4639e 100755 --- a/test/get_previous_releases.py +++ b/test/get_previous_releases.py @@ -141,20 +141,9 @@ def download_binary(tag, args) -> int: print('Fetching: {tarballUrl}'.format(tarballUrl=tarballUrl)) - header, status = subprocess.Popen( - ['curl', '--head', tarballUrl], stdout=subprocess.PIPE).communicate() - if re.search("404 Not Found", header.decode("utf-8")): - print("Binary tag was not found") - return 1 - - curlCmds = [ - ['curl', '-L', '--remote-name', tarballUrl] - ] - - for cmd in curlCmds: - ret = subprocess.run(cmd).returncode - if ret: - return ret + ret = subprocess.run(['curl', '--fail', '-L', '--remote-name', tarballUrl]).returncode + if ret: + return ret hasher = hashlib.sha256() with open(tarball, "rb") as afile: From f86c1100b1edccb116bf13bafc5c1d90b27750ce Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Fri, 3 May 2024 12:36:56 -0400 Subject: [PATCH 02/10] Merge bitcoin/bitcoin#30024: doc: replace remaining "520" magic nums with MAX_SCRIPT_ELEMENT_SIZE ffc674595cb19b2fdc5705b355bdd3e7f724b860 Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZE (Jon Atack) Pull request description: Noticed these while reviewing BIPs yesterday. It would be clearer and more future-proof to refer to their constant name. ACKs for top commit: instagibbs: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 sipa: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 achow101: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 glozow: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860, agree it's clearer for these comments to refer to the greppable name of the limit rather than the number Tree-SHA512: 462afc1c64543877ac58cb3acdb01d42c6d08abfb362802f29f3482d75401a2a8adadbc2facd222a9a9fefcaab6854865ea400f50ad60bec17831d29f7798afe --- src/net_processing.cpp | 2 +- src/outputtype.cpp | 2 +- src/policy/policy.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 3f8ba2e11b48..c8f85e533e11 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -5365,7 +5365,7 @@ void PeerManagerImpl::ProcessMessage( std::vector vData; vRecv >> vData; - // Nodes must NEVER send a data item > 520 bytes (the max size for a script data object, + // Nodes must NEVER send a data item > MAX_SCRIPT_ELEMENT_SIZE bytes (the max size for a script data object, // and thus, the maximum size any matched object can have) in a filteradd message bool bad = false; if (vData.size() > MAX_SCRIPT_ELEMENT_SIZE) { diff --git a/src/outputtype.cpp b/src/outputtype.cpp index 2a48c1e3c776..c94610888766 100644 --- a/src/outputtype.cpp +++ b/src/outputtype.cpp @@ -59,7 +59,7 @@ CTxDestination AddAndGetDestinationForScript(FillableSigningProvider& keystore, // Add script to keystore keystore.AddCScript(script); ScriptHash sh(script); - // Note that scripts over 520 bytes are not yet supported. + // Note that scripts over MAX_SCRIPT_ELEMENT_SIZE bytes are not yet supported. switch (type) { case OutputType::LEGACY: keystore.AddCScript(GetScriptForDestination(sh)); diff --git a/src/policy/policy.cpp b/src/policy/policy.cpp index c91471f8d959..b9fbaf0c4e60 100644 --- a/src/policy/policy.cpp +++ b/src/policy/policy.cpp @@ -89,7 +89,7 @@ bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, const CFeeR for (const CTxIn& txin : tx.vin) { // Biggest 'standard' txin involving only keys is a 15-of-15 P2SH - // multisig with compressed keys (remember the 520 byte limit on + // multisig with compressed keys (remember the MAX_SCRIPT_ELEMENT_SIZE byte limit on // redeemScript size). That works out to a (15*(33+1))+3=513 byte // redeemScript, 513+1+15*(73+1)+3=1627 bytes of scriptSig, which // we round off to 1650(MAX_STANDARD_SCRIPTSIG_SIZE) bytes for From c0fc74fbd5174acd23954842b9b4d17f6c46c727 Mon Sep 17 00:00:00 2001 From: merge-script Date: Mon, 6 May 2024 21:02:30 +0800 Subject: [PATCH 03/10] Merge bitcoin/bitcoin#29773: build, bench, msvc: Add missing benchmarks 31a15f0aff79d2b34a9640909b9e6fb39a647b60 bench: Disable WalletCreate* benchmarks when building with MSVC (Hennadii Stepanov) 23dc0c19acd54cad1bed2f14df024b6b533f2330 msvc, bench: Add missing source files to bench_bitcoin project (Hennadii Stepanov) Pull request description: On the master branch, the `bench_bitcoin.vcxproj` MSVC project misses wallet-specific source files. This PR fixes this issue. Benchmark run on Windows: ``` > src\bench_bitcoin.exe -filter="CoinSelection|BnBExhaustion|Wallet.*" | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 398,800.00 | 2,507.52 | 1.5% | 0.01 | `BnBExhaustion` | 584,450.00 | 1,711.01 | 1.5% | 0.01 | `CoinSelection` | 86,603,650.00 | 11.55 | 0.4% | 1.91 | `WalletAvailableCoins` | 7,604.00 | 131,509.73 | 0.9% | 0.01 | `WalletBalanceClean` | 124,028.57 | 8,062.66 | 2.6% | 0.01 | `WalletBalanceDirty` | 7,587.12 | 131,802.30 | 1.9% | 0.01 | `WalletBalanceMine` | 48.58 | 20,583,872.99 | 0.9% | 0.01 | `WalletBalanceWatch` | 2,371,060.00 | 421.75 | 1.3% | 0.13 | `WalletCreateTxUseOnlyPresetInputs` | 96,861,760.00 | 10.32 | 0.9% | 5.31 | `WalletCreateTxUsePresetInputsAndCoinSelection` | 280.71 | 3,562,424.13 | 1.5% | 0.01 | `WalletIsMineDescriptors` | 1,033.47 | 967,618.32 | 0.3% | 0.01 | `WalletIsMineLegacy` | 282.36 | 3,541,599.91 | 0.5% | 0.01 | `WalletIsMineMigratedDescriptors` | 484,547,300.00 | 2.06 | 1.0% | 2.43 | `WalletLoadingDescriptors` | 29,924,300.00 | 33.42 | 0.4% | 0.15 | `WalletLoadingLegacy` ``` ACKs for top commit: maflcko: lgtm ACK 31a15f0aff79d2b34a9640909b9e6fb39a647b60 Tree-SHA512: 0241af06126edf612489322cdce66ba43792066b5400b1719a8b9d1ec62030e8a9d497e2f01e38290e94c387db59ccf2a458f4b35d3dc8030a1a1413d89eb792 --- src/bench/wallet_create.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp index f900a7e90edb..f9132ff15262 100644 --- a/src/bench/wallet_create.cpp +++ b/src/bench/wallet_create.cpp @@ -48,9 +48,14 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted) static void WalletCreatePlain(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/false); } static void WalletCreateEncrypted(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/true); } +#ifndef _MSC_VER +// TODO: Being built with MSVC, the fs::remove_all() call in +// the WalletCreate() fails with the error "The process cannot +// access the file because it is being used by another process." #ifdef USE_SQLITE BENCHMARK(WalletCreatePlain, benchmark::PriorityLevel::HIGH); BENCHMARK(WalletCreateEncrypted, benchmark::PriorityLevel::HIGH); #endif +#endif } // namespace wallet From 41d01859cde168da57123656070668a09a0c45c8 Mon Sep 17 00:00:00 2001 From: merge-script Date: Thu, 2 May 2024 11:21:24 +0800 Subject: [PATCH 04/10] Merge bitcoin/bitcoin#29707: depends: build miniupnpc with CMake 5195baa60087ee366290887ad982fc491e14c111 depends: fix miniupnpc snprintf usage on Windows (fanquake) 3c2d440f1497f60bb444326f51383df244dcdfe9 depends: switch miniupnpc to CMake (Cory Fields) f5618c79d9e7af05c2987044dc2da03697f8bb89 depends: add upstream CMake patch to miniupnpc (fanquake) 6866b571ab96f03ca0775424e45458c5731f230f depends: miniupnpc 2.2.7 (fanquake) Pull request description: This picks up one of the changes from #29232, which is a switch to building miniupnpc with CMake. It includes an update to the most recent version of miniupnpc (2.2.7), which means we can drop one patch from that commit, and includes a new patch for a change I've upstreamed https://github.com/miniupnp/miniupnp/pull/721, as well as some suggestions from the previous PR. ACKs for top commit: theuni: ACK 5195baa60087ee366290887ad982fc491e14c111. TheCharlatan: utACK 5195baa60087ee366290887ad982fc491e14c111 Tree-SHA512: 5b27e132cd5eed285e9be34c8b96893417d92a1ae55c99345c9a89e1c1c5e40e4bc840bc061b879758b2b11fcb520cd98c3da985c1e153f2e5380cf63efe2d69 --- depends/packages/miniupnpc.mk | 29 ++++++++++--------- .../miniupnpc/cmake_get_src_addr.patch | 22 ++++++++++++++ .../patches/miniupnpc/dont_leak_info.patch | 26 ++++++++--------- .../miniupnpc/fix_windows_snprintf.patch | 25 ++++++++++++++++ depends/patches/miniupnpc/no_libtool.patch | 15 ---------- .../miniupnpc/respect_mingw_cflags.patch | 23 --------------- doc/dependencies.md | 2 +- 7 files changed, 76 insertions(+), 66 deletions(-) create mode 100644 depends/patches/miniupnpc/cmake_get_src_addr.patch create mode 100644 depends/patches/miniupnpc/fix_windows_snprintf.patch delete mode 100644 depends/patches/miniupnpc/no_libtool.patch delete mode 100644 depends/patches/miniupnpc/respect_mingw_cflags.patch diff --git a/depends/packages/miniupnpc.mk b/depends/packages/miniupnpc.mk index b3b3e999d3f0..e63752406107 100644 --- a/depends/packages/miniupnpc.mk +++ b/depends/packages/miniupnpc.mk @@ -1,32 +1,33 @@ package=miniupnpc -$(package)_version=2.2.2 +$(package)_version=2.2.7 $(package)_download_path=https://miniupnp.tuxfamily.org/files/ $(package)_file_name=$(package)-$($(package)_version).tar.gz -$(package)_sha256_hash=888fb0976ba61518276fe1eda988589c700a3f2a69d71089260d75562afd3687 -$(package)_patches=dont_leak_info.patch respect_mingw_cflags.patch no_libtool.patch +$(package)_sha256_hash=b0c3a27056840fd0ec9328a5a9bac3dc5e0ec6d2e8733349cf577b0aa1e70ac1 +$(package)_patches=dont_leak_info.patch cmake_get_src_addr.patch fix_windows_snprintf.patch +$(package)_build_subdir=build -# Next time this package is updated, ensure that _WIN32_WINNT is still properly set. -# See discussion in https://github.com/bitcoin/bitcoin/pull/25964. define $(package)_set_vars -$(package)_build_opts=CC="$($(package)_cc)" -$(package)_build_opts_mingw32=-f Makefile.mingw CFLAGS="$($(package)_cflags) -D_WIN32_WINNT=0x0A00" -$(package)_build_env+=CFLAGS="$($(package)_cflags) $($(package)_cppflags)" AR="$($(package)_ar)" +$(package)_config_opts = -DUPNPC_BUILD_SAMPLE=OFF -DUPNPC_BUILD_SHARED=OFF +$(package)_config_opts += -DUPNPC_BUILD_STATIC=ON -DUPNPC_BUILD_TESTS=OFF +$(package)_config_opts_mingw32 += -DMINIUPNPC_TARGET_WINDOWS_VERSION=0x0A00 endef define $(package)_preprocess_cmds patch -p1 < $($(package)_patch_dir)/dont_leak_info.patch && \ - patch -p1 < $($(package)_patch_dir)/respect_mingw_cflags.patch && \ - patch -p1 < $($(package)_patch_dir)/no_libtool.patch + patch -p1 < $($(package)_patch_dir)/cmake_get_src_addr.patch && \ + patch -p1 < $($(package)_patch_dir)/fix_windows_snprintf.patch +endef + +define $(package)_config_cmds + $($(package)_cmake) -S .. -B . endef define $(package)_build_cmds - $(MAKE) libminiupnpc.a $($(package)_build_opts) + $(MAKE) endef define $(package)_stage_cmds - mkdir -p $($(package)_staging_prefix_dir)/include/miniupnpc $($(package)_staging_prefix_dir)/lib &&\ - install *.h $($(package)_staging_prefix_dir)/include/miniupnpc &&\ - install libminiupnpc.a $($(package)_staging_prefix_dir)/lib + cmake --install . --prefix $($(package)_staging_prefix_dir) endef define $(package)_postprocess_cmds diff --git a/depends/patches/miniupnpc/cmake_get_src_addr.patch b/depends/patches/miniupnpc/cmake_get_src_addr.patch new file mode 100644 index 000000000000..bae1b738f3f3 --- /dev/null +++ b/depends/patches/miniupnpc/cmake_get_src_addr.patch @@ -0,0 +1,22 @@ +commit cb2026239c2a3aff393952ccb0ee1c448189402d +Author: fanquake +Date: Fri Mar 22 14:03:54 2024 +0000 + + build: add MINIUPNPC_GET_SRC_ADDR to CMake build + + This mirrors the autotools build. + + See https://github.com/miniupnp/miniupnp/pull/721. + +diff --git a/CMakeLists.txt b/CMakeLists.txt +index 1aa95a8..0cacf3e 100644 +--- a/CMakeLists.txt ++++ b/CMakeLists.txt +@@ -31,6 +31,7 @@ endif () + if (NOT WIN32) + target_compile_definitions(miniupnpc-private INTERFACE + MINIUPNPC_SET_SOCKET_TIMEOUT ++ MINIUPNPC_GET_SRC_ADDR + _BSD_SOURCE _DEFAULT_SOURCE) + if (NOT APPLE AND NOT CMAKE_SYSTEM_NAME MATCHES ".*BSD" AND NOT CMAKE_SYSTEM_NAME STREQUAL "SunOS") + # add_definitions (-D_POSIX_C_SOURCE=200112L) diff --git a/depends/patches/miniupnpc/dont_leak_info.patch b/depends/patches/miniupnpc/dont_leak_info.patch index 512f9c50ea8d..95a09a26dce1 100644 --- a/depends/patches/miniupnpc/dont_leak_info.patch +++ b/depends/patches/miniupnpc/dont_leak_info.patch @@ -1,31 +1,31 @@ -commit 8815452257437ba36607d0e2381c01142d1c7bb0 +commit 51f6dd991c29af66fb4f64c6feb2787cce23a1a7 Author: fanquake -Date: Thu Nov 19 10:51:19 2020 +0800 +Date: Mon Jan 8 11:21:40 2024 +0000 Don't leak OS and miniupnpc version info in User-Agent -diff --git a//minisoap.c b/minisoap.c -index 7860667..775580b 100644 ---- a/minisoap.c -+++ b/minisoap.c +diff --git a/src/minisoap.c b/src/minisoap.c +index 903ac5f..046e0ea 100644 +--- a/src/minisoap.c ++++ b/src/minisoap.c @@ -90,7 +90,7 @@ int soapPostSubmit(SOCKET fd, headerssize = snprintf(headerbuf, sizeof(headerbuf), "POST %s HTTP/%s\r\n" "Host: %s%s\r\n" -- "User-Agent: " OS_STRING ", " UPNP_VERSION_STRING ", MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n" +- "User-Agent: " OS_STRING " " UPNP_VERSION_STRING " MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n" + "User-Agent: " UPNP_VERSION_STRING "\r\n" "Content-Length: %d\r\n" + #if (UPNP_VERSION_MAJOR == 1) && (UPNP_VERSION_MINOR == 0) "Content-Type: text/xml\r\n" - "SOAPAction: \"%s\"\r\n" -diff --git a/miniwget.c b/miniwget.c -index d5b7970..05aeb9c 100644 ---- a/miniwget.c -+++ b/miniwget.c +diff --git a/src/miniwget.c b/src/miniwget.c +index e76a5e5..0cc36fe 100644 +--- a/src/miniwget.c ++++ b/src/miniwget.c @@ -444,7 +444,7 @@ miniwget3(const char * host, "GET %s HTTP/%s\r\n" "Host: %s:%d\r\n" "Connection: Close\r\n" -- "User-Agent: " OS_STRING ", " UPNP_VERSION_STRING ", MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n" +- "User-Agent: " OS_STRING " " UPNP_VERSION_STRING " MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n" + "User-Agent: " UPNP_VERSION_STRING "\r\n" "\r\n", diff --git a/depends/patches/miniupnpc/fix_windows_snprintf.patch b/depends/patches/miniupnpc/fix_windows_snprintf.patch new file mode 100644 index 000000000000..ff9e26231e73 --- /dev/null +++ b/depends/patches/miniupnpc/fix_windows_snprintf.patch @@ -0,0 +1,25 @@ +commit a1e9de80ab99b4c956a6a4e21d3e0de6f7a1014d +Author: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> +Date: Sat Apr 20 15:14:47 2024 +0100 + + Fix macro expression that guards `snprintf` for Windows + + Otherwise, the `snprintf` is still wrongly emulated for the following + cases: + - mingw-w64 6.0.0 or new with ucrt + - mingw-w64 8.0.0 or new with iso c ext + +--- a/src/win32_snprintf.h ++++ b/src/win32_snprintf.h +@@ -23,9 +23,9 @@ + (defined(_MSC_VER) && _MSC_VER < 1900) /* Visual Studio older than 2015 */ || \ + (defined(__MINGW32__) && !defined(__MINGW64_VERSION_MAJOR) && defined(__NO_ISOCEXT)) /* mingw32 without iso c ext */ || \ + (defined(__MINGW64_VERSION_MAJOR) && /* mingw-w64 not ... */ !( \ +- (defined (__USE_MINGW_ANSI_STDIO) && __USE_MINGW_ANSI_STDIO != 0)) /* ... with ansi stdio */ || \ ++ (defined (__USE_MINGW_ANSI_STDIO) && __USE_MINGW_ANSI_STDIO != 0) /* ... with ansi stdio */ || \ + (__MINGW64_VERSION_MAJOR >= 6 && defined(_UCRT)) /* ... at least 6.0.0 with ucrt */ || \ +- (__MINGW64_VERSION_MAJOR >= 8 && !defined(__NO_ISOCEXT)) /* ... at least 8.0.0 with iso c ext */ || \ ++ (__MINGW64_VERSION_MAJOR >= 8 && !defined(__NO_ISOCEXT))) /* ... at least 8.0.0 with iso c ext */ || \ + 0) || \ + 0) + diff --git a/depends/patches/miniupnpc/no_libtool.patch b/depends/patches/miniupnpc/no_libtool.patch deleted file mode 100644 index bb7d4a87ef7c..000000000000 --- a/depends/patches/miniupnpc/no_libtool.patch +++ /dev/null @@ -1,15 +0,0 @@ -diff -ruN miniupnpc-2.2.2/Makefile miniupnpc-2.2.2.new/Makefile ---- miniupnpc-2.2.2/Makefile 2020-11-27 18:25:02.000000000 +0000 -+++ miniupnpc-2.2.2.new/Makefile 2024-01-23 20:58:08.387188527 +0000 -@@ -298,11 +298,7 @@ - makedepend -Y -- $(CFLAGS) -- $(SRCS) 2>/dev/null - - $(LIBRARY): $(LIBOBJS) --ifneq (, $(findstring darwin, $(OS))) -- $(LIBTOOL) -static -o $@ $? --else - $(AR) crs $@ $? --endif - - $(SHAREDLIBRARY): $(LIBOBJS) - ifneq (, $(findstring darwin, $(OS))) diff --git a/depends/patches/miniupnpc/respect_mingw_cflags.patch b/depends/patches/miniupnpc/respect_mingw_cflags.patch deleted file mode 100644 index a44580ddab65..000000000000 --- a/depends/patches/miniupnpc/respect_mingw_cflags.patch +++ /dev/null @@ -1,23 +0,0 @@ -commit fec515a7ac9991a0ee91068fda046b54b191155e -Author: fanquake -Date: Wed Jul 27 15:52:37 2022 +0100 - - build: respect CFLAGS in makefile.mingw - - Similar to the other Makefile. - - Cherry-pick of https://github.com/miniupnp/miniupnp/pull/619. - -diff --git a/Makefile.mingw b/Makefile.mingw -index 2bff7bd..88430d2 100644 ---- a/Makefile.mingw -+++ b/Makefile.mingw -@@ -19,7 +19,7 @@ else - RM = rm -f - endif - #CFLAGS = -Wall -g -DDEBUG -D_WIN32_WINNT=0X501 --CFLAGS = -Wall -W -Wstrict-prototypes -Os -DNDEBUG -D_WIN32_WINNT=0X501 -+CFLAGS ?= -Wall -W -Wstrict-prototypes -Os -DNDEBUG -D_WIN32_WINNT=0X501 - LDLIBS = -lws2_32 -liphlpapi - # -lwsock32 - # -liphlpapi is needed for GetBestRoute() and GetIpAddrTable() diff --git a/doc/dependencies.md b/doc/dependencies.md index 2a2b7ec28370..4a2c6528c38c 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -39,7 +39,7 @@ You can find installation instructions in the `build-*.md` file for your platfor | Dependency | Releases | Version used | Minimum required | Runtime | | --- | --- | --- | --- | --- | | [libnatpmp](../depends/packages/libnatpmp.mk) | [link](https://github.com/miniupnp/libnatpmp/) | commit [07004b9...](https://github.com/miniupnp/libnatpmp/tree/07004b97cf691774efebe70404cf22201e4d330d) | | No | -| [MiniUPnPc](../depends/packages/miniupnpc.mk) | [link](https://miniupnp.tuxfamily.org/) | [2.2.2](https://github.com/bitcoin/bitcoin/pull/20421) | 2.1 | No | +| [MiniUPnPc](../depends/packages/miniupnpc.mk) | [link](https://miniupnp.tuxfamily.org/) | [2.2.2](https://github.com/bitcoin/bitcoin/pull/29707) | 2.1 | No | ### Notifications | Dependency | Releases | Version used | Minimum required | Runtime | From ae3ccb4ce587877f62abc664d1679edd27110d1f Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 8 May 2024 17:52:58 -0400 Subject: [PATCH 05/10] Merge bitcoin/bitcoin#28336: rpc: parse legacy pubkeys consistently with specific error messages 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e test: add coverage for parsing cryptographically invalid pubkeys (Sebastian Falbesoner) c740b154d193b91ca42f18759098d3fef6eaab05 rpc: use `HexToPubKey` helper for all legacy pubkey-parsing RPCs (Sebastian Falbesoner) 100e8a75bf5d8196c005331bd8f2ed42ada6d8d0 rpc: check and throw specific pubkey parsing errors in `HexToPubKey` (Sebastian Falbesoner) Pull request description: Parsing legacy public keys can fail for three reasons (in this order): - pubkey is not in hex - pubkey has an invalid length (not 33 or 65 bytes for compressed/uncompressed, respectively) - pubkey is crytographically invalid, i.e. is not on curve (`CPubKey.IsFullyValid()` check) Many RPCs currently perform these checks manually with different error messages, even though we already have a `HexToPubKey` helper. This PR puts all three checks in this helper (the length check was done on the call-sites before), adds specific error messages for each case, and consequently uses it for all RPCs that parse legacy pubkeys. This leads to deduplicated code and also to more consistent and detailed error messages for the user. Affected RPC calls are `createmultisig`, `addmultisigaddress`, `importpubkey`, `importmulti`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send` and `sendall`. Note that the error code (-5 a.k.a. `RPC_INVALID_ADDRESS_OR_KEY`) doesn't change in any of the causes, so the changes are not breaking RPC API compatibility. Only the messages are more specific. The last commits adds test coverage for the cryptographically invalid (not-on-curve) pubkey case which wasn't exercised before. ACKs for top commit: stratospher: tested ACK 98570fe. davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Eunovo: Tested ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e achow101: ACK 98570fe29bb08d7edc48011aa6b9731c6ab4ed2e Tree-SHA512: cfa474176e95b5b18f3a9da28fdd9e87195cd58994c1331198f2840925fff322fd323a6371feab74a1b32e4b9ea58a6dc732fa751b4cdd45402c1029af609ece --- src/rpc/output_script.cpp | 6 +----- src/rpc/util.cpp | 7 +++++-- src/wallet/rpc/backup.cpp | 17 ++--------------- src/wallet/rpc/spend.cpp | 10 +--------- test/functional/rpc_rawtransaction.py | 4 ++-- test/functional/wallet_basic.py | 9 ++++++--- test/functional/wallet_fundrawtransaction.py | 4 ++-- 7 files changed, 19 insertions(+), 38 deletions(-) diff --git a/src/rpc/output_script.cpp b/src/rpc/output_script.cpp index 2ad98f709d38..ac26d6f6d5ec 100644 --- a/src/rpc/output_script.cpp +++ b/src/rpc/output_script.cpp @@ -106,11 +106,7 @@ static RPCHelpMan createmultisig() const UniValue& keys = request.params[1].get_array(); std::vector pubkeys; for (unsigned int i = 0; i < keys.size(); ++i) { - if (IsHex(keys[i].get_str()) && (keys[i].get_str().length() == 66 || keys[i].get_str().length() == 130)) { - pubkeys.push_back(HexToPubKey(keys[i].get_str())); - } else { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("Invalid public key: %s\n.", keys[i].get_str())); - } + pubkeys.push_back(HexToPubKey(keys[i].get_str())); } // Construct using pay-to-script-hash: diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 738a5dea162e..2f8ca883d731 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -229,11 +229,14 @@ std::string HelpExampleRpcNamed(const std::string& methodname, const RPCArgList& CPubKey HexToPubKey(const std::string& hex_in) { if (!IsHex(hex_in)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must be a hex string"); + } + if (hex_in.length() != 66 && hex_in.length() != 130) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must have a length of either 33 or 65 bytes"); } CPubKey vchPubKey(ParseHex(hex_in)); if (!vchPubKey.IsFullyValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid public key: " + hex_in); + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + hex_in + "\" must be cryptographically valid."); } return vchPubKey; } diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 666516516923..9def574298a8 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -453,12 +453,7 @@ RPCHelpMan importpubkey() throw JSONRPCError(RPC_WALLET_ERROR, "Wallet is currently rescanning. Abort existing rescan or wait."); } - if (!IsHex(request.params[0].get_str())) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey must be a hex string"); - std::vector data(ParseHex(request.params[0].get_str())); - CPubKey pubKey(data); - if (!pubKey.IsFullyValid()) - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey is not a valid public key"); + CPubKey pubKey = HexToPubKey(request.params[0].get_str()); { LOCK(pwallet->cs_wallet); @@ -1217,15 +1212,7 @@ static UniValue ProcessImportLegacy(ImportData& import_data, std::map(parsed_redeemscript.begin(), parsed_redeemscript.end()); } for (size_t i = 0; i < pubKeys.size(); ++i) { - const auto& str = pubKeys[i].get_str(); - if (!IsHex(str)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" must be a hex string"); - } - auto parsed_pubkey = ParseHex(str); - CPubKey pubkey(parsed_pubkey); - if (!pubkey.IsFullyValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Pubkey \"" + str + "\" is not a valid public key"); - } + CPubKey pubkey = HexToPubKey(pubKeys[i].get_str()); pubkey_map.emplace(pubkey.GetID(), pubkey); ordered_pubkeys.push_back(pubkey.GetID()); } diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index de9385e82b5d..2212e3f989d2 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -583,15 +583,7 @@ void FundTransaction(CWallet& wallet, CMutableTransaction& tx, CAmount& fee_out, const UniValue solving_data = options["solving_data"].get_obj(); if (solving_data.exists("pubkeys")) { for (const UniValue& pk_univ : solving_data["pubkeys"].get_array().getValues()) { - const std::string& pk_str = pk_univ.get_str(); - if (!IsHex(pk_str)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' is not hex", pk_str)); - } - const std::vector data(ParseHex(pk_str)); - const CPubKey pubkey(data.begin(), data.end()); - if (!pubkey.IsFullyValid()) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("'%s' is not a valid public key", pk_str)); - } + const CPubKey pubkey = HexToPubKey(pk_univ.get_str()); coinControl.m_external_provider.pubkeys.emplace(pubkey.GetID(), pubkey); // Add script for pubkeys const CScript pk_script = GetScriptForDestination(PKHash(pubkey)); diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index ad0c55ba82e3..84535653f8e0 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -341,11 +341,11 @@ def raw_multisig_transaction_legacy_tests(self): addr2Obj = self.nodes[2].getaddressinfo(addr2) # Tests for createmultisig and addmultisigaddress - assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"]) + assert_raises_rpc_error(-5, 'Pubkey "01020304" must have a length of either 33 or 65 bytes', self.nodes[0].createmultisig, 1, ["01020304"]) # createmultisig can only take public keys self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here - assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) + assert_raises_rpc_error(-5, f'Pubkey "{addr1}" must be a hex string', self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address'] diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 4b8bc4a328e9..1df602735b07 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -499,10 +499,13 @@ def run_test(self): assert_raises_rpc_error(-5, "Invalid Dash address or script", self.nodes[0].importaddress, "invalid") # This will raise an exception for attempting to import a pubkey that isn't in hex - assert_raises_rpc_error(-5, "Pubkey must be a hex string", self.nodes[0].importpubkey, "not hex") + assert_raises_rpc_error(-5, 'Pubkey "not hex" must be a hex string', self.nodes[0].importpubkey, "not hex") - # This will raise an exception for importing an invalid pubkey - assert_raises_rpc_error(-5, "Pubkey is not a valid public key", self.nodes[0].importpubkey, "5361746f736869204e616b616d6f746f") + # This will raise exceptions for importing a pubkeys with invalid length / invalid coordinates + too_short_pubkey = "5361746f736869204e616b616d6f746f" + assert_raises_rpc_error(-5, f'Pubkey "{too_short_pubkey}" must have a length of either 33 or 65 bytes', self.nodes[0].importpubkey, too_short_pubkey) + not_on_curve_pubkey = bytes([4] + [0]*64).hex() # pubkey with coordinates (0,0) is not on curve + assert_raises_rpc_error(-5, f'Pubkey "{not_on_curve_pubkey}" must be cryptographically valid', self.nodes[0].importpubkey, not_on_curve_pubkey) # Import address and private key to check correct behavior of spendable unspents # 1. Send some coins to generate new UTXO diff --git a/test/functional/wallet_fundrawtransaction.py b/test/functional/wallet_fundrawtransaction.py index f97b54ecec09..3506a4abf65e 100755 --- a/test/functional/wallet_fundrawtransaction.py +++ b/test/functional/wallet_fundrawtransaction.py @@ -962,8 +962,8 @@ def test_external_inputs(self): assert_raises_rpc_error(-4, "Insufficient funds", wallet.fundrawtransaction, raw_tx) # Error conditions - assert_raises_rpc_error(-5, "'not a pubkey' is not hex", wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["not a pubkey"]}}) - assert_raises_rpc_error(-5, "'01234567890a0b0c0d0e0f' is not a valid public key", wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["01234567890a0b0c0d0e0f"]}}) + assert_raises_rpc_error(-5, 'Pubkey "not a pubkey" must be a hex string', wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["not a pubkey"]}}) + assert_raises_rpc_error(-5, 'Pubkey "01234567890a0b0c0d0e0f" must have a length of either 33 or 65 bytes', wallet.fundrawtransaction, raw_tx, {"solving_data": {"pubkeys":["01234567890a0b0c0d0e0f"]}}) assert_raises_rpc_error(-5, "'not a script' is not hex", wallet.fundrawtransaction, raw_tx, {"solving_data": {"scripts":["not a script"]}}) assert_raises_rpc_error(-8, "Unable to parse descriptor 'not a descriptor'", wallet.fundrawtransaction, raw_tx, {"solving_data": {"descriptors":["not a descriptor"]}}) assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", wallet.fundrawtransaction, raw_tx, {"input_sizes": [{"txid": ext_utxo["txid"]}]}) From 6052205bcbf4ea060cda52cb8d3d55cc12bfc5d8 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 8 May 2024 18:11:35 -0400 Subject: [PATCH 06/10] Merge bitcoin/bitcoin#29335: test: Handle functional test disk-full error 357ad110548d726021547d85b5b2bfcf3191d7e3 test: Handle functional test disk-full error (Brandon Odiwuor) Pull request description: Fixes: https://github.com/bitcoin/bitcoin/issues/23099 Handle disk-full more gracefully in functional tests ACKs for top commit: itornaza: re-ACK 357ad110548d726021547d85b5b2bfcf3191d7e3 achow101: reACK 357ad110548d726021547d85b5b2bfcf3191d7e3 cbergqvist: reACK 357ad110548d726021547d85b5b2bfcf3191d7e3. Looks good! tdb3: re ACK for 357ad110548d726021547d85b5b2bfcf3191d7e3 Tree-SHA512: 9bb0d3fbe84600c88873b9f55d4b5d1443f79ec303467680c301be2b4879201387f203d9d1984169461f321037189b5e10a6a4b9d61750de638f072d2f95d77e --- test/functional/test_runner.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 026ed289447c..7649bd991012 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -29,6 +29,13 @@ os.environ["REQUIRE_WALLET_TYPE_SET"] = "1" +# Minimum amount of space to run the tests. +MIN_FREE_SPACE = 1.1 * 1024 * 1024 * 1024 +# Additional space to run an extra job. +ADDITIONAL_SPACE_PER_JOB = 100 * 1024 * 1024 +# Minimum amount of space required for --nocleanup +MIN_NO_CLEANUP_SPACE = 12 * 1024 * 1024 * 1024 + # Formatting. Default colors to empty strings. DEFAULT, BOLD, GREEN, RED = ("", ""), ("", ""), ("", ""), ("", "") try: @@ -448,6 +455,8 @@ def main(): parser.add_argument('--failfast', '-F', action='store_true', help='stop execution after the first test failure') parser.add_argument('--filter', help='filter scripts to run by regular expression') parser.add_argument('--skipunit', '-u', action='store_true', help='skip unit tests for the test framework') + parser.add_argument("--nocleanup", dest="nocleanup", default=False, action="store_true", + help="Leave bitcoinds and test.* datadir on exit or error") args, unknown_args = parser.parse_known_args() @@ -542,6 +551,13 @@ def main(): subprocess.check_call([sys.executable, os.path.join(config["environment"]["SRCDIR"], 'test', 'functional', test_list[0].split()[0]), '-h']) sys.exit(0) + # Warn if there is not enough space on tmpdir to run the tests with --nocleanup + if args.nocleanup: + if shutil.disk_usage(tmpdir).free < MIN_NO_CLEANUP_SPACE: + print(f"{BOLD[1]}WARNING!{BOLD[0]} There may be insufficient free space in {tmpdir} to run the functional test suite with --nocleanup. " + f"A minimum of {MIN_NO_CLEANUP_SPACE // (1024 * 1024 * 1024)} GB of free space is required.") + passon_args.append("--nocleanup") + check_script_list(src_dir=config["environment"]["SRCDIR"], fail_on_warn=args.ci) check_script_prefixes() @@ -580,6 +596,11 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, attempts=1, enab if os.path.isdir(cache_dir): print("%sWARNING!%s There is a cache directory here: %s. If tests fail unexpectedly, try deleting the cache directory." % (BOLD[1], BOLD[0], cache_dir)) + # Warn if there is not enough space on the testing dir + min_space = MIN_FREE_SPACE + (jobs - 1) * ADDITIONAL_SPACE_PER_JOB + if shutil.disk_usage(tmpdir).free < min_space: + print(f"{BOLD[1]}WARNING!{BOLD[0]} There may be insufficient free space in {tmpdir} to run the Bitcoin functional test suite. " + f"Running the test suite with fewer than {min_space // (1024 * 1024)} MB of free space might cause tests to fail.") tests_dir = src_dir + '/test/functional/' # This allows `test_runner.py` to work from an out-of-source build directory using a symlink, @@ -659,6 +680,11 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, attempts=1, enab logging.debug("Early exiting after test failure") break + if "[Errno 28] No space left on device" in stdout: + sys.exit(f"Early exiting after test failure due to insuffient free space in {tmpdir}\n" + f"Test execution data left in {tmpdir}.\n" + f"Additional storage is needed to execute testing.") + print_results(test_results, max_len_name, (int(time.time() - start_time))) if coverage: From 001ad48790d7e4493570c4e06e994ea515c617c5 Mon Sep 17 00:00:00 2001 From: merge-script Date: Thu, 9 May 2024 11:11:57 +0800 Subject: [PATCH 07/10] Merge bitcoin/bitcoin#30063: build, test: Remove unused `TIMEOUT` environment variable 189d0da3f6f561c808fdd9fbd4dfd34ccfa23fe1 build, test: Remove unused `TIMEOUT` environment variable (Hennadii Stepanov) Pull request description: Setting the `TIMEOUT` environment variable has been a noop in both cases since its introduction. It seems to have been inadvertently copy-pasted from existed code. For example, in commit d80e3cbece857b293a4903ef49c4d543bb2cfb7f, it was needlessly copied from a valid case a few lines above for the `qa/pull-tester/run-bitcoind-for-test.sh` script. ACKs for top commit: maflcko: utACK 189d0da3f6f561c808fdd9fbd4dfd34ccfa23fe1 edilmedeiros: ACK 189d0da3f6f561c808fdd9fbd4dfd34ccfa23fe1 Tree-SHA512: 61111eba30e0c82a0220bea48eba451cd9caa68785b48ec8a91059ca5aadfaff2f6d2ccdc5aa737c5cefa33579cb735431bb9e94bda8fa047825d7bd28d542fb --- Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index ea2cba9fd2fa..d16a7081bda6 100644 --- a/Makefile.am +++ b/Makefile.am @@ -194,7 +194,7 @@ baseline_filtered.info: baseline.info $(LCOV) -a $@ $(LCOV_OPTS) -o $@ fuzz.info: baseline_filtered.info - @TIMEOUT=15 test/fuzz/test_runner.py $(DIR_FUZZ_SEED_CORPUS) -l DEBUG + @test/fuzz/test_runner.py $(DIR_FUZZ_SEED_CORPUS) -l DEBUG $(LCOV) -c $(LCOV_OPTS) -d $(abs_builddir)/src --t fuzz-tests -o $@ $(LCOV) -z $(LCOV_OPTS) -d $(abs_builddir)/src @@ -212,7 +212,7 @@ test_dash_filtered.info: test_dash.info $(LCOV) -a $@ $(LCOV_OPTS) -o $@ functional_test.info: test_dash_filtered.info - @TIMEOUT=15 test/functional/test_runner.py $(EXTENDED_FUNCTIONAL_TESTS) + @test/functional/test_runner.py $(EXTENDED_FUNCTIONAL_TESTS) $(LCOV) -c $(LCOV_OPTS) -d $(abs_builddir)/src --t functional-tests -o $@ $(LCOV) -z $(LCOV_OPTS) -d $(abs_builddir)/src From 708db0d55f12d6b4faaedc4f371e2b13bb7dbb19 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 9 May 2024 18:31:03 -0400 Subject: [PATCH 08/10] 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 07add980924b35b24cd219a82ea64eb2a1cdaadd Mon Sep 17 00:00:00 2001 From: glozow Date: Fri, 7 Jun 2024 14:22:14 +0100 Subject: [PATCH 09/10] Merge bitcoin/bitcoin#30161: util: add VecDeque 7b8eea067f188c0b0e52ef21b01aedd37667a237 tests: add fuzz tests for VecDeque (Pieter Wuille) 62fd24af6a3fe1569662c2802f59bb68a0172087 util: add VecDeque (Pieter Wuille) Pull request description: Extracted from #30126. This adds a `VecDeque` data type, inspired by `std::deque`, but backed by a single allocated memory region used as a ring buffer instead of a linked list of arrays. This gives better memory locality and less allocation overhead, plus better guarantees (some C++ standard library implementations, though not libstdc++ and libc++, use a separate allocation per element in a deque). It is intended for the candidate set search queue in #30126, but may be useful as a replacement for `std::deque` in other places too. It's not a full drop-in replacement, as I did not add iteration support which is unnecessary for the intended use case, but nothing prevents adding that if needed. Everything is tested through a simulation-based fuzz test that compares the behavior with normal `std::deque` equivalent operations, both for trivially-copyable/destructible types and others. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/30161/commits/7b8eea067f188c0b0e52ef21b01aedd37667a237 cbergqvist: re-ACK 7b8eea067f188c0b0e52ef21b01aedd37667a237 hebasto: re-ACK 7b8eea067f188c0b0e52ef21b01aedd37667a237, I've verified changes since my recent [review](https://github.com/bitcoin/bitcoin/pull/30161#pullrequestreview-2103018546) with glozow: ACK 7b8eea067f Tree-SHA512: 1b62f3ba1a43a1293d8c9de047e2399442e74c46de2df81406151fe27538716ce265f35fb6779ee56d77a39cddf8fb4b4e15bda8f04ebf3b149e2f05fa55cb21 --- src/Makefile.am | 1 + src/Makefile.test.include | 1 + src/test/fuzz/vecdeque.cpp | 491 +++++++++++++++++++++++++++++++++++++ src/util/vecdeque.h | 316 ++++++++++++++++++++++++ 4 files changed, 809 insertions(+) create mode 100644 src/test/fuzz/vecdeque.cpp create mode 100644 src/util/vecdeque.h diff --git a/src/Makefile.am b/src/Makefile.am index a0eca537b48d..0ce854f87a51 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -430,6 +430,7 @@ BITCOIN_CORE_H = \ util/translation.h \ util/types.h \ util/ui_change_type.h \ + util/vecdeque.h \ util/vector.h \ util/wpipe.h \ validation.h \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 375c9940d059..cd0c61f0f6a8 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -377,6 +377,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/utxo_snapshot.cpp \ test/fuzz/utxo_total_supply.cpp \ test/fuzz/validation_load_mempool.cpp \ + test/fuzz/vecdeque.cpp \ test/fuzz/versionbits.cpp endif # ENABLE_FUZZ_BINARY diff --git a/src/test/fuzz/vecdeque.cpp b/src/test/fuzz/vecdeque.cpp new file mode 100644 index 000000000000..1d9a98931fb7 --- /dev/null +++ b/src/test/fuzz/vecdeque.cpp @@ -0,0 +1,491 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include + +#include +#include + +namespace { + +/** The maximum number of simultaneous buffers kept by the test. */ +static constexpr size_t MAX_BUFFERS{3}; +/** How many elements are kept in a buffer at most. */ +static constexpr size_t MAX_BUFFER_SIZE{48}; +/** How many operations are performed at most on the buffers in one test. */ +static constexpr size_t MAX_OPERATIONS{1024}; + +/** Perform a simulation fuzz test on VecDeque type T. + * + * T must be constructible from a uint64_t seed, comparable to other T, copyable, and movable. + */ +template +void TestType(Span buffer, uint64_t rng_tweak) +{ + FuzzedDataProvider provider(buffer.data(), buffer.size()); + // Local RNG, only used for the seeds to initialize T objects with. + XoRoShiRo128PlusPlus rng(provider.ConsumeIntegral() ^ rng_tweak); + + // Real circular buffers. + std::vector> real; + real.reserve(MAX_BUFFERS); + // Simulated circular buffers. + std::vector> sim; + sim.reserve(MAX_BUFFERS); + // Temporary object of type T. + std::optional tmp; + + // Compare a real and a simulated buffer. + auto compare_fn = [](const VecDeque& r, const std::deque& s) { + assert(r.size() == s.size()); + assert(r.empty() == s.empty()); + assert(r.capacity() >= r.size()); + if (s.size() == 0) return; + assert(r.front() == s.front()); + assert(r.back() == s.back()); + for (size_t i = 0; i < s.size(); ++i) { + assert(r[i] == s[i]); + } + }; + + LIMITED_WHILE(provider.remaining_bytes(), MAX_OPERATIONS) { + int command = provider.ConsumeIntegral() % 64; + unsigned idx = real.empty() ? 0 : provider.ConsumeIntegralInRange(0, real.size() - 1); + const size_t num_buffers = sim.size(); + // Pick one operation based on value of command. Not all operations are always applicable. + // Loop through the applicable ones until command reaches 0 (which avoids the need to + // compute the number of applicable commands ahead of time). + const bool non_empty{num_buffers != 0}; + const bool non_full{num_buffers < MAX_BUFFERS}; + const bool partially_full{non_empty && non_full}; + const bool multiple_exist{num_buffers > 1}; + const bool existing_buffer_non_full{non_empty && sim[idx].size() < MAX_BUFFER_SIZE}; + const bool existing_buffer_non_empty{non_empty && !sim[idx].empty()}; + assert(non_full || non_empty); + while (true) { + if (non_full && command-- == 0) { + /* Default construct. */ + real.emplace_back(); + sim.emplace_back(); + break; + } + if (non_empty && command-- == 0) { + /* resize() */ + compare_fn(real[idx], sim[idx]); + size_t new_size = provider.ConsumeIntegralInRange(0, MAX_BUFFER_SIZE); + real[idx].resize(new_size); + sim[idx].resize(new_size); + assert(real[idx].size() == new_size); + break; + } + if (non_empty && command-- == 0) { + /* clear() */ + compare_fn(real[idx], sim[idx]); + real[idx].clear(); + sim[idx].clear(); + assert(real[idx].empty()); + break; + } + if (non_empty && command-- == 0) { + /* Copy construct default. */ + compare_fn(real[idx], sim[idx]); + real[idx] = VecDeque(); + sim[idx].clear(); + assert(real[idx].size() == 0); + break; + } + if (non_empty && command-- == 0) { + /* Destruct. */ + compare_fn(real.back(), sim.back()); + real.pop_back(); + sim.pop_back(); + break; + } + if (partially_full && command-- == 0) { + /* Copy construct. */ + real.emplace_back(real[idx]); + sim.emplace_back(sim[idx]); + break; + } + if (partially_full && command-- == 0) { + /* Move construct. */ + VecDeque copy(real[idx]); + real.emplace_back(std::move(copy)); + sim.emplace_back(sim[idx]); + break; + } + if (multiple_exist && command-- == 0) { + /* swap() */ + swap(real[idx], real[(idx + 1) % num_buffers]); + swap(sim[idx], sim[(idx + 1) % num_buffers]); + break; + } + if (multiple_exist && command-- == 0) { + /* Copy assign. */ + compare_fn(real[idx], sim[idx]); + real[idx] = real[(idx + 1) % num_buffers]; + sim[idx] = sim[(idx + 1) % num_buffers]; + break; + } + if (multiple_exist && command-- == 0) { + /* Move assign. */ + VecDeque copy(real[(idx + 1) % num_buffers]); + compare_fn(real[idx], sim[idx]); + real[idx] = std::move(copy); + sim[idx] = sim[(idx + 1) % num_buffers]; + break; + } + if (non_empty && command-- == 0) { + /* Self swap() */ + swap(real[idx], real[idx]); + break; + } + if (non_empty && command-- == 0) { + /* Self-copy assign. */ + real[idx] = real[idx]; + break; + } + if (non_empty && command-- == 0) { + /* Self-move assign. */ + // Do not use std::move(real[idx]) here: -Wself-move correctly warns about that. + real[idx] = static_cast&&>(real[idx]); + break; + } + if (non_empty && command-- == 0) { + /* reserve() */ + size_t res_size = provider.ConsumeIntegralInRange(0, MAX_BUFFER_SIZE); + size_t old_cap = real[idx].capacity(); + size_t old_size = real[idx].size(); + real[idx].reserve(res_size); + assert(real[idx].size() == old_size); + assert(real[idx].capacity() == std::max(old_cap, res_size)); + break; + } + if (non_empty && command-- == 0) { + /* shrink_to_fit() */ + size_t old_size = real[idx].size(); + real[idx].shrink_to_fit(); + assert(real[idx].size() == old_size); + assert(real[idx].capacity() == old_size); + break; + } + if (existing_buffer_non_full && command-- == 0) { + /* push_back() (copying) */ + tmp = T(rng()); + size_t old_size = real[idx].size(); + size_t old_cap = real[idx].capacity(); + real[idx].push_back(*tmp); + sim[idx].push_back(*tmp); + assert(real[idx].size() == old_size + 1); + if (old_cap > old_size) { + assert(real[idx].capacity() == old_cap); + } else { + assert(real[idx].capacity() > old_cap); + assert(real[idx].capacity() <= 2 * (old_cap + 1)); + } + break; + } + if (existing_buffer_non_full && command-- == 0) { + /* push_back() (moving) */ + tmp = T(rng()); + size_t old_size = real[idx].size(); + size_t old_cap = real[idx].capacity(); + sim[idx].push_back(*tmp); + real[idx].push_back(std::move(*tmp)); + assert(real[idx].size() == old_size + 1); + if (old_cap > old_size) { + assert(real[idx].capacity() == old_cap); + } else { + assert(real[idx].capacity() > old_cap); + assert(real[idx].capacity() <= 2 * (old_cap + 1)); + } + break; + } + if (existing_buffer_non_full && command-- == 0) { + /* emplace_back() */ + uint64_t seed{rng()}; + size_t old_size = real[idx].size(); + size_t old_cap = real[idx].capacity(); + sim[idx].emplace_back(seed); + real[idx].emplace_back(seed); + assert(real[idx].size() == old_size + 1); + if (old_cap > old_size) { + assert(real[idx].capacity() == old_cap); + } else { + assert(real[idx].capacity() > old_cap); + assert(real[idx].capacity() <= 2 * (old_cap + 1)); + } + break; + } + if (existing_buffer_non_full && command-- == 0) { + /* push_front() (copying) */ + tmp = T(rng()); + size_t old_size = real[idx].size(); + size_t old_cap = real[idx].capacity(); + real[idx].push_front(*tmp); + sim[idx].push_front(*tmp); + assert(real[idx].size() == old_size + 1); + if (old_cap > old_size) { + assert(real[idx].capacity() == old_cap); + } else { + assert(real[idx].capacity() > old_cap); + assert(real[idx].capacity() <= 2 * (old_cap + 1)); + } + break; + } + if (existing_buffer_non_full && command-- == 0) { + /* push_front() (moving) */ + tmp = T(rng()); + size_t old_size = real[idx].size(); + size_t old_cap = real[idx].capacity(); + sim[idx].push_front(*tmp); + real[idx].push_front(std::move(*tmp)); + assert(real[idx].size() == old_size + 1); + if (old_cap > old_size) { + assert(real[idx].capacity() == old_cap); + } else { + assert(real[idx].capacity() > old_cap); + assert(real[idx].capacity() <= 2 * (old_cap + 1)); + } + break; + } + if (existing_buffer_non_full && command-- == 0) { + /* emplace_front() */ + uint64_t seed{rng()}; + size_t old_size = real[idx].size(); + size_t old_cap = real[idx].capacity(); + sim[idx].emplace_front(seed); + real[idx].emplace_front(seed); + assert(real[idx].size() == old_size + 1); + if (old_cap > old_size) { + assert(real[idx].capacity() == old_cap); + } else { + assert(real[idx].capacity() > old_cap); + assert(real[idx].capacity() <= 2 * (old_cap + 1)); + } + break; + } + if (existing_buffer_non_empty && command-- == 0) { + /* front() [modifying] */ + tmp = T(rng()); + size_t old_size = real[idx].size(); + assert(sim[idx].front() == real[idx].front()); + sim[idx].front() = *tmp; + real[idx].front() = std::move(*tmp); + assert(real[idx].size() == old_size); + break; + } + if (existing_buffer_non_empty && command-- == 0) { + /* back() [modifying] */ + tmp = T(rng()); + size_t old_size = real[idx].size(); + assert(sim[idx].back() == real[idx].back()); + sim[idx].back() = *tmp; + real[idx].back() = *tmp; + assert(real[idx].size() == old_size); + break; + } + if (existing_buffer_non_empty && command-- == 0) { + /* operator[] [modifying] */ + tmp = T(rng()); + size_t pos = provider.ConsumeIntegralInRange(0, sim[idx].size() - 1); + size_t old_size = real[idx].size(); + assert(sim[idx][pos] == real[idx][pos]); + sim[idx][pos] = *tmp; + real[idx][pos] = std::move(*tmp); + assert(real[idx].size() == old_size); + break; + } + if (existing_buffer_non_empty && command-- == 0) { + /* pop_front() */ + assert(sim[idx].front() == real[idx].front()); + size_t old_size = real[idx].size(); + sim[idx].pop_front(); + real[idx].pop_front(); + assert(real[idx].size() == old_size - 1); + break; + } + if (existing_buffer_non_empty && command-- == 0) { + /* pop_back() */ + assert(sim[idx].back() == real[idx].back()); + size_t old_size = real[idx].size(); + sim[idx].pop_back(); + real[idx].pop_back(); + assert(real[idx].size() == old_size - 1); + break; + } + } + } + + /* Fully compare the final state. */ + for (unsigned i = 0; i < sim.size(); ++i) { + // Make sure const getters work. + const VecDeque& realbuf = real[i]; + const std::deque& simbuf = sim[i]; + compare_fn(realbuf, simbuf); + for (unsigned j = 0; j < sim.size(); ++j) { + assert((realbuf == real[j]) == (simbuf == sim[j])); + assert(((realbuf <=> real[j]) >= 0) == (simbuf >= sim[j])); + assert(((realbuf <=> real[j]) <= 0) == (simbuf <= sim[j])); + } + // Clear out the buffers so we can check below that no objects exist anymore. + sim[i].clear(); + real[i].clear(); + } + + if constexpr (CheckNoneLeft) { + tmp = std::nullopt; + T::CheckNoneExist(); + } +} + +/** Data structure with built-in tracking of all existing objects. */ +template +class TrackedObj +{ + static_assert(Size > 0); + + /* Data type for map that actually stores the object data. + * + * The key is a pointer to the TrackedObj, the value is the uint64_t it was initialized with. + * Default-constructed and moved-from objects hold an std::nullopt. + */ + using track_map_type = std::map*, std::optional>; + +private: + + /** Actual map. */ + static inline track_map_type g_tracker; + + /** Iterators into the tracker map for this object. + * + * This is an array of size Size, all holding the same value, to give the object configurable + * size. The value is g_tracker.end() if this object is not fully initialized. */ + typename track_map_type::iterator m_track_entry[Size]; + + void Check() const + { + auto it = g_tracker.find(this); + for (size_t i = 0; i < Size; ++i) { + assert(m_track_entry[i] == it); + } + } + + /** Create entry for this object in g_tracker and populate m_track_entry. */ + void Register() + { + auto [it, inserted] = g_tracker.emplace(this, std::nullopt); + assert(inserted); + for (size_t i = 0; i < Size; ++i) { + m_track_entry[i] = it; + } + } + + void Deregister() + { + Check(); + assert(m_track_entry[0] != g_tracker.end()); + g_tracker.erase(m_track_entry[0]); + for (size_t i = 0; i < Size; ++i) { + m_track_entry[i] = g_tracker.end(); + } + } + + /** Get value corresponding to this object in g_tracker. */ + std::optional& Deref() + { + Check(); + assert(m_track_entry[0] != g_tracker.end()); + return m_track_entry[0]->second; + } + + /** Get value corresponding to this object in g_tracker. */ + const std::optional& Deref() const + { + Check(); + assert(m_track_entry[0] != g_tracker.end()); + return m_track_entry[0]->second; + } + +public: + ~TrackedObj() { Deregister(); } + TrackedObj() { Register(); } + + TrackedObj(uint64_t value) + { + Register(); + Deref() = value; + } + + TrackedObj(const TrackedObj& other) + { + Register(); + Deref() = other.Deref(); + } + + TrackedObj(TrackedObj&& other) + { + Register(); + Deref() = other.Deref(); + other.Deref() = std::nullopt; + } + + TrackedObj& operator=(const TrackedObj& other) + { + if (this == &other) return *this; + Deref() = other.Deref(); + return *this; + } + + TrackedObj& operator=(TrackedObj&& other) + { + if (this == &other) return *this; + Deref() = other.Deref(); + other.Deref() = std::nullopt; + return *this; + } + + friend bool operator==(const TrackedObj& a, const TrackedObj& b) + { + return a.Deref() == b.Deref(); + } + + friend std::strong_ordering operator<=>(const TrackedObj& a, const TrackedObj& b) + { + // Libc++ 15 & 16 do not support std::optional::operator<=> yet. See + // https://reviews.llvm.org/D146392. + if (!a.Deref().has_value() || !b.Deref().has_value()) { + return a.Deref().has_value() <=> b.Deref().has_value(); + } + return *a.Deref() <=> *b.Deref(); + } + + static void CheckNoneExist() + { + assert(g_tracker.empty()); + } +}; + +} // namespace + +FUZZ_TARGET(vecdeque) +{ + // Run the test with simple uints (which satisfy all the trivial properties). + static_assert(std::is_trivially_copyable_v); + static_assert(std::is_trivially_destructible_v); + TestType(buffer, 1); + TestType(buffer, 2); + TestType(buffer, 3); + TestType(buffer, 4); + + // Run the test with TrackedObjs (which do not). + static_assert(!std::is_trivially_copyable_v>); + static_assert(!std::is_trivially_destructible_v>); + TestType, true>(buffer, 5); + TestType, true>(buffer, 6); + TestType, true>(buffer, 7); +} diff --git a/src/util/vecdeque.h b/src/util/vecdeque.h new file mode 100644 index 000000000000..b5e727847347 --- /dev/null +++ b/src/util/vecdeque.h @@ -0,0 +1,316 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_VECDEQUE_H +#define BITCOIN_UTIL_VECDEQUE_H + +#include + +#include +#include + +/** Data structure largely mimicking std::deque, but using single preallocated ring buffer. + * + * - More efficient and better memory locality than std::deque. + * - Most operations ({push_,pop_,emplace_,}{front,back}(), operator[], ...) are O(1), + * unless reallocation is needed (in which case they are O(n)). + * - Supports reserve(), capacity(), shrink_to_fit() like vectors. + * - No iterator support. + * - Data is not stored in a single contiguous block, so no data(). + */ +template +class VecDeque +{ + /** Pointer to allocated memory. Can contain constructed and uninitialized T objects. */ + T* m_buffer{nullptr}; + /** m_buffer + m_offset points to first object in queue. m_offset = 0 if m_capacity is 0; + * otherwise 0 <= m_offset < m_capacity. */ + size_t m_offset{0}; + /** Number of objects in the container. 0 <= m_size <= m_capacity. */ + size_t m_size{0}; + /** The size of m_buffer, expressed as a multiple of the size of T. */ + size_t m_capacity{0}; + + /** Returns the number of populated objects between m_offset and the end of the buffer. */ + size_t FirstPart() const noexcept { return std::min(m_capacity - m_offset, m_size); } + + void Reallocate(size_t capacity) + { + Assume(capacity >= m_size); + Assume((m_offset == 0 && m_capacity == 0) || m_offset < m_capacity); + // Allocate new buffer. + T* new_buffer = capacity ? std::allocator().allocate(capacity) : nullptr; + if (capacity) { + if constexpr (std::is_trivially_copyable_v) { + // When T is trivially copyable, just copy the data over from old to new buffer. + size_t first_part = FirstPart(); + if (first_part != 0) { + std::memcpy(new_buffer, m_buffer + m_offset, first_part * sizeof(T)); + } + if (first_part != m_size) { + std::memcpy(new_buffer + first_part, m_buffer, (m_size - first_part) * sizeof(T)); + } + } else { + // Otherwise move-construct in place in the new buffer, and destroy old buffer objects. + size_t old_pos = m_offset; + for (size_t new_pos = 0; new_pos < m_size; ++new_pos) { + std::construct_at(new_buffer + new_pos, std::move(*(m_buffer + old_pos))); + std::destroy_at(m_buffer + old_pos); + ++old_pos; + if (old_pos == m_capacity) old_pos = 0; + } + } + } + // Deallocate old buffer and update housekeeping. + std::allocator().deallocate(m_buffer, m_capacity); + m_buffer = new_buffer; + m_offset = 0; + m_capacity = capacity; + Assume((m_offset == 0 && m_capacity == 0) || m_offset < m_capacity); + } + + /** What index in the buffer does logical entry number pos have? */ + size_t BufferIndex(size_t pos) const noexcept + { + Assume(pos < m_capacity); + // The expression below is used instead of the more obvious (pos + m_offset >= m_capacity), + // because the addition there could in theory overflow with very large deques. + if (pos >= m_capacity - m_offset) { + return (m_offset + pos) - m_capacity; + } else { + return m_offset + pos; + } + } + + /** Specialization of resize() that can only shrink. Separate so that clear() can call it + * without requiring a default T constructor. */ + void ResizeDown(size_t size) noexcept + { + Assume(size <= m_size); + if constexpr (std::is_trivially_destructible_v) { + // If T is trivially destructible, we do not need to do anything but update the + // housekeeping record. Default constructor or zero-filling will be used when + // the space is reused. + m_size = size; + } else { + // If not, we need to invoke the destructor for every element separately. + while (m_size > size) { + std::destroy_at(m_buffer + BufferIndex(m_size - 1)); + --m_size; + } + } + } + +public: + VecDeque() noexcept = default; + + /** Resize the deque to be exactly size size (adding default-constructed elements if needed). */ + void resize(size_t size) + { + if (size < m_size) { + // Delegate to ResizeDown when shrinking. + ResizeDown(size); + } else if (size > m_size) { + // When growing, first see if we need to allocate more space. + if (size > m_capacity) Reallocate(size); + while (m_size < size) { + std::construct_at(m_buffer + BufferIndex(m_size)); + ++m_size; + } + } + } + + /** Resize the deque to be size 0. The capacity will remain unchanged. */ + void clear() noexcept { ResizeDown(0); } + + /** Destroy a deque. */ + ~VecDeque() + { + clear(); + Reallocate(0); + } + + /** Copy-assign a deque. */ + VecDeque& operator=(const VecDeque& other) + { + if (&other == this) [[unlikely]] return *this; + clear(); + Reallocate(other.m_size); + if constexpr (std::is_trivially_copyable_v) { + size_t first_part = other.FirstPart(); + Assume(first_part > 0 || m_size == 0); + if (first_part != 0) { + std::memcpy(m_buffer, other.m_buffer + other.m_offset, first_part * sizeof(T)); + } + if (first_part != other.m_size) { + std::memcpy(m_buffer + first_part, other.m_buffer, (other.m_size - first_part) * sizeof(T)); + } + m_size = other.m_size; + } else { + while (m_size < other.m_size) { + std::construct_at(m_buffer + BufferIndex(m_size), other[m_size]); + ++m_size; + } + } + return *this; + } + + /** Swap two deques. */ + void swap(VecDeque& other) noexcept + { + std::swap(m_buffer, other.m_buffer); + std::swap(m_offset, other.m_offset); + std::swap(m_size, other.m_size); + std::swap(m_capacity, other.m_capacity); + } + + /** Non-member version of swap. */ + friend void swap(VecDeque& a, VecDeque& b) noexcept { a.swap(b); } + + /** Move-assign a deque. */ + VecDeque& operator=(VecDeque&& other) noexcept + { + swap(other); + return *this; + } + + /** Copy-construct a deque. */ + VecDeque(const VecDeque& other) { *this = other; } + /** Move-construct a deque. */ + VecDeque(VecDeque&& other) noexcept { swap(other); } + + /** Equality comparison between two deques (only compares size+contents, not capacity). */ + bool friend operator==(const VecDeque& a, const VecDeque& b) + { + if (a.m_size != b.m_size) return false; + for (size_t i = 0; i < a.m_size; ++i) { + if (a[i] != b[i]) return false; + } + return true; + } + + /** Comparison between two deques, implementing lexicographic ordering on the contents. */ + std::strong_ordering friend operator<=>(const VecDeque& a, const VecDeque& b) + { + size_t pos_a{0}, pos_b{0}; + while (pos_a < a.m_size && pos_b < b.m_size) { + auto cmp = a[pos_a++] <=> b[pos_b++]; + if (cmp != 0) return cmp; + } + return a.m_size <=> b.m_size; + } + + /** Increase the capacity to capacity. Capacity will not shrink. */ + void reserve(size_t capacity) + { + if (capacity > m_capacity) Reallocate(capacity); + } + + /** Make the capacity equal to the size. The contents does not change. */ + void shrink_to_fit() + { + if (m_capacity > m_size) Reallocate(m_size); + } + + /** Construct a new element at the end of the deque. */ + template + void emplace_back(Args&&... args) + { + if (m_size == m_capacity) Reallocate((m_size + 1) * 2); + std::construct_at(m_buffer + BufferIndex(m_size), std::forward(args)...); + ++m_size; + } + + /** Move-construct a new element at the end of the deque. */ + void push_back(T&& elem) { emplace_back(std::move(elem)); } + + /** Copy-construct a new element at the end of the deque. */ + void push_back(const T& elem) { emplace_back(elem); } + + /** Construct a new element at the beginning of the deque. */ + template + void emplace_front(Args&&... args) + { + if (m_size == m_capacity) Reallocate((m_size + 1) * 2); + std::construct_at(m_buffer + BufferIndex(m_capacity - 1), std::forward(args)...); + if (m_offset == 0) m_offset = m_capacity; + --m_offset; + ++m_size; + } + + /** Copy-construct a new element at the beginning of the deque. */ + void push_front(const T& elem) { emplace_front(elem); } + + /** Move-construct a new element at the beginning of the deque. */ + void push_front(T&& elem) { emplace_front(std::move(elem)); } + + /** Remove the first element of the deque. Requires !empty(). */ + void pop_front() + { + Assume(m_size); + std::destroy_at(m_buffer + m_offset); + --m_size; + ++m_offset; + if (m_offset == m_capacity) m_offset = 0; + } + + /** Remove the last element of the deque. Requires !empty(). */ + void pop_back() + { + Assume(m_size); + std::destroy_at(m_buffer + BufferIndex(m_size - 1)); + --m_size; + } + + /** Get a mutable reference to the first element of the deque. Requires !empty(). */ + T& front() noexcept + { + Assume(m_size); + return m_buffer[m_offset]; + } + + /** Get a const reference to the first element of the deque. Requires !empty(). */ + const T& front() const noexcept + { + Assume(m_size); + return m_buffer[m_offset]; + } + + /** Get a mutable reference to the last element of the deque. Requires !empty(). */ + T& back() noexcept + { + Assume(m_size); + return m_buffer[BufferIndex(m_size - 1)]; + } + + /** Get a const reference to the last element of the deque. Requires !empty(). */ + const T& back() const noexcept + { + Assume(m_size); + return m_buffer[BufferIndex(m_size - 1)]; + } + + /** Get a mutable reference to the element in the deque at the given index. Requires idx < size(). */ + T& operator[](size_t idx) noexcept + { + Assume(idx < m_size); + return m_buffer[BufferIndex(idx)]; + } + + /** Get a const reference to the element in the deque at the given index. Requires idx < size(). */ + const T& operator[](size_t idx) const noexcept + { + Assume(idx < m_size); + return m_buffer[BufferIndex(idx)]; + } + + /** Test whether the contents of this deque is empty. */ + bool empty() const noexcept { return m_size == 0; } + /** Get the number of elements in this deque. */ + size_t size() const noexcept { return m_size; } + /** Get the capacity of this deque (maximum size it can have without reallocating). */ + size_t capacity() const noexcept { return m_capacity; } +}; + +#endif // BITCOIN_UTIL_VECDEQUE_H From 29d76307af92053443317348dca137e8ad926404 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 11 Jun 2024 17:28:51 -0400 Subject: [PATCH 10/10] Merge bitcoin/bitcoin#30160: util: add BitSet 47f705b33fc1381d96c99038e2110e6fe2b2f883 tests: add fuzz tests for BitSet (Pieter Wuille) 59a6df6bd584701f820ad60a10d9d477bf0236b5 util: add BitSet (Pieter Wuille) Pull request description: Extracted from #30126. This introduces the `BitSet` data structure, inspired by `std::bitset`, but with a few features that cannot be implemented on top without efficiency loss: * Finding the first set bit (`First`) * Finding the last set bit (`Last`) * Iterating over all set bits (`begin` and `end`). And a few other operators/member functions that help readability for #30126: * `operator-` for set subtraction * `Overlaps()` for testing whether intersection is non-empty * `IsSupersetOf()` for testing (non-strict) supersetness * `IsSubsetOf()` for testing (non-strict) subsetness * `Fill()` to construct a set with all numbers from 0 to n-1, inclusive * `Singleton()` to construct a set with one specific element. Everything is tested through a simulation-based fuzz test that compares the behavior with normal `std::bitset` equivalent operations. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/30160/commits/47f705b33fc1381d96c99038e2110e6fe2b2f883 achow101: ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883 cbergqvist: re-ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883 theStack: Code-review ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883 Tree-SHA512: e451bf4b801f193239ee434b6b614f5a2ac7bb49c70af5aba24c2ac0c54acbef4672556800e4ac799ae835632bdba716209c5ca8c37433a6883dab4eb7cd67c1 --- src/Makefile.am | 1 + src/Makefile.test.include | 1 + src/test/fuzz/bitset.cpp | 316 ++++++++++++++++++ src/util/bitset.h | 527 ++++++++++++++++++++++++++++++ test/sanitizer_suppressions/ubsan | 1 + 5 files changed, 846 insertions(+) create mode 100644 src/test/fuzz/bitset.cpp create mode 100644 src/util/bitset.h diff --git a/src/Makefile.am b/src/Makefile.am index 0ce854f87a51..d7bac02e356a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -391,6 +391,7 @@ BITCOIN_CORE_H = \ undo.h \ unordered_lru_cache.h \ util/bip32.h \ + util/bitset.h \ util/bytevectorhash.h \ util/check.h \ util/edge.h \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index cd0c61f0f6a8..a3564c4e4e8c 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -283,6 +283,7 @@ test_fuzz_fuzz_SOURCES = \ test/fuzz/base_encode_decode.cpp \ test/fuzz/bech32.cpp \ test/fuzz/bip324.cpp \ + test/fuzz/bitset.cpp \ test/fuzz/block.cpp \ test/fuzz/block_header.cpp \ test/fuzz/blockfilter.cpp \ diff --git a/src/test/fuzz/bitset.cpp b/src/test/fuzz/bitset.cpp new file mode 100644 index 000000000000..98fcddfb8dd6 --- /dev/null +++ b/src/test/fuzz/bitset.cpp @@ -0,0 +1,316 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include + +#include +#include + +namespace { + +/** Pop the first byte from a Span, and return it. */ +uint8_t ReadByte(Span& buffer) +{ + if (buffer.empty()) return 0; + uint8_t ret = buffer.front(); + buffer = buffer.subspan(1); + return ret; +} + +/** Perform a simulation fuzz test on BitSet type S. */ +template +void TestType(Span buffer) +{ + /** This fuzz test's design is based on the assumption that the actual bits stored in the + * bitsets and their simulations do not matter for the purpose of detecting edge cases, thus + * these are taken from a deterministically-seeded RNG instead. To provide some level of + * variation however, pick the seed based on the buffer size and size of the chosen bitset. */ + XoRoShiRo128PlusPlus rng(buffer.size() + 0x10000 * S::Size()); + + using Sim = std::bitset; + // Up to 4 real BitSets (initially 2). + std::vector real(2); + // Up to 4 std::bitsets with the same corresponding contents. + std::vector sim(2); + + /* Compare sim[idx] with real[idx], using all inspector operations. */ + auto compare_fn = [&](unsigned idx) { + /* iterators and operator[] */ + auto it = real[idx].begin(); + unsigned first = S::Size(); + unsigned last = S::Size(); + for (unsigned i = 0; i < S::Size(); ++i) { + bool match = (it != real[idx].end()) && *it == i; + assert(sim[idx][i] == real[idx][i]); + assert(match == real[idx][i]); + assert((it == real[idx].end()) != (it != real[idx].end())); + if (match) { + ++it; + if (first == S::Size()) first = i; + last = i; + } + } + assert(it == real[idx].end()); + assert(!(it != real[idx].end())); + /* Any / None */ + assert(sim[idx].any() == real[idx].Any()); + assert(sim[idx].none() == real[idx].None()); + /* First / Last */ + if (sim[idx].any()) { + assert(first == real[idx].First()); + assert(last == real[idx].Last()); + } + /* Count */ + assert(sim[idx].count() == real[idx].Count()); + }; + + LIMITED_WHILE(buffer.size() > 0, 1000) { + // Read one byte to determine which operation to execute on the BitSets. + int command = ReadByte(buffer) % 64; + // Read another byte that determines which bitsets will be involved. + unsigned args = ReadByte(buffer); + unsigned dest = ((args & 7) * sim.size()) >> 3; + unsigned src = (((args >> 3) & 7) * sim.size()) >> 3; + unsigned aux = (((args >> 6) & 3) * sim.size()) >> 2; + // Args are in range for non-empty sim, or sim is completely empty and will be grown + assert((sim.empty() && dest == 0 && src == 0 && aux == 0) || + (!sim.empty() && dest < sim.size() && src < sim.size() && aux < sim.size())); + + // Pick one operation based on value of command. Not all operations are always applicable. + // Loop through the applicable ones until command reaches 0 (which avoids the need to + // compute the number of applicable commands ahead of time). + while (true) { + if (dest < sim.size() && command-- == 0) { + /* Set() (true) */ + unsigned val = ReadByte(buffer) % S::Size(); + assert(sim[dest][val] == real[dest][val]); + sim[dest].set(val); + real[dest].Set(val); + break; + } else if (dest < sim.size() && command-- == 0) { + /* Reset() */ + unsigned val = ReadByte(buffer) % S::Size(); + assert(sim[dest][val] == real[dest][val]); + sim[dest].reset(val); + real[dest].Reset(val); + break; + } else if (dest < sim.size() && command-- == 0) { + /* Set() (conditional) */ + unsigned val = ReadByte(buffer) % S::Size(); + assert(sim[dest][val] == real[dest][val]); + sim[dest].set(val, args >> 7); + real[dest].Set(val, args >> 7); + break; + } else if (sim.size() < 4 && command-- == 0) { + /* Construct empty. */ + sim.resize(sim.size() + 1); + real.resize(real.size() + 1); + break; + } else if (sim.size() < 4 && command-- == 0) { + /* Construct singleton. */ + unsigned val = ReadByte(buffer) % S::Size(); + std::bitset newset; + newset[val] = true; + sim.push_back(newset); + real.push_back(S::Singleton(val)); + break; + } else if (dest < sim.size() && command-- == 0) { + /* Make random. */ + compare_fn(dest); + sim[dest].reset(); + real[dest] = S{}; + for (unsigned i = 0; i < S::Size(); ++i) { + if (rng() & 1) { + sim[dest][i] = true; + real[dest].Set(i); + } + } + break; + } else if (dest < sim.size() && command-- == 0) { + /* Assign initializer list. */ + unsigned r1 = rng() % S::Size(); + unsigned r2 = rng() % S::Size(); + unsigned r3 = rng() % S::Size(); + compare_fn(dest); + sim[dest].reset(); + real[dest] = {r1, r2, r3}; + sim[dest].set(r1); + sim[dest].set(r2); + sim[dest].set(r3); + break; + } else if (!sim.empty() && command-- == 0) { + /* Destruct. */ + compare_fn(sim.size() - 1); + sim.pop_back(); + real.pop_back(); + break; + } else if (sim.size() < 4 && src < sim.size() && command-- == 0) { + /* Copy construct. */ + sim.emplace_back(sim[src]); + real.emplace_back(real[src]); + break; + } else if (src < sim.size() && dest < sim.size() && command-- == 0) { + /* Copy assign. */ + compare_fn(dest); + sim[dest] = sim[src]; + real[dest] = real[src]; + break; + } else if (src < sim.size() && dest < sim.size() && command-- == 0) { + /* swap() function. */ + swap(sim[dest], sim[src]); + swap(real[dest], real[src]); + break; + } else if (sim.size() < 4 && command-- == 0) { + /* Construct with initializer list. */ + unsigned r1 = rng() % S::Size(); + unsigned r2 = rng() % S::Size(); + sim.emplace_back(); + sim.back().set(r1); + sim.back().set(r2); + real.push_back(S{r1, r2}); + break; + } else if (dest < sim.size() && command-- == 0) { + /* Fill() + copy assign. */ + unsigned len = ReadByte(buffer) % S::Size(); + compare_fn(dest); + sim[dest].reset(); + for (unsigned i = 0; i < len; ++i) sim[dest][i] = true; + real[dest] = S::Fill(len); + break; + } else if (src < sim.size() && command-- == 0) { + /* Iterator copy based compare. */ + unsigned val = ReadByte(buffer) % S::Size(); + /* In a first loop, compare begin..end, and copy to it_copy at some point. */ + auto it = real[src].begin(), it_copy = it; + for (unsigned i = 0; i < S::Size(); ++i) { + if (i == val) it_copy = it; + bool match = (it != real[src].end()) && *it == i; + assert(match == sim[src][i]); + if (match) ++it; + } + assert(it == real[src].end()); + /* Then compare from the copied point again to end. */ + for (unsigned i = val; i < S::Size(); ++i) { + bool match = (it_copy != real[src].end()) && *it_copy == i; + assert(match == sim[src][i]); + if (match) ++it_copy; + } + assert(it_copy == real[src].end()); + break; + } else if (src < sim.size() && dest < sim.size() && command-- == 0) { + /* operator|= */ + compare_fn(dest); + sim[dest] |= sim[src]; + real[dest] |= real[src]; + break; + } else if (src < sim.size() && dest < sim.size() && command-- == 0) { + /* operator&= */ + compare_fn(dest); + sim[dest] &= sim[src]; + real[dest] &= real[src]; + break; + } else if (src < sim.size() && dest < sim.size() && command-- == 0) { + /* operator-= */ + compare_fn(dest); + sim[dest] &= ~sim[src]; + real[dest] -= real[src]; + break; + } else if (src < sim.size() && dest < sim.size() && command-- == 0) { + /* operator^= */ + compare_fn(dest); + sim[dest] ^= sim[src]; + real[dest] ^= real[src]; + break; + } else if (src < sim.size() && dest < sim.size() && aux < sim.size() && command-- == 0) { + /* operator| */ + compare_fn(dest); + sim[dest] = sim[src] | sim[aux]; + real[dest] = real[src] | real[aux]; + break; + } else if (src < sim.size() && dest < sim.size() && aux < sim.size() && command-- == 0) { + /* operator& */ + compare_fn(dest); + sim[dest] = sim[src] & sim[aux]; + real[dest] = real[src] & real[aux]; + break; + } else if (src < sim.size() && dest < sim.size() && aux < sim.size() && command-- == 0) { + /* operator- */ + compare_fn(dest); + sim[dest] = sim[src] & ~sim[aux]; + real[dest] = real[src] - real[aux]; + break; + } else if (src < sim.size() && dest < sim.size() && aux < sim.size() && command-- == 0) { + /* operator^ */ + compare_fn(dest); + sim[dest] = sim[src] ^ sim[aux]; + real[dest] = real[src] ^ real[aux]; + break; + } else if (src < sim.size() && aux < sim.size() && command-- == 0) { + /* IsSupersetOf() and IsSubsetOf() */ + bool is_superset = (sim[aux] & ~sim[src]).none(); + bool is_subset = (sim[src] & ~sim[aux]).none(); + assert(real[src].IsSupersetOf(real[aux]) == is_superset); + assert(real[src].IsSubsetOf(real[aux]) == is_subset); + assert(real[aux].IsSupersetOf(real[src]) == is_subset); + assert(real[aux].IsSubsetOf(real[src]) == is_superset); + break; + } else if (src < sim.size() && aux < sim.size() && command-- == 0) { + /* operator== and operator!= */ + assert((sim[src] == sim[aux]) == (real[src] == real[aux])); + assert((sim[src] != sim[aux]) == (real[src] != real[aux])); + break; + } else if (src < sim.size() && aux < sim.size() && command-- == 0) { + /* Overlaps() */ + assert((sim[src] & sim[aux]).any() == real[src].Overlaps(real[aux])); + assert((sim[src] & sim[aux]).any() == real[aux].Overlaps(real[src])); + break; + } + } + } + /* Fully compare the final state. */ + for (unsigned i = 0; i < sim.size(); ++i) { + compare_fn(i); + } +} + +} // namespace + +FUZZ_TARGET(bitset) +{ + unsigned typdat = ReadByte(buffer) % 8; + if (typdat == 0) { + /* 16 bits */ + TestType>(buffer); + TestType>(buffer); + } else if (typdat == 1) { + /* 32 bits */ + TestType>(buffer); + TestType>(buffer); + } else if (typdat == 2) { + /* 48 bits */ + TestType>(buffer); + } else if (typdat == 3) { + /* 64 bits */ + TestType>(buffer); + TestType>(buffer); + TestType>(buffer); + TestType>(buffer); + } else if (typdat == 4) { + /* 96 bits */ + TestType>(buffer); + } else if (typdat == 5) { + /* 128 bits */ + TestType>(buffer); + TestType>(buffer); + } else if (typdat == 6) { + /* 192 bits */ + TestType>(buffer); + } else if (typdat == 7) { + /* 256 bits */ + TestType>(buffer); + } +} diff --git a/src/util/bitset.h b/src/util/bitset.h new file mode 100644 index 000000000000..6f9e808c371e --- /dev/null +++ b/src/util/bitset.h @@ -0,0 +1,527 @@ +// Copyright (c) The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_BITSET_H +#define BITCOIN_UTIL_BITSET_H + +#include + +#include +#include +#include +#include +#include + +/* This file provides data types similar to std::bitset, but adds the following functionality: + * + * - Efficient iteration over all set bits (compatible with range-based for loops). + * - Efficient search for the first and last set bit (First() and Last()). + * - Efficient set subtraction: (a - b) implements "a and not b". + * - Efficient non-strict subset/superset testing: IsSubsetOf() and IsSupersetOf(). + * - Efficient set overlap testing: a.Overlaps(b) + * - Efficient construction of set containing 0..N-1 (S::Fill). + * - Efficient construction of a single set (S::Singleton). + * - Construction from initializer lists. + * + * Other differences: + * - BitSet is a bitset that supports at least N elements, but may support more (Size() reports + * the actual number). Because the actual number is unpredictable, there are no operations that + * affect all positions (like std::bitset's operator~, flip(), or all()). + * - Various other unimplemented features. + */ + +namespace bitset_detail { + +/** Count the number of bits set in an unsigned integer type. */ +template +unsigned inline constexpr PopCount(I v) +{ + static_assert(std::is_integral_v && std::is_unsigned_v && std::numeric_limits::radix == 2); + constexpr auto BITS = std::numeric_limits::digits; + // Algorithms from https://en.wikipedia.org/wiki/Hamming_weight#Efficient_implementation. + // These seem to be faster than std::popcount when compiling for non-SSE4 on x86_64. + if constexpr (BITS <= 32) { + v -= (v >> 1) & 0x55555555; + v = (v & 0x33333333) + ((v >> 2) & 0x33333333); + v = (v + (v >> 4)) & 0x0f0f0f0f; + if constexpr (BITS > 8) v += v >> 8; + if constexpr (BITS > 16) v += v >> 16; + return v & 0x3f; + } else { + static_assert(BITS <= 64); + v -= (v >> 1) & 0x5555555555555555; + v = (v & 0x3333333333333333) + ((v >> 2) & 0x3333333333333333); + v = (v + (v >> 4)) & 0x0f0f0f0f0f0f0f0f; + return (v * uint64_t{0x0101010101010101}) >> 56; + } +} + +/** A bitset implementation backed by a single integer of type I. */ +template +class IntBitSet +{ + // Only binary, unsigned, integer, types allowed. + static_assert(std::is_integral_v && std::is_unsigned_v && std::numeric_limits::radix == 2); + /** The maximum number of bits this bitset supports. */ + static constexpr unsigned MAX_SIZE = std::numeric_limits::digits; + /** Integer whose bits represent this bitset. */ + I m_val; + /** Internal constructor with a given integer as contents. */ + IntBitSet(I val) noexcept : m_val{val} {} + /** Dummy type to return using end(). Only used for comparing with Iterator. */ + class IteratorEnd + { + friend class IntBitSet; + constexpr IteratorEnd() = default; + public: + constexpr IteratorEnd(const IteratorEnd&) = default; + }; + /** Iterator type returned by begin(), which efficiently iterates all 1 positions. */ + class Iterator + { + friend class IntBitSet; + I m_val; /**< The original integer's remaining bits. */ + unsigned m_pos; /** Last reported 1 position (if m_pos != 0). */ + constexpr Iterator(I val) noexcept : m_val(val), m_pos(0) + { + if (m_val != 0) m_pos = std::countr_zero(m_val); + } + public: + /** Do not allow external code to construct an Iterator. */ + Iterator() = delete; + // Copying is allowed. + constexpr Iterator(const Iterator&) noexcept = default; + constexpr Iterator& operator=(const Iterator&) noexcept = default; + /** Test whether we are done (can only compare with IteratorEnd). */ + constexpr friend bool operator==(const Iterator& a, const IteratorEnd&) noexcept + { + return a.m_val == 0; + } + /** Progress to the next 1 bit (only if != IteratorEnd). */ + constexpr Iterator& operator++() noexcept + { + Assume(m_val != 0); + m_val &= m_val - I{1U}; + if (m_val != 0) m_pos = std::countr_zero(m_val); + return *this; + } + /** Get the current bit position (only if != IteratorEnd). */ + constexpr unsigned operator*() const noexcept + { + Assume(m_val != 0); + return m_pos; + } + }; + +public: + /** Construct an all-zero bitset. */ + constexpr IntBitSet() noexcept : m_val{0} {} + /** Copy construct a bitset. */ + constexpr IntBitSet(const IntBitSet&) noexcept = default; + /** Construct from a list of values. */ + constexpr IntBitSet(std::initializer_list ilist) noexcept : m_val(0) + { + for (auto pos : ilist) Set(pos); + } + /** Copy assign a bitset. */ + constexpr IntBitSet& operator=(const IntBitSet&) noexcept = default; + /** Assign from a list of positions (which will be made true, all others false). */ + constexpr IntBitSet& operator=(std::initializer_list ilist) noexcept + { + m_val = 0; + for (auto pos : ilist) Set(pos); + return *this; + } + /** Construct a bitset with the singleton i. */ + static constexpr IntBitSet Singleton(unsigned i) noexcept + { + Assume(i < MAX_SIZE); + return IntBitSet(I(1U) << i); + } + /** Construct a bitset with bits 0..count-1 (inclusive) set to 1. */ + static constexpr IntBitSet Fill(unsigned count) noexcept + { + IntBitSet ret; + Assume(count <= MAX_SIZE); + if (count) ret.m_val = I(~I{0}) >> (MAX_SIZE - count); + return ret; + } + /** Set a bit to 1. */ + constexpr void Set(unsigned pos) noexcept + { + Assume(pos < MAX_SIZE); + m_val |= I{1U} << pos; + } + /** Set a bit to the specified value. */ + constexpr void Set(unsigned pos, bool val) noexcept + { + Assume(pos < MAX_SIZE); + m_val = (m_val & ~I(I{1U} << pos)) | (I(val) << pos); + } + /** Set a bit to 0. */ + constexpr void Reset(unsigned pos) noexcept + { + Assume(pos < MAX_SIZE); + m_val &= ~I(I{1U} << pos); + } + /** Retrieve a bit at the given position. */ + constexpr bool operator[](unsigned pos) const noexcept + { + Assume(pos < MAX_SIZE); + return (m_val >> pos) & 1U; + } + /** Compute the number of 1 bits in the bitset. */ + constexpr unsigned Count() const noexcept { return PopCount(m_val); } + /** Return the number of bits that this object holds. */ + static constexpr unsigned Size() noexcept { return MAX_SIZE; } + /** Check if all bits are 0. */ + constexpr bool None() const noexcept { return m_val == 0; } + /** Check if any bits are 1. */ + constexpr bool Any() const noexcept { return !None(); } + /** Return an object that iterates over all 1 bits (++ and * only allowed when != end()). */ + constexpr Iterator begin() const noexcept { return Iterator(m_val); } + /** Return a dummy object to compare Iterators with. */ + constexpr IteratorEnd end() const noexcept { return IteratorEnd(); } + /** Find the first element (requires Any()). */ + constexpr unsigned First() const noexcept + { + Assume(m_val != 0); + return std::countr_zero(m_val); + } + /** Find the last element (requires Any()). */ + constexpr unsigned Last() const noexcept + { + Assume(m_val != 0); + return std::bit_width(m_val) - 1; + } + /** Set this object's bits to be the binary AND between respective bits from this and a. */ + constexpr IntBitSet& operator|=(const IntBitSet& a) noexcept { m_val |= a.m_val; return *this; } + /** Set this object's bits to be the binary OR between respective bits from this and a. */ + constexpr IntBitSet& operator&=(const IntBitSet& a) noexcept { m_val &= a.m_val; return *this; } + /** Set this object's bits to be the binary AND NOT between respective bits from this and a. */ + constexpr IntBitSet& operator-=(const IntBitSet& a) noexcept { m_val &= ~a.m_val; return *this; } + /** Set this object's bits to be the binary XOR between respective bits from this as a. */ + constexpr IntBitSet& operator^=(const IntBitSet& a) noexcept { m_val ^= a.m_val; return *this; } + /** Check if the intersection between two sets is non-empty. */ + constexpr bool Overlaps(const IntBitSet& a) const noexcept { return m_val & a.m_val; } + /** Return an object with the binary AND between respective bits from a and b. */ + friend constexpr IntBitSet operator&(const IntBitSet& a, const IntBitSet& b) noexcept { return I(a.m_val & b.m_val); } + /** Return an object with the binary OR between respective bits from a and b. */ + friend constexpr IntBitSet operator|(const IntBitSet& a, const IntBitSet& b) noexcept { return I(a.m_val | b.m_val); } + /** Return an object with the binary AND NOT between respective bits from a and b. */ + friend constexpr IntBitSet operator-(const IntBitSet& a, const IntBitSet& b) noexcept { return I(a.m_val & ~b.m_val); } + /** Return an object with the binary XOR between respective bits from a and b. */ + friend constexpr IntBitSet operator^(const IntBitSet& a, const IntBitSet& b) noexcept { return I(a.m_val ^ b.m_val); } + /** Check if bitset a and bitset b are identical. */ + friend constexpr bool operator==(const IntBitSet& a, const IntBitSet& b) noexcept = default; + /** Check if bitset a is a superset of bitset b (= every 1 bit in b is also in a). */ + constexpr bool IsSupersetOf(const IntBitSet& a) const noexcept { return (a.m_val & ~m_val) == 0; } + /** Check if bitset a is a subset of bitset b (= every 1 bit in a is also in b). */ + constexpr bool IsSubsetOf(const IntBitSet& a) const noexcept { return (m_val & ~a.m_val) == 0; } + /** Swap two bitsets. */ + friend constexpr void swap(IntBitSet& a, IntBitSet& b) noexcept { std::swap(a.m_val, b.m_val); } +}; + +/** A bitset implementation backed by N integers of type I. */ +template +class MultiIntBitSet +{ + // Only binary, unsigned, integer, types allowed. + static_assert(std::is_integral_v && std::is_unsigned_v && std::numeric_limits::radix == 2); + // Cannot be empty. + static_assert(N > 0); + /** The number of bits per integer. */ + static constexpr unsigned LIMB_BITS = std::numeric_limits::digits; + /** Number of elements this set type supports. */ + static constexpr unsigned MAX_SIZE = LIMB_BITS * N; + // No overflow allowed here. + static_assert(MAX_SIZE / LIMB_BITS == N); + /** Array whose member integers store the bits of the set. */ + std::array m_val; + /** Dummy type to return using end(). Only used for comparing with Iterator. */ + class IteratorEnd + { + friend class MultiIntBitSet; + constexpr IteratorEnd() = default; + public: + constexpr IteratorEnd(const IteratorEnd&) = default; + }; + /** Iterator type returned by begin(), which efficiently iterates all 1 positions. */ + class Iterator + { + friend class MultiIntBitSet; + const std::array* m_ptr; /**< Pointer to array to fetch bits from. */ + I m_val; /**< The remaining bits of (*m_ptr)[m_idx]. */ + unsigned m_pos; /**< The last reported position. */ + unsigned m_idx; /**< The index in *m_ptr currently being iterated over. */ + constexpr Iterator(const std::array& ref) noexcept : m_ptr(&ref), m_idx(0) + { + do { + m_val = (*m_ptr)[m_idx]; + if (m_val) { + m_pos = std::countr_zero(m_val) + m_idx * LIMB_BITS; + break; + } + ++m_idx; + } while(m_idx < N); + } + + public: + /** Do not allow external code to construct an Iterator. */ + Iterator() = delete; + // Copying is allowed. + constexpr Iterator(const Iterator&) noexcept = default; + constexpr Iterator& operator=(const Iterator&) noexcept = default; + /** Test whether we are done (can only compare with IteratorEnd). */ + friend constexpr bool operator==(const Iterator& a, const IteratorEnd&) noexcept + { + return a.m_idx == N; + } + /** Progress to the next 1 bit (only if != IteratorEnd). */ + constexpr Iterator& operator++() noexcept + { + Assume(m_idx < N); + m_val &= m_val - I{1U}; + if (m_val == 0) { + while (true) { + ++m_idx; + if (m_idx == N) break; + m_val = (*m_ptr)[m_idx]; + if (m_val) { + m_pos = std::countr_zero(m_val) + m_idx * LIMB_BITS; + break; + } + } + } else { + m_pos = std::countr_zero(m_val) + m_idx * LIMB_BITS; + } + return *this; + } + /** Get the current bit position (only if != IteratorEnd). */ + constexpr unsigned operator*() const noexcept + { + Assume(m_idx < N); + return m_pos; + } + }; + +public: + /** Construct an all-zero bitset. */ + constexpr MultiIntBitSet() noexcept : m_val{} {} + /** Copy construct a bitset. */ + constexpr MultiIntBitSet(const MultiIntBitSet&) noexcept = default; + /** Copy assign a bitset. */ + constexpr MultiIntBitSet& operator=(const MultiIntBitSet&) noexcept = default; + /** Set a bit to 1. */ + void constexpr Set(unsigned pos) noexcept + { + Assume(pos < MAX_SIZE); + m_val[pos / LIMB_BITS] |= I{1U} << (pos % LIMB_BITS); + } + /** Set a bit to the specified value. */ + void constexpr Set(unsigned pos, bool val) noexcept + { + Assume(pos < MAX_SIZE); + m_val[pos / LIMB_BITS] = (m_val[pos / LIMB_BITS] & ~I(I{1U} << (pos % LIMB_BITS))) | + (I{val} << (pos % LIMB_BITS)); + } + /** Construct a bitset from a list of values. */ + constexpr MultiIntBitSet(std::initializer_list ilist) noexcept : m_val{} + { + for (auto pos : ilist) Set(pos); + } + /** Set a bitset to a list of values. */ + constexpr MultiIntBitSet& operator=(std::initializer_list ilist) noexcept + { + m_val.fill(0); + for (auto pos : ilist) Set(pos); + return *this; + } + /** Set a bit to 0. */ + void constexpr Reset(unsigned pos) noexcept + { + Assume(pos < MAX_SIZE); + m_val[pos / LIMB_BITS] &= ~I(I{1U} << (pos % LIMB_BITS)); + } + /** Retrieve a bit at the given position. */ + bool constexpr operator[](unsigned pos) const noexcept + { + Assume(pos < MAX_SIZE); + return (m_val[pos / LIMB_BITS] >> (pos % LIMB_BITS)) & 1U; + } + /** Construct a bitset with the singleton pos. */ + static constexpr MultiIntBitSet Singleton(unsigned pos) noexcept + { + Assume(pos < MAX_SIZE); + MultiIntBitSet ret; + ret.m_val[pos / LIMB_BITS] = I{1U} << (pos % LIMB_BITS); + return ret; + } + /** Construct a bitset with bits 0..count-1 (inclusive) set to 1. */ + static constexpr MultiIntBitSet Fill(unsigned count) noexcept + { + Assume(count <= MAX_SIZE); + MultiIntBitSet ret; + if (count) { + unsigned i = 0; + while (count > LIMB_BITS) { + ret.m_val[i++] = ~I{0}; + count -= LIMB_BITS; + } + ret.m_val[i] = I(~I{0}) >> (LIMB_BITS - count); + } + return ret; + } + /** Return the number of bits that this object holds. */ + static constexpr unsigned Size() noexcept { return MAX_SIZE; } + /** Compute the number of 1 bits in the bitset. */ + unsigned constexpr Count() const noexcept + { + unsigned ret{0}; + for (I v : m_val) ret += PopCount(v); + return ret; + } + /** Check if all bits are 0. */ + bool constexpr None() const noexcept + { + for (auto v : m_val) { + if (v != 0) return false; + } + return true; + } + /** Check if any bits are 1. */ + bool constexpr Any() const noexcept { return !None(); } + /** Return an object that iterates over all 1 bits (++ and * only allowed when != end()). */ + Iterator constexpr begin() const noexcept { return Iterator(m_val); } + /** Return a dummy object to compare Iterators with. */ + IteratorEnd constexpr end() const noexcept { return IteratorEnd(); } + /** Find the first element (requires Any()). */ + unsigned constexpr First() const noexcept + { + unsigned p = 0; + while (m_val[p] == 0) { + ++p; + Assume(p < N); + } + return std::countr_zero(m_val[p]) + p * LIMB_BITS; + } + /** Find the last element (requires Any()). */ + unsigned constexpr Last() const noexcept + { + unsigned p = N - 1; + while (m_val[p] == 0) { + Assume(p > 0); + --p; + } + return std::bit_width(m_val[p]) - 1 + p * LIMB_BITS; + } + /** Set this object's bits to be the binary OR between respective bits from this and a. */ + constexpr MultiIntBitSet& operator|=(const MultiIntBitSet& a) noexcept + { + for (unsigned i = 0; i < N; ++i) { + m_val[i] |= a.m_val[i]; + } + return *this; + } + /** Set this object's bits to be the binary AND between respective bits from this and a. */ + constexpr MultiIntBitSet& operator&=(const MultiIntBitSet& a) noexcept + { + for (unsigned i = 0; i < N; ++i) { + m_val[i] &= a.m_val[i]; + } + return *this; + } + /** Set this object's bits to be the binary AND NOT between respective bits from this and a. */ + constexpr MultiIntBitSet& operator-=(const MultiIntBitSet& a) noexcept + { + for (unsigned i = 0; i < N; ++i) { + m_val[i] &= ~a.m_val[i]; + } + return *this; + } + /** Set this object's bits to be the binary XOR between respective bits from this and a. */ + constexpr MultiIntBitSet& operator^=(const MultiIntBitSet& a) noexcept + { + for (unsigned i = 0; i < N; ++i) { + m_val[i] ^= a.m_val[i]; + } + return *this; + } + /** Check whether the intersection between two sets is non-empty. */ + constexpr bool Overlaps(const MultiIntBitSet& a) const noexcept + { + for (unsigned i = 0; i < N; ++i) { + if (m_val[i] & a.m_val[i]) return true; + } + return false; + } + /** Return an object with the binary AND between respective bits from a and b. */ + friend constexpr MultiIntBitSet operator&(const MultiIntBitSet& a, const MultiIntBitSet& b) noexcept + { + MultiIntBitSet r; + for (unsigned i = 0; i < N; ++i) { + r.m_val[i] = a.m_val[i] & b.m_val[i]; + } + return r; + } + /** Return an object with the binary OR between respective bits from a and b. */ + friend constexpr MultiIntBitSet operator|(const MultiIntBitSet& a, const MultiIntBitSet& b) noexcept + { + MultiIntBitSet r; + for (unsigned i = 0; i < N; ++i) { + r.m_val[i] = a.m_val[i] | b.m_val[i]; + } + return r; + } + /** Return an object with the binary AND NOT between respective bits from a and b. */ + friend constexpr MultiIntBitSet operator-(const MultiIntBitSet& a, const MultiIntBitSet& b) noexcept + { + MultiIntBitSet r; + for (unsigned i = 0; i < N; ++i) { + r.m_val[i] = a.m_val[i] & ~b.m_val[i]; + } + return r; + } + /** Return an object with the binary XOR between respective bits from a and b. */ + friend constexpr MultiIntBitSet operator^(const MultiIntBitSet& a, const MultiIntBitSet& b) noexcept + { + MultiIntBitSet r; + for (unsigned i = 0; i < N; ++i) { + r.m_val[i] = a.m_val[i] ^ b.m_val[i]; + } + return r; + } + /** Check if bitset a is a superset of bitset b (= every 1 bit in b is also in a). */ + constexpr bool IsSupersetOf(const MultiIntBitSet& a) const noexcept + { + for (unsigned i = 0; i < N; ++i) { + if (a.m_val[i] & ~m_val[i]) return false; + } + return true; + } + /** Check if bitset a is a subset of bitset b (= every 1 bit in a is also in b). */ + constexpr bool IsSubsetOf(const MultiIntBitSet& a) const noexcept + { + for (unsigned i = 0; i < N; ++i) { + if (m_val[i] & ~a.m_val[i]) return false; + } + return true; + } + /** Check if bitset a and bitset b are identical. */ + friend constexpr bool operator==(const MultiIntBitSet& a, const MultiIntBitSet& b) noexcept = default; + /** Swap two bitsets. */ + friend constexpr void swap(MultiIntBitSet& a, MultiIntBitSet& b) noexcept { std::swap(a.m_val, b.m_val); } +}; + +} // namespace bitset_detail + +// BitSet dispatches to IntBitSet or MultiIntBitSet as appropriate for the requested minimum number +// of bits. Use IntBitSet up to 32-bit, or up to 64-bit on 64-bit platforms; above that, use a +// MultiIntBitSet of size_t. +template +using BitSet = std::conditional_t<(BITS <= 32), bitset_detail::IntBitSet, + std::conditional_t<(BITS <= std::numeric_limits::digits), bitset_detail::IntBitSet, + bitset_detail::MultiIntBitSet::digits - 1) / std::numeric_limits::digits>>>; + +#endif // BITCOIN_UTIL_BITSET_H diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 1d1f2ad352b5..3a14b22264a3 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -52,6 +52,7 @@ unsigned-integer-overflow:EvalScript unsigned-integer-overflow:txmempool.cpp unsigned-integer-overflow:util/strencodings.cpp unsigned-integer-overflow:xoroshiro128plusplus.h +unsigned-integer-overflow:bitset_detail::PopCount implicit-integer-sign-change:addrman.h implicit-integer-sign-change:bech32.cpp implicit-integer-sign-change:compat/stdin.cpp