backport: Merge bitcoin#27005, 28644, 26940, 27511, 28403, 28361, 28409

[vijaydasmp]

bitcoin backports

[github-actions]

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

[thepastaclaw]

✅ Review complete (commit 3ded43e)

[coderabbitai]

Walkthrough

This pull request primarily reorganizes test infrastructure and introduces a new RPC command. It moves random utility functions from setup_common.h to a dedicated test/util/random.h header and updates ~40 test files to include this header. A new test/util/coins.cpp utility file is created for generating test coins. The Timer implementation switches from default-initialized microseconds to optional steady_clock timestamps. A new hidden RPC command getaddrmaninfo is added to expose address manager statistics. The P2P test framework's sync method is simplified by consolidating sync_send_with_ping() into sync_with_ping(). Wallet functional tests have unlock timeout values increased from short durations to 999000 seconds. Minor improvements include fuzzing enhancements for public key construction and platform detection updates.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested reviewers

• UdjinM6

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 2.86% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description 'bitcoin backports' is minimal but directly related to the changeset, which exclusively contains backported changes from bitcoin/bitcoin.
Title check	✅ Passed	The PR title lists multiple Bitcoin PR numbers being backported, which is directly supported by the commit messages and file changes that implement these specific features (steady clock timer, test utilities reorganization, getaddrmaninfo RPC command, fuzz improvements, and sync helper consolidation).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

test/functional/p2p_ibd_stalling.py (1)

161-164: Consider renaming the helper for clarity.

The method now calls sync_with_ping(), so all_sync_send_with_ping is slightly misleading. A rename would reduce cognitive overhead in this test.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/functional/p2p_ibd_stalling.py` around lines 161 - 164, Rename the
helper function all_sync_send_with_ping to a clearer name that reflects it calls
each peer's sync_with_ping method (e.g., sync_all_with_ping or
peers_sync_with_ping); update the function definition (all_sync_send_with_ping)
and every call site to the new name, and ensure any tests or imports referencing
the old name are adjusted so behavior remains identical and no references remain
to all_sync_send_with_ping.

src/test/fuzz/util.cpp (1)

319-329: Guard against short byte_data spans in ConstructPubKeyBytes.

This helper assumes at least 33/65 bytes and does unchecked iterator arithmetic. Adding a precondition assert makes the function safer for future reuse.

♻️ Proposed hardening

 std::vector<uint8_t> ConstructPubKeyBytes(FuzzedDataProvider& fuzzed_data_provider, Span<const uint8_t> byte_data, const bool compressed) noexcept
 {
+    const size_t needed = compressed ? CPubKey::COMPRESSED_SIZE : CPubKey::SIZE;
+    assert(byte_data.size() >= needed);
     uint8_t pk_type;
     if (compressed) {
         pk_type = fuzzed_data_provider.PickValueInArray({0x02, 0x03});
     } else {
         pk_type = fuzzed_data_provider.PickValueInArray({0x04, 0x06, 0x07});
     }
-    std::vector<uint8_t> pk_data{byte_data.begin(), byte_data.begin() + (compressed ? CPubKey::COMPRESSED_SIZE : CPubKey::SIZE)};
+    std::vector<uint8_t> pk_data{byte_data.begin(), byte_data.begin() + needed};
     pk_data[0] = pk_type;
     return pk_data;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/fuzz/util.cpp` around lines 319 - 329, ConstructPubKeyBytes assumes
