backport: Merge bitcoin#28755, 28865, 31225, 31718, 28966

[vijaydasmp]

bitcoin back ports

[github-actions]

⚠️ Potential Merge Conflicts Detected

This PR has potential conflicts with the following open PRs:

• backport: Merge bitcoin#30703, 30024, 29707, 28336, 29335, 30063, 3006, 30161, 30160 #7308 - backport: Merge bitcoin#30703, 30024, 29707, 28336, 29335, 30063, 3006, 30161, 30160
• backport: Merge bitcoin/bitcoin#28671, 28831, 28966 #7177 - backport: Merge bitcoin/bitcoin#28671, 28831, 28966

Please coordinate with the authors of these PRs to avoid merge conflicts.

[thepastaclaw]

✅ Review complete (commit 2e0dcdc)

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 50ba1f51-b9e1-4c0b-9a44-18c10e93e75d

📥 Commits

Reviewing files that changed from the base of the PR and between cad4245 and 2e0dcdc.

📒 Files selected for processing (1)

• test/sanitizer_suppressions/ubsan

✅ Files skipped from review due to trivial changes (1)

• test/sanitizer_suppressions/ubsan

---

Walkthrough

This pull request makes several independent improvements: documentation typos and wording are corrected across ci/README.md, contrib/guix/README.md, contrib/tracing/README.md, contrib/tracing/log_raw_p2p_msgs.py, and doc/multisig-tutorial.md. In configure.ac, the optional UPnP (miniupnpc) and NAT-PMP (natpmp) library detection is refactored from a combined header-and-library check into a two-step flow: header availability is checked first, and only on success is the library symbol checked. The test/sanitizer_suppressions/ubsan file replaces broad, file-based UBSan suppression entries with more targeted, symbol-scoped suppressions for unsigned-integer-overflow, implicit-integer-sign-change, and shift-base categories.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

• PastaPastaPasta
• knst
• UdjinM6

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The description 'bitcoin back ports' is vague and uses non-descriptive terms that don't convey meaningful information about the specific changes or commit numbers mentioned in the title.	Expand the description to explain which Bitcoin commits are being backported and what problems they address, such as mentioning the UBSan suppression fixes or documentation clarifications.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title references specific Bitcoin commit numbers being backported, directly relating to the changes which include documentation updates and configuration refactoring from those commits.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ 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.}

[thepastaclaw]

Code Review

Faithful backport of four small upstream PRs: typo/grammar doc fixes (bitcoin#31718, bitcoin#31225), configure.ac restructure for miniupnpc (bitcoin#28755), and per-symbol UBSan suppressions migration (bitcoin#28865). All agent findings were verified against upstream diffs and current source; none represent defects introduced by the Dash adaptation. The flagged 'incomplete' UBSan migration for policy/fees.cpp is in fact a faithful match of upstream — that line was intentionally not migrated in bitcoin#28865 (the narrowing to TxConfirmStats::removeTx happened in the later, unbackported bitcoin#28966). The packages.md/tracing-script notes are upstream prerequisite gaps that do not affect this PR.

[thepastaclaw]

Code Review

Incremental + cumulative re-review of 2e0dcdc0 after prior clean review at cad42458. No carried-forward prior findings exist, and CodeRabbit had zero inline findings. The latest delta only retunes test/sanitizer_suppressions/ubsan; the UBSan suppression state matches the relevant upstream backport after verification.

One cumulative backport completeness issue remains: the PR claims bitcoin#31718, but omits its doc/policy/packages.md hunk because Dash is missing the prerequisite package-RBF documentation from bitcoin#28984. This is documentation-only and not a build/runtime defect, but for a full upstream backport it should either bring the prerequisite doc state or explicitly document the intentional exclusion.

🔴 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 `doc/policy/packages.md`:
- [BLOCKING] doc/policy/packages.md:38-39: Missing prerequisite: bitcoin#28984
  bitcoin#31718 changes five files upstream, including doc/policy/packages.md. The Dash cherry-pick applies the other four doc/tracing typo fixes but omits the packages.md hunk. Upstream pre-31718 has a package replacement subsection after the package conflict rules, including the typo line `Replacements must pay more total total fees at the incremental relay fee`; Dash current still jumps from the older package conflict/ancestor-limit rules to `When packages are evaluated against ancestor/descendant limits...` and has no package replacement subsection to fix. Upstream blame shows the omitted paragraph was introduced by commit afd52d8e63 (`doc: update package RBF comment`), first merged via 41544b8f96 (`Merge bitcoin/bitcoin#28984: Cluster size 2 package rbf`). This is a soft prerequisite gap: the current cherry-pick handles it by omitting a documentation-only hunk, so it should not break build/runtime behavior, but the claimed full backport of bitcoin#31718 is incomplete without either bitcoin#28984's relevant package-RBF doc state or an explicit exclusion of that hunk.

  ---
  **Policy gate (backport-prereq-restore):** For full upstream backport PRs, a missing prerequisite is blocking unless the finding is explicitly allowlisted (e.g. `intentional_exclusion: true` or a matching entry in `policy_overrides`). Either backport the prerequisite or annotate the intentional exclusion in the PR description.

[thepastaclaw]

🔴 Blocking: 28865: kept broader policy/fees.cpp suppression instead of upstream's TxConfirmStats::removeTx narrowing

Upstream PR 28865 replaces 'implicit-integer-sign-change:policy/fees.cpp' with the narrower per-symbol 'implicit-integer-sign-change:TxConfirmStats::removeTx'. The Dash backport (commit 73add31) keeps the original file-scoped 'implicit-integer-sign-change:policy/fees.cpp' entry and does not add 'TxConfirmStats::removeTx'. This is the only ubsan line where Dash diverges from upstream's per-symbol migration. It is more permissive (suppresses any sign-change UBSan finding in fees.cpp) rather than less, so it is not a missing prerequisite and will not break the build or tests. Worth a note in case the divergence is unintentional — upstream's stated goal of 28865 was precisely to narrow these suppressions to specific symbols.

---

Policy gate (backport-prereq-restore): For full upstream backport PRs, a missing prerequisite is blocking unless the finding is explicitly allowlisted (e.g. intentional_exclusion: true or a matching entry in policy_overrides). The agent's original evidence above is the basis for this block; either backport the prerequisite or annotate the intentional exclusion in the PR description.

_{source: ['claude-backport-reviewer']}