💬 l0rinc commented on pull request "fuzz: Add target for coins database":
(https://github.com/bitcoin/bitcoin/pull/32602#discussion_r2104819427)
can you please add a short comment to the `best_block = uint256::ONE` cases?

💬 rkrux commented on pull request "wallet, rpc, doc: various legacy wallet removal cleanups in RPCs":
(https://github.com/bitcoin/bitcoin/pull/32596#discussion_r2104831591)
Ah, I note now that its presence was conditional on legacy wallets.

💬 maflcko commented on pull request "test: Fix `system_tests/run_command` test":
(https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2104835433)
Then it seems like it should be fixed in `RunCommandParseJSON`, because real users could run into the bug as well?

`RunCommandParseJSON` should just take a string and then pass it to the shell as-is?

💬 hebasto commented on pull request "subprocess: Let shell parse command on non-Windows systems":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2904837194)
> * It would probably be good if the new behavior were controlled by a shell boolean option like python & the upstream library

Reverting 633e45b2e2728efcb0637afa94fcbd5756dfbe76 is still an option on the table. https://github.com/bitcoin/bitcoin/pull/32566 may also [benefit](https://github.com/bitcoin/bitcoin/pull/32566#discussion_r2099983137) from it.

The issues mentioned in https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2899448860 might be addressed by switching to an alterna
...

💬 instagibbs commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2904837679)
Dropping seemingly-unsolicited compact blocks seems fine, we already do that for the parallel portion, this should be a change just to the first one? blocksonly nodes should never set cb peers.

💬 hebasto commented on pull request "test: Fix `system_tests/run_command` test":
(https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2104848828)
That was the approach in https://github.com/bitcoin/bitcoin/pull/32577.

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2104866651)
There was a build failure on 32 bit systems because of the alignment changes - fixed it by moving the `std::assume_aligned` check inside the alignment condition itself.
Remeasured the benchmarks (all 3 compilers are the same as the previous push) and redid the endianness check (as described in the PR) and updated the https://godbolt.org/z/35nveanf5 again.

💬 m3dwards commented on issue "test: `tool_wallet.py` references no-longer used CI":
(https://github.com/bitcoin/bitcoin/issues/32576#issuecomment-2904880163)
Removing '666' fails tests on windows: https://github.com/m3dwards/bitcoin/actions/runs/15213032706/job/42791654918

Suggest just changing comment to:

`666 on Windows`

💬 maflcko commented on pull request "ci: Downgrade DEBUG=1 to -D_GLIBCXX_ASSERTIONS in centos task":
(https://github.com/bitcoin/bitcoin/pull/32586#issuecomment-2904920622)
(edited a prior comment of mine, which said there are two libc++ debug runs in the CI. In reality there is the libc++ debug build (msan) and the glibc++ debug build (32-bit).)

💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2904926114)
The 32-bit GLIBCXX debug run (gcc-13) is also passing fine: https://cirrus-ci.com/task/6401825881980928?logs=ci#L3769

💬 maflcko commented on pull request "test: Fix `system_tests/run_command` test":
(https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2104898126)
Yes, but it is unclear to me why it was closed, when it looks like the correct approach

💬 maflcko commented on pull request "validation: Do less work in NeedsRedownload":
(https://github.com/bitcoin/bitcoin/pull/31714#issuecomment-2905035622)
I still don't understand why this would be safe to remove the check, or just check a single block (start or end), when it is required to check all blocks. Otherwise, upgrading from an ancient blocksdir could lead to errors and bugs down the line. I know this is an edge case, but we are talking about consensus code, so if the introduced failure mode is intentional, it would be good to at least mention or document it.

The exact steps to reproduce the new bug are:

* (optional) Download any nu
...

🤔 murchandamus reviewed a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2865141901)
ReACK d73e4f878d143fe86d84e0e6f18c2a88c0cb8609

💬 murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2105070757)
I think I just didn’t fully understand what was happening here then.

💬 murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r2105072678)
It would probably be best documented by updating the description of the `GetDebit` function whenever someone changes code nearby.

💬 mzumsande commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-2905198756)
Some discussion about the privacy implications for blocksonly peers in IRC: https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2025-05-23#1123715;

Dropping unsolicted `cmpctblock` messages makes sense to me as well (and doesn't appear to be at odds with anything in BIP152).

📝 hebasto reopened a pull request: "subprocess: Let shell parse command on non-Windows systems"
(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
...

💬 hebasto commented on pull request "subprocess: Let shell parse command on non-Windows systems":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2905201466)
Reopened per [request](https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2104898126).

💬 hebasto commented on pull request "test: Fix `system_tests/run_command` test":
(https://github.com/bitcoin/bitcoin/pull/32601#discussion_r2105083977)
> Yes, but it is unclear to me why it was closed, when it looks like the correct approach

Reopened.

💬 jonatack commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2905206387)
Concept ACK, need to review this