byte_data has at least CPubKey::COMPRESSED_SIZE or CPubKey::SIZE bytes; add a
guard at the start of ConstructPubKeyBytes that checks byte_data.size() >=
(compressed ? CPubKey::COMPRESSED_SIZE : CPubKey::SIZE) and assert or return an
empty/std::vector<uint8_t>() if the precondition fails, then proceed to
PickValueInArray and slice using the validated size (referencing
ConstructPubKeyBytes, fuzzed_data_provider, CPubKey::COMPRESSED_SIZE,
CPubKey::SIZE).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/get_previous_releases.py`:
- Around line 267-270: The platforms mapping added an entry for
'powerpc64le-*-linux-*' -> 'powerpc64le-linux-gnu' but there are no
corresponding SHA256_SUMS entries and download_binary() rejects tarballs without
checksum entries; either revert/remove the 'powerpc64le-*-linux-*' entry from
the platforms dict (so host detection keeps it unmapped) or add the official
Dash release assets and matching checksum lines for 'powerpc64le-linux-gnu' into
the SHA256_SUMS data used by download_binary() and ensure the checksum keys
match exactly the platform string; update the SHA256_SUMS source and test
download_binary() to accept those checksums if you choose to add support.

---

Nitpick comments:
In `@src/test/fuzz/util.cpp`:
- Around line 319-329: ConstructPubKeyBytes assumes byte_data has at least
CPubKey::COMPRESSED_SIZE or CPubKey::SIZE bytes; add a guard at the start of
ConstructPubKeyBytes that checks byte_data.size() >= (compressed ?
CPubKey::COMPRESSED_SIZE : CPubKey::SIZE) and assert or return an
empty/std::vector<uint8_t>() if the precondition fails, then proceed to
PickValueInArray and slice using the validated size (referencing
ConstructPubKeyBytes, fuzzed_data_provider, CPubKey::COMPRESSED_SIZE,
CPubKey::SIZE).

In `@test/functional/p2p_ibd_stalling.py`:
- Around line 161-164: Rename the helper function all_sync_send_with_ping to a
clearer name that reflects it calls each peer's sync_with_ping method (e.g.,
sync_all_with_ping or peers_sync_with_ping); update the function definition
(all_sync_send_with_ping) and every call site to the new name, and ensure any
tests or imports referencing the old name are adjusted so behavior remains
identical and no references remain to all_sync_send_with_ping.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 600d104e-11b6-4612-9e78-a9f8b7ab2e16

📥 Commits

Reviewing files that changed from the base of the PR and between 56bceb4 and a040529.

📒 Files selected for processing (62)

• src/Makefile.test_util.include
• src/logging/timer.h
• src/rpc/net.cpp
• src/test/base58_tests.cpp
• src/test/bip324_tests.cpp
• src/test/blockencodings_tests.cpp
• src/test/bloom_tests.cpp
• src/test/checkqueue_tests.cpp
• src/test/coins_tests.cpp
• src/test/crypto_tests.cpp
• src/test/cuckoocache_tests.cpp
• src/test/dbwrapper_tests.cpp
• src/test/fuzz/rpc.cpp
• src/test/fuzz/util.cpp
• src/test/hash_tests.cpp
• src/test/key_tests.cpp
• src/test/logging_tests.cpp
• src/test/merkle_tests.cpp
• src/test/miner_tests.cpp
• src/test/minisketch_tests.cpp
• src/test/net_tests.cpp
• src/test/orphanage_tests.cpp
• src/test/pmt_tests.cpp
• src/test/pool_tests.cpp
• src/test/pow_tests.cpp
• src/test/prevector_tests.cpp
• src/test/script_tests.cpp
• src/test/serfloat_tests.cpp
• src/test/sighash_tests.cpp
• src/test/skiplist_tests.cpp
• src/test/streams_tests.cpp
• src/test/transaction_tests.cpp
• src/test/txpackage_tests.cpp
• src/test/util/blockfilter.cpp
• src/test/util/coins.cpp
• src/test/util/coins.h
• src/test/util/llmq_tests.h
• src/test/util/random.h
• src/test/util/setup_common.h
• src/test/util_tests.cpp
• src/test/validation_block_tests.cpp
• src/test/validation_chainstate_tests.cpp
• src/test/validation_chainstatemanager_tests.cpp
• src/test/validation_flush_tests.cpp
• src/test/versionbits_tests.cpp
• src/wallet/test/wallet_crypto_tests.cpp
• test/functional/p2p_addrfetch.py
• test/functional/p2p_blocksonly.py
• test/functional/p2p_compactblocks_blocksonly.py
• test/functional/p2p_filter.py
• test/functional/p2p_ibd_stalling.py
• test/functional/p2p_ibd_txrelay.py
• test/functional/rpc_net.py
• test/functional/test_framework/p2p.py
• test/functional/wallet_createwallet.py
• test/functional/wallet_descriptor.py
• test/functional/wallet_dump.py
• test/functional/wallet_encryption.py
• test/functional/wallet_fundrawtransaction.py
• test/functional/wallet_keypool.py
• test/fuzz/test_runner.py
• test/get_previous_releases.py

💤 Files with no reviewable changes (2)

• src/test/util/blockfilter.cpp
• src/test/util/setup_common.h

[coderabbitai]

⚠️ Potential issue | 🟠 Major

powerpc64le now passes host detection but still can't download any Dash release.

Line 269 enables the platform, but this file still has no powerpc64le-linux-gnu entries in SHA256_SUMS, and download_binary() rejects tarballs without a matching checksum entry. On a powerpc64le host this now gets past detection only to fail later for every default tag. Please either add the matching release assets/checksums or keep the host unmapped until Dash publishes supported binaries.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/get_previous_releases.py` around lines 267 - 270, The platforms mapping
added an entry for 'powerpc64le-*-linux-*' -> 'powerpc64le-linux-gnu' but there
are no corresponding SHA256_SUMS entries and download_binary() rejects tarballs
without checksum entries; either revert/remove the 'powerpc64le-*-linux-*' entry
from the platforms dict (so host detection keeps it unmapped) or add the
official Dash release assets and matching checksum lines for
'powerpc64le-linux-gnu' into the SHA256_SUMS data used by download_binary() and
ensure the checksum keys match exactly the platform string; update the
SHA256_SUMS source and test download_binary() to accept those checksums if you
choose to add support.

