💬 fjahr commented on pull request "init: Split file path handling out of -asmap option":
(https://github.com/bitcoin/bitcoin/pull/33631#issuecomment-3554829049)
> But in the medium turn with embedded asmap data present but not turned on by default, you dislike the fact that -asmap=1 would load a file called 1 if we don't take additional steps to prevent this.

Yes, this is what has bothered me most about the arg for a long time and why I had included my minimal fix for this in the original embedding PR already, just implemented a bit differently than you suggest but with the same effect. I think it's just not acceptable that you couldn't even use the
...

🤔 mzumsande reviewed a pull request: "test: add `-alertnotify` test for large work invalid chain warning"
(https://github.com/bitcoin/bitcoin/pull/33893#pullrequestreview-3484869685)
ACK c5b301f6ac0d3f882cc55397e3fb0cf95058704b

Fun fact: The 6-block warning is very old, introduced by Satoshi in https://github.com/bitcoin/bitcoin/commit/94cfec07fd302c9ff9b6a80c47418d4fe56596ae

💬 mzumsande commented on pull request "test: add `-alertnotify` test for large work invalid chain warning":
(https://github.com/bitcoin/bitcoin/pull/33893#discussion_r2543670655)
nit: isn't "at least" imprecise, and you need _more than_ 6 blocks?
The validation codes uses `>` instead of `>=` and the test fails if I create just 6 blocks on top of the tip.

🤔 hebasto reviewed a pull request: "depends: update xcb-util packages to latest versions"
(https://github.com/bitcoin/bitcoin/pull/33851#pullrequestreview-3484940469)
Maybe update `xcb_proto` as well?

💬 hebasto commented on pull request "depends: update xcb-util packages to latest versions":
(https://github.com/bitcoin/bitcoin/pull/33851#discussion_r2543728983)
Why not [1.17](https://gitlab.freedesktop.org/xorg/lib/libxcb/-/commits/libxcb-1.17.0?ref_type=tags)?

💬 achow101 commented on pull request "wallet: Add separate balance info for non-mempool wallet txs":
(https://github.com/bitcoin/bitcoin/pull/33671#issuecomment-3554858459)
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.

From a cursory glance at the code, I think 2 transactions that conflict with each other but aren't in the mempool would both get counted towar
...

💬 fjahr commented on pull request "test: Fix race condition in IPC interface block progation test":
(https://github.com/bitcoin/bitcoin/pull/33880#discussion_r2543746081)
Fixed

💬 hebasto commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#discussion_r2543752271)
It would be helpful if someone could report this upstream.

💬 fanquake commented on pull request "depends: update xcb-util packages to latest versions":
(https://github.com/bitcoin/bitcoin/pull/33851#discussion_r2543759301)
That required adding new dependencies, which I didn't want to do here. It's the same reason libxkbcommon isn't updated further, as that would require a new build tool/system (meson), and new dependencies.

💬 fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2543764811)
Ah, I see, thanks for clarifying! Done.

💬 achow101 commented on pull request "wallet: don't consider unconfirmed TRUC coins with ancestors":
(https://github.com/bitcoin/bitcoin/pull/33528#issuecomment-3554911366)
ACK dcd42d6d8f160ae8bc12c152099a6e6473658e30

✅ waketraindev closed a pull request: "net: Decouple `CConnman::GetAddresses` from `CNode`"
(https://github.com/bitcoin/bitcoin/pull/33900)

💬 waketraindev commented on pull request "net: Decouple `CConnman::GetAddresses` from `CNode`":
(https://github.com/bitcoin/bitcoin/pull/33900#issuecomment-3554921486)
Closed. Going to let Net Split WG handle this one :) thanks for the comments.

👍 brunoerg approved a pull request: "test: add `-alertnotify` test for large work invalid chain warning"
(https://github.com/bitcoin/bitcoin/pull/33893#pullrequestreview-3485040821)
reACK c5b301f6ac0d3f882cc55397e3fb0cf95058704b

💬 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.