💬 tdb3 commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-2103631437)
> Actually, since these don't change on their own... maybe a separate RPC altogether? (later extended to allow some changes?)

This option also makes sense to me. Maybe something like `getmempoolpolicy` or similar?

🤔 ajtowns reviewed a pull request: "test, subprocess: Improve coverage report correctness"
(https://github.com/bitcoin/bitcoin/pull/30075#pullrequestreview-2049083052)
Approach ACK

💬 ajtowns commented on pull request "test, subprocess: Improve coverage report correctness":
(https://github.com/bitcoin/bitcoin/pull/30075#discussion_r1596149065)
The code in https://inbox.sourceware.org/gcc-cvs/20200505141638.8B852388F43D@sourceware.org/T/ suggests that `__gcov_dump()` should be called before exec() because otherwise a successful exec will cause the information to be discarded, and `__gcov_reset()` should be called after exec() returns, because that means exec failed and the information wasn't discarded, so coverage stats should continue to be gathered without duplicating the info that's already been dumped. Doing the reset unconditional
...

💬 ajtowns commented on pull request "test, subprocess: Improve coverage report correctness":
(https://github.com/bitcoin/bitcoin/pull/30075#discussion_r1596146092)
These lines are indented too much compared to surrounding code.

Are they necessary at all?

⚠️ grizznaut opened an issue: "RPC: Internal bug in `walletprocesspsbt` when non_witness_utxo is not provided and a witness signature is invalid"
(https://github.com/bitcoin/bitcoin/issues/30077)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Calling `walletprocesspsbt` on a signed PSBT that only includes a `witness_utxo` (no `non_witness_utxo`) with an invalid signature produces an internal bug (`CHECK_NONFATAL`):

```
❯ bitcoin-cli walletprocesspsbt "cHNidP8BAJoCAAAAAq3g8bgaDs84DkYQ6urdGcuevx4mTtc5ImnEuw/RKiZ2AAAAAAD9////O+qpBnMQ44R69kLMIMPrviyL4ml4zSC1iA8Ec7ux/8YBAAAAAP3///8ClnsOAAAAAAAWABQ1vvbTc5DqIMQ7qVHMKjCDEWY5m+DhDQA
...

💬 fanquake commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1596208350)
Thanks, addressed both.

📝 fanquake opened a pull request: "depends: set AR & RANLIB for CMake"
(https://github.com/bitcoin/bitcoin/pull/30078)
Needed for #21778. Should be more correct in any case.

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1596235623)
Yea. Split into #30078.

💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2103855244)
Darwin Guix build (aarch64):
```bash
1de4299604e1711c90134a94a7ba6bc2685cc43e4acb283a7b3689afba883205 guix-build-c5f2df00a26c/output/arm64-apple-darwin/SHA256SUMS.part
7dcb77afb8dfa36c39be686827bec06bd39d197faed179b6e17d62492567ce9a guix-build-c5f2df00a26c/output/arm64-apple-darwin/bitcoin-c5f2df00a26c-arm64-apple-darwin-unsigned.tar.gz
2f2bcf41e7b1f7ac28258e6db4600b0230db515f9549dae1c714948c9986c433 guix-build-c5f2df00a26c/output/arm64-apple-darwin/bitcoin-c5f2df00a26c-arm64-apple-darwin
...

💬 fanquake commented on pull request "build: swap cctools otool for llvm-objdump":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2103926877)
> There is a mismatch on the windows debug zip :/

I've rebuilt, and see matching now (you and Sjors):
```bash
67f4db93b4f985590948dc344306042841ab747b40ad73bf97ba62d77eb7bfd4 guix-build-7f5ac4520d15/output/aarch64-linux-gnu/SHA256SUMS.part
62e019f164d3c8cf8a1a649b2caa500dd85b00a319910feaf97658020b8cd1e3 guix-build-7f5ac4520d15/output/aarch64-linux-gnu/bitcoin-7f5ac4520d15-aarch64-linux-gnu-debug.tar.gz
c2227a058cf91c0bc3eb244cb254c3c6fcecddc21ba619bca8839d9d79bb1961 guix-build-7f5ac452
...

💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2104027691)
@wiz the v28 release isn't going to happen sooner, so I assume you'd like this merged so people can more easily test on the master branch? That's probably fine - worst case people need to delete their `~/.bitcoin/testnet4` directory when the release comes out.

💬 Sjors commented on issue "BIP352 tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28536#issuecomment-2104063959)
You can point to the actual BIP now in the description 🎉

💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1596384279)
I fully agree what you've said. I'm leaving as is for now. If #26593 is merged and I retouch this, I'll reconsider changing this. I'll leave this as "unresolved" for now.

💬 Sjors commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#issuecomment-2104114821)
tACK 57a06646952fed98c1c281f02fe58a0758a8ed5a

It seems that BIP341 introduced H as just an example of a NUMS point:

> One example of such a point is H

The BIP got it from libsecp256k1 which IIUC only uses it for tests. As does Bitcoin Core so far.

[BIP352](https://github.com/bitcoin/bips/blob/master/bip-0352.mediawiki) gives it special status:

> The one exception is script path spends that use NUMS point H as their internal key

💬 S3RK commented on pull request "crypto: add `NUMS_H` const":
(https://github.com/bitcoin/bitcoin/pull/30048#issuecomment-2104126580)
Code Review ACK 57a06646952fed98c1c281f02fe58a0758a8ed5a

The constant definition is consistent with BIP-341. PR removes code duplication and provides a single place to refer to `NUMS_H` either as vector of bytes or x-only pubkey.

👍 laanwj approved a pull request: "net: log connections failures via SOCKS5 with less severity"
(https://github.com/bitcoin/bitcoin/pull/30064#pullrequestreview-2049515955)
Agree these don't need to be logged at high severity, this matches our general idea that network problems on the open internet are normal and there's no reason to scream at the user for them.
ACK 7524aaf58f5dbe077d4997b3a5dcf635a027a659

💬 willcl-ark commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29888#issuecomment-2104165366)
reACK bd5860bc7a892c6bcffe313246dd6b81b973b9c6

💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1596441523)
dropped in 8109319e102c41d3aeed0ecfbc3a0e23b7fea807

👍 kristapsk approved a pull request: "net: log connections failures via SOCKS5 with less severity"
(https://github.com/bitcoin/bitcoin/pull/30064#pullrequestreview-2049587375)
cr utACK 7524aaf58f5dbe077d4997b3a5dcf635a027a659

👍 stickies-v approved a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29888#pullrequestreview-2049591642)
re-ACK bd5860bc7a892c6bcffe313246dd6b81b973b9c6