💬 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
```

💬 davidgumberg commented on pull request "rpc: parse legacy pubkeys consistently with specific error messages":
(https://github.com/bitcoin/bitcoin/pull/28336#discussion_r1589810999)
Nit-question: Should there be a test added here that checks for pubkeys that are not valid points as in https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e?

Or, since this PR deduplicates pubkey parsing, is this test of the length check redundant and worthy of being removed?

💬 davidgumberg commented on pull request "rpc: parse legacy pubkeys consistently with specific error messages":
(https://github.com/bitcoin/bitcoin/pull/28336#discussion_r1589811146)
see: https://github.com/bitcoin/bitcoin/pull/28336/commits/100e8a75bf5d8196c005331bd8f2ed42ada6d8d0#r1589810999

💬 davidgumberg commented on pull request "rpc: parse legacy pubkeys consistently with specific error messages":
(https://github.com/bitcoin/bitcoin/pull/28336#issuecomment-2093897482)
ACK https://github.com/bitcoin/bitcoin/pull/28336/commits/98570fe29bb08d7edc48011aa6b9731c6ab4ed2e
The deduplication of pubkey parsing here is a big improvement, and granular pubkey error messages make the rpc interface easier to use. As far as I could find, no instances of pubkey parsing in the rpc code were missed in this PR.

I tested this PR manually by compiling with legacy wallet support and passing bad pubkeys with the `importpubkey` rpc.

💬 fanquake commented on pull request "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set":
(https://github.com/bitcoin/bitcoin/pull/25972#issuecomment-2093918590)
> Would it make sense to mention that -Wno-error=... should be passed, when needed, in the compile docs?

Maybe into the CI or dev docs, given we'll never use -Werror by default for compilation?

✅ fanquake closed an issue: "build: CXXFLAGS with depends"
(https://github.com/bitcoin/bitcoin/issues/18092)

🚀 fanquake merged a pull request: "build: no-longer disable WARN_CXXFLAGS when CXXFLAGS is set"
(https://github.com/bitcoin/bitcoin/pull/25972)