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

💬 Michealflair commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-2094073473)
bc1quc0kme44uzqsqtpwfm93lfe8zgh5xrgv7zjtzy

💬 Emzy commented on issue "DNS seed "seed.bitcoinstats.com" doesn't support filtering while the comments says it does":
(https://github.com/bitcoin/bitcoin/issues/29911#issuecomment-2094078985)
> I have stopped using these DNS seeds. I found only 2 useful when I last tested:
>
> seed.bitcoin.sipa.be seed.bitcoin.wiz.biz

These have a very low TTL set, which is fine.

The DNS seeds using the DNSSEC setup https://github.com/sipa/bitcoin-seeder/pull/85 will renew the DNS zone every hour and have a TTL of 1h set.

The DNS seeder https://github.com/sipa/bitcoin-seeder is only providing long running Bitcoin nodes. So caching for one hour is not an issue.

💬 maflcko commented on pull request "msvc: Compile `test\fuzz\miniscript.cpp`":
(https://github.com/bitcoin/bitcoin/pull/30031#issuecomment-2094079027)
ACK 9155b733e153e799f09cc7f7e9199ad776b2cbb1

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

Not sure. `acceptnonstdtxn` is limited in what it allows. There are many non-standard examples that are not even exposed as a setting.