[28.x] More backports

fanquake commented at 11:09 am on September 17, 2025: member

Further backports for 28.x:

fanquake added this to the milestone 28.3 on Sep 17, 2025

DrahtBot added the label Backport on Sep 17, 2025

DrahtBot commented at 11:09 am on September 17, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33415.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	willcl-ark, achow101
Stale ACK	glozow

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label CI failed on Sep 17, 2025

fanquake marked this as ready for review on Sep 17, 2025

fanquake force-pushed on Sep 17, 2025

willcl-ark approved

willcl-ark commented at 2:39 pm on September 17, 2025: member

ACK 7575828dd2ea539e103067cd35e31333797d22e3

Backports look good. Release notes contain all commits and authors.

fanquake force-pushed on Sep 17, 2025

achow101 commented at 6:19 pm on September 17, 2025: member

Should we also backport #33106 if we’re going to be doing a 28.3 anyways?

glozow commented at 8:43 pm on September 17, 2025: member

Should we also backport #33106 if we’re going to be doing a 28.3 anyways?

Here is a branch to do this: https://github.com/glozow/bitcoin/tree/2025-09-28.3-backport

Similar to #33226, needed to do some test massaging. I also needed #30948 and #30784 for the test utils/refactors. Feel free to pull; I updated the final changes as well.

fanquake commented at 9:32 am on September 18, 2025: member

Here is a branch to do this: https://github.com/glozow/bitcoin/tree/2025-09-28.3-backport

Thanks. Looks like there are some issues with this branch. i.e:

0test/mempool_tests.cpp:434: Entering test case "MempoolSizeLimitTest"
1<snip>
2test/mempool_tests.cpp:562: error: in "mempool_tests/MempoolSizeLimitTest": check pool.GetMinFee(1).GetFeePerK() == maxFeeRateRemoved.GetFeePerK() + 1000 has failed [19941 != 20841]
32025-09-18T09:27:52.459923Z (mocktime: 1970-01-01T12:00:42Z) [test] [validationinterface.cpp:228] [MempoolTransactionsRemovedForBlock] [validation] Enqueuing MempoolTransactionsRemovedForBlock: block height=1 txs removed=0
4test/mempool_tests.cpp:566: error: in "mempool_tests/MempoolSizeLimitTest": check pool.GetMinFee(1).GetFeePerK() == llround((maxFeeRateRemoved.GetFeePerK() + 1000)/2.0) has failed [9971 != 10421]
5test/mempool_tests.cpp:570: error: in "mempool_tests/MempoolSizeLimitTest": check pool.GetMinFee(pool.DynamicMemoryUsage() * 5 / 2).GetFeePerK() == llround((maxFeeRateRemoved.GetFeePerK() + 1000)/4.0) has failed [4985 != 5210]
6test/mempool_tests.cpp:574: error: in "mempool_tests/MempoolSizeLimitTest": check pool.GetMinFee(pool.DynamicMemoryUsage() * 9 / 2).GetFeePerK() == llround((maxFeeRateRemoved.GetFeePerK() + 1000)/8.0) has failed [2493 != 2605]
7test/mempool_tests.cpp:578: error: in "mempool_tests/MempoolSizeLimitTest": check pool.GetMinFee(1).GetFeePerK() == 1000 has failed [100 != 1000]

Want to PR / debug that separately?

glozow commented at 2:25 pm on September 23, 2025: member

Thanks. Looks like there are some issues with this branch. i.e:

Ah sorry about that! It’s fixed on the branch now.

Want to PR / debug that separately?

Happy to review this and do a separate PR if that’s easier. The default changes also require manpage modifications, so it might be better to defer the rc final changes if you’d like a separate PR.

fanquake commented at 2:26 pm on September 23, 2025: member

Ok, I’ll just pull the branch in here (shortly).

fanquake force-pushed on Sep 23, 2025

glozow commented at 6:26 pm on September 23, 2025: member

ACK 1b1f359fc43385cb4d465b15754dac84eef06873

Could probably have someone double check the min feerate backport. The CI failure is related to #31210 / #30125. IIUC we skipped backporting #30125 as it’s quite involved (also see #31422#pullrequestreview-2607160362)

DrahtBot requested review from willcl-ark on Sep 23, 2025

in test/functional/wallet_fundrawtransaction.py:48 in f8006d9df0 outdated

43@@ -44,6 +44,10 @@ def add_options(self, parser):
44 
45     def set_test_params(self):
46         self.num_nodes = 4
47+        self.extra_args = [[
48+            "-deprecatedrpc=settxfee",

achow101 commented at 8:33 pm on September 23, 2025:

In f8006d9df08597bc2f4e85d32bff677c27c36ced “[prep/test] make wallet_fundrawtransaction’s minrelaytxfee assumption explicit”

settxfee is not deprecated in 28.x, so including this change is unnecessary.

glozow commented at 2:32 pm on September 26, 2025:

fixed in #33476

achow101 commented at 8:58 pm on September 23, 2025: member

It looks like 3eab8b724044dc321f70e5eed66b149713158a04 was skipped in the backport? It doesn’t seem like there are significant conflicts and it makes 2e756e8b02d30e9baacd36d4e519ab64421778ba easier to review.

IIUC we skipped backporting #30125 as it’s quite involved (also see #31422#pullrequestreview-2607160362)

It backports without issue to 28.x. For 27.x, the second commit is complicated, but I think it could’ve been skipped.

willcl-ark approved

willcl-ark commented at 9:01 pm on September 23, 2025: member

ACK 1b1f359fc43385cb4d465b15754dac84eef06873

#33106 backport is naturally a bit tricky to review, especially as I didn’t review the original PR.

That said, git-range-diff helped out a fair bit e.g. git range-diff e5f896bb1f052fb8c7811c6024cb49143b427512..ba84a25deec0b3b9b94ee51b373e715fec995791 6090af0d350..2e756e8b02d uses the range-diff algo on the two commit ranges, which is pretty neat! It helped me identify the “massaging” that was needed for the backport, which looks fine to me.

I wonder if we want to also include the getmininginfo followup here too? :hide: 9169a50 (#33189)

willcl-ark commented at 9:03 pm on September 23, 2025: member

It looks like 3eab8b7 was skipped in the backport? It doesn’t seem like there are significant conflicts and it makes 2e756e8 easier to review.

Ah, I was just complaining about this being tricky to review offline, but now see it in the range-diff too. That would have indeed made it easier for me to review.

0 6:  3eab8b72404 <  -:  ----------- [prep/test] replace magic number 1000 with respective feerate vars

glozow commented at 9:09 pm on September 23, 2025: member

It looks like https://github.com/bitcoin/bitcoin/commit/3eab8b724044dc321f70e5eed66b149713158a04 was skipped in the backport? It doesn’t seem like there are significant conflicts and it makes https://github.com/bitcoin/bitcoin/commit/2e756e8b02d30e9baacd36d4e519ab64421778ba easier to review.

My bad - I missed that commit and it didn’t hit me that there was a separate commit while I was resolving the conflicts.

maflcko removed the label CI failed on Sep 24, 2025

Fix benchmark CSV output

The `SHA256AutoDetect` return output is used, among other use cases, to
name benchmarks. Using a comma breaks the CSV output.

This change replaces the comma with a semicolon, which fixes the issue.

Github-Pull: #33340
Rebased-From: 790b440197bde322432a5bab161f1869b667e681

a381de750d

net: Do not apply whitelist permission to onion inbounds

Tor inbound connections do not reveal the peer's actual network address.
Therefore do not apply whitelist permissions to them.

Co-authored-by: Vasil Dimov <vd@FreeBSD.org>

Github-Pull: #33395
Rebased-From: f563ce90818d486d2a199439d2f6ba39cd106352

9e56d8889a

doc: Remove wrong and redundant doxygen tag

Remove it in feerate.

Fix it in the other places.

Github-Pull: #33236
Rebased-From: 966666de9a6211b8748f43d682490c924e132e58

4598dfcfde

in doc/man/bitcoin-qt.1:320 in 1b1f359fc4

316@@ -317,10 +317,6 @@ created within past week. 0 = no limit (default: 0M). Optional
317 suffix units [k|K|m|M|g|G|t|T] (default: M). Lowercase is 1000
318 base while uppercase is 1024 base
319 .HP
320-\fB\-natpmp\fR

achow101 commented at 6:24 pm on September 24, 2025:

In 1b1f359fc43385cb4d465b15754dac84eef06873 “[doc] manpages for 28.3rc1”

I don’t think this is supposed to be removed from the manage.

Same with -upnp.

fanquake force-pushed on Sep 24, 2025

glozow commented at 7:18 pm on September 24, 2025: member

Opened #33476 to follow

doc: update release notes for 28.x a5e4fec494

fanquake force-pushed on Sep 24, 2025

willcl-ark approved

willcl-ark commented at 12:39 pm on September 25, 2025: member

ACK a5e4fec4949f61ebbd7d6696f39da26df04515f9

This now only includes #33236, #33395 and #33340, with the other changes moving into #33476.

Backports look fine, 966666de was the only commit that appears to have needed resolution.

All backports are credited in the release notes.

DrahtBot requested review from glozow on Sep 25, 2025

achow101 commented at 8:22 pm on September 25, 2025: member

ACK a5e4fec4949f61ebbd7d6696f39da26df04515f9

achow101 merged this on Sep 25, 2025

achow101 closed this on Sep 25, 2025

fanquake deleted the branch on Sep 26, 2025

[28.x] More backports #33415

Code Coverage & Benchmarks

Reviews