[thepastaclaw]

Code Review

Validated the candidate findings against the full upstream/develop...HEAD range; one blocking regression remains in test/get_previous_releases.py, and the backport-prerequisite note is a pre-existing gap rather than a defect introduced by this PR.

Reviewed commit: a040529

🔴 1 blocking

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `test/get_previous_releases.py`:
- [BLOCKING] line 269: `powerpc64le` downloads will always fail checksum validation
  Adding the `powerpc64le-*-linux-*` mapping makes `--download-binary` request `dashcore-<tag>-powerpc64le-linux-gnu.tar.gz`, but this file has no matching `powerpc64le-linux-gnu` entries anywhere in `SHA256_SUMS`. As a result, every `powerpc64le` download now falls through to the checksum gate at lines 164-169 and exits with `Checksum for given version doesn't exist`, so the new host mapping changes the failure from "unsupported host" to a later runtime error without actually enabling working downloads.

[thepastaclaw]

🔴 Blocking: powerpc64le downloads will always fail checksum validation

Adding the powerpc64le-*-linux-* mapping makes --download-binary request dashcore-<tag>-powerpc64le-linux-gnu.tar.gz, but this file has no matching powerpc64le-linux-gnu entries anywhere in SHA256_SUMS. As a result, every powerpc64le download now falls through to the checksum gate at lines 164-169 and exits with Checksum for given version doesn't exist, so the new host mapping changes the failure from "unsupported host" to a later runtime error without actually enabling working downloads.

_{source: ['codex-general']}

🤖 Fix this with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `test/get_previous_releases.py`:
- [BLOCKING] line 269: `powerpc64le` downloads will always fail checksum validation
  Adding the `powerpc64le-*-linux-*` mapping makes `--download-binary` request `dashcore-<tag>-powerpc64le-linux-gnu.tar.gz`, but this file has no matching `powerpc64le-linux-gnu` entries anywhere in `SHA256_SUMS`. As a result, every `powerpc64le` download now falls through to the checksum gate at lines 164-169 and exits with `Checksum for given version doesn't exist`, so the new host mapping changes the failure from "unsupported host" to a later runtime error without actually enabling working downloads.

[thepastaclaw]

Code Review

Backport bundle of 7 Bitcoin Core PRs — overwhelmingly mechanical test/fuzz/logging infrastructure plus one hidden test-only RPC and a BCLog::Timer steady_clock swap. No consensus or merge-resolution defects found. The getaddrmaninfo RPC accounting in rpc/net.cpp:1158-1171 is self-consistent with the test in rpc_net.py:408-428 (routable networks + all_networks aggregate). The powerpc64le platform mapping lands without matching SHA256_SUMS entries, but this mirrors upstream exactly (Bitcoin doesn't ship ppc64le binaries either) — it's a suggestion, not a blocker. Minor style/robustness nits only.

Reviewed commit: a040529

🟡 2 suggestion(s) | 💬 2 nitpick(s)

1 additional finding

🟡 suggestion: powerpc64le platform mapping has no matching SHA256_SUMS entries

test/get_previous_releases.py (line 269)

Adding powerpc64le-*-linux-* to the platforms dict makes --download-binary request dashcore-<tag>-powerpc64le-linux-gnu.tar.gz, but SHA256_SUMS contains no powerpc64le-linux-gnu entries (grep confirms the only occurrence in the file is this new line). A powerpc64le user invoking --download-binary will fall through to the checksum gate at lines 164-170 and exit with Checksum for given version doesn't exist. This matches upstream bitcoin#28352, which also shipped without prebuilt ppc64le binaries (the mapping exists to support --depends builds and future binary releases). Not a regression that blocks merging, but worth either (a) adding ppc64le SHA entries when Dash ships such tarballs, or (b) a comment clarifying that ppc64le is depends-only.

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `test/get_previous_releases.py`:
- [SUGGESTION] line 269: powerpc64le platform mapping has no matching SHA256_SUMS entries
  Adding `powerpc64le-*-linux-*` to the `platforms` dict makes `--download-binary` request `dashcore-<tag>-powerpc64le-linux-gnu.tar.gz`, but SHA256_SUMS contains no `powerpc64le-linux-gnu` entries (grep confirms the only occurrence in the file is this new line). A powerpc64le user invoking `--download-binary` will fall through to the checksum gate at lines 164-170 and exit with `Checksum for given version doesn't exist`. This matches upstream bitcoin#28352, which also shipped without prebuilt ppc64le binaries (the mapping exists to support `--depends` builds and future binary releases). Not a regression that blocks merging, but worth either (a) adding ppc64le SHA entries when Dash ships such tarballs, or (b) a comment clarifying that ppc64le is depends-only.

In `test/functional/rpc_net.py`:
- [SUGGESTION] lines 408-428: test_getaddrmaninfo relies on exact addrman state from preceding subtest
  The hard-coded expectations `ipv4.new=1`, `ipv4.tried=1`, `all_networks.total=2` depend on `test_addpeeraddress` having run immediately before and having populated nodes[1]'s addrman with exactly one new (2.0.0.0) and one tried (1.2.3.4) address. Dash's `NetTest` extends `DashTestFramework` and carries masternode wiring that could in the future inject additional addr activity into nodes[1] via p2p `addr` messages, silently breaking these assertions. Making the subtest self-contained (snapshot baseline with `getaddrmaninfo`, then `addpeeraddress` a fresh address and diff) would make it resilient. Non-blocking — passes as-is given current subtest ordering.

[thepastaclaw]

🟡 Suggestion: test_getaddrmaninfo relies on exact addrman state from preceding subtest

The hard-coded expectations ipv4.new=1, ipv4.tried=1, all_networks.total=2 depend on test_addpeeraddress having run immediately before and having populated nodes[1]'s addrman with exactly one new (2.0.0.0) and one tried (1.2.3.4) address. Dash's NetTest extends DashTestFramework and carries masternode wiring that could in the future inject additional addr activity into nodes[1] via p2p addr messages, silently breaking these assertions. Making the subtest self-contained (snapshot baseline with getaddrmaninfo, then addpeeraddress a fresh address and diff) would make it resilient. Non-blocking — passes as-is given current subtest ordering.

_{source: ['claude']}

🤖 Fix this with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `test/functional/rpc_net.py`:
- [SUGGESTION] lines 408-428: test_getaddrmaninfo relies on exact addrman state from preceding subtest
  The hard-coded expectations `ipv4.new=1`, `ipv4.tried=1`, `all_networks.total=2` depend on `test_addpeeraddress` having run immediately before and having populated nodes[1]'s addrman with exactly one new (2.0.0.0) and one tried (1.2.3.4) address. Dash's `NetTest` extends `DashTestFramework` and carries masternode wiring that could in the future inject additional addr activity into nodes[1] via p2p `addr` messages, silently breaking these assertions. Making the subtest self-contained (snapshot baseline with `getaddrmaninfo`, then `addpeeraddress` a fresh address and diff) would make it resilient. Non-blocking — passes as-is given current subtest ordering.

[thepastaclaw]

Leaving this one as-is for this backport batch. The test currently passes on the intended ordered setup, and I do not want to add extra structure churn beyond the upstream backport unless this becomes a real flake.

[thepastaclaw]

💬 Nitpick: test/util/coins.cpp inserted out of alphabetical order in libtest_util_a_SOURCES

The libtest_util_a_SOURCES list is otherwise alphabetically sorted (blockfilter, index, json, logging, mining, net, script, setup_common, str, transaction_utils, txmempool, validation, wallet). The new test/util/coins.cpp was inserted between index.cpp and json.cpp instead of between blockfilter.cpp and index.cpp. The header-side entry (test/util/coins.h) is placed correctly at line 13. Upstream bitcoin#26940 placed it correctly. Build is unaffected.

💡 Suggested change

Suggested change

	libtest_util_a_SOURCES = \
	test/util/blockfilter.cpp \
	test/util/index.cpp \
	test/util/coins.cpp \
	test/util/json.cpp \
	test/util/logging.cpp \
	libtest_util_a_SOURCES = \
	test/util/blockfilter.cpp \
	test/util/coins.cpp \
	test/util/index.cpp \
	test/util/json.cpp \
	test/util/logging.cpp \
	test/util/mining.cpp \

_{source: ['claude']}

[thepastaclaw]

Leaving this as-is. It is a harmless ordering nit in a backport bundle, and I do not want to respin the branch just for local source-list ordering.

[thepastaclaw]

💬 Nitpick: sync_with_ping docstring grammar: "is called" should be "are called"

The merged sync_with_ping now guarantees both ProcessMessages and SendMessages were invoked (by sending two back-to-back pings), but the docstring still reads "Ensure ProcessMessages and SendMessages is called on this connection" — subject-verb disagreement. The inline comment below correctly explains the two-ping rationale. Nonce=0 sentinel is safe because ping_counter starts at 1, so no collision is possible.

💡 Suggested change

Suggested change

	def sync_with_ping(self, *, timeout=60):
	"""Ensure ProcessMessages and SendMessages is called on this connection"""
	def sync_with_ping(self, *, timeout=60):
	"""Ensure ProcessMessages and SendMessages are called on this connection."""

_{source: ['claude']}

[thepastaclaw]

Leaving this as-is too. Good nit, but it is docstring-only churn on an otherwise green backport branch, so I am not respinning for it.

[thepastaclaw]

Code Review

Backport batch of 7 Bitcoin Core PRs (test-infra refactors, steady_clock Timer, hidden getaddrmaninfo RPC, sync_with_ping consolidation, wallet unlock timeout bumps). Both agents independently verified prerequisites are present and Dash adaptations are correct; no blocking or high-signal issues identified. One nitpick about Makefile alphabetical ordering was dropped per the backport skill's 'do not flag cosmetic divergences' guidance.

Reviewed commit: 3ded43e

[vijaydasmp]

Hello @PastaPastaPasta @thepastaclaw @knst requesting review

[thepastaclaw]

Revalidated current head 3ded43e2 in an isolated worktree.

The bundle still looks like a clean, mechanically faithful backport set: steady_clock timer swap, test-util extraction, hidden getaddrmaninfo RPC + functional coverage, wallet timeout hardening, fuzz pubkey helper, and the sync_with_ping() consolidation all line up with the intended upstream behavior. CI is green; the only red surface is predict_conflicts, which is informational-only.