💬 fanquake commented on pull request "Add createwallet, createwalletdescriptor, and migratewallet to history filter":
(https://github.com/bitcoin-core/gui/pull/901#issuecomment-3485578511)
Backported to `30.x` in https://github.com/bitcoin/bitcoin/pull/33609.
(https://github.com/bitcoin-core/gui/pull/901#issuecomment-3485578511)
Backported to `30.x` in https://github.com/bitcoin/bitcoin/pull/33609.
💬 l0rinc commented on pull request "[30.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3485583769)
Can you please add https://github.com/bitcoin/bitcoin/pull/33336 to the queue as well?
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3485583769)
Can you please add https://github.com/bitcoin/bitcoin/pull/33336 to the queue as well?
💬 kristapsk commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-3485606391)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-3485606391)
Concept ACK
💬 stringintech commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3485634089)
re-ACK 6c7a34f
<details>
<summary>nits if/when you have to retouch</summary>
```diff
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
index 8bba3cf1c0..07b59d9543 100644
--- a/src/kernel/bitcoinkernel.cpp
+++ b/src/kernel/bitcoinkernel.cpp
@@ -602,11 +602,11 @@ void btck_transaction_output_destroy(btck_TransactionOutput* output)
}
int btck_script_pubkey_verify(const btck_ScriptPubkey* script_pubkey,
- const int64_t amount,
+
...
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3485634089)
re-ACK 6c7a34f
<details>
<summary>nits if/when you have to retouch</summary>
```diff
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
index 8bba3cf1c0..07b59d9543 100644
--- a/src/kernel/bitcoinkernel.cpp
+++ b/src/kernel/bitcoinkernel.cpp
@@ -602,11 +602,11 @@ void btck_transaction_output_destroy(btck_TransactionOutput* output)
}
int btck_script_pubkey_verify(const btck_ScriptPubkey* script_pubkey,
- const int64_t amount,
+
...
💬 hebasto commented on pull request "ci: Add Windows + UCRT jobs for cross-compiling and native testing":
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3485712938)
> > This one simply addresses your comments [#33593 (comment)](https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3460836590) and [#33593 (review)](https://github.com/bitcoin/bitcoin/pull/33593#pullrequestreview-3398698978).
>
> Somewhat, but if this is the approach, it means that not only the compiler, but also the version of the headers being tested isn't going to match Guix? (I'm wondering if internalising the headers might be a better approach, as that would be more flexible, and,
...
(https://github.com/bitcoin/bitcoin/pull/33764#issuecomment-3485712938)
> > This one simply addresses your comments [#33593 (comment)](https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3460836590) and [#33593 (review)](https://github.com/bitcoin/bitcoin/pull/33593#pullrequestreview-3398698978).
>
> Somewhat, but if this is the approach, it means that not only the compiler, but also the version of the headers being tested isn't going to match Guix? (I'm wondering if internalising the headers might be a better approach, as that would be more flexible, and,
...
💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3485734385)
CI failed, limits of the 3rd for cycle were off; fixed
```diff
int count_diff_smaller_16 = 0;
int max_diff = 0;
int total_diff = 0;
- for(int i{1}; i < n; ++i) {
+ for(size_t i{0}; i < skip_diffs.size(); ++i) {
auto diff = skip_diffs[i];
if (diff < 16) {
++count_diff_smaller_16;
```
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3485734385)
CI failed, limits of the 3rd for cycle were off; fixed
```diff
int count_diff_smaller_16 = 0;
int max_diff = 0;
int total_diff = 0;
- for(int i{1}; i < n; ++i) {
+ for(size_t i{0}; i < skip_diffs.size(); ++i) {
auto diff = skip_diffs[i];
if (diff < 16) {
++count_diff_smaller_16;
```
📝 hebasto opened a pull request: "cmake: Move IPC tests to `ipc/test`"
(https://github.com/bitcoin/bitcoin/pull/33774)
This PR follows up on https://github.com/bitcoin/bitcoin/pull/33445 and:
1. Organizes the IPC tests in the same way as the wallet tests.
2. Removes no longer needed `src/test/.clang-tidy.in`.
See the previous discussion:
- https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2379651340
- https://github.com/bitcoin/bitcoin/pull/33445#pullrequestreview-3411868329
(https://github.com/bitcoin/bitcoin/pull/33774)
This PR follows up on https://github.com/bitcoin/bitcoin/pull/33445 and:
1. Organizes the IPC tests in the same way as the wallet tests.
2. Removes no longer needed `src/test/.clang-tidy.in`.
See the previous discussion:
- https://github.com/bitcoin/bitcoin/pull/33445#discussion_r2379651340
- https://github.com/bitcoin/bitcoin/pull/33445#pullrequestreview-3411868329
💬 hebasto commented on pull request "cmake: Move IPC tests to `ipc/test`":
(https://github.com/bitcoin/bitcoin/pull/33774#issuecomment-3485938369)
cc @ryanofsky
(https://github.com/bitcoin/bitcoin/pull/33774#issuecomment-3485938369)
cc @ryanofsky
📝 fanquake opened a pull request: "guix: use GCC 14.3.0 over 13.3.0"
(https://github.com/bitcoin/bitcoin/pull/33775)
Switching to using GCC 14.x for release builds has come up multiple times recently. It will eventually be needed for #25573, and could also be useful for #30210.
(https://github.com/bitcoin/bitcoin/pull/33775)
Switching to using GCC 14.x for release builds has come up multiple times recently. It will eventually be needed for #25573, and could also be useful for #30210.
💬 hebasto commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3485952872)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/33775#issuecomment-3485952872)
Concept ACK.
💬 hebasto commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2490493926)
Why 14.3.0 specifically, and not 14.2.0?
(asking because 14.2.x is mentioned in https://gist.github.com/hebasto/dee3c918da49ee767ccf5eea43276407).
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2490493926)
Why 14.3.0 specifically, and not 14.2.0?
(asking because 14.2.x is mentioned in https://gist.github.com/hebasto/dee3c918da49ee767ccf5eea43276407).
💬 instagibbs commented on something "":
(https://github.com/bitcoin/bitcoin/commit/51d213d1449b61665628ce9b4e0209cff3c4f818#r169601405)
51d213d1449b61665628ce9b4e0209cff3c4f818
If we're doing this, I think the AcceptSubPackage instance of ClearSubPackage is a redundant?
(https://github.com/bitcoin/bitcoin/commit/51d213d1449b61665628ce9b4e0209cff3c4f818#r169601405)
51d213d1449b61665628ce9b4e0209cff3c4f818
If we're doing this, I think the AcceptSubPackage instance of ClearSubPackage is a redundant?
📝 maflcko opened a pull request: "ci: Lint follow-ups"
(https://github.com/bitcoin/bitcoin/pull/33776)
This contains a few follow-ups to https://github.com/bitcoin/bitcoin/pull/33744:
* Rewrite the actions Bash snippet to Python. I've confirmed it still works in https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/19067932430 (scroll down).
* Add a lint-build retry to avoid issues such as https://github.com/bitcoin/bitcoin/issues/33640 for the lint task as well.
* Finally, run the `debug_pull_request_number_str` annotation on all checks, to ensure they are present even when GitHub de
...
(https://github.com/bitcoin/bitcoin/pull/33776)
This contains a few follow-ups to https://github.com/bitcoin/bitcoin/pull/33744:
* Rewrite the actions Bash snippet to Python. I've confirmed it still works in https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/19067932430 (scroll down).
* Add a lint-build retry to avoid issues such as https://github.com/bitcoin/bitcoin/issues/33640 for the lint task as well.
* Finally, run the `debug_pull_request_number_str` annotation on all checks, to ensure they are present even when GitHub de
...
💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3486049487)
https://github.com/bitcoin/bitcoin/commit/51d213d1449b61665628ce9b4e0209cff3c4f818#r169601405 left a comment in the non-PR review mode, just making sure it gets noticed
(https://github.com/bitcoin/bitcoin/pull/33629#issuecomment-3486049487)
https://github.com/bitcoin/bitcoin/commit/51d213d1449b61665628ce9b4e0209cff3c4f818#r169601405 left a comment in the non-PR review mode, just making sure it gets noticed
💬 laanwj commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3486050199)
Code review ACK 6c7a34f3b0bd39ef7a1520aac56e12f78e5cc969
i think this is a good start for the C API, and ready for merge.
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3486050199)
Code review ACK 6c7a34f3b0bd39ef7a1520aac56e12f78e5cc969
i think this is a good start for the C API, and ready for merge.
💬 TheBlueMatt commented on issue "Batch tx broadcast RPC":
(https://github.com/bitcoin/bitcoin/issues/33700#issuecomment-3486063102)
> What would you prefer to happen if a transaction fails? Is it fine if we quit altogether or do you want the node to keep trying with everything else?
Parameter as mentioned by @ajtowns would be fine, but my general view of transaction relay is its all fallible anyway (nodes may be partitioned/fees may spike/we may be disconnected from the internet/etc/etc) so I want to just send a bunch of transaction hex into the void and Bitcoin Core is responsible for getting as much as possible confirmed.
...
(https://github.com/bitcoin/bitcoin/issues/33700#issuecomment-3486063102)
> What would you prefer to happen if a transaction fails? Is it fine if we quit altogether or do you want the node to keep trying with everything else?
Parameter as mentioned by @ajtowns would be fine, but my general view of transaction relay is its all fallible anyway (nodes may be partitioned/fees may spike/we may be disconnected from the internet/etc/etc) so I want to just send a bunch of transaction hex into the void and Bitcoin Core is responsible for getting as much as possible confirmed.
...
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3486069744)
@ajtowns, thanks for looking into this. Some replies inline:
> Not sure the internal implementation around `BroadcastTransaction()` fully makes sense -- I could see `BroadcastTransaction()` having the knowledge of how transactions should be broadcast being better than having every caller of it specify it.
In `master` it is the latter - callers of `BroadcastTransaction()` tell it whether to broadcast and/or put in the mempool. I guess this can be changed, at least for the private broadcast
...
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3486069744)
@ajtowns, thanks for looking into this. Some replies inline:
> Not sure the internal implementation around `BroadcastTransaction()` fully makes sense -- I could see `BroadcastTransaction()` having the knowledge of how transactions should be broadcast being better than having every caller of it specify it.
In `master` it is the latter - callers of `BroadcastTransaction()` tell it whether to broadcast and/or put in the mempool. I guess this can be changed, at least for the private broadcast
...
💬 pinheadmz commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3486092759)
> While I no longer support the Bitcoin Core project
@luke-jr I think you should open a PR to remove your seed for this reason. It would be much more reasonable than someone else opening the PR on your behalf.
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3486092759)
> While I no longer support the Bitcoin Core project
@luke-jr I think you should open a PR to remove your seed for this reason. It would be much more reasonable than someone else opening the PR on your behalf.
👍 dergoegge approved a pull request: "ci: run native fuzz with MSAN job"
(https://github.com/bitcoin/bitcoin/pull/33626#pullrequestreview-3416571878)
utACK 1e6e32fa8a64daa21c9c9de437f7a12745ed4a4e
(https://github.com/bitcoin/bitcoin/pull/33626#pullrequestreview-3416571878)
utACK 1e6e32fa8a64daa21c9c9de437f7a12745ed4a4e
💬 fanquake commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2490601023)
I'd rather we use the version of the compiler with more bug fixes, and less known bugs.
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2490601023)
I'd rather we use the version of the compiler with more bug fixes, and less known bugs.