💬 optout21 commented on pull request "Improve LastCommonAncestor performance + add tests":
(https://github.com/bitcoin/bitcoin/pull/33515#issuecomment-3371426440)
> However, timing the tests (pre and post optimization) I could not measure a difference.

Most of the CPU time in the test is taken by the `NaiveGetAncestor` and `NaiveLastCommonAncestor` naive implementations. Taken out calls to those, I have measured a 47% speedup in the execution of the test (user time).

💬 rkrux commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3371437153)
> Sidenote: currently it's unlikely you'll accidentally spend via an `older()` or `after()` script path. This is because you need to manually set the input `sequence` field in the `send` (etc.) RPC (and manually adjust the fee rate). Found out the hard way while testing 🤦 (there's currently no useful feedback when this happens).

**Scenario that I have not tested but expect to behave in this way:** 3 signers (A, B, C) form a MuSig (ABC) in the `keypath` and also have 3 non-Musig multi-sig scr
...

💬 rkrux commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3371447238)
If #32876 is not garnering reviews, can consider undrafting this PR itself because it contains 7 commits only.
Allowing skipping script path spending optionally seems reasonable to me and a ready-to-review PR is more inviting than a draft one.

💬 ryanofsky commented on pull request "argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)":
(https://github.com/bitcoin/bitcoin/pull/33540#issuecomment-3371531256)
Haven't looked at code yet, so forgive if these are dumb questions, but could be nice if PR description addressed these:

- Is there a way to pass `bitcoin-cli` RPC arguments that begin with a `-` character? Can you escape these arguments with `--` for example?

- What's the PR's goal for backwards compatibility? Is the PR only allowing command lines that were disallowed before, and therefore fully backwards compatible? Or are there some command lines which used to be accepted which will now
...

💬 ryanofsky commented on pull request "argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)":
(https://github.com/bitcoin/bitcoin/pull/33540#issuecomment-3371562956)
Concept ACK on the idea. I think it probably makes sense to allow options arguments to follow non-option arguments even this change is not strictly backwards compatible, as long as there is some way to escape non-option arguments beginning with `-`, such as with a `--` separator.

📝 maflcko opened a pull request: "ci: Use ARM instead of Intel in macOS cross task"
(https://github.com/bitcoin/bitcoin/pull/33549)
Cross compiling to Intel macOS seems fine, but it has to be switched at some point, see https://en.wikipedia.org/wiki/Mac_transition_to_Apple_silicon#Timeline.


Also, if the executable were to be run:

* Running the Intel executable in Rosetta 2 is slower
* It is harder to find native Intel macOS hardware (E.g. GitHub is in the process of dropping it: https://github.blog/changelog/2025-07-11-upcoming-changes-to-macos-hosted-runners-macos-latest-migration-and-xcode-support-policy-updates/#
...

💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3371932129)
ACK 0f1f02995b715a83f71b1b8fc92fa520625ec427

`git range-diff master a161f058089dfe5f3aa7d3b9635f0400f0147b5d 0f1f02995b715a83f71b1b8fc92fa520625ec427`

💬 ryanofsky commented on pull request "argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)":
(https://github.com/bitcoin/bitcoin/pull/33540#issuecomment-3372043307)
Assuming we are ok with losing some backwards compatibility by starting to treating arguments that begin with `-` as options instead of non-options when they follow arguments that don't begin with `-`, there could be other interesting ways to extend this PR.

For example, `bitcoin-cli` could distinguish between options that begin with single and double dashes, and treat options after the RPC method name that begin with double dashes as RPC named parameters. In other words, accept:

```bash

...

✅ maflcko closed a pull request: "ci: Check macos-cross executables on macOS"
(https://github.com/bitcoin/bitcoin/pull/33509)

💬 maflcko commented on pull request "ci: Check macos-cross executables on macOS":
(https://github.com/bitcoin/bitcoin/pull/33509#issuecomment-3372189103)
> Since we don't have high hopes of this actually detecting any issues that the rest of the CI would not have found, seems to make more sense as a nightly job?

Thx, done in https://github.com/bitcoin/bitcoin/pull/33549 and https://github.com/maflcko/b-c-nightly/actions/runs/18282485376/job/52056370818

📝 ryanofsky opened a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550)
As reported by hebasto in https://github.com/bitcoin/bitcoin/issues/33545, newer libc++ versions implementing https://wg21.link/lwg3430 will no longer implicitly convert `fs::path` objects to `std::filesystem::path` objects when constructing `std::ifstream` and `std::ofstream` types.

This is not a problem in Unix systems since `fs::path` objects use `std::string` as their native string type, but it causes compile errors on Windows which use `std::wstring` as their string type, since `fstream`
...

💬 ryanofsky commented on issue "Implicit conversion from `fs::path` to `std::string` when constructing file streams":
(https://github.com/bitcoin/bitcoin/issues/33545#issuecomment-3372209399)
#33550 could be a good fix for this

🤔 hebasto reviewed a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305372031)
Concept ACK.

Maybe consider removing these lines now:https://github.com/bitcoin/bitcoin/blob/f39d3e1a2f2b9cdcf0e77942fd69eb92a58ce77f/src/util/fs.h#L79-L81 ?

💬 hebasto commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372250792)
CI [fails](https://github.com/bitcoin/bitcoin/actions/runs/18285430177/job/52059106747):
```
/home/admin/actions-runner/_work/_temp/src/wallet/test/init_test_fixture.cpp:39:21: error: conversion function from 'mapped_type' (aka 'fs::path') to 'const string' (aka 'const basic_string<char>') invokes a deleted function
39 | std::ofstream f{m_walletdir_path_cases["file"]};
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/admin/actions-runner/_work/_temp/src/util/fs.h:65:
...

💬 maflcko commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372288022)
review ACK f39d3e1a2f2b9cdcf0e77942fd69eb92a58ce77f, but CI fails

💬 ryanofsky commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372301217)
<!-- begin push-2 -->
Updated f39d3e1a2f2b9cdcf0e77942fd69eb92a58ce77f -> b0113afd44b4c7c0d0da9883424bd2978de3d18c ([`pr/winstream.1`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.1) -> [`pr/winstream.2`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/winstream.1..pr/winstream.2))<!-- end --> to fix CI failures caused by missing std_path replacement

💬 ryanofsky commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372384215)
re: https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305372031

> Maybe consider removing these lines now:

Nice suggestion. Attempted this in new commit.

<!-- begin push-3 -->
Added 1 commits b0113afd44b4c7c0d0da9883424bd2978de3d18c -> d405c76d2e32e957a72c042f1fb82fd0cfcc8e6d ([`pr/winstream.2`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.2) -> [`pr/winstream.3`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.3), [compare](https://github.com/ryan
...

💬 hebasto commented on pull request "Fix windows libc++ `fs::path` `fstream` compile errors":
(https://github.com/bitcoin/bitcoin/pull/33550#issuecomment-3372389642)
> re: [#33550 (review)](https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305372031)
>
> > Maybe consider removing these lines now:
>
> Nice suggestion. Attempted this in new commit.
>
> Added 1 commits [b0113af](https://github.com/bitcoin/bitcoin/commit/b0113afd44b4c7c0d0da9883424bd2978de3d18c) -> [d405c76](https://github.com/bitcoin/bitcoin/commit/d405c76d2e32e957a72c042f1fb82fd0cfcc8e6d) ([`pr/winstream.2`](https://github.com/ryanofsky/bitcoin/commits/pr/winstream.2) -
...

📝 fanquake opened a pull request: "[29.x] Finalise 29.2"
(https://github.com/bitcoin/bitcoin/pull/33551)
I'm optimistic that 29.2 wont need an `rc3`.

🤔 hebasto reviewed a pull request: "Fix windows libc++ `fs::path` `fstream` compile errors"
(https://github.com/bitcoin/bitcoin/pull/33550#pullrequestreview-3305683677)
Tested b0113afd44b4c7c0d0da9883424bd2978de3d18c by cross-compiling for `x86_64-w64-mingw32` and `aarch64-w64-mingw32` targets using the [LLVM MinGW](https://github.com/mstorsjo/llvm-mingw) toolchain.