💬 hebasto commented on pull request "test: Suppress Windows abort message in CI":
(https://github.com/bitcoin/bitcoin/pull/32409#issuecomment-2855259932)
Not related to this PR, but the [error in the `wallet_fees` target](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14801894543/job/41562322239#step:8:484):
```
wallet_fees: succeeded against 23 files in 0s.
Run wallet_fees with args ['D:\\a\\bitcoin-core-nightly\\bitcoin-core-nightly\\build\\bin\\Debug\\fuzz.exe', WindowsPath('D:/a/_temp/qa-assets/fuzz_corpora/wallet_fees')]
\u26a0\ufe0f Failure generated from target with exit code 3221225477: ['D:\\a\\bitcoin-core-nightly\\bit
...

🤔 fanquake reviewed a pull request: "test: Suppress Windows abort message in CI"
(https://github.com/bitcoin/bitcoin/pull/32409#pullrequestreview-2818307258)
Not sure about compiler specific changes, that rely on the presence of an undocumented environment variable (which we don't explicitly set as far as I can tell?), to make tests work properly.

💬 fanquake commented on pull request "test: Suppress Windows abort message in CI":
(https://github.com/bitcoin/bitcoin/pull/32409#discussion_r2075491685)
The commit message says "On Windows", but the code is using `_MSC_VER`, not `WIN32`.
Is that a typo, or does this issue only affect MSVC compiled binaries; in which case, why is there a difference in runtime behaviour between the MSVC compiled test binaries & our release compiled test binaries?

💬 theStack commented on pull request "psbt: clarify PSBT, PSBTInput, PSBTOutput unserialization flows":
(https://github.com/bitcoin/bitcoin/pull/32419#issuecomment-2855291909)
Concept ACK

Makes sense to me to provide at least a brief description of how the key data is structured and deserialized, since this is not intuitive at all (as I also noticed in the course of reviewing https://github.com/bitcoin/bitcoin/pull/31247 a while ago).

💬 pablomartin4btc commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2075889487)
nit: the port is optional, no?
```suggestion
"ip[:<port>]|path";
```

🤔 pablomartin4btc reviewed a pull request: "config: allow setting -proxy per network"
(https://github.com/bitcoin/bitcoin/pull/32425#pullrequestreview-2818995555)
If the user sets `-proxy=addr:port=tor` then `-onion=` shouldn't be allowed, no? Otherwise one could set 2 diff proxies for Tor(?).

Also, in [Tor's documentation](https://github.com/bitcoin/bitcoin/tree/master/doc/tor.md):

```
-proxy=ip:port Set the proxy server. If SOCKS5 is selected (default), this proxy
server will be used to try to reach .onion addresses as well.
```

So, shouldn't `-proxy=0=cjdns` be the default if `-cjdnsreachable` when `-proxy=addr:port`?

💬 pablomartin4btc commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2075906033)
nit: for the port value maybe use DEFAULT_TOR_SOCKS_PORT from `torcontrol.cpp`?

💬 yancyribbens commented on pull request "rpc: Undeprecate rpcuser/rpcpassword, store all credentials hashed in memory":
(https://github.com/bitcoin/bitcoin/pull/32423#issuecomment-2855300344)
> As for the security risk, in many kinds of setups (no wallet, containerized, single-user-single-application, local-only, etc) it is an unlikely point of escalation

This probably applies to testnet and regtest too.

🚀 fanquake merged a pull request: "[29.x] qt: 29.1 translations update"
(https://github.com/bitcoin/bitcoin/pull/32352)

🚀 fanquake merged a pull request: "Shuffle depends instructions and recommend modern make for macOS"
(https://github.com/bitcoin/bitcoin/pull/32086)

👍 fanquake approved a pull request: "build: Fix `macdeployqtplus` after switching to Qt 6"
(https://github.com/bitcoin/bitcoin/pull/32287#pullrequestreview-2819054059)
ACK 84de8c93e7d4979575161a2bb8f7eb64e1317b89

✅ fanquake closed an issue: "build: `--target deploy` failure on macOS"
(https://github.com/bitcoin/bitcoin/issues/32267)

🚀 fanquake merged a pull request: "build: Fix `macdeployqtplus` after switching to Qt 6"
(https://github.com/bitcoin/bitcoin/pull/32287)

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2075937910)
I think that this wouldn't be correct: We use `CBlockIndexWorkComparator` during normal adding to `setBlockIndexCandidates` ([here](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/src/validation.cpp#L3840C41-L3840C79)) and also use it while asserting that blocks that sort worse are not in the set [here](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/src/validation.cpp#L5412). If we'd insert blocks in the set that have the same `n
...

💬 maflcko commented on issue "Intermittent timeout in tsan feature_init.py":
(https://github.com/bitcoin/bitcoin/issues/30586#issuecomment-2855360892)
Not sure why CI runs into this so rarely. This reproduces easily locally on a fresh install of Ubuntu 25.04 Plucky:

```
1 apt update && apt install -y git ccache build-essential cmake pkg-config python3-zmq libevent-dev libboost-dev qt6-tools-dev qt6-l10n-tools qt6-base-dev systemtap-sdt-dev libqrencode-dev libzmq3-dev libsqlite3-dev
2 git clone https://github.com/bitcoin/bitcoin --depth=1 ./bitcoin-core
3 cd bitcoin-core/
4 apt install -y libc++abi-dev libc++-dev clang ll
...

✅ fanquake closed an issue: "gui: translation spam?"
(https://github.com/bitcoin/bitcoin/issues/32295)

💬 fanquake commented on issue "gui: translation spam?":
(https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2855376554)
Closing for now given #32352.

💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2075959357)
> Actually, it looks like https://github.com/bitcoin/bitcoin/commit/79bef1bc52afab860782d6d9a6016b93f94c4503 is still here

oops, fixed

> releasing cs_main so another thread can change the BI

I wasn't suggesting this, the reason why we are releasing cs_main frequently during invalidateblock in the first place is so that the node/GUI doesn't become unresponsive in case many blocks in a row are disconnected.

In the past, there was a problem with the intermediate state in `InvalidateBloc
...

💬 mzumsande commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2855430075)
> These limits are easily bypassed by both direct submission to miner mempools (e.g. MARA Slipstream)

💬 ryanofsky commented on issue "Depends toolchain doesn't contain enough info to build from depends on a fresh NixOS install":
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2855454246)
I can confirm Will's commit in 437a4da93f437f9588ff32d19ac59fea3802c676 fixes the problem for me and seems like a good solution. But I think it can be simplified to only just `list(APPEND CMAKE_PREFIX_PATH "${CMAKE_CURRENT_LIST_DIR}")` and the if() guard should be unnecessary.

I also confirmed the other fix appending `-DCMAKE_PREFIX_PATH=/` to the command line from https://github.com/hebasto/bitcoin/issues/121#issuecomment-2015274066 works. I guess this works because cmake appends CMAKE_FIND_RO
...