📝 fjahr opened a pull request: "test: Use rpc_deprecated only for testing deprecation"
(https://github.com/bitcoin/bitcoin/pull/31977)
The comment in `functional/rpc_deprecated.py` says "This test should be used to verify correct behaviour of deprecated RPC methods with and without the -deprecatedrpc flags." I think we can get rid of the "with" part since we can assume that every RPC is already tested in at least one other functional test. (I didn't look but I could verify in our coverage if someone has doubts about that.) In order for this test to continue working, the flag will need to be used there. Otherwise this seems to p
...

💬 fjahr commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#discussion_r1978196051)
Note that I think it's enough to test the deprecation error there, see #31977

💬 laanwj commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-2695543354)
> Nevertheless, I could revert that change if you prefer it remain appended to the versions header, WDYT?

i agree an alphabet soup like "nbwcl2" does not say anything without a legend what every character means.

To be clear, i'm not against adding it, it's only one line, it was more of a general concern about the direction.

📝 glozow opened a pull request: "kernel: pre-29.x chainparams and headerssync update"
(https://github.com/bitcoin/bitcoin/pull/31978)
Update chainparams and headerssync config for v29.0 (see [release process](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off)).

💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-2695553587)
I've updated the pull description to be clearer about it.

💬 glozow commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#issuecomment-2695559684)
These are from my nodes except for the mainnet `m_assumed_blockchain_size` which is from @sr-gi (thanks!)

💬 pinheadmz commented on issue "test: WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=32090)":
(https://github.com/bitcoin/bitcoin/issues/30764#issuecomment-2695593851)
I might be seeing this today as well: https://gist.github.com/pinheadmz/08758d268ec1741afc8c4f181c0e97d2

My branch forks off of master at db36a92c02

💬 fjahr commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2695596738)
I gave it a shot and changed it to this:

"Due to a bug the default block reserved weight for fixed-size block header, transactions count, and coinbase transaction was reserved twice and could not be lowered further. As a result the total reserved weight was always `8,000 WU`, meaning that even when specifying a `-blockmaxweight` higher than the default (even to the max `4,000,000 WU`), the actual block size never exceeded `3,992,000 WU`. The fix consolidates the reservation into a single pla
...

💬 Sjors commented on pull request "Drop testnet3":
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2695598930)
I split 5d6aa13cf047e31005afca919ca9cf181e30905b into smaller commits. If we decide to punt this until after v30 then I'll PR some of these separately. E.g. there's some tests that can use testnet4 instead of testnet3 and perhaps we can drop GUI support earlier.

💬 ismaelsadeeq commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2695600516)
Thanks for the ping @glozow I will amend the suggestions in the wiki

💬 davidgumberg commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#discussion_r1978281970)
Note: `WriteSettingsFile()` throws if the `-nosettings` argument is set, but in that case we would never find a value for `upnp` above, and `settings_changed` won't be true, so that edge case is handled well.

💬 Crypt-iQ commented on issue "compact blocks in IBD resets m_stalling_since":
(https://github.com/bitcoin/bitcoin/issues/31962#issuecomment-2695634708)
Ah, I did not consider the minchainwork.

> So it shouldn't be possible to trigger this by sending header messages, but the two attacking peers could be be randomly selected next to each other.

I could have made that more clear in the description that the attacker needed two peers to be selected in sequence for the `SendMessages` call. That's what I meant by "subsequent" but I think it was unclear.

> dependent on already_in_flight - could we unconditionally skip out here if CanDirectFetch() re
...

💬 sipa commented on pull request "cluster mempool: introduce TxGraph":
(https://github.com/bitcoin/bitcoin/pull/31363#issuecomment-2695648258)
I have dropped the `QualityLevel::NEEDS_SPLIT_OPTIMAL` value, and made the "post-linearize after remove, before split" apply to `NEEDS_SPLIT_ACCEPTABLE` instead, with updated comments about it.

💬 instagibbs commented on issue "compact blocks in IBD resets m_stalling_since":
(https://github.com/bitcoin/bitcoin/issues/31962#issuecomment-2695653952)
That code block appears to be in since the dawn of compact blocks: https://github.com/bitcoin/bitcoin/commit/d25cd3ec4e8#diff-34d21af3c614ea3cee120df276c9c4ae95053830d7f1d3deaf009a4625409ad2R5312

I might be missing something of course.

> At least removing the condition doesn't break any existing tests.

Not very comforting :)

💬 laanwj commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#discussion_r1978311245)
Yes, i used to have an explicit check around this logic for dynamic settings to be enabled (so that it won't throw), but removed it after a comment by @ryanofsky. It indeed doesn't seem to be necessary because the dynamic settings won't be populated in the first place.

💬 davidgumberg commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#discussion_r1978313107)
Thanks, I missed that discussion. (https://github.com/bitcoin/bitcoin/pull/31916#discussion_r1965808053)

💬 laanwj commented on pull request "delete release note fragments for v29":
(https://github.com/bitcoin/bitcoin/pull/31976#issuecomment-2695681192)
ACK ae92bd8e1b2c144697662ba0358c27f5c1892bb2

💬 davidgumberg commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#issuecomment-2695688502)
lightly reviewed code, tested ACK https://github.com/bitcoin/bitcoin/commit/44041ae0eca9d2034b7c2bdef24724809cc35e90

There are a lot of possible combinations of settings in bitcoin.conf, commandline arguments, and GUI settings, I sanity tested the happy path of `upnp=true` in settings.json, and a few cases I thought would be tricky with `-nosettings` and `-upnp` as a cli argument and in bitcoin.conf, and this solution handles them all well, and I can't think of any unhandled cases from review
...

👍 laanwj approved a pull request: "torcontrol: Add comment explaining Proxy credential randomization for Tor privacy"
(https://github.com/bitcoin/bitcoin/pull/31973#pullrequestreview-2655481086)
Code review ACK 65e503e8499f18e48d7801a298d9acdd6ed62d0b

💬 darosior commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1978342771)
> Curious to know the reason behind using UTx0 instead of UTXO.

:shrug: doesn't matter, but as it is an acronym it makes more sense to me to capitalize the first letter of each word an not the others.

> Can this be made more understandable?

That's a pretty vague request.. I tried to be as detailed as possible in the commit message since the change is not obvious. Could you point to what you don't understand exactly?

> What I can gather is this seems to make the test less tight aroun
...

💬 achow101 commented on pull request "seeds: update makeseeds regex and DNS fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/31960#issuecomment-2695738356)
Could also do testnet4, my seeder publishes a seeds.txt.gz at https://testnet4.achownodes.xyz/seeds.txt.gz