💬 fanquake commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2404501882)
> What is the status/expectation of this? I presume at some point the switch will happen either way and right now it is just waiting for a good enough reason?

The status is mostly that. It has been useful to push occasionally and see what breaks. If there are some new compelling reasons to switch we could consider it. I'll take it out of draft, and if anyone wants to leave (conceptual) review they can.

💬 fanquake commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2404502351)
cc @maflcko. Do you have an opinion here?

👋 fanquake's pull request is ready for review: "guix: use GCC 13 to build releases"
(https://github.com/bitcoin/bitcoin/pull/29881)

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