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

✅ fanquake closed an issue: "`test/streams_tests.cpp` fails to compile on SunOS / illumos"
(https://github.com/bitcoin/bitcoin/issues/29884)

🚀 fanquake merged a pull request: "test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos"
(https://github.com/bitcoin/bitcoin/pull/29907)

🚀 fanquake merged a pull request: "miniscript: make operator""_mst consteval"
(https://github.com/bitcoin/bitcoin/pull/28657)

💬 fanquake commented on pull request "msvc: Compile `test\fuzz\miniscript.cpp`":
(https://github.com/bitcoin/bitcoin/pull/30031#issuecomment-2093940009)
> Based on https://github.com/bitcoin/bitcoin/pull/28657.

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2093953949)
> We made `acceptnonstdtxn=0` the default for testnet3 in #28354. We could flip it back on. Or we could make it easier to preferentially peer with no-standardness nodes (and miners) to get those in. I don't think relying on low difficulty blocks is the right solution, at least I don't think it's a good enough reason to keep them around.

Yeah, I tend to agree, I thought about flipping it back on earlier as well: https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2041139129 but we weren
...

💬 fanquake commented on pull request "guix: build with glibc 2.31":
(https://github.com/bitcoin/bitcoin/pull/29987#issuecomment-2093954870)
Rebased and pulled in 1 more commit from the 2.31 branch.

💬 ajtowns commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2094003382)
> and `acceptnonstdtxn=1` as default is still a good solution

We've had a case where having this setting as default rendered [$76M of bitcoin inaccessible for months](https://github.com/bitcoin/bitcoin/pull/26348), so that doesn't seem like a good solution to me. If you're testing something that will require miners with meaningful hashpower running non-default things on mainnet, finding a miner who'll do the same thing on testnet first doesn't seem that unreasonable to me. You can always test
...

👋 hebasto's pull request is ready for review: "msvc: Compile `test\fuzz\miniscript.cpp`"
(https://github.com/bitcoin/bitcoin/pull/30031)

💬 hebasto commented on pull request "msvc: Compile `test\fuzz\miniscript.cpp`":
(https://github.com/bitcoin/bitcoin/pull/30031#issuecomment-2094071793)
> > Based on #28657.
>
> Can be rebased now.

Rebased and undrafted.