👍 laanwj approved a pull request: "subprocess: Backport upstream changes"
(https://github.com/bitcoin/bitcoin/pull/32567#pullrequestreview-2857235239)
Code review ACK e63a7034f0386789628dcb940d99ec6436d21128

💬 laanwj commented on pull request "Use subprocess library for notifications":
(https://github.com/bitcoin/bitcoin/pull/32566#discussion_r2099983137)
TIL that upstream subprocess has a `shell` option that does the equivalent (https://github.com/arun11299/cpp-subprocess/blob/master/cpp-subprocess/subprocess.hpp#L1630) . But we removed it.

Unfortunately, currently it doesn't support Windows. Could revert the option and switch to using that, when it does. Or, unless they had reason to not do it in the trivial way we do, it seems easy to add.

💬 laanwj commented on pull request "ci: Avoid && dropping errors":
(https://github.com/bitcoin/bitcoin/pull/32573#issuecomment-2897540714)
> My preference would be to just stop using bash

Yes.

> but writing this one in Python or Rust will bloat the code. 🤷‍♂️

That's good and bad, i mean bloated code that is straightforward to review might be preferable to a one-line magic incantation that has all kinds of unintuitive, hidden oopses.

👍 laanwj approved a pull request: "ci: Avoid && dropping errors"
(https://github.com/bitcoin/bitcoin/pull/32573#pullrequestreview-2857276533)
ACK fab97f583f119f43da352774479dd78e39729632

🚀 fanquake merged a pull request: "subprocess: Backport upstream changes"
(https://github.com/bitcoin/bitcoin/pull/32567)

💬 janb84 commented on pull request "ci: Avoid && dropping errors":
(https://github.com/bitcoin/bitcoin/pull/32573#issuecomment-2897546693)
> > My preference would be to just stop using bash
>
> Yes.
>
> > but writing this one in Python or Rust will bloat the code. 🤷‍♂️
>
> That's good and bad, i mean bloated code that is straightforward to review might be preferable to a one-line magic incantation that has all kinds of unintuitive, hidden oopses.

I second this !

🚀 fanquake merged a pull request: "ci: Avoid && dropping errors"
(https://github.com/bitcoin/bitcoin/pull/32573)

💬 laanwj commented on pull request "fs: remove `_POSIX_C_SOURCE` defining":
(https://github.com/bitcoin/bitcoin/pull/32460#issuecomment-2897556590)
> Removing the fallback would make review easier here, because if the code compiles, it is more likely to be correct.

Oof. That is a good point. i hadn't realized that the detection will just make this compile even if `posix_fallocate` doesn't end up used, so it might result in a hidden performance regression instead of just a compile error.

💬 ismaelsadeeq commented on pull request "fees: rpc: `estimatesmartfee` now returns a fee rate estimate during low network activity":
(https://github.com/bitcoin/bitcoin/pull/32395#discussion_r2100028491)
Thanks @DrahtBot this is fixed.

💬 laanwj commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2897593795)
> Yea, on our side, this should be enough to close https://github.com/bitcoin/bitcoin/issues/32391.

Yes, the issue refers to our specific use, so imo this closes it.

From the point of "would our GUIX release run on an OS which actually removed that function", it's different, but given how much breakage that would cause to windows software all over the place it would be idiotic for Microsoft to do that any time soon. i was just curious to see if there were more uses (and if so, where).

💬 janb84 commented on pull request "doc: Remove stale sections in dev notes":
(https://github.com/bitcoin/bitcoin/pull/32572#discussion_r2100039328)
Small suggestion also add in CONTRIBUTING to wait for CI to pass. This creates a nice sort of checklist and is inline with the suggested changes to remove the statement from developer-notes and add it to the readme.

Lines 136-137:
- Push changes to your fork
- Wait for the CI to pass
- Create pull request

💬 rkrux commented on issue "Awesome multisig PR labyrinth guide":
(https://github.com/bitcoin/bitcoin/issues/24861#issuecomment-2897605677)
> Alice calls getxpub m/87h/0h/0h on the new wallet

Is this supposed to use the not yet present `gethdkey` RPC instead? Asking because afaik there is no `getxpub` in the core wallet.

💬 vasild commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#issuecomment-2897613233)
`98bba932bf...22d4c57a99`: rebase due to conflicts

📝 glozow converted_to_draft a pull request: "node: cap `-maxmempool` and `-dbcache` values for 32-bit"
(https://github.com/bitcoin/bitcoin/pull/32530)
32-bit architecture is limited to 4GiB of RAM, so it doesn't make sense to set a too high value.

👍 laanwj approved a pull request: "random: Use modern Windows randomness functions"
(https://github.com/bitcoin/bitcoin/pull/32400#pullrequestreview-2857356065)
re-ACK 6b4bcc16234575108bb691c15c3532198d9bf98a
`git range-diff 5b8046a6e893b7fad5a93631e6d1e70db31878af..345944bd6cf8be0c8b85b6d3ccb593ce26c598aa 31d3eebfb92ae0521e18225d69be95e78fb02672..6b4bcc16234575108bb691c15c3532198d9bf98a`:
Only difference: rebase for move of `symbol-check.py`

💬 fanquake commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2100060153)
In 1e826bf91932e2d76969ae40a0c0ef85a990cf2f: Can the commit be updated to better explain why this is needed. Looking at mingw-w64, `GetACP` has returned `CP_UTF8` since it was introduced. I'm guessing the same is happening with MSVC (although that's less relevant given it doesn't effect releases)? Given that, and the fact that according to https://github.com/bitcoin/bitcoin/pull/32380/files#r2096044476, we are just going to assume that a user is running a new enough version of Windows, and throw
...

💬 Sjors commented on issue "Awesome multisig PR labyrinth guide":
(https://github.com/bitcoin/bitcoin/issues/24861#issuecomment-2897630755)
@rkrux yes, fixed. HWI has a `getxpub` command, but in Bitcoin Core it would probably be called `gethdkey` (since it might also fetch the extended _private_ key).

✅ cdecker closed a pull request: "chainparams: Re-add seed.bitcoinstats.com"
(https://github.com/bitcoin/bitcoin/pull/31086)

💬 laanwj commented on pull request "Use subprocess library for notifications":
(https://github.com/bitcoin/bitcoin/pull/32566#issuecomment-2897638499)
Rebased for #32567: [ceb1783c1af07f59885f51fdd15e7143fefdd821 → 82aeba60c2b3a166302e10fbf6e3eaf0593fab4d](https://github.com/bitcoin/bitcoin/compare/ceb1783c1af07f59885f51fdd15e7143fefdd821..82aeba60c2b3a166302e10fbf6e3eaf0593fab4d), which dropped one commit.

📝 hebasto opened a pull request: "subprocess: Handle quoted tokens properly"
(https://github.com/bitcoin/bitcoin/pull/32577)
Fixes https://github.com/bitcoin/bitcoin/issues/32574.

The `subprocess::Popen` constructor has two overloads: https://github.com/bitcoin/bitcoin/blob/7763e86afa045910a14ac9b2cd644927a447370b/src/util/subprocess.h#L938-L941

During the migration from Boost.Process in https://github.com/bitcoin/bitcoin/pull/28981, the second was chosen for two reasons: (1) it minimized changes at the call sites, and (2) it [addressed](https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1837238921) quot
...