💬 hebasto commented on pull request "depends: update xcb-util packages to latest versions":
(https://github.com/bitcoin/bitcoin/pull/33851#discussion_r2543816478)
> That required adding new dependencies...

Such as?

FWIW, [this](https://github.com/hebasto/bitcoin/commits/pr33851/1119/) branch seem to build fine.

💬 fjahr commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2543817367)
> I can't remember where we landed on making this is a build option. Has anyone asked for the ability to turn this off at build time, as opposed to just at runtime (if it becomes the default)

We didn't really land anywhere. My last response on it was [here](https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-2962598663) and then the discussion didn't continue, I think. Similar to the default location discussion I am not going to die on this hill, I just want to give users the options t
...

💬 achow101 commented on pull request "wallet: remove redundant sighash calculation in Musig2 signing flow":
(https://github.com/bitcoin/bitcoin/pull/33665#issuecomment-3554963168)
> The original intent of this refactor to reduce seeing duplicate code

Considering how much extra is being changed to avoid the duplication, I don't really think this is worth it.

> but it should also help in improving performance of the wallet signing operations.

I don't think that's the case. The MuSig2 functions that are all recomputing the same sighash are never actually doing so in the same call to `SignMuSig2`. `CreateMuSig2AggregateSig` exits early when there aren't enough pubnon
...

💬 D33r-Gee commented on pull request "Interfaces: Expose UTXO Snapshot Loading and Add Progress Notifications":
(https://github.com/bitcoin/bitcoin/pull/33117#issuecomment-3555056928)
with [d0c32c2](https://github.com/bitcoin/bitcoin/commit/d0c32c28e59d3076100bf4c12d94f8b2d4060943) fixed RPC command issue that surfaced after rebase

💬 fjahr commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2543875825)
I think this is just poor wording on my part, I don't mean a completely new process there, I mean the process that is described in this document can be run again at any time so that there is a new map created that can be used for a release. If that answers your question then I will amend that part of the document to be more clear.

I will still give short answer to the questions in case above doesn't resolve them. As mentioned previously here: https://github.com/bitcoin/bitcoin/pull/28792/file
...

💬 achow101 commented on pull request "wallet: Expand MuSig test coverage and follow-ups":
(https://github.com/bitcoin/bitcoin/pull/33636#issuecomment-3555082242)
Concept ACK

💬 fjahr commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3555104094)
Rebased since https://github.com/bitcoin/bitcoin/pull/33026 was merged, of course still based on https://github.com/bitcoin/bitcoin/pull/33878.

💬 achow101 commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#issuecomment-3555111778)
ACK fb72cc33be570d12fc6b76146fc89047e58f5aaf

💬 achow101 commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#issuecomment-3555127585)
Silent merge conflict?

```
Running 2 test cases...
./test/descriptor_tests.cpp(1272): fatal error: in "descriptor_tests/descriptor_older_warnings": critical check !descs.empty() has failed

*** 1 failure is detected in the test module "Bitcoin Core Test Suite"
```

💬 achow101 commented on pull request "kernel: handle null or empty directories in implementation":
(https://github.com/bitcoin/bitcoin/pull/33867#issuecomment-3555163702)
ACK 6657bcbdb4d0359c1843ca31fb3670c7c0c260d5

💬 achow101 commented on pull request "clang-format: Set InsertNewlineAtEOF: true":
(https://github.com/bitcoin/bitcoin/pull/33896#issuecomment-3555168238)
ACK fa1bf6818f0910f997e5235a197ff51f2e18780d

🚀 achow101 merged a pull request: "kernel: handle null or empty directories in implementation"
(https://github.com/bitcoin/bitcoin/pull/33867)

🚀 achow101 merged a pull request: "clang-format: Set InsertNewlineAtEOF: true"
(https://github.com/bitcoin/bitcoin/pull/33896)

💬 w0xlt commented on pull request "kernel: add context‑free block validation API (`btck_check_block_context_free`) with POW/Merkle flags":
(https://github.com/bitcoin/bitcoin/pull/33908#issuecomment-3555189629)
Thanks for the review @yuvicc and @purpleKarrot .

Great idea exposing the chain parameter class.
I’ve updated the PR to follow this approach and split it into three commits for better readability and easier reviewing.
I also had to update `BlockValidationState` so the reference parameter works correctly.
I kept `btck_BlockCheckFlags` to avoid unclear boolean parameters.

💬 151henry151 commented on issue "Load PSBT error: Unable to decode PSBT":
(https://github.com/bitcoin/bitcoin/issues/30070#issuecomment-3555230079)
I've started some work on this. I modified `WalletModel::prepareTransaction` in `src/qt/walletmodel.h` (line 99) and `src/qt/walletmodel.cpp` (line 150) to accept an optional `sign` parameter (defaults to `false`). In lines 206-208 of `walletmodel.cpp`, I changed the logic so it only signs when explicitly requested via this parameter, rather than always signing when private keys are available. Then in `src/qt/sendcoinsdialog.cpp`, I updated the call to `prepareTransaction` on line 293 to pass `s
...

💬 ajtowns commented on pull request "wallet: Add separate balance info for non-mempool wallet txs":
(https://github.com/bitcoin/bitcoin/pull/33671#issuecomment-3555267523)
> Concept NACK-ish
>
> There's a lot of reasons that an unconfirmed transaction might be in the wallet but not the mempool, and I think giving an accurate balance of those transactions will be hard to get right. We absolutely do not want to mislead users into thinking they have more coins than they actually do, so I am hesitant to add something like this.

We currently mislead users into thinking they have less coins than they actually do (due to transactions in the wallet with long mempool
...

💬 danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3555600586)
Hey, sorry this took a while. I tried rebasing onto #33785 but it wouldn't fix the warning.

In my last push (de4242f):
- I didn't rebase, to hopefully make re-reviewing easier
- I removed `const` as suggested in https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2509736434, I like it a bit better than the previous solution
- I squashed the fix inside the last commit instead pushing as a separate one

> Technically I think the repo policy is that every commit should build on all C
...

💬 stratospher commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-3555608000)
Thank you all for the very helpful feedback on this! and sorry for getting late with the update.

I've pushed an [update](https://github.com/bitcoin/bitcoin/compare/27e90008835a4c980447d97e80910742e041dfaf..14ee29dedc36611f9d33a207b7c5b25a7c9b89ea) addressing all the review comments except @mzumsande's conceptual concern about whether
`v2onlyclearnet` should accept v2 inbounds as well so that the network doesn't face a shortage of listening capacity in the worst case scenario where a huge fra
...

💬 151henry151 commented on issue "Standardness policy rules for legacy Multisig script is incoherent":
(https://github.com/bitcoin/bitcoin/issues/33882#issuecomment-3555753329)
P2SH allows non-solvable redeem scripts with low sigop counts (PR #4365), while legacy scripts still require solvability. This inconsistency is hard to justify, especially since both rely on sigop limits for security.

Aligning legacy script redemption with P2SH policy would remove the asymmetry and fix the Counterparty UTXOs without special-casing, while preserving security through existing sigop limits.

If hybrid key bans are reconsidered (as ajtowns suggested), that could simplify further, b
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2544448081)
Stopping the SOCKS5 proxy works. Leaving it like that, but just to mention, another alternative that occurred to me would be to `self.nodes[0].setnetworkactive(state=False)` before the restart.