🤔 hebasto reviewed a pull request: "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings"
(https://github.com/bitcoin/bitcoin/pull/33247#pullrequestreview-3254612614)
Approach ACK 07027afa155f8ec9715c761d80630b31723c2c32.

The comment "`TODO: The `CMAKE_SKIP_BUILD_RPATH` variable setting can be deleted...`" does not imply that anything is deprecated. Could you please update the PR description and commit message accordingly?

💬 hebasto commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#discussion_r2369869582)
It's outside the scope of this PR, but `CMAKE_SKIP_INSTALL_RPATH` should be set in the Guix script, keeping the build system free of hardcoded behaviour.

💬 hodlinator commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#issuecomment-3320876003)
Latest push has somewhat simpler code (no `found`-list). Tested performance again, similarly to https://github.com/bitcoin/bitcoin/pull/33423#pullrequestreview-3244741286:
```
₿ git co 2025/09/assert_debug_log_rebased~3
₿ hyperfine -r 30 -N ./build/test/functional/feature_assumevalid.py
Benchmark 1: ./build/test/functional/feature_assumevalid.py
Time (mean ± σ): 6.034 s ± 0.047 s [User: 4.260 s, System: 0.488 s]
Range (min … max): 5.964 s … 6.146 s 30 runs

₿ git co
...

📝 theStack opened a pull request: "doc: remove unrelated `bitcoin-wallet` binary from `libbitcoin_ipc` description"
(https://github.com/bitcoin/bitcoin/pull/33459)
`bitcoin-wallet` as-is is merely an offline wallet inspection tool (introduced more than 9 years ago in PR #13926) that doesn't have any relation with IPC/multiprocess, so remove it from the list of binaries that use `libbitcoin_ipc`.

👍 ryanofsky approved a pull request: "rpc: Handle -named argument parsing where '=' character is used"
(https://github.com/bitcoin/bitcoin/pull/32821#pullrequestreview-3254620368)
Code review ACK ad6d740623f01bbe90cbf328590ccfae602c2a3f, just updating documentation and converting class into a namespace since last review.

I left some comments below, but feel free to ignore them, they are just explaining previous suggestions not adding anything new.

💬 ryanofsky commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2369951983)
In commit "rpc: Handle -named argument parsing where '=' character is used" (8eafb1e9ee4fff3a9ab4a7dc899bd2d57c29d04f)

New example below is nice, and feel free to ignore this feedback, but IMO lines 437-447 are not as good as the earlier suggestion for a few reasons:

- They is saying what code is doing without reasons for doing those things.
- They are not acknowledging the -named syntax is ambiguous, which seems an important and non-obvious fact.
- The first two sentences seem to contra
...

💬 ryanofsky commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2369876273)
In commit "rpc: Handle -named argument parsing where '=' character is used" (8eafb1e9ee4fff3a9ab4a7dc899bd2d57c29d04f)

Note: namespace contents is typically not indented: https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c

I also think entire namespace could be replaced with a `ParamFind` function

https://github.com/bitcoin/bitcoin/blob/da919fb63c519ac91c9704c58f1bbe522d7a8879/src/rpc/client.cpp#L384-L389

Which would deduplicate and remove a level of
...

💬 ryanofsky commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2369939396)
In commit "rpc: Handle -named argument parsing where '=' character is used" (8eafb1e9ee4fff3a9ab4a7dc899bd2d57c29d04f)

Woud suggest replacing `parameter with PARAM_NAME="my".` with `parameter named "my"` so this is shorter and more understandable without the previous context.

💬 moonsettler commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3321131775)
> I would prefer a PR improving the deprecation message to make it clear that we are discouraging the use of the option, but not planning to remove it (for the moment).

That is also fine. I have not changed my mind on this, I considered removing the option a bad move to begin with. This option being configurable made even less practical sense in the past than it has going forward.

👍 hodlinator approved a pull request: "Modernize use of UTF-8 in Windows code"
(https://github.com/bitcoin/bitcoin/pull/32380#pullrequestreview-3254889896)
ACK b74a92cdb2c4d9871865a67162f47f97fb7e642c

Latest push adds missing/previously avoided call to `SetupEnvironment()` (discovered in https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2367130713), this entails also adding a dependency from bitcoin.exe on bitcoin_common.

Tested Windows-native build, running `bitcoin node`.

💬 Dorex45 commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3321181329)
Cant wait for the release of V 30,Bitcoin sidechains are in for a massive ride,Bitcoin utilities are yet to be unlocked tremendously.
V30 will champion the future of Bitcoin.

💬 BitcoinMechanic commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3321194786)
Slating configurability for removal is one of three issues with https://github.com/bitcoin/bitcoin/pull/32406 which this fixes and is strictly an improvement.

The other two issues - datacarriersize no longer working correctly and the setting of it to a malicious default - both remain.

In that context, all this specific PR serves to do is help manufacture consent for an unjustified change in attitude towards arbitrary data by making the most mild concession possible to those against it, whi
...

⚠️ RandyMcMillan opened an issue: "cli:passing non-integers to datacarriersize"
(https://github.com/bitcoin/bitcoin/issues/33460)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

While testing #33453

<img width="1847" height="1043" alt="Image" src="https://github.com/user-attachments/assets/a6ef5108-4ec1-4802-b061-14335cd8e84e" />


recommend reverting https://github.com/bitcoin/bitcoin/commit/63091b79e70b8e230a122fa6fb3dac91c80638e7
and adding additional tests.

cc: @bitschmidty @hebasto



This reminds me of an old issue. Passing di-graphs as values.

### Ex
...

💬 hebasto commented on pull request "msvc: Update vcpkg manifest":
(https://github.com/bitcoin/bitcoin/pull/33408#issuecomment-3321239131)
> nit: Discovered image blurring/smoothing worsened from master to PR. Here are two native Windows builds:

A quick search turned up a similar bug report: https://bugreports.qt.io/browse/QTBUG-136807.

💬 Dorex45 commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3321249513)
Release schedule for v30.0 ACK.

💬 bitschmidty commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3321265268)
> this specific PR

This specific PR seeks clarity in the datacarrier options' future availability, in favor of removing the deprecation, as the deprecation designation definition is unclear, potentially incorrect (if the definition is will be removed), and causes uncertainty for Bitcoin Core users around timing.

💬 hodlinator commented on pull request "msvc: Update vcpkg manifest":
(https://github.com/bitcoin/bitcoin/pull/33408#issuecomment-3321301575)
> A quick search turned up a similar bug report: https://bugreports.qt.io/browse/QTBUG-136807.

Yes, eerily similar, but it's not explicitly comparing *6.8* to 6.9 (although the bug is on 6.9). It's comparing FreeType to native Windows font rendering.

Also notice that even the cut off corners in the Qt logo are different in my screenshots, which I'm guessing is not font-related.

💬 Ademan commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3321385143)
utACK 451ba9a

I'd strongly caution against backporting this unless there's clear precedent, however. (I'm not claiming there is or isn't clear precedent)

💬 fanquake commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#issuecomment-3321448261)
Backported to 30.x in #33424.

💬 fanquake commented on pull request "system: improve handling around GetTotalRAM()":
(https://github.com/bitcoin/bitcoin/pull/33435#issuecomment-3321448632)
Backported to 30.x in #33435.