💬 fanquake commented on pull request "test: add coverage for unknown address type for `createwalletdescriptor`":
(https://github.com/bitcoin/bitcoin/pull/31635#issuecomment-2582849166)
https://github.com/bitcoin/bitcoin/actions/runs/12711073565/job/35433597593?pr=31635#step:7:44312:
```bash
test 2025-01-10T14:30:30.975000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/runner/work/_temp/test/functional/test_framework/util.py", line 160, in try_rpc
fun(*args, **kwds)
File "/home/runne
...

💬 fanquake commented on issue "cmake inconsistently overriding `-O3` (sometimes)":
(https://github.com/bitcoin/bitcoin/issues/31491#issuecomment-2582866149)
> Instead, I think we only want to do that if we're not overriding the user. Essentially the same as: https://github.com/bitcoin/bitcoin/blob/28.x/configure.ac#L298

So should we also be documenting that our `-DCMAKE_BUILD_TYPE=Release` doesn't give `-O3` (given it's the expected CMake default)? As users explicitly setting that are expecting `-O3`, without having to also provide it via `CMAKE_CXX_FLAGS_RELEASE`, but still wont get it via master or your branch above.

💬 brunoerg commented on pull request "test: add coverage for unknown address type for `createwalletdescriptor`":
(https://github.com/bitcoin/bitcoin/pull/31635#issuecomment-2582866679)
Thanks, @fanquake. Fixed.

💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2582882222)
I guess `bitcoin` is intentionally not added to `installable_targets` because it makes reference to the multiprocess binaries that aren't shipped?

I made a Windows guix build for 044c1129db06983da598f427dff85513d8480b3a. It got misidentified as `Trojan:Script/Wavatac.B1ml` again. But since the `bitcoin` binary isn't included yet, it can't be because of the `execvp` call.

Just to be sure, I also tried a guix build for master @ 62bd61de110b057cbfd6e31e4d0b727d93119c72 that this PR is based o
...

👍 brunoerg approved a pull request: "descriptor: Add proper Clone function to miniscript::Node"
(https://github.com/bitcoin/bitcoin/pull/30866#pullrequestreview-2542646880)
code review ACK 352391c2cf1a45231ae92ca92d2415b3786ab9ad

💬 stickies-v commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1910489320)
Everything in your observation looks correct to me, yeah. It's fine in master but not in this PR.

📝 crStiv opened a pull request: "fix: grammatical errors"
(https://github.com/bitcoin/bitcoin/pull/31636)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 Sjors commented on issue "Stratum v2 via IPC Mining Interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/31098#issuecomment-2582939898)
I moved Windows support to "can wait for later releases" and expanded the item a bit.

👍 hebasto approved a pull request: "depends: add *FLAGS to gen_id"
(https://github.com/bitcoin/bitcoin/pull/31125#pullrequestreview-2542706349)
ACK 01df180bfb82c7eafac4638ced249bee4409784b.

It might be better to place this [comment](https://github.com/bitcoin/bitcoin/pull/31125/files#r1878548250) in a more prominent location, such as the PR description or within a source code comment.

🚀 fanquake merged a pull request: "wallet: Cleanup accidental encryption keys in watchonly wallets"
(https://github.com/bitcoin/bitcoin/pull/28724)

💬 Sjors commented on pull request "Add multiprocess binaries to release build (except Windows)":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2582967477)
Rebased to see if nothing new breaks. Clarified the PR description to indicate Windows support (probably) won't make it into this PR.

As described in the TODO, before marking this as ready review, there's one UndefinedBehaviorSanitizer left to resolve upstream in libmultiprocess. I might also wait for #31375 and #31161 to land first (added to description).

💬 0xB10C commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2582976824)
Concept ACK

⚠️ Lin-web-AI opened an issue: "比特币"
(https://github.com/bitcoin/bitcoin/issues/31637)
********@@********

✅ pinheadmz closed an issue: "比特币"
(https://github.com/bitcoin/bitcoin/issues/31637)

:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/31637)

✅ maflcko closed a pull request: "fix: grammatical errors"
(https://github.com/bitcoin/bitcoin/pull/31636)

💬 maflcko commented on pull request "fix: grammatical errors":
(https://github.com/bitcoin/bitcoin/pull/31636#issuecomment-2583000205)
historic release notes are archived and should not be modified

💬 fanquake commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#issuecomment-2583006911)
https://github.com/bitcoin/bitcoin/actions/runs/12700499877/job/35403281996?pr=31622#step:12:8987:
```bash
test 2025-01-10T00:28:24.738000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "D:\a\bitcoin\bitcoin\build\test\functional\test_framework\test_framework.py", line 135, in main
self.run_test()
File "D:\
...

💬 fanquake commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2583015317)
https://cirrus-ci.com/task/6223844022681600?logs=ci#L2898:
```bash
[08:40:37.725] + LD_LIBRARY_PATH=/ci_container_base/depends/i686-pc-linux-gnu/lib
[08:40:37.725] + test/functional/test_runner.py --ci -j10 --tmpdirprefix /ci_container_base/ci/scratch/test_runner/ --ansi --combinedlogslen=99999999 --timeout-factor=40 --v2transport --quiet --failfast
[08:40:38.317] 2025-01-10T13:40:38.202000Z TestFramework (INFO): PRNG seed is: 5569585375096173377
[08:40:38.317] 2025-01-10T13:40:38.206000Z T
...

💬 Sjors commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2583073982)
Concept ACK

Lightly tested 8c44a947b501c8417454527637bd3adb3ce2ff4e by building on macOS 15.2 (M4). I like how the documentation is changed with a scripted-diff. The build system changes in the first commit look reasonable to me at first glance, but did not review thoroughly.

@ryanofsky wrote:

> Possibly, it might make sense to build bitcoin-chainstate.exe and bitcoinkernel.dll to libexec/ instead of bin/.

I like that as well. If we add them to the release later, they won't clutter `
...