💬 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

💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1795202925)
> Addressed in https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2402990346.

How was it addressed? I still see the same output using 2d009aa8e861c161820a4c0619ecd4b2329267af:
```bash
make -C depends -j12 HOST=x86_64-apple-darwin
gmake: Entering directory '/bitcoin/depends'
Fetching qtbase-everywhere-src-6.7.3.tar.xz from https://download.qt.io/official_releases/qt/6.7/6.7.3/submodules
% Total % Received % Xferd Average Speed Time Time Time Current

...

💬 maflcko commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2404783263)
> I think we could consider removing the honggfuzz netdriver instructions in a follow

Not sure about touching real code with the motivation to make instructions happy that may be removed anyway.

If the changes are meaningful for the fuzz target, then it may be fine.

> It also makes the `p2p_transport_serialization` fuzz target simpler since we do not need to assist the network magic and checksum creation anymore.

Can you provide more details here? IIUC a hash will still be calculated
...

👍 itornaza approved a pull request: "rpc: getorphantxs follow-up"
(https://github.com/bitcoin/bitcoin/pull/31043#pullrequestreview-2360026747)
Concept ACK ae17f9050e0c2a6fd31cd9f5d2f3ad8066f02cc9

Enriches the functional tests for the the `getorphantxs` rpc and offers added functionality to conveniently access the expiration times of transactions in the orphanage.

Broadening the context with refactoring from `rpc_getorphantxs.py` to `rpc_orphans.py` as a future provision for more orphanage rpcs looks good to me.