💬 davidgumberg commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2470468329)
https://github.com/bitcoin/bitcoin/pull/32380/commits/f366408492f6205ee20fe23e5104813de45dd4b1 (_cmake: Set process code page to UTF-8 on Windows_)
This commit should also remove the skips of manifest checks for these executables:
https://github.com/bitcoin/bitcoin/blob/5a58d4915e5ce53b922961b16840709686ce9996/.github/workflows/ci.yml#L282-L286
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2470468329)
https://github.com/bitcoin/bitcoin/pull/32380/commits/f366408492f6205ee20fe23e5104813de45dd4b1 (_cmake: Set process code page to UTF-8 on Windows_)
This commit should also remove the skips of manifest checks for these executables:
https://github.com/bitcoin/bitcoin/blob/5a58d4915e5ce53b922961b16840709686ce9996/.github/workflows/ci.yml#L282-L286
💬 ismaelsadeeq commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470677387)
which line?
you highlighted four lines.
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470677387)
which line?
you highlighted four lines.
💬 Crypt-iQ commented on pull request "p2p: reduce false-positives in addr rate-limiting":
(https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3457987486)
Ignoring self-announcements seems to occur only in the time between our inbound peer (outbound from their perspective) 1) calling `SetupAddressRelay` when receiving version, and 2) receiving verack where it will set `fSuccessfullyConnected = true` and call `SendMessages` followed by `MaybeSendAddr`. The address entries are sent in order of insert, but then shuffled before processing since https://github.com/bitcoin/bitcoin/pull/22387 ([see rationale](https://github.com/bitcoin/bitcoin/pull/22387
...
(https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3457987486)
Ignoring self-announcements seems to occur only in the time between our inbound peer (outbound from their perspective) 1) calling `SetupAddressRelay` when receiving version, and 2) receiving verack where it will set `fSuccessfullyConnected = true` and call `SendMessages` followed by `MaybeSendAddr`. The address entries are sent in order of insert, but then shuffled before processing since https://github.com/bitcoin/bitcoin/pull/22387 ([see rationale](https://github.com/bitcoin/bitcoin/pull/22387
...
💬 ismaelsadeeq commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470687187)
Interesting (that should never happen), can you share the logs and steps to reproduce?
I can verify that it failed for me.
```
2025-10-28T18:42:24.302035Z TestFramework (INFO): PRNG seed is: 1841123518053179096
2025-10-28T18:42:24.302446Z TestFramework (INFO): Initializing test directory /var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/bitcoin_func_test_kcqf80gj
2025-10-28T18:42:25.106568Z TestFramework (INFO): Running echo test
2025-10-28T18:42:25.113270Z TestFramework (INFO): Running m
...
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470687187)
Interesting (that should never happen), can you share the logs and steps to reproduce?
I can verify that it failed for me.
```
2025-10-28T18:42:24.302035Z TestFramework (INFO): PRNG seed is: 1841123518053179096
2025-10-28T18:42:24.302446Z TestFramework (INFO): Initializing test directory /var/folders/dj/d8p8jhd172n7wnq81ryfl6rc0000gn/T/bitcoin_func_test_kcqf80gj
2025-10-28T18:42:25.106568Z TestFramework (INFO): Running echo test
2025-10-28T18:42:25.113270Z TestFramework (INFO): Running m
...
💬 ismaelsadeeq commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470690366)
Good idea, will do if retouching.
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470690366)
Good idea, will do if retouching.
💬 niteshbalusu11 commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458027438)
Concept ACK
Because it's been verified that this happens.
> Have independently confirmed that `dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us` is not returning any Core v30 nodes.
If @luke-jr is willing to change this then we don't have to remove.
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458027438)
Concept ACK
Because it's been verified that this happens.
> Have independently confirmed that `dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us` is not returning any Core v30 nodes.
If @luke-jr is willing to change this then we don't have to remove.
💬 hsjoberg commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458038934)
utACK 9c3291ff9bd713e1a92dae2c388a83e4a107bf7e.
Luke has repeatedly called "Bitcoin Core" bad actors in public.
Given this hostility, it stands to reason that his DNS seed should not be in the software.
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458038934)
utACK 9c3291ff9bd713e1a92dae2c388a83e4a107bf7e.
Luke has repeatedly called "Bitcoin Core" bad actors in public.
Given this hostility, it stands to reason that his DNS seed should not be in the software.
📝 isrod opened a pull request: "Log ZMQ bind error at Error level, abort startup on ZMQ init error (#33715)"
(https://github.com/bitcoin/bitcoin/pull/33727)
Fixes #33715
(https://github.com/bitcoin/bitcoin/pull/33727)
Fixes #33715
💬 maflcko commented on issue "ci: Where to run heavy and high-maintenance CI tasks?":
(https://github.com/bitcoin/bitcoin/issues/33668#issuecomment-3458046048)
I went ahead and pushed the msan -O0, and the s390x task to https://github.com/maflcko/bitcoin-core-nightly/actions/runs/18885767497/job/53900576177. However, they are so slow on GHA, that they time out after 6 hours.
Possibly with a CTest Dashboard to upload results, those tasks could be run on faster or at least dedicated hardware without timeouts.
(https://github.com/bitcoin/bitcoin/issues/33668#issuecomment-3458046048)
I went ahead and pushed the msan -O0, and the s390x task to https://github.com/maflcko/bitcoin-core-nightly/actions/runs/18885767497/job/53900576177. However, they are so slow on GHA, that they time out after 6 hours.
Possibly with a CTest Dashboard to upload results, those tasks could be run on faster or at least dedicated hardware without timeouts.
💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470732607)
> Could the same be done via a bool parameter to Stop to skip joining?
If we do that, the threads wouldn't remain in the `m_workers` vector anymore. They'd be swapped out on the first call. That means we wouldn't be able to wait for them to finish later on, which would be quite bad since we’d lose track of when all requests are fulfilled before destroying the backend objects (that's basically what joining the threads mean for us right now). And this could lead to requests accessing null point
...
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470732607)
> Could the same be done via a bool parameter to Stop to skip joining?
If we do that, the threads wouldn't remain in the `m_workers` vector anymore. They'd be swapped out on the first call. That means we wouldn't be able to wait for them to finish later on, which would be quite bad since we’d lose track of when all requests are fulfilled before destroying the backend objects (that's basically what joining the threads mean for us right now). And this could lead to requests accessing null point
...
💬 pinheadmz commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458058470)
> [!CAUTION]
> Ok at this point there is NO LONGER A NEED for more comments about an individual's trustworthiness.
This PR is starting to get brigaded and comments that do not ADD ADDITIONAL INFORMATION or help the maintainers do their job will probably get hidden and may result in a ban
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458058470)
> [!CAUTION]
> Ok at this point there is NO LONGER A NEED for more comments about an individual's trustworthiness.
This PR is starting to get brigaded and comments that do not ADD ADDITIONAL INFORMATION or help the maintainers do their job will probably get hidden and may result in a ban
💬 Eunovo commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470739542)
Comment out line 497 and rebuild. The interface_ipc tests still pass successfully. They fail if you modify them to use two waitNext calls like so
```diff
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index 0c9de28da0..a3dc559008 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -199,10 +199,14 @@ class IPCInterfaceTest(BitcoinTestFramework):
wait_task = asyncio.create_task(wait_for_block())
...
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470739542)
Comment out line 497 and rebuild. The interface_ipc tests still pass successfully. They fail if you modify them to use two waitNext calls like so
```diff
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index 0c9de28da0..a3dc559008 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -199,10 +199,14 @@ class IPCInterfaceTest(BitcoinTestFramework):
wait_task = asyncio.create_task(wait_for_block())
...
💬 Eunovo commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470743677)
Line 198
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470743677)
Line 198
💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470743689)
sounds good, taken. thanks!
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470743689)
sounds good, taken. thanks!
💬 john-moffett commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458071231)
Concept ACK based on @achow101 's comment and the currently not "fairly selected" Bitcoin nodes.
Here's what I get with multiple runs:
| Seed | 30.99.0 | 30.0.0 | 29.2.0 | 29.1.0 | 29.0.0 | 28.99.0 | 28.2.0 | 28.1.0 | 28.0.0 | 27.99.0 | 27.2.0 | 27.1.0 | 27.0.0 | 26.2.0 | 26.1.0 | 26.0.0 | 25.1.0 | 25.0.0 | 24.0.1 | 24.0.0 | 23.1.0 | 23.0.0 | 22.0.0 | 0.21.1 | 0.20.1 | 0.20.0 | 0.17.1 | 0.12.1 |
|---|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:
...
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458071231)
Concept ACK based on @achow101 's comment and the currently not "fairly selected" Bitcoin nodes.
Here's what I get with multiple runs:
| Seed | 30.99.0 | 30.0.0 | 29.2.0 | 29.1.0 | 29.0.0 | 28.99.0 | 28.2.0 | 28.1.0 | 28.0.0 | 27.99.0 | 27.2.0 | 27.1.0 | 27.0.0 | 26.2.0 | 26.1.0 | 26.0.0 | 25.1.0 | 25.0.0 | 24.0.1 | 24.0.0 | 23.1.0 | 23.0.0 | 22.0.0 | 0.21.1 | 0.20.1 | 0.20.0 | 0.17.1 | 0.12.1 |
|---|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:
...
💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470749427)
Sure, pushed. Thanks!
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470749427)
Sure, pushed. Thanks!
💬 andrewtoth commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470750271)
I was thinking like this
```diff
diff --git a/src/util/threadpool.h b/src/util/threadpool.h
index 94409facd5..dc1a218abd 100644
--- a/src/util/threadpool.h
+++ b/src/util/threadpool.h
@@ -123,19 +123,19 @@ public:
*
* Must be called from a controller (non-worker) thread.
*/
- void Stop() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+ void Stop(bool join_threads = true) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
{
// Notify workers and join them.
std::ve
...
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470750271)
I was thinking like this
```diff
diff --git a/src/util/threadpool.h b/src/util/threadpool.h
index 94409facd5..dc1a218abd 100644
--- a/src/util/threadpool.h
+++ b/src/util/threadpool.h
@@ -123,19 +123,19 @@ public:
*
* Must be called from a controller (non-worker) thread.
*/
- void Stop() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
+ void Stop(bool join_threads = true) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
{
// Notify workers and join them.
std::ve
...
💬 gmaxwell commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458091139)
ACK 9c3291ff9bd713e1a92dae2c388a83e4a107bf7e
This DNS seed is clearly violating policy-- which was put into place to make discourage abuse of seeds for attempting eclipse attacks and network surveillance. Removing it should be entirely uncontroversial. The change should also be backported.
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458091139)
ACK 9c3291ff9bd713e1a92dae2c388a83e4a107bf7e
This DNS seed is clearly violating policy-- which was put into place to make discourage abuse of seeds for attempting eclipse attacks and network surveillance. Removing it should be entirely uncontroversial. The change should also be backported.
💬 ismaelsadeeq commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470765602)
Thanks for your swift response, you highlighted multiple lines, so I commented them all that’s why it failed for me.
It should pass because we still return (The issue with commenting that line is that subsequent wait for same block template will return instantly). We can add a test case to verify that after interrupting a wait, you can wait again.
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2470765602)
Thanks for your swift response, you highlighted multiple lines, so I commented them all that’s why it failed for me.
It should pass because we still return (The issue with commenting that line is that subsequent wait for same block template will return instantly). We can add a test case to verify that after interrupting a wait, you can wait again.
💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470765839)
You know, we could call `ProcessTask()` during `Stop()` too, which would be a strict improvement over the current locking-wait behavior in master (since we would shut down faster by actively processing pending requests on the shutdown thread as well). This is something that hasn’t been possible until now.
Still, this is material for a follow-up; would say that it is better to keep this PR as contained as possible.
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2470765839)
You know, we could call `ProcessTask()` during `Stop()` too, which would be a strict improvement over the current locking-wait behavior in master (since we would shut down faster by actively processing pending requests on the shutdown thread as well). This is something that hasn’t been possible until now.
Still, this is material for a follow-up; would say that it is better to keep this PR as contained as possible.