💬 maflcko commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1795043415)
> Suggested fix to address both:

Good catch. Though, I don't think the fix fixes all issues. There was a third that the `translatable` member field was unused. I've pushed a fix to fix all three issues.

💬 Sjors commented on pull request "ci: Add missing -DWERROR=ON to test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/31045#issuecomment-2404573726)
It only impacts CI, so is fine by me.

💬 fanquake commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2404575815)
> could we add a safety harness to make sure it's never being used accidentally for a live node?
> Maybe somewhere in init.cpp or bitcoind.cpp:
> Edit: I realize that bitcoind should be disabled in that case. This would be belt-and-suspenders to make sure that always holds.

Just want to note that putting something in either of these files wouldn't be enough to prevent someone producing a kernel lib with the fuzzing code.

💬 fanquake commented on pull request "build: scripted-diff: drop config/ subdir for bitcoin-config.h":
(https://github.com/bitcoin/bitcoin/pull/30937#issuecomment-2404578174)
Guix Build
```bash
a78d6d030a78972bf2ebab6d3599a3ce8b325e5422e192ab97d65966f3ee0ae4 guix-build-09b0161c4a25/output/aarch64-linux-gnu/SHA256SUMS.part
a3ff2330715466bef18e536f5ef4c6c00c9a359e11f896b3afade7364b4e40eb guix-build-09b0161c4a25/output/aarch64-linux-gnu/bitcoin-09b0161c4a25-aarch64-linux-gnu-debug.tar.gz
dcedb6cea8f7799d34d64b9a73deda40452887898a44e6b5a2048b7ebf304924 guix-build-09b0161c4a25/output/aarch64-linux-gnu/bitcoin-09b0161c4a25-aarch64-linux-gnu.tar.gz
aa2b1c6e8fc747872
...

💬 fanquake commented on pull request "build: scripted-diff: drop config/ subdir for bitcoin-config.h":
(https://github.com/bitcoin/bitcoin/pull/30937#issuecomment-2404581733)
```bash
bitcoin/src/common/netif.cpp:5:10: fatal error: 'config/bitcoin-config.h' file not found
5 | #include <config/bitcoin-config.h> // IWYU pragma: keep
| ^~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
```

💬 TheCharlatan commented on pull request "init: Correct coins db cache size setting":
(https://github.com/bitcoin/bitcoin/pull/31064#issuecomment-2404584380)
Updated e24783efe5cc17c17349b8ed96ef852e73ff8309 -> 3a4a788ee0db83d20607f14801dbed2ee932943c ([patchCoinsDBCacheSizeInit_0](https://github.com/TheCharlatan/bitcoin/tree/patchCoinsDBCacheSizeInit_0) -> [patchCoinsDBCacheSizeInit_1](https://github.com/TheCharlatan/bitcoin/tree/patchCoinsDBCacheSizeInit_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/patchCoinsDBCacheSizeInit_0..patchCoinsDBCacheSizeInit_1))

* Addressed @fjahr's [comment](https://github.com/bitcoin/bitcoin/pull/310
...

💬 Sjors commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#issuecomment-2404586122)
Thanks for the feedback, and great job finding that bug @tdb3.

(It might be two weeks before I get to updating this PR)

💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1795064562)
Not sure, maybe 29 existed in an earlier rebase. It might just change it to 29.

💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1795066784)
It should match the spec. I'm guessing the comment is wrong, because otherwise it wouldn't work against SRI, but will check.

💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1795071626)
It's only used in tests, but I think we should review the code in both directions under the assumption that they'll be used in production.

💬 l0rinc commented on pull request "init: Correct coins db cache size setting":
(https://github.com/bitcoin/bitcoin/pull/31064#discussion_r1795072559)
Is this the same as
```suggestion
auto init_cache_fraction = chainman.IsSnapshotActive() ? 0.2 : 1.0;
```
?

💬 TheCharlatan commented on pull request "init: Correct coins db cache size setting":
(https://github.com/bitcoin/bitcoin/pull/31064#discussion_r1795086831)
I think in practice it is, since we call this after `DetectSnapshotChainstate()`, which in turn calls `ActivateExistingSnapshot` if there is a snapshot chainstate, but checking if there is more than one chainstate seems more defensive to me, since it relies on fewer assumptions.

💬 fanquake commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2404642324)
> It's ready for review, but not for merge:

> > The parent PR may need more conceptual review

This should probably be a draft then. Re more conceptual review, I think it's been made (fairly?) clear by multiple contributors (and from the progress of the sidecar PRs from #29432) which approach is favoured, and it doesn't seem to be the one involving this PR, and implementing all the SV2 logic inside Core.

✅ fanquake closed a pull request: "Add -pausebackgroundsync startup option"
(https://github.com/bitcoin/bitcoin/pull/31023)

💬 fanquake commented on pull request "Add -pausebackgroundsync startup option":
(https://github.com/bitcoin/bitcoin/pull/31023#issuecomment-2404649271)
Going to close this for now, given the feedback.

💬 hodlinator commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2404665924)
One of the [OpenRPC examples](https://github.com/open-rpc/examples/blob/dce69463ba9a3ca2232506b734606fa97f25dd45/service-descriptions/petstore-openrpc.json) has:
```json5
...
"components": {
"contentDescriptors": {
"PetId": {
"name": "petId",
"required": true,
"description": "The id of the pet to retrieve",
"schema": {
"$ref": "#/components/schemas/PetId"
}
}
},
"schemas": {
"PetId": {
"ty
...

💬 fanquake commented on pull request "build: Bump minimum supported macOS to 12.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2404671671)
> Running Bitcoin Core on unsupported OSes may expose users to security issues.
> macOS Big Sur 11 received its last security update ([11.7.10](https://support.apple.com/en-us/106338)) over a year ago.

If this is the reasoning we are going to use, shouldn't this be bumping to `10.13`, given `10.12` is also "unsupported" in terms of security updates?

💬 theStack commented on pull request "build: scripted-diff: drop config/ subdir for bitcoin-config.h":
(https://github.com/bitcoin/bitcoin/pull/30937#issuecomment-2404737373)
Rebased on master and added another commit fixing the outdated comment in the linter (https://github.com/bitcoin/bitcoin/pull/30937#discussion_r1773075606).

💬 theStack commented on pull request "build: scripted-diff: drop config/ subdir for bitcoin-config.h":
(https://github.com/bitcoin/bitcoin/pull/30937#discussion_r1795181976)
Fixed now.

💬 naumenkogs commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2404771253)
ACK ab45a0654c189da3013ef0c8d30478601052d922