👍 TheCharlatan approved a pull request: "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set"
(https://github.com/bitcoin/bitcoin/pull/25972#pullrequestreview-2038887677)
ACK f0e22be69a15248c42964d57f44ce8c37a36081d

📝 paplorinc opened a pull request: "test: Add a few more corner cases to the base58 test suite"
(https://github.com/bitcoin/bitcoin/pull/30035)
Split out the additional tests from the base58 optimization PR, see: https://github.com/bitcoin/bitcoin/pull/29473#discussion_r1526210147

🤔 sr-gi reviewed a pull request: "p2p: index TxOrphanage by wtxid, allow entries with same txid"
(https://github.com/bitcoin/bitcoin/pull/30000#pullrequestreview-2038712299)
Concept ACK

💬 sr-gi commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1589529436)
In 646aa4da91c03a0e72d086cae281aa0688f2f41d

This is only used for logging now. Do we care about logging the `txid` now that the map is keyed by `wtxid`?

💬 sr-gi commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1589549760)
In 646aa4da91c03a0e72d086cae281aa0688f2f41d

Same here, do we still care about logging `txids`? I guess this applies to all the file, so not pointing it out again

💬 sr-gi commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1589569008)
In 646aa4da91c03a0e72d086cae281aa0688f2f41d

This should not be called `orphanHash` anymore, but also, this could be inlined

💬 sr-gi commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1589547282)
In 646aa4da91c03a0e72d086cae281aa0688f2f41d

nit: `nErased` is a counter right? Shouldn't this read `txs` instead?

💬 sr-gi commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1589571339)
Also, in L231:

`for (const auto& orphanHash : vOrphanErase)` -> `for (const auto& wtxid : vOrphanErase)`

💬 Sjors commented on pull request "depends: swap some cctools tools for LLVM tools":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2093692188)
@fanquake I get the same hashes on AMD Ubuntu 24.04

⚠️ IAmAdamRest opened an issue: "Possible to Ban Clients by Name?"
(https://github.com/bitcoin/bitcoin/issues/30036)
### Please describe the feature you'd like to see added.

Thousands of "/Satoshi-BTC(Bitcoin Finance):0.15.1/" peers are suddenly connecting to my two nodes and sending GIGABYTES of information even though my nodes are fully synced, this doesn't feel like expected behavior.

Can we have a way to ban something like this? They are connecting from only cloud providers or inside mainland China. I can barely keep up banning these. Is it possible to have a way to reject this sort of thing from happe
...

💬 IAmAdamRest commented on issue "Manually Banning Peers Results in Crash":
(https://github.com/bitcoin/bitcoin/issues/29916#issuecomment-2093708486)
> @iotamega @IAmAdamRest
>
> Does the issue happen with Bitcoin Core v26.1, v25.2?

I don't know, I can download one of those. I am likely just giving up though as this network seems to be full of spam trash nodes that seem intent on just overwhelming machines. I have spent all day banning peers that are sending gigs of data even though I am fully synced and wasting my bandwidth.

💬 pinheadmz commented on issue "Possible to Ban Clients by Name?":
(https://github.com/bitcoin/bitcoin/issues/30036#issuecomment-2093720851)
You can not ban by user agent (that is very easily spoofed) but you can ban a range of IPs. What exactly is happening here? What are you receiving? Bitcoin Core already has lots of DoS mitigation mechanisms.

⚠️ mzumsande opened an issue: "RfC: increase minimum prune target?"
(https://github.com/bitcoin/bitcoin/issues/30037)
The minimum pruning target of `550MiB` was chosen based on a physical block size of `1 MiB`, see  https://github.com/bitcoin/bitcoin/blob/f5b6f621fff7540ca97974b26a66ca1cc6db018c/src/validation.h#L69-L76
We never prune files within 288 blocks from the tip, so the limit will not be respected if these blocks take up more space.

Since physical block sizes have increased since segwit, I think that there is no benefit to choosing `-prune=550` as opposed to `-prune=1000`:
Running with the smaller
...

🤔 tdb3 reviewed a pull request: "test: Handle functional test disk-full error"
(https://github.com/bitcoin/bitcoin/pull/29335#pullrequestreview-2038975683)
re ACK for 357ad110548d726021547d85b5b2bfcf3191d7e3

Retested https://github.com/bitcoin/bitcoin/pull/29335#pullrequestreview-1999297020 (and received the same results, which warn the user of insufficient free space and early exit due to insufficient space).

🤔 tdb3 reviewed a pull request: "rpc: return warnings as an array instead of just a single one"
(https://github.com/bitcoin/bitcoin/pull/29845#pullrequestreview-2039013213)
Re ACK for 42fb5311b19582361409d65c6fddeadbee14bb97

Ran the tests in https://github.com/bitcoin/bitcoin/pull/29845#issuecomment-2081107100 and https://github.com/bitcoin/bitcoin/pull/29845#pullrequestreview-1999801519 again (no warnings, one warning, two warnings, and with `-deprecatedrpc` with one warning). All behaved as expected.

💬 brunoerg commented on issue "Possible to Ban Clients by Name?":
(https://github.com/bitcoin/bitcoin/issues/30036#issuecomment-2093786404)
I don't think it would be effective. If we implemented something like it and, if they're bad/malicious peers, they can just vary it and bypass this ban.

✅ brunoerg closed an issue: "wallet, coin selection: config option to prioritize 'no change' when possible"
(https://github.com/bitcoin/bitcoin/issues/23372)

💬 kevkevinpal commented on pull request "test: use assert_greater_than util":
(https://github.com/bitcoin/bitcoin/pull/30019#discussion_r1589760663)
that is fair I initially did it like this because there was one scenario where we used two `<` operators but I now switched to using `assert_greater_than` and now broke that statement up to two statements

💬 kevkevinpal commented on pull request "test: use assert_greater_than util":
(https://github.com/bitcoin/bitcoin/pull/30019#issuecomment-2093824753)
pushed to [fe07516](https://github.com/bitcoin/bitcoin/pull/30019/commits/fe075160f04c3e57fab9571891b17a4ce9f8bec3)

we now use `assert_greater_than` instead of creating a new util

[04d8f07](https://github.com/bitcoin/bitcoin/pull/30019/commits/04d8f07c4317ec4c517ae060a1c2fea2b01416df) contains a scripted-diff which looks for `assert.*< ` and then swaps the two values and replaces the `assert` with `assert_greater_than`

💬 davidgumberg commented on pull request "rpc: parse legacy pubkeys consistently with specific error messages":
(https://github.com/bitcoin/bitcoin/pull/28336#discussion_r1589807191)
Nit: IMO, the length check should happen before the `IsHex` check, since a string with an odd-numbered length of valid hex characters will fail with the slightly confusing error message that it "must be a hex string." For example, 67 zeroes:
```console
$ bitcoin-cli importpubkey 0000000000000000000000000000000000000000000000000000000000000000000
error code: -5
error message:
Pubkey "0000000000000000000000000000000000000000000000000000000000000000000" must be a hex string
```