💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1858340882)
The caching has been [added](https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2500496959).

💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1858341129)
[Reworked](https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2500496959).

💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1858341637)
[Reworked](https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2500496959).

💬 Sjors commented on pull request "guix: swap `moreutils` for just `sponge`":
(https://github.com/bitcoin/bitcoin/pull/31323#issuecomment-2500514151)
Getting the same guix hashes on `x86_64` as @TheCharlatan, still working on my amd64 guix system.

💬 TheCharlatan commented on pull request "kernel, refactor: Replace `goto` with RAII in `bitcoin-chainstate`":
(https://github.com/bitcoin/bitcoin/pull/31362#issuecomment-2500562746)
There is some context on both the usage of `goto` and tests in the original PR https://github.com/bitcoin/bitcoin/pull/24304#discussion_r804072128, and https://github.com/bitcoin/bitcoin/pull/24304#issuecomment-1042814237 and https://github.com/bitcoin/bitcoin/pull/24304#issuecomment-1042920837 respectively. If you look at aj's original suggested [solution](https://github.com/ajtowns/bitcoin/commit/5d7d8de713961a88a40c236346bbfad4ad2906f0#diff-04e685224f1ac5bfd91d47d8d7528a2e44f94fab5535d4b6b5af
...

💬 hebasto commented on pull request "refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors":
(https://github.com/bitcoin/bitcoin/pull/31364#discussion_r1858402086)
I think so.

💬 willcl-ark commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2500591909)
Just wanted to clarify that I'd love to see this get in sooner rather than later, as it can provide a valuable speedup and I'd be ready to ACK with a reduced (default) size selected, for the reasons I outlined above.

We can in tandem discuss making this value configurable over in #30059.

💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1858444085)
Tempted to leave the PR as is for now as it's already quite large.

💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1858444169)
Reverted this change (and its corresponding test).

💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1858445736)
I added a commit to improve this message.

💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2500659531)
Rebased and addressed feedback.

💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1858447297)
Fixed

💬 jonatack commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#issuecomment-2500679564)
Concept ACK, if this patchset can meaningfully improve performance and reliably refresh wallet utxos to keep the in-memory cache in sync.

💬 jonatack commented on pull request "wallet: Track no-longer-spendable TXOs separately":
(https://github.com/bitcoin/bitcoin/pull/27865#issuecomment-2500682230)
Concept ACK, pending https://github.com/bitcoin/bitcoin/pull/27286, if this improves performance for wallets with many transactions.

💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1858472581)
remove wine?

💬 maflcko commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#discussion_r1858474155)
Would be nice to run the util and functional tests as well

👍 maflcko approved a pull request: "ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176#pullrequestreview-2461424433)
lgtm

💬 theStack commented on pull request "doc, test: more ephemeral dust follow-ups":
(https://github.com/bitcoin/bitcoin/pull/31371#discussion_r1858489166)
Note that this fee is set initially for a tx with a single (non-dust) output, and any additional outputs after are created in a way that the total tx fee stays constant (see doc-string of function `add_output_to_create_multi_result`: `Add output without changing absolute tx fee`). So the parameter denotes indeed the fee for the whole tx.

💬 jonatack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-2500731539)
Concept ACK

> Given BIP373 doesn't have test vectors, it would be very useful that either this PR or the BIP include some hard coded PSBT examples to ensure every implementations are on the same page.

Good point (the BIP373 test vector section currently states "TBD" and seems worth completing even if the implementation here also has tests).

💬 stickies-v commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r1858493399)
> You'd probably want to pass this function a script verify object that has already passed through the required pre-checks.

I 100% agree with this approach. Adding extra types makes the API more cumbersome to use, but I think it does make it more safe, and the extra verbosity should be quite easy to hide in client libraries.

> But then you're forced to check them here again.

I think that can be avoided by having the prechecks function return a `ScriptPreChecksPassed*` (which references
...