🤔 furszy reviewed a pull request: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960#pullrequestreview-1884993957)
ACK 316c7c8450
(https://github.com/bitcoin/bitcoin/pull/28960#pullrequestreview-1884993957)
ACK 316c7c8450
💬 furszy commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492391457)
I'm still unsure about the way this is written. Maybe better to write something like "`func` usually contains a loop that dispatches a single validation event to all subscribers sequentially. This will block validation if it is processed synchronously".
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492391457)
I'm still unsure about the way this is written. Maybe better to write something like "`func` usually contains a loop that dispatches a single validation event to all subscribers sequentially. This will block validation if it is processed synchronously".
🚀 fanquake merged a pull request: "rpc: Drop migratewallet experimental warning"
(https://github.com/bitcoin/bitcoin/pull/28037)
(https://github.com/bitcoin/bitcoin/pull/28037)
💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492401601)
Would be a nice follow-up to remove the non-deterministic scheduler from all tests, unless they are explicitly testing the scheduler.
cc @aureleoules , who was asking for a solution to this for the corecheck infra
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492401601)
Would be a nice follow-up to remove the non-deterministic scheduler from all tests, unless they are explicitly testing the scheduler.
cc @aureleoules , who was asking for a solution to this for the corecheck infra
💬 maflcko commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492409498)
Are you sure this is correct? Would it be possible that it is called once for each validation interface event, and the inserted function would then iterate over each subscriber?
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492409498)
Are you sure this is correct? Would it be possible that it is called once for each validation interface event, and the inserted function would then iterate over each subscriber?
🤔 maflcko reviewed a pull request: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960#pullrequestreview-1885045429)
ACK 316c7c845036cbffa22b9e44f31dca8573ffb639 🔁
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 316c7c845036cbffa22b9e44f3
...
(https://github.com/bitcoin/bitcoin/pull/28960#pullrequestreview-1885045429)
ACK 316c7c845036cbffa22b9e44f31dca8573ffb639 🔁
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 316c7c845036cbffa22b9e44f3
...
💬 furszy commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492412912)
https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492391457
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492412912)
https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492391457
💬 maflcko commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948330593)
> What are we supposed to do to automatically avoid redundant / unneeded includes?
Iwyu is a tool to detect redundant and unneeded includes, so it can be used. However, there may be false warnings in regard to this specific header. Thus, an iwyu pragma can be used to avoid incorrect removal of needed includes. Should I push a commit here?
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948330593)
> What are we supposed to do to automatically avoid redundant / unneeded includes?
Iwyu is a tool to detect redundant and unneeded includes, so it can be used. However, there may be false warnings in regard to this specific header. Thus, an iwyu pragma can be used to avoid incorrect removal of needed includes. Should I push a commit here?
💬 josibake commented on pull request "wallet: cache IsMine scriptPubKeys to improve performance of descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1948331732)
ACK https://github.com/bitcoin/bitcoin/pull/26008/commits/ac246f68299d0dc208ae513dfad1a8fc91b5e6d4
(https://github.com/bitcoin/bitcoin/pull/26008#issuecomment-1948331732)
ACK https://github.com/bitcoin/bitcoin/pull/26008/commits/ac246f68299d0dc208ae513dfad1a8fc91b5e6d4
💬 maflcko commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948352540)
Done
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948352540)
Done
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-1948355393)
Added `m_clients_mutex` similar to the one in `Connman`. Fixed a deadlock on `g_best_block_mutex` when we the block from a peer at the same time as we receive `SubmitSolution` from the pool / job declarator client.
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-1948355393)
Added `m_clients_mutex` similar to the one in `Connman`. Fixed a deadlock on `g_best_block_mutex` when we the block from a peer at the same time as we receive `SubmitSolution` from the pool / job declarator client.
🤔 brunoerg reviewed a pull request: "fuzz: Generate with random libFuzzer settings"
(https://github.com/bitcoin/bitcoin/pull/28178#pullrequestreview-1885135549)
light ACK fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde
I did a light test of these values using differential fuzzing and mutation testing. I applied differential fuzzing between the original coin selection functions and their mutants. In general:
1. Running from seed corpus, `-runs=100000` was able to kill most mutants (>55%) but `-max_total_time=6000` is so much more effective (>90%), as expected.
2. Running without seed corpus, `-runs=100000` was useless, `-max_total_time=6000` was effectiv
...
(https://github.com/bitcoin/bitcoin/pull/28178#pullrequestreview-1885135549)
light ACK fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde
I did a light test of these values using differential fuzzing and mutation testing. I applied differential fuzzing between the original coin selection functions and their mutants. In general:
1. Running from seed corpus, `-runs=100000` was able to kill most mutants (>55%) but `-max_total_time=6000` is so much more effective (>90%), as expected.
2. Running without seed corpus, `-runs=100000` was useless, `-max_total_time=6000` was effectiv
...
🚀 fanquake merged a pull request: "[26.x] more backports"
(https://github.com/bitcoin/bitcoin/pull/29209)
(https://github.com/bitcoin/bitcoin/pull/29209)
💬 fanquake commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1948410187)
> Removal happens at some unknown time after deprecation,
No, removal is going to happen pretty immediately after the 27.x branch off.
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1948410187)
> Removal happens at some unknown time after deprecation,
No, removal is going to happen pretty immediately after the 27.x branch off.
💬 hebasto commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948433154)
> > What are we supposed to do to automatically avoid redundant / unneeded includes?
>
> Iwyu is a tool to detect redundant and unneeded includes, so it can be used. However, there may be false warnings in regard to this specific header. Thus, an iwyu pragma can be used to avoid incorrect removal of needed includes. Should I push a commit here?
IWYU falsely suggests to remove `#include <config/bitcoin-config.h>` in any case where the corresponding macro is commented out in the `config/bit
...
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948433154)
> > What are we supposed to do to automatically avoid redundant / unneeded includes?
>
> Iwyu is a tool to detect redundant and unneeded includes, so it can be used. However, there may be false warnings in regard to this specific header. Thus, an iwyu pragma can be used to avoid incorrect removal of needed includes. Should I push a commit here?
IWYU falsely suggests to remove `#include <config/bitcoin-config.h>` in any case where the corresponding macro is commented out in the `config/bit
...
💬 hebasto commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948460459)
> > What are we supposed to do to automatically avoid redundant / unneeded includes?
>
> Iwyu is a tool to detect redundant and unneeded includes, so it can be used. However, there may be false warnings in regard to this specific header. Thus, an iwyu pragma can be used to avoid incorrect removal of needed includes. Should I push a commit here?
I assume that both macros `HAVE_TIMINGSAFE_BCMP` and `HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR` are commented out in `config/bitcoin-config.h` in
...
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948460459)
> > What are we supposed to do to automatically avoid redundant / unneeded includes?
>
> Iwyu is a tool to detect redundant and unneeded includes, so it can be used. However, there may be false warnings in regard to this specific header. Thus, an iwyu pragma can be used to avoid incorrect removal of needed includes. Should I push a commit here?
I assume that both macros `HAVE_TIMINGSAFE_BCMP` and `HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR` are commented out in `config/bitcoin-config.h` in
...
💬 ryanofsky commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492523347)
re: https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492409498
Yeah the sentence "When used in the ValidationSignals synchronous processing will block validation. It is called for each subscriber on each validation interface event." is a little misleading because only one task per event is inserted, regardless of number of subscribers. In the future we might want to change this so indexes/wallets don't block each other, but that is the current implementation.
I would fix this by
...
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492523347)
re: https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1492409498
Yeah the sentence "When used in the ValidationSignals synchronous processing will block validation. It is called for each subscriber on each validation interface event." is a little misleading because only one task per event is inserted, regardless of number of subscribers. In the future we might want to change this so indexes/wallets don't block each other, but that is the current implementation.
I would fix this by
...
👍 vasild approved a pull request: "net: call `Select` with reachable networks in `ThreadOpenConnections`"
(https://github.com/bitcoin/bitcoin/pull/29436#pullrequestreview-1885261790)
ACK 7edb07ca800991f7c88d582b880a392efe17f31d
About the graph in the OP: "... until it finds an address from a network that compose 20% ...". The chance of not getting the result after `N` tries is `(8/10)^N`. For example for `N=25` that is less than `0.004`. What is the explanation of having so many dots so high? Uneven distribution within addrman?
(https://github.com/bitcoin/bitcoin/pull/29436#pullrequestreview-1885261790)
ACK 7edb07ca800991f7c88d582b880a392efe17f31d
About the graph in the OP: "... until it finds an address from a network that compose 20% ...". The chance of not getting the result after `N` tries is `(8/10)^N`. For example for `N=25` that is less than `0.004`. What is the explanation of having so many dots so high? Uneven distribution within addrman?
📝 glozow opened a pull request: "[26.1] final changes for 26.1rc1"
(https://github.com/bitcoin/bitcoin/pull/29440)
Final changes to tag a 26.1rc1.
Bumps version numbers, man pages, adds release notes etc.
(https://github.com/bitcoin/bitcoin/pull/29440)
Final changes to tag a 26.1rc1.
Bumps version numbers, man pages, adds release notes etc.
💬 vasild commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1492529744)
Does not matter now, but it should work with vector as well.
(https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1492529744)
Does not matter now, but it should work with vector as well.