👋 hebasto's pull request is ready for review: "ci: Test cross-built Windows executables on Windows natively"
(https://github.com/bitcoin/bitcoin/pull/31176)
(https://github.com/bitcoin/bitcoin/pull/31176)
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2500496959)
Reworked. The GHA job now re-uses `ci/test/00_setup_env_win64.sh`.
Added caching for `depends/built` and Ccache.
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2500496959)
Reworked. The GHA job now re-uses `ci/test/00_setup_env_win64.sh`.
Added caching for `depends/built` and Ccache.
💬 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).
(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).
(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).
(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.
(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
...
(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.
(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.
(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.
(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).
(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.
(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.
(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
(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.
(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.
(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?
(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
(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
(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.
(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.