👍 hebasto approved a pull request: "build: add root dir to CMAKE_SYSTEM_PREFIX_PATH in toolchain"
(https://github.com/bitcoin/bitcoin/pull/32798#pullrequestreview-2953832427)
ACK e27a94596f2a1f5e04722a16165717cc6e891d36, I have reviewed the code and it looks OK.

💬 hebasto commented on pull request "build: add root dir to CMAKE_SYSTEM_PREFIX_PATH in toolchain":
(https://github.com/bitcoin/bitcoin/pull/32798#discussion_r2164000499)
To be precise, "Adding it back ... on other platforms" does not occur on every platform that supports our depends build subsystem. According to the CMake source code, [`Platform/UnixPaths`](https://gitlab.kitware.com/cmake/cmake/-/blob/master/Modules/Platform/UnixPaths.cmake) is processed on all platforms: Linux, Darwin, and all *BSD.

⚠️ plebhash opened an issue: "IPC via TCP Sockets"
(https://github.com/bitcoin/bitcoin/issues/32802)
### Please describe the feature you'd like to see added.

Interprocess communication via TCP Sockets

### Is your feature related to a problem, if so please describe it.

I'm dockerizing @Sjors 's Stratum V2 sidecar (`bitcoin-mine`) such that it's able to reach a Bitcoin Core node running on the host.

My host is macOS, and Docker is Linux, which means they don't have a shared kernel and their Unix socket descriptors don't match.

### Describe the solution you'd like

IPC over TCP sockets.

###
...

💬 instagibbs commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3000571610)
concept and approach ACK

I have a preference for breaking out the new checks into its own function since if we intend it to someday soon(TM) be a consensus rule it would have to be pulled out anyways. https://github.com/instagibbs/bitcoin/commit/5d63372f75c4a2403c4032b63e5c604ee96c5a40

💬 instagibbs commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2164070208)
Could also just Assume it's higher than `FeeFrac{1, 1}` like later

💬 Sjors commented on issue "IPC via TCP Sockets":
(https://github.com/bitcoin/bitcoin/issues/32802#issuecomment-3000589480)
`bitcoin-mine` refers to the Template Provider application I wrote in https://github.com/Sjors/bitcoin/pull/48. This is not intended to be include in Bitcoin Core.

It's based on the `bitcoin-mine` demo proposed by @ryanofsky in #30437. This may or may not be included in v30, but it doesn't really matter if this particular demo adds TCP support.

What does matter is that the `-ipcbind` argument of `bitcoin-node` gets TCP support.

🤔 rkrux reviewed a pull request: "descriptors: MuSig2"
(https://github.com/bitcoin/bitcoin/pull/31244#pullrequestreview-2953212032)
I have finished reviewing the `descriptor: Add MuSigPubkeyProvider` commit - 4039be3f9b78c1ddb75c04f4a8002d36c4b05f79

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2163953475)
I see, `out` does need to be filled. In that case, there is an option to cache the `dummy` signing provider on line 629 when the aggregate pubkey & provider are being created (and cached) in case of non ranged participants. This could later be `FlatSigningProvider::Merge`'d with the `out` signing provider on line 672 so that `out` has information for both the participants and the aggregate.

The reason I find moving this snippet inside the `else` block here appealing is that we don't need to d
...

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2163599033)
Nice defaulting to using the public key incase private key is not present. This is a more forward facing solution that aligns with the resolution of the existing issue #32078.

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2163760571)
Nit: can add an `Assume` here for `m_path` being empty keeping in mind this error.

> "musig(): Cannot have ranged participant keys if musig() also has derivation"

💬 rkrux commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2164092082)
```diff
- KeyOriginInfo info;
- CKeyID keyid = aggregate_pubkey->GetID();
- std::copy(keyid.begin(), keyid.begin() + sizeof(info.fingerprint), info.fingerprint);
- out.origins.emplace(keyid, std::make_pair(*aggregate_pubkey, info));
- out.pubkeys.emplace(aggregate_pubkey->GetID(), *aggregate_pubkey);
+ auto temp_aggregate_provider = std::make_unique<ConstPubkeyProvider>(m_expr_index, aggregate_pubkey.value(), /*xonly=*/false
...

💬 josibake commented on pull request "build: add root dir to CMAKE_PREFIX_PATH in toolchain":
(https://github.com/bitcoin/bitcoin/pull/32798#issuecomment-3000812206)
reACK https://github.com/bitcoin/bitcoin/commit/e27a94596f2a1f5e04722a16165717cc6e891d36

The only change is to use `CMAKE_PREFIX_PATH` over `CMAKE_SYSTEM_PREFIX_PATH`, which is more in-line with CMake's documentation as mentioned in https://github.com/bitcoin/bitcoin/pull/32798#pullrequestreview-2953470821

💬 polespinasa commented on pull request "init: make `-blockmaxweight` startup option debug only":
(https://github.com/bitcoin/bitcoin/pull/32654#discussion_r2164247671)
That's right. It is checked during stopping of the node. Had to introduce that in https://github.com/bitcoin/bitcoin/commit/bf194c920cf768d1339d41aef1441a78e2f5fcbe#diff-4a04bc0b355c780033960e8c261ee9b6d3c452897e1dcd88a15d272512266c76

Didn't find a better way to handle it though (at that moment). Probably we could just remove that stderr check and restart the node manually using first stop and then start. That could be clearer?

👍 l0rinc approved a pull request: "headerssync: Preempt unrealistic unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-2954075447)
I find the current layout of tiny, focused commits a lot easier to follow, thanks!
Left a few comments, after that I will test it again locally and I can ACK.

💬 l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2164197118)
this comment is obvious from the code now: `/*exp_locator_hash=*/Params().GenesisBlock().GetHash()` or `/*exp_locator_hash=*/genesis_hash` later in 65a96b507c11b3f34efa996919892fa1a2fcf49c

💬 l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2164220997)
is this comment accurate?

💬 l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2164235547)
```suggestion
// 5. Sets the redownload buffer size to be large enough that we
```

💬 l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2164194785)
would't an `.at(0)` suffice?

💬 l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2164239931)
nice, this is exactly how comments should be used to augment what the code cannot easily tell <3

💬 l0rinc commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2164212315)
I'm not sure what "this many headers *on top* have been received"