From 03e0bd3347ec17ce4f2324da065b35e8fd0b6164 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 8 Feb 2024 13:41:02 -0500 Subject: [PATCH 01/18] Merge bitcoin/bitcoin#27319: addrman, refactor: improve stochastic test in `AddSingle` e064487ca28c12ba774c2f43a3c7acbdb1a278c9 addrman, refactor: improve stochastic test in `AddSingle` (brunoerg) Pull request description: This PR changes this algorithm to be O(1) instead of O(n). Also, in the current implementation, if `pinfo->nRefCount` is 0, we created an unnecessary variable (`nFactor`), this changes it. the change is relatively simple and does not cause conflicts. ACKs for top commit: achow101: ACK e064487ca28c12ba774c2f43a3c7acbdb1a278c9 amitiuttarwar: ACK e064487ca28c12ba774c2f43a3c7acbdb1a278c9 stratospher: ACK e064487ca28c12ba774c2f43a3c7acbdb1a278c9. simple use of << instead of a loop, didn't observe any behaviour difference before and after. Tree-SHA512: ff0a65155e47f65d2ce3cb5a3fd7a86efef1861181143df13a9d8e59cb16aee9be2f8801457bba8478b17fac47b015bff5cc656f6fac2ccc071ee7178a38d291 --- src/addrman.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index 95bf5eb9c693..835b9aa6fe46 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -581,11 +581,10 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, int64_ return false; // stochastic test: previous nRefCount == N: 2^N times harder to increase it - int nFactor = 1; - for (int n = 0; n < pinfo->nRefCount; n++) - nFactor *= 2; - if (nFactor > 1 && (insecure_rand.randrange(nFactor) != 0)) - return false; + if (pinfo->nRefCount > 0) { + const int nFactor{1 << pinfo->nRefCount}; + if (insecure_rand.randrange(nFactor) != 0) return false; + } } else { pinfo = Create(addr, source, &nId); pinfo->nTime = std::max((int64_t)0, (int64_t)pinfo->nTime - nTimePenalty); From f0968807bc535606968d4c5a5f9dceadc121413a Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Thu, 8 Feb 2024 18:01:32 -0500 Subject: [PATCH 02/18] Merge bitcoin/bitcoin#29377: test: Add makefile target for running unit tests 5ca9b24da18e842e7a093dc44f6b222af73e92cf test: Add makefile target for running unit tests (TheCharlatan) Pull request description: `make check` runs a bunch of other subtree tests that exercise code that is hardly ever changed and have a comparatively long runtime. There seems to be no target for running just the unit tests, so add one. Alternatively the secp256k1 tests could be removed from the `check-local` target, reducing its runtime. This was rejected before though in https://github.com/bitcoin/bitcoin/pull/20264. ACKs for top commit: delta1: utACK 5ca9b24da18e842e7a093dc44f6b222af73e92cf edilmedeiros: Tested ACK 5ca9b24da18e842e7a093dc44f6b222af73e92cf achow101: ACK 5ca9b24da18e842e7a093dc44f6b222af73e92cf ryanofsky: Tested ACK 5ca9b24da18e842e7a093dc44f6b222af73e92cf. Tree-SHA512: 470969d44585d7cc33ad038a16e791db9e2be8469f52ddf122c46f20776fad34e6a48f988861a132c42540158fed05f3cf66fcc3bea05708253daaa35af54339 --- src/Makefile.test.include | 4 +++- src/test/README.md | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 9c7a0a4a249e..1a4cc323e292 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -367,7 +367,9 @@ dash_test_check: $(TEST_BINARY) FORCE dash_test_clean : FORCE rm -f $(CLEAN_BITCOIN_TEST) $(test_test_dash_OBJECTS) $(TEST_BINARY) -check-local: $(BITCOIN_TESTS:.cpp=.cpp.test) +check-unit: $(BITCOIN_TESTS:.cpp=.cpp.test) + +check-local: check-unit if BUILD_BITCOIN_TX @echo "Running test/util/bitcoin-util-test.py..." $(PYTHON) $(top_builddir)/test/util/bitcoin-util-test.py diff --git a/src/test/README.md b/src/test/README.md index cf76f12007c1..2f69c24f86bc 100644 --- a/src/test/README.md +++ b/src/test/README.md @@ -15,7 +15,8 @@ that runs all of the unit tests. The main source file for the test library is fo Unit tests will be automatically compiled if dependencies were met in `./configure` and tests weren't explicitly disabled. -After configuring, they can be run with `make check`. +After configuring, they can be run with `make check`, which includes unit tests from +subtrees, or `make && make -C src check-unit` for just the unit tests. To run the unit tests manually, launch `src/test/test_dash`. To recompile after a test file was modified, run `make` and then run the test again. If you From b719883081471cc1c373e8b95af3c8571fe9bbab Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 12 Feb 2024 09:01:30 -0300 Subject: [PATCH 03/18] Merge bitcoin/bitcoin#29399: test: Fix utxo set hash serialisation signedness fa0ceae970242d8d6bdef150c98f04c67b06e20c test: Fix utxo set hash serialisation signedness (MarcoFalke) Pull request description: It is unsigned in Bitcoin Core, so the tests should match it: https://github.com/bitcoin/bitcoin/blob/5b8990a1f3c49b0b02b7383c69e95320acbda13e/src/kernel/coinstats.cpp#L54 Large positive values for the block height are too difficult to hit in tests, but it still seems fine to fix this. The bug was introduced when the code was written in 6ccc8fc067bf516cda7bc5d7d721945be5ac2003. (Lowercase `i` means signed, see https://docs.python.org/3/library/struct.html#format-characters) ACKs for top commit: epiccurious: Tested ACK fa0ceae970242d8d6bdef150c98f04c67b06e20c. fjahr: utACK fa0ceae970242d8d6bdef150c98f04c67b06e20c Tree-SHA512: ab4405c74fb191fff8520b456d3a800cd084d616bb9ddca27d56b8e5c8969bd537490f6e204c1870dbb09a3e130b03b22a27b6644252a024059c200bbd9004e7 --- test/functional/feature_utxo_set_hash.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/functional/feature_utxo_set_hash.py b/test/functional/feature_utxo_set_hash.py index 5f36f5dc9817..450e55ed348d 100755 --- a/test/functional/feature_utxo_set_hash.py +++ b/test/functional/feature_utxo_set_hash.py @@ -4,8 +4,6 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test UTXO set hash value calculation in gettxoutsetinfo.""" -import struct - from test_framework.messages import ( CBlock, COutPoint, @@ -58,7 +56,7 @@ def test_muhash_implementation(self): continue data = COutPoint(int(tx.rehash(), 16), n).serialize() - data += struct.pack(" Date: Tue, 13 Feb 2024 09:40:22 -0300 Subject: [PATCH 04/18] Merge bitcoin/bitcoin#29425: test: fix intermittent failure in wallet_reorgrestore.py 44d11532f80705b790bc6e28df9a96ac54b25f9b test: fix intermittent failure in wallet_reorgrestore.py (Martin Zumsande) Pull request description: By adding a missing `sync_blocks` call. There was a race at `node2` between connecting the block produced by `node0`, and using `-generate` to create new blocks itself. In the failed run, block generation started before connecting the block, resulting in a final block height that was smaller by 1 than expected. See https://github.com/bitcoin/bitcoin/issues/29392#issuecomment-1939541603 for a more detailed analysis of the failed run. Can be reproduced by adding a sleep to [this spot](https://github.com/bitcoin/bitcoin/blob/6ff0aa089c01ff3e610ecb47814ed739d685a14c/src/validation.cpp#L4217) in `ChainstateManager::ProcessNewBlock()`: ``` if (util::ThreadGetInternalName() == "msghand") { std::this_thread::sleep_for(0.2s); } ``` which fails for me on master and succeeds with the fix. Fixes #29392 ACKs for top commit: maflcko: lgtm ACK 44d11532f80705b790bc6e28df9a96ac54b25f9b Tree-SHA512: c08699e5ae348d4c0626022b519449d052f511d3f44601bcd8dac836a130a3f67fca149532e1e3690367ebfdcbcdd32e527170d039209c1f599ce861136ae29f --- test/functional/wallet_reorgsrestore.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/functional/wallet_reorgsrestore.py b/test/functional/wallet_reorgsrestore.py index 9a86ede5f94f..86c8f9bca833 100755 --- a/test/functional/wallet_reorgsrestore.py +++ b/test/functional/wallet_reorgsrestore.py @@ -44,6 +44,7 @@ def run_test(self): txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10")) tx = self.nodes[0].gettransaction(txid) self.generate(self.nodes[0], 4, sync_fun=self.no_op) + self.sync_blocks([self.nodes[0], self.nodes[2]]) tx_before_reorg = self.nodes[0].gettransaction(txid) assert_equal(tx_before_reorg["confirmations"], 4) From 9963e6bc2861958a4ff5072f78b660eae55d1dec Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 13 Feb 2024 11:46:56 -0300 Subject: [PATCH 05/18] Merge bitcoin/bitcoin#29413: fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse` 864e2e9097de8f1fda63137f803687dd5cc96c03 fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse` (brunoerg) Pull request description: The string `s` represents the value from `-whitelist`/`-whitebind` (e.g. "bloom,forcerelay,noban@1.2.3.4:32") and it is used in `NetWhitelistPermissions::TryParse` and `NetWhitebindPermissions::TryParse`. However, a max length of 32 is not enough to cover a lot of cases. Even disconsidering the permissions, 32 would not be enough to cover a lot of addresses. This PR fixes it. ACKs for top commit: maflcko: lgtm ACK 864e2e9097de8f1fda63137f803687dd5cc96c03 epiccurious: utACK 864e2e9097de8f1fda63137f803687dd5cc96c03. vasild: ACK 864e2e9097de8f1fda63137f803687dd5cc96c03 Tree-SHA512: 2b89031b9f2ea92d636f05fd167b1e5ac726742a7e7c1af8ddaeaf90236e659731aaa6b7c23f65ec16ce52ac1b9e68e7b16e23c59e355312d057e001976d172a --- src/test/fuzz/net_permissions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/net_permissions.cpp b/src/test/fuzz/net_permissions.cpp index 6ea79464d019..1e078b93854a 100644 --- a/src/test/fuzz/net_permissions.cpp +++ b/src/test/fuzz/net_permissions.cpp @@ -16,7 +16,7 @@ FUZZ_TARGET(net_permissions) { FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); - const std::string s = fuzzed_data_provider.ConsumeRandomLengthString(32); + const std::string s = fuzzed_data_provider.ConsumeRandomLengthString(1000); const NetPermissionFlags net_permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); NetWhitebindPermissions net_whitebind_permissions; From 9b6a05df66cc4662b7fd5efd4b0d4eb13e8c7e72 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 26 Feb 2024 11:30:43 +0000 Subject: [PATCH 06/18] Merge bitcoin/bitcoin#29443: depends: fix BDB compilation on OpenBSD 0fbf051fec723f86f49ab14ea15c91bb1435c656 depends: fix BDB compilation on OpenBSD (Sebastian Falbesoner) Pull request description: Compiling C++ code with `-D_XOPEN_SOURCE=600` causes problems on OpenBSD. If that define is set, the C++ standard header detection routine in BDB's configure script fails due to a missing type name for `locale_t` (see https://gist.github.com/theStack/b41884e31ebc5cdca3220bcaa674cb70 for the relevant config.log part). This results in `HAVE_CXX_STDHEADERS` not being defined, which then it turn leads to the inclusion of `` (rather than ``), which doesn't exist, as described in #28963. According to a mailing list post discussing a similar problem [1], "OpenBSD provides the POSIX APIs by default", so we don't need this define anyway and can remove it. This fixes the BDB build problem as described in issue #28963. See also https://github.com/google/flatbuffers/pull/6205/commits/f87e75ae71a2301daf2ad59b180977fc30c6abe4 for a similar fix for google's flatbuffer project. Tested on OpenBSD 7.4 with clang 13.0.0. Fixes #28963. [1] https://www.mail-archive.com/tech@openbsd.org/msg63386.html ACKs for top commit: fanquake: ACK 0fbf051fec723f86f49ab14ea15c91bb1435c656 Tree-SHA512: 02139e9081ed855e067bfba8c81b54c657417576e553cc1035a916ada9be049358f5e14d756d5f234c5226bd7e943f61c6ae8990c1b152f9125681b7b777c9b3 --- depends/packages/bdb.mk | 1 - 1 file changed, 1 deletion(-) diff --git a/depends/packages/bdb.mk b/depends/packages/bdb.mk index 6a8de3922113..b57858376418 100644 --- a/depends/packages/bdb.mk +++ b/depends/packages/bdb.mk @@ -13,7 +13,6 @@ $(package)_cflags+=-Wno-error=implicit-function-declaration -Wno-error=format-se $(package)_cxxflags+=-std=c++17 $(package)_cppflags_freebsd=-D_XOPEN_SOURCE=600 -D__BSD_VISIBLE=1 $(package)_cppflags_netbsd=-D_XOPEN_SOURCE=600 -$(package)_cppflags_openbsd=-D_XOPEN_SOURCE=600 $(package)_cppflags_mingw32=-DUNICODE -D_UNICODE endef From 92bad90e6cb8960014de1f92c60df5c3e91c0abe Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 27 Feb 2024 09:03:11 +0000 Subject: [PATCH 07/18] Merge bitcoin/bitcoin#28178: fuzz: Generate with random libFuzzer settings fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde fuzz: Set -rss_limit_mb=8000 for generate as well (MarcoFalke) fa4e396e1da8e5b04a5f906b95017b969ea37bae fuzz: Generate with random libFuzzer settings (MarcoFalke) Pull request description: Sometimes a libFuzzer setting like `-use_value_profile=1` helps [0], sometimes it hurts [1]. [0] https://github.com/bitcoin/bitcoin/pull/20789#issuecomment-752961937 [1] https://github.com/bitcoin/bitcoin/pull/27888#issuecomment-1645976254 By picking a random value, it is ensured that at least some of the runs will have the beneficial configuration set. Also, set `-max_total_time` to prevent slow fuzz targets from getting a larger time share, or possibly peg to a single core for a long time and block the python script from exiting for a long time. This can be improved in the future. For example, the python script can exit after some time (https://github.com/bitcoin/bitcoin/pull/20752#discussion_r549248791). Alternatively, it can measure if coverage progress was made and run for less time if no progress has been made recently anyway, so that more time can be spent on targets that are new or still make progress. ACKs for top commit: murchandamus: utACK fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde dergoegge: utACK fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde brunoerg: light ACK fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde Tree-SHA512: bfd04a76ca09aec612397bae5f3f263a608faa7087697169bd4c506c8195c4d2dd84ddc7fcd3ebbc75771eab618fad840af819114968ca3668fc730092376768 --- test/fuzz/test_runner.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index 581fb36c53c4..33ea7b583216 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -10,6 +10,7 @@ import configparser import logging import os +import random import subprocess import sys @@ -207,9 +208,13 @@ def job(command, t): for target in targets: target_corpus_dir = os.path.join(corpus_dir, target) os.makedirs(target_corpus_dir, exist_ok=True) + use_value_profile = int(random.random() < .3) command = [ os.path.join(build_dir, 'src', 'test', 'fuzz', 'fuzz'), - "-runs=100000", + "-rss_limit_mb=8000", + "-max_total_time=6000", + "-reload=0", + f"-use_value_profile={use_value_profile}", target_corpus_dir, ] futures.append(fuzz_pool.submit(job, command, target)) From a23b342d7d052b611160bf0d5fe4189983aa60cb Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 27 Feb 2024 22:29:37 +0000 Subject: [PATCH 08/18] Merge bitcoin/bitcoin#29475: doc: Fix Broken Links MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 6fa61e35320ac2bc623a9c9ca11b270b34e2d05a doc: Fix Broken Links (Justin Dhillon) Pull request description: ### Summery Here is what I have fixed: http://voorloopnul.com/blog/a-python-netstat-in-less-than-100-lines-of-code/ --> https://web.archive.org/web/20190424172231/http://voorloopnul.com/blog/a-python-netstat-in-less-than-100-lines-of-code/ ### Support my work These links were found with [link-inspector](https://github.com/justindhillon/link-inspector). If you find this PR useful, give the repo a ⭐ ACKs for top commit: fjahr: ACK 6fa61e35320ac2bc623a9c9ca11b270b34e2d05a Tree-SHA512: ba83badfc8a89f33813801f749bcd7ad41d4c9c817ece76f3bb1b60f24c28e99cfccc485a0ba059ec2c1134e8ffb5fa37fdc9835e553229ee5b1167c9b2e8d1f --- test/functional/test_framework/netutil.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/test_framework/netutil.py b/test/functional/test_framework/netutil.py index 21145f90e990..8431b9020ee4 100644 --- a/test/functional/test_framework/netutil.py +++ b/test/functional/test_framework/netutil.py @@ -4,7 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Linux network utilities. -Roughly based on http://voorloopnul.com/blog/a-python-netstat-in-less-than-100-lines-of-code/ by Ricardo Pascal +Roughly based on https://web.archive.org/web/20190424172231/http://voorloopnul.com/blog/a-python-netstat-in-less-than-100-lines-of-code/ by Ricardo Pascal """ import sys From fdac2b3286ebd9134c417613466ec2e5eac949ca Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 28 Feb 2024 22:06:20 +0000 Subject: [PATCH 09/18] Merge bitcoin/bitcoin#29493: subtree: update crc32c subtree 5d45552fd4303f8d668ffbc50cce1053485aeead Squashed 'src/crc32c/' changes from 0bac72c455..b60d2b7334 (fanquake) Pull request description: Update the crc32c subtree. Includes: * https://github.com/bitcoin-core/crc32c-subtree/pull/6 Which fixes #29178. ACKs for top commit: hebasto: ACK 359a8d98468aa4f00be349ccbfc869d797ee807d. theuni: ACK 359a8d98468aa4f00be349ccbfc869d797ee807d dergoegge: ACK 359a8d98468aa4f00be349ccbfc869d797ee807d Tree-SHA512: 2cec81a34ad26bbbc298aea5daffa41e56114d31cc2eb5fe486f46a77c3467bba22bdeca1c52ae97220e119d98818304272fc6337442af55282accabcd4c5833 --- src/crc32c/src/crc32c_arm64.cc | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/crc32c/src/crc32c_arm64.cc b/src/crc32c/src/crc32c_arm64.cc index 1da04ed34a3c..711616cd2f53 100644 --- a/src/crc32c/src/crc32c_arm64.cc +++ b/src/crc32c/src/crc32c_arm64.cc @@ -12,6 +12,7 @@ #include #include +#include #include "./crc32c_internal.h" #ifdef CRC32C_HAVE_CONFIG_H @@ -29,14 +30,14 @@ // compute 8bytes for each segment parallelly #define CRC32C32BYTES(P, IND) \ do { \ - crc1 = __crc32cd( \ - crc1, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 1 + (IND))); \ - crc2 = __crc32cd( \ - crc2, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 2 + (IND))); \ - crc3 = __crc32cd( \ - crc3, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 3 + (IND))); \ - crc0 = __crc32cd( \ - crc0, *((const uint64_t *)(P) + (SEGMENTBYTES / 8) * 0 + (IND))); \ + std::memcpy(&d64, (P) + SEGMENTBYTES * 1 + (IND) * 8, sizeof(d64)); \ + crc1 = __crc32cd(crc1, d64); \ + std::memcpy(&d64, (P) + SEGMENTBYTES * 2 + (IND) * 8, sizeof(d64)); \ + crc2 = __crc32cd(crc2, d64); \ + std::memcpy(&d64, (P) + SEGMENTBYTES * 3 + (IND) * 8, sizeof(d64)); \ + crc3 = __crc32cd(crc3, d64); \ + std::memcpy(&d64, (P) + SEGMENTBYTES * 0 + (IND) * 8, sizeof(d64)); \ + crc0 = __crc32cd(crc0, d64); \ } while (0); // compute 8*8 bytes for each segment parallelly @@ -68,6 +69,9 @@ uint32_t ExtendArm64(uint32_t crc, const uint8_t *data, size_t size) { int64_t length = size; uint32_t crc0, crc1, crc2, crc3; uint64_t t0, t1, t2; + uint16_t d16; + uint32_t d32; + uint64_t d64; // k0=CRC(x^(3*SEGMENTBYTES*8)), k1=CRC(x^(2*SEGMENTBYTES*8)), // k2=CRC(x^(SEGMENTBYTES*8)) @@ -88,7 +92,8 @@ uint32_t ExtendArm64(uint32_t crc, const uint8_t *data, size_t size) { t2 = (uint64_t)vmull_p64(crc2, k2); t1 = (uint64_t)vmull_p64(crc1, k1); t0 = (uint64_t)vmull_p64(crc0, k0); - crc = __crc32cd(crc3, *(uint64_t *)data); + std::memcpy(&d64, data, sizeof(d64)); + crc = __crc32cd(crc3, d64); data += sizeof(uint64_t); crc ^= __crc32cd(0, t2); crc ^= __crc32cd(0, t1); @@ -98,18 +103,21 @@ uint32_t ExtendArm64(uint32_t crc, const uint8_t *data, size_t size) { } while (length >= 8) { - crc = __crc32cd(crc, *(uint64_t *)data); + std::memcpy(&d64, data, sizeof(d64)); + crc = __crc32cd(crc, d64); data += 8; length -= 8; } if (length & 4) { - crc = __crc32cw(crc, *(uint32_t *)data); + std::memcpy(&d32, data, sizeof(d32)); + crc = __crc32cw(crc, d32); data += 4; } if (length & 2) { - crc = __crc32ch(crc, *(uint16_t *)data); + std::memcpy(&d16, data, sizeof(d16)); + crc = __crc32ch(crc, d16); data += 2; } From 910a7d6cbf8701be1cb87096dbe36646a6c5056c Mon Sep 17 00:00:00 2001 From: fanquake Date: Wed, 6 Mar 2024 12:05:47 +0000 Subject: [PATCH 10/18] Merge bitcoin/bitcoin#29529: fuzz: restrict fopencookie usage to Linux & FreeBSD 312f3381a2d3a7afb7c81b4662214d4d67b4e84a fuzz: restrict fopencookie usage to Linux & FreeBSD (fanquake) Pull request description: Should fix the GCC compilation portion of https://github.com/bitcoin/bitcoin/issues/29517#issuecomment-1973573314. See also: https://www.gnu.org/software/gnulib/manual/html_node/fopencookie.html. ACKs for top commit: m3dwards: ACK https://github.com/bitcoin/bitcoin/pull/29529/commits/312f3381a2d3a7afb7c81b4662214d4d67b4e84a TheCharlatan: utACK 312f3381a2d3a7afb7c81b4662214d4d67b4e84a Tree-SHA512: aa8ff20c3fa735415d05a93b8855877035c300f4d2dfd82f65fd9ed5b5c96ab619b4d84eef114ed0013dc5ff0800cb628ed3801e1efde0cfb0d426930d1673d5 --- src/test/fuzz/util.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 3cfa9467fb9f..fb2f9c05fe9b 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -438,7 +438,7 @@ FILE* FuzzedFileProvider::open() [&] { mode = "a+"; }); -#if defined _GNU_SOURCE && !defined __ANDROID__ +#if defined _GNU_SOURCE && (defined(__linux__) || defined(__FreeBSD__)) const cookie_io_functions_t io_hooks = { FuzzedFileProvider::read, FuzzedFileProvider::write, From 4dce690a5edeb3025028d15afa3248d9ac3bc87a Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 7 Mar 2024 09:42:22 +0000 Subject: [PATCH 11/18] Merge bitcoin/bitcoin#29576: Update functional test runner to return error code when no tests are found to run 33268a855883142a039a7a7b14eb1345e52809fd test: exit with code 1 when no fn tests are found (Max Edwards) Pull request description: As discussed in the following PR comment: https://github.com/bitcoin/bitcoin/pull/29535#issuecomment-1979259786 Prevents the test_runner from exiting silently with code 0 when no tests were found which has recently happened after a GHA runner update such as in this run: https://github.com/bitcoin/bitcoin/actions/runs/8131828989/job/22239779585#step:27:63 ACKs for top commit: TheCharlatan: ACK 33268a855883142a039a7a7b14eb1345e52809fd theStack: lgtm ACK 33268a855883142a039a7a7b14eb1345e52809fd Tree-SHA512: d389e9f5e4da7ce1627fb2fad9b33baf0b04e75dbdbfc0dbf5f1e3e2b0ae1e79721476c5668476055b0f7de29723ed02c8d7e420081a555030cb784886e240fc --- test/functional/test_runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 42e54fc5783a..13fee193f147 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -453,7 +453,7 @@ def main(): if not enable_bitcoind: print("No functional tests to run.") print("Rerun ./configure with --with-daemon and then make") - sys.exit(0) + sys.exit(1) # Build list of tests test_list = [] @@ -502,7 +502,7 @@ def main(): if not test_list: print("No valid test scripts specified. Check that your test is in one " "of the test lists in test_runner.py, or run test_runner.py with no arguments to run all tests") - sys.exit(0) + sys.exit(1) if args.help: # Print help for test_runner.py, then print help of the first script (with args removed) and exit. From 8d6e5e7d67453e167f75b6422ced945a94e1bffe Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 8 Mar 2024 10:13:09 +0000 Subject: [PATCH 12/18] Merge bitcoin/bitcoin#29583: fuzz: Apply fuzz env (suppressions, etc.) when fetching harness list 738a53720e7df70a23709f7a26e4467bbe36db9c [fuzz] Apply fuzz env (suppressions, etc.) when fetching harness list (dergoegge) Pull request description: The fuzz test runner does not add the UBSan suppressions when fetching the harness list. We can observe this in CI as lots of UBSan errors prior to the harnesses actually executing: https://api.cirrus-ci.com/v1/task/5678606140047360/logs/ci.log ``` + test/fuzz/test_runner.py -j10 -l DEBUG /ci_container_base/ci/scratch/qa-assets/fuzz_seed_corpus/ --empty_min_time=60 /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:38: runtime error: unsigned integer overflow: 12 - 23 cannot be represented in type 'size_type' (aka 'unsigned long') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:38 in /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:33: runtime error: implicit conversion from type 'size_type' (aka 'unsigned long') of value 18446744073709551605 (64-bit, unsigned) to type 'const difference_type' (aka 'const long') changed the value to -11 (64-bit, signed) SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /usr/bin/../lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/string_view:578:33 in crypto/sha256.cpp:75:57: runtime error: left shift of 1359893119 by 26 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:57 in crypto/sha256.cpp:75:79: runtime error: left shift of 1359893119 by 21 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:79 in crypto/sha256.cpp:75:101: runtime error: left shift of 1359893119 by 7 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:75:101 in crypto/sha256.cpp:82:47: runtime error: unsigned integer overflow: 2968370640 + 2483695512 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:47 in crypto/sha256.cpp:74:57: runtime error: left shift of 1779033703 by 30 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:57 in crypto/sha256.cpp:74:79: runtime error: left shift of 1779033703 by 19 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:79 in crypto/sha256.cpp:74:101: runtime error: left shift of 1779033703 by 10 places cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:74:101 in crypto/sha256.cpp:83:29: runtime error: unsigned integer overflow: 3458249854 + 980412007 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:83:29 in crypto/sha256.cpp:82:21: runtime error: unsigned integer overflow: 528734635 + 4228187651 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:21 in crypto/sha256.cpp:84:7: runtime error: unsigned integer overflow: 1013904242 + 3720769133 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:84:7 in crypto/sha256.cpp:85:12: runtime error: unsigned integer overflow: 3720769133 + 2654153126 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:85:12 in crypto/sha256.cpp:82:33: runtime error: unsigned integer overflow: 4165002546 + 1259303586 cannot be represented in type 'uint32_t' (aka 'unsigned int') SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:82:33 in crypto/sha256.cpp:125:50: runtime error: unsigned integer overflow: 3835390401 + 1367343104 cannot be represented in type 'unsigned int' SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior crypto/sha256.cpp:125:50 in crypto/sha256.cpp:77:58: runtime error: left shift of 1367343104 by 15 places cannot be represented in type 'uint32_t' (aka 'unsigned int') ... ``` To fix this we simply apply the usual fuzz env variables (that apply the suppressions) when fetching the harness list as well. ACKs for top commit: ismaelsadeeq: Tested ACK 738a53720e7df70a23709f7a26e4467bbe36db9c fanquake: ACK 738a53720e7df70a23709f7a26e4467bbe36db9c Tree-SHA512: befebaeb4ee5f2eddca67fc6dc69e997c6a250ea54844e5e6e93d1f6a13be49364a3ace31eaa942b02dcf73612af29ec4ace86c9eb7567b92f6f5dc3ea14dc11 --- test/fuzz/test_runner.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/fuzz/test_runner.py b/test/fuzz/test_runner.py index 33ea7b583216..9b8970045523 100755 --- a/test/fuzz/test_runner.py +++ b/test/fuzz/test_runner.py @@ -94,7 +94,10 @@ def main(): sys.exit(1) # Build list of tests - test_list_all = parse_test_list(fuzz_bin=os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz')) + test_list_all = parse_test_list( + fuzz_bin=os.path.join(config["environment"]["BUILDDIR"], 'src', 'test', 'fuzz', 'fuzz'), + source_dir=config['environment']['SRCDIR'], + ) if not test_list_all: logging.error("No fuzz targets found") @@ -303,11 +306,12 @@ def job(t, args): sys.exit(1) -def parse_test_list(*, fuzz_bin): +def parse_test_list(*, fuzz_bin, source_dir): test_list_all = subprocess.run( fuzz_bin, env={ - 'PRINT_ALL_FUZZ_TARGETS_AND_ABORT': '' + 'PRINT_ALL_FUZZ_TARGETS_AND_ABORT': '', + **get_fuzz_env(target="", source_dir=source_dir) }, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, From c9617558e3d0f798ff0b9607b5ea2dc80d88c972 Mon Sep 17 00:00:00 2001 From: fanquake Date: Fri, 8 Mar 2024 10:15:19 +0000 Subject: [PATCH 13/18] Merge bitcoin/bitcoin#29595: doc: Wrap flags with code in developer-notes.md 4f1753deaa2481839a0cc4c690d703b3dfebcb5f doc: Wrap flags with code in developer-notes.md (spicyzboss) Pull request description: Before wrap code block image After wrap code block image ACKs for top commit: fanquake: ACK 4f1753deaa2481839a0cc4c690d703b3dfebcb5f Tree-SHA512: 15a123f3a13c62d9cfdf62d5df351b8c2b3f9f666fba5a2722325600d802a29da61773ad32fb9f8483a915f59dd42731ba724b81b7874d39cb9627f0266b5713 --- doc/developer-notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 5d6fd6d420e8..36c0112534fa 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -326,7 +326,7 @@ produce better debugging builds. ### Show sources in debugging If you have ccache enabled, absolute paths are stripped from debug information -with the -fdebug-prefix-map and -fmacro-prefix-map options (if supported by the +with the `-fdebug-prefix-map` and `-fmacro-prefix-map` options (if supported by the compiler). This might break source file detection in case you move binaries after compilation, debug from the directory other than the project root or use an IDE that only supports absolute paths for debugging. From bd607f049dd5621a6bb9514ebe0711fef8caa0ac Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Fri, 8 Mar 2024 20:59:26 -0500 Subject: [PATCH 14/18] Merge bitcoin/bitcoin#29393: i2p: log connection was refused due to arbitrary port 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 i2p: log connection was refused due to arbitrary port (brunoerg) Pull request description: For I2P, we do not try to connect if port is != 0. However, we do not have anything that indicates it or any error when trying to connect with port != 0. This PR adds a log for it. Also, it improves the functional test. With this log we can ensure the reason we won't connect is the port, in the current test, we cannot ensure it. ACKs for top commit: jonatack: ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 epiccurious: re-ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796. achow101: ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 kristapsk: re-ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 vasild: ACK 5b358cdd1a5f5d2fe87a9e41c638996eab2e2796 Tree-SHA512: 027245afa771c9295fff0bfd17c251dca4a9f4c739e5773922de3c030a65ef05d96291edcbdeeaa50ba3add61f75f28d8c00be503e03fc33d3491d1956fc549f --- src/i2p.cpp | 1 + test/functional/p2p_i2p_ports.py | 18 +++++------------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/i2p.cpp b/src/i2p.cpp index 1175366eb5d3..6713fdcea890 100644 --- a/src/i2p.cpp +++ b/src/i2p.cpp @@ -186,6 +186,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error) // Refuse connecting to arbitrary ports. We don't specify any destination port to the SAM proxy // when connecting (SAM 3.1 does not use ports) and it forces/defaults it to I2P_SAM31_PORT. if (to.GetPort() != I2P_SAM31_PORT) { + Log("Error connecting to %s, connection refused due to arbitrary port %s", to.ToStringAddrPort(), to.GetPort()); proxy_error = false; return false; } diff --git a/test/functional/p2p_i2p_ports.py b/test/functional/p2p_i2p_ports.py index 13188b930577..20dcb50a5749 100755 --- a/test/functional/p2p_i2p_ports.py +++ b/test/functional/p2p_i2p_ports.py @@ -6,36 +6,28 @@ Test ports handling for I2P hosts """ -import re from test_framework.test_framework import BitcoinTestFramework +PROXY = "127.0.0.1:60000" class I2PPorts(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 # The test assumes that an I2P SAM proxy is not listening here. - self.extra_args = [["-i2psam=127.0.0.1:60000"]] + self.extra_args = [[f"-i2psam={PROXY}"]] def run_test(self): node = self.nodes[0] self.log.info("Ensure we don't try to connect if port!=0") addr = "zsxwyo6qcn3chqzwxnseusqgsnuw3maqnztkiypyfxtya4snkoka.b32.i2p:8333" - raised = False - try: - with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}"]): - node.addnode(node=addr, command="onetry") - except AssertionError as e: - raised = True - if not re.search(r"Expected messages .* does not partially match log", str(e)): - raise AssertionError(f"Assertion raised as expected, but with an unexpected message: {str(e)}") - if not raised: - raise AssertionError("Assertion should have been raised") + with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}, connection refused due to arbitrary port 8333"]): + node.addnode(node=addr, command="onetry") self.log.info("Ensure we try to connect if port=0 and get an error due to missing I2P proxy") addr = "h3r6bkn46qxftwja53pxiykntegfyfjqtnzbm6iv6r5mungmqgmq.b32.i2p:0" - with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}"]): + with node.assert_debug_log(expected_msgs=[f"Error connecting to {addr}: Cannot connect to {PROXY}"]): node.addnode(node=addr, command="onetry") From 045fa5f57eb51209d565c7679a73e3cf974ed27d Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 11 Mar 2024 07:52:02 -0400 Subject: [PATCH 15/18] Merge bitcoin/bitcoin#29514: tests: Provide more helpful assert_equal errors a3badf75f6fd88d465e59f46f0336a0c1eacb7de tests: Provide more helpful assert_equal errors (Anthony Towns) Pull request description: In the functional tests, we often compare dicts with assert_equal, but the output makes it very hard to tell exactly which entry in the dicts don't match when there are a lot of entries and only minor differences. Change the output to make it clearer. ACKs for top commit: achow101: ACK a3badf75f6fd88d465e59f46f0336a0c1eacb7de vasild: ACK a3badf75f6fd88d465e59f46f0336a0c1eacb7de brunoerg: utACK a3badf75f6fd88d465e59f46f0336a0c1eacb7de josibake: ACK https://github.com/bitcoin/bitcoin/pull/29514/commits/a3badf75f6fd88d465e59f46f0336a0c1eacb7de BrandonOdiwuor: Code Review ACK a3badf75f6fd88d465e59f46f0336a0c1eacb7de Tree-SHA512: 1d4b4a3b2e2e28ab09f10b41b04b52b37f64e0d8a54e2306f37de0c3eb3299a7ad4ba225b9efa67057a75e90d008a17385c810a32c9b212d240be280c2dcf2e5 --- test/functional/test_framework/util.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py index 010193ac4303..9b427cf40b5d 100644 --- a/test/functional/test_framework/util.py +++ b/test/functional/test_framework/util.py @@ -46,7 +46,24 @@ def assert_fee_amount(fee, tx_size, fee_per_kB): raise AssertionError("Fee of %s DASH too high! (Should be %s DASH)" % (str(fee), str(target_fee))) +def summarise_dict_differences(thing1, thing2): + if not isinstance(thing1, dict) or not isinstance(thing2, dict): + return thing1, thing2 + d1, d2 = {}, {} + for k in sorted(thing1.keys()): + if k not in thing2: + d1[k] = thing1[k] + elif thing1[k] != thing2[k]: + d1[k], d2[k] = summarise_dict_differences(thing1[k], thing2[k]) + for k in sorted(thing2.keys()): + if k not in thing1: + d2[k] = thing2[k] + return d1, d2 + def assert_equal(thing1, thing2, *args): + if thing1 != thing2 and not args and isinstance(thing1, dict) and isinstance(thing2, dict): + d1,d2 = summarise_dict_differences(thing1, thing2) + raise AssertionError("not(%s == %s)\n in particular not(%s == %s)" % (thing1, thing2, d1, d2)) if thing1 != thing2 or any(thing1 != arg for arg in args): raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)) From d0e15d59f8a449e0c3e47513776892713187c10f Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Wed, 13 Mar 2024 08:18:01 -0400 Subject: [PATCH 16/18] Merge bitcoin/bitcoin#29606: refactor: Reserve memory for ToLower/ToUpper conversions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 6f2f4a4d096a3b261258c8cdd96cca532988d1d3 Reserve memory for ToLower/ToUpper conversions (Lőrinc) Pull request description: Similarly to https://github.com/bitcoin/bitcoin/pull/29458, we're preallocating the result string based on the input string's length. The methods were already [covered by tests](https://github.com/bitcoin/bitcoin/blob/master/src/test/util_tests.cpp#L1250-L1276). ACKs for top commit: tdb3: ACK for 6f2f4a4d096a3b261258c8cdd96cca532988d1d3 maflcko: lgtm ACK 6f2f4a4d096a3b261258c8cdd96cca532988d1d3 achow101: ACK 6f2f4a4d096a3b261258c8cdd96cca532988d1d3 Empact: Code Review ACK https://github.com/bitcoin/bitcoin/pull/29606/commits/6f2f4a4d096a3b261258c8cdd96cca532988d1d3 stickies-v: ACK 6f2f4a4d096a3b261258c8cdd96cca532988d1d3 Tree-SHA512: e3ba7af77decdc73272d804c94fef0b11028a85f3c0ea1ed6386672611b1c35fce151f02e64f5bb5acb5ba506aaa54577719b07925b9cc745143cf5c7e5eb262 --- src/util/strencodings.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 95050042fb3a..4fea3925fd0e 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -476,6 +476,7 @@ bool ParseFixedPoint(const std::string &val, int decimals, int64_t *amount_out) std::string ToLower(const std::string& str) { std::string r; + r.reserve(str.size()); for (auto ch : str) r += ToLower(ch); return r; } @@ -483,6 +484,7 @@ std::string ToLower(const std::string& str) std::string ToUpper(const std::string& str) { std::string r; + r.reserve(str.size()); for (auto ch : str) r += ToUpper(ch); return r; } From 6d7aa3d978b4f442a2f8568ea6e23859521b24bc Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 14 Mar 2024 09:55:10 +0000 Subject: [PATCH 17/18] Merge bitcoin/bitcoin#29497: test: simplify test_runner.py 0831b54dfca1b9e728295fff500215da14589fc0 test: simplify test_runner.py (tdb3) Pull request description: Implements the simplifications to test_runner.py proposed by sipa in PR #23995. Remove the num_running variable as it can be implied by the length of the jobs list. Remove the i variable as it can be implied by the length of the test_results list. Instead of counting results to determine if finished, make the queue object itself responsible (by looking at running jobs and jobs left). ACKs for top commit: mzumsande: re-ACK 0831b54 davidgumberg: reACK https://github.com/bitcoin/bitcoin/commit/0831b54dfca1b9e728295fff500215da14589fc0 marcofleon: re-ACK 0831b54dfca1b9e728295fff500215da14589fc0 Tree-SHA512: e5473e68d49cd779b29d97635329283ae7195412cb1e92461675715ca7eedb6519a1a93ba28d40ca6f015d270f7bcd3e77cef279d9cd655155ab7805b49638f1 --- test/functional/test_runner.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 13fee193f147..84b08c6f5596 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -592,14 +592,12 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, attempts=1, enab max_len_name = len(max(test_list, key=len)) test_count = len(test_list) all_passed = True - i = 0 - while i < test_count: + while not job_queue.done(): if failfast and not all_passed: break for test_result, testdir, stdout, stderr in job_queue.get_next(): test_results.append(test_result) - i += 1 - done_str = "{}/{} - {}{}{}".format(i, test_count, BOLD[1], test_result.name, BOLD[0]) + done_str = f"{len(test_results)}/{test_count} - {BOLD[1]}{test_result.name}{BOLD[0]}" if test_result.status == "Passed": logging.debug("%s passed, Duration: %s s" % (done_str, test_result.time)) elif test_result.status == "Skipped": @@ -684,15 +682,16 @@ def __init__(self, *, num_tests_parallel, tests_dir, tmpdir, test_list, flags, u self.tmpdir = tmpdir self.test_list = test_list self.flags = flags - self.num_running = 0 self.jobs = [] self.use_term_control = use_term_control self.attempts = attempts + def done(self): + return not (self.jobs or self.test_list) + def get_next(self): - while self.num_running < self.num_jobs and self.test_list: + while len(self.jobs) < self.num_jobs and self.test_list: # Add tests - self.num_running += 1 test = self.test_list.pop(0) portseed = len(self.test_list) portseed_arg = ["--portseed={}".format(portseed)] @@ -764,7 +763,6 @@ def get_next(self): continue else: status = "Failed" - self.num_running -= 1 self.jobs.remove(job) if self.use_term_control: clearline = '\r' + (' ' * dot_count) + '\r' From b70e091374ee1a5db4f8299e693eabd7df0a225e Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 19 Mar 2024 12:20:27 +0000 Subject: [PATCH 18/18] Merge bitcoin/bitcoin#29667: fuzz: actually test garbage >64b in p2p transport test 626f8e398e219b84907ccaad036f69177d39284c fuzz: actually test garbage >64b in p2p transport test (Pieter Wuille) Pull request description: This fixes an oversight from #28196: in the `p2p_transport_bidirectional_v2` fuzz test, when the desired garbage length is over 64 bytes, the code would actually use garbage length 0. Fix this. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/29667/commits/626f8e398e219b84907ccaad036f69177d39284c brunoerg: crACK 626f8e398e219b84907ccaad036f69177d39284c Tree-SHA512: f6346367adb10464b6c9d20aef43625531d2a4d8110887ad03214b8c1907b83560f2dd5b5415e2180a40b4cd276d51881b32b60c740471b5c6bb218aa19848d8 --- src/test/fuzz/p2p_transport_serialization.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/fuzz/p2p_transport_serialization.cpp b/src/test/fuzz/p2p_transport_serialization.cpp index 530ce8524d8d..f5f8486e5f3f 100644 --- a/src/test/fuzz/p2p_transport_serialization.cpp +++ b/src/test/fuzz/p2p_transport_serialization.cpp @@ -349,6 +349,7 @@ std::unique_ptr MakeV2Transport(NodeId nodeid, bool initiator, RNG& r } else { // If it's longer, generate it from the RNG. This avoids having large amounts of // (hopefully) irrelevant data needing to be stored in the fuzzer data. + garb.resize(garb_len); for (auto& v : garb) v = uint8_t(rng()); } // Retrieve entropy