💬 Sjors commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2313830013)
A slightly different approach, which avoids confusing `NLMSG_DONE` with `done`:
```diff
diff --git a/src/common/netif.cpp b/src/common/netif.cpp
index 77246980c3..3f3b8fdcea 100644
--- a/src/common/netif.cpp
+++ b/src/common/netif.cpp
@@ -117,9 +117,10 @@ std::optional<CNetAddr> QueryDefaultGatewayImpl(sa_family_t family)
// Receive response.
char response[4096];
ssize_t total_bytes_read{0};
- bool done{false};
- bool multi_part{false};
- while (!done) {
+
...
(https://github.com/bitcoin/bitcoin/pull/32159#discussion_r2313830013)
A slightly different approach, which avoids confusing `NLMSG_DONE` with `done`:
```diff
diff --git a/src/common/netif.cpp b/src/common/netif.cpp
index 77246980c3..3f3b8fdcea 100644
--- a/src/common/netif.cpp
+++ b/src/common/netif.cpp
@@ -117,9 +117,10 @@ std::optional<CNetAddr> QueryDefaultGatewayImpl(sa_family_t family)
// Receive response.
char response[4096];
ssize_t total_bytes_read{0};
- bool done{false};
- bool multi_part{false};
- while (!done) {
+
...
💬 Sjors commented on pull request "net, pcp: handle multi-part responses and filter for default route while querying default gateway":
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-3242198039)
utACK 4c531782569b42fc47478a468f4a79e0e2d93946
Tested (rebased) with a router running OPNsense 25.7.2 with miniupnpd 2.3.9 using pf backend.
From a macOS 15.6.1 machine and macOS 13.7.8 machine I can see it opens a port and I'm (still) getting inbound connections. This is unsurprising, because the changes here don't impact macOS.
I also tested on Ubuntu 25.04
(https://github.com/bitcoin/bitcoin/pull/32159#issuecomment-3242198039)
utACK 4c531782569b42fc47478a468f4a79e0e2d93946
Tested (rebased) with a router running OPNsense 25.7.2 with miniupnpd 2.3.9 using pf backend.
From a macOS 15.6.1 machine and macOS 13.7.8 machine I can see it opens a port and I'm (still) getting inbound connections. This is unsurprising, because the changes here don't impact macOS.
I also tested on Ubuntu 25.04
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313845884)
Done, just reverted it.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313845884)
Done, just reverted it.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313846144)
Done.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313846144)
Done.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313846492)
Done.
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313846492)
Done.
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#issuecomment-3242208777)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313144770, https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313159186 and https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313164738.
(https://github.com/bitcoin/bitcoin/pull/33210#issuecomment-3242208777)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313144770, https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313159186 and https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2313164738.
✅ maflcko closed an issue: "intermittent TSAN failure in lockmanager_tests::blockmanager_readblock_hash_mismatch"
(https://github.com/bitcoin/bitcoin/issues/33150)
(https://github.com/bitcoin/bitcoin/issues/33150)
🤔 janb84 reviewed a pull request: "ci: Migrate CI to hosted Cirrus Runners"
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3173431453)
ACK 8a7eb6ef2c226459739bdfe050b735a5a0d0b771
PR changes CI from self hosted instances to hosted Cirrus Runners. In addition to migrating to a more-commonly-used GitHub workflow syntax.
The migration to the more-commonly-used GitHub workflow syntax makes the whole CI proces easier more structured, easier to understand, this will benefit future maintenance. In addition to be less reliant on (centralised) self-hosting with the ability to easily switch-out Cirrus in the future, will be a be
...
(https://github.com/bitcoin/bitcoin/pull/32989#pullrequestreview-3173431453)
ACK 8a7eb6ef2c226459739bdfe050b735a5a0d0b771
PR changes CI from self hosted instances to hosted Cirrus Runners. In addition to migrating to a more-commonly-used GitHub workflow syntax.
The migration to the more-commonly-used GitHub workflow syntax makes the whole CI proces easier more structured, easier to understand, this will benefit future maintenance. In addition to be less reliant on (centralised) self-hosting with the ability to easily switch-out Cirrus in the future, will be a be
...
💬 janb84 commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2313808003)
```suggestion
needs: [runners, lint]
```
Nit / Question: Why not wait on successful run of linter before running more heavier / costlier runs ?
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2313808003)
```suggestion
needs: [runners, lint]
```
Nit / Question: Why not wait on successful run of linter before running more heavier / costlier runs ?
💬 brunoerg commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#issuecomment-3242299628)
Oops, forgot to fix the unit test, doing this.
(https://github.com/bitcoin/bitcoin/pull/33210#issuecomment-3242299628)
Oops, forgot to fix the unit test, doing this.
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3242320535)
> Just some style fixups, but it looks like out-of-storage errors are happening:
>
> ```
> /home/admin/actions-runner/_work/_temp/depends/work/build/arm-linux-gnueabihf/qt/6.7.3-634ec665c7f/qtbase/src/gui/util/qvalidator.h:163:2: fatal error: cannot write PCH file: No space left on device
> ```
For reference, https://github.com/bitcoin/bitcoin/actions/runs/17373684837/job/49315124575?pr=32989#step:9:1483:
```
+ echo 'Free disk space:'
+ df -h
Free disk space:
+ [[ ci_arm_linux ==
...
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3242320535)
> Just some style fixups, but it looks like out-of-storage errors are happening:
>
> ```
> /home/admin/actions-runner/_work/_temp/depends/work/build/arm-linux-gnueabihf/qt/6.7.3-634ec665c7f/qtbase/src/gui/util/qvalidator.h:163:2: fatal error: cannot write PCH file: No space left on device
> ```
For reference, https://github.com/bitcoin/bitcoin/actions/runs/17373684837/job/49315124575?pr=32989#step:9:1483:
```
+ echo 'Free disk space:'
+ df -h
Free disk space:
+ [[ ci_arm_linux ==
...
💬 Sjors commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3242352376)
The following patches fix the linter:
```diff
iff --git a/src/bitcoin.cpp b/src/bitcoin.cpp
index c1a5fce33a..aa74f3e54e 100644
--- a/src/bitcoin.cpp
+++ b/src/bitcoin.cpp
@@ -166,7 +166,7 @@ bool UseMultiprocess(const CommandLine& cmd)
// If any -ipc* options are set these need to be processed by a
// multiprocess-capable binary.
- return args.IsArgSet("-ipcbind") || args.IsArgSet("-ipcconnect") || args.IsArgSet("-ipcfd");
+ return args.IsArgSet("-ipcbind");
}
...
(https://github.com/bitcoin/bitcoin/pull/33229#issuecomment-3242352376)
The following patches fix the linter:
```diff
iff --git a/src/bitcoin.cpp b/src/bitcoin.cpp
index c1a5fce33a..aa74f3e54e 100644
--- a/src/bitcoin.cpp
+++ b/src/bitcoin.cpp
@@ -166,7 +166,7 @@ bool UseMultiprocess(const CommandLine& cmd)
// If any -ipc* options are set these need to be processed by a
// multiprocess-capable binary.
- return args.IsArgSet("-ipcbind") || args.IsArgSet("-ipcconnect") || args.IsArgSet("-ipcfd");
+ return args.IsArgSet("-ipcbind");
}
...
💬 Sjors commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2313941771)
5a28d0b027fdca07c55c853a550324c0aa1c450f: did you mean to add `-ipcconnect` and `-ipcfd` here?
(https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2313941771)
5a28d0b027fdca07c55c853a550324c0aa1c450f: did you mean to add `-ipcconnect` and `-ipcfd` here?
💬 furszy commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2313947931)
> So I dunno I guess "thread" and "worker" are redundant in the name of a thread?
Sure, done. Now the previous "indexes_threadpool_worker_[num]" will be "indexes_pool_[num]".
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2313947931)
> So I dunno I guess "thread" and "worker" are redundant in the name of a thread?
Sure, done. Now the previous "indexes_threadpool_worker_[num]" will be "indexes_pool_[num]".
💬 ismaelsadeeq commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#issuecomment-3242381132)
> This picks up #13990? If yes, it may be good to explain the differences.
This is different from #13990 in approach:
It omits the consistency check and avoids extending the fee rate buckets when the min bucket fee rate changes.
Instead, it builds on #29702 by simply bumping the fees file version, which ensures that estimates saved in previous files are not read. As such after a restart, the estimator starts afresh.
(https://github.com/bitcoin/bitcoin/pull/33199#issuecomment-3242381132)
> This picks up #13990? If yes, it may be good to explain the differences.
This is different from #13990 in approach:
It omits the consistency check and avoids extending the fee rate buckets when the min bucket fee rate changes.
Instead, it builds on #29702 by simply bumping the fees file version, which ensures that estimates saved in previous files are not read. As such after a restart, the estimator starts afresh.
💬 Sjors commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2313967295)
231fba1f067211b889080bd97c8f58df1500ecf1: I'm confused, should this be `bitcoin-qt`?
I don't see it appear when I run `build/bin/bitcoin gui --version`
(https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2313967295)
231fba1f067211b889080bd97c8f58df1500ecf1: I'm confused, should this be `bitcoin-qt`?
I don't see it appear when I run `build/bin/bitcoin gui --version`
💬 Sjors commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2313969321)
`bitcoin node` command?
(https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2313969321)
`bitcoin node` command?
📝 fanquake opened a pull request: "kernel: chainparams & headersync updates for 30.0"
(https://github.com/bitcoin/bitcoin/pull/33274)
Also adds assumeutxo params for mainnet at `910'000` & testnet4 & `90'000`.
(https://github.com/bitcoin/bitcoin/pull/33274)
Also adds assumeutxo params for mainnet at `910'000` & testnet4 & `90'000`.
💬 fkorotkov commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3242472637)
@maflcko apologies for the disk size on Arm runners issue. It should be fixed. Please try to re-run the job.
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3242472637)
@maflcko apologies for the disk size on Arm runners issue. It should be fixed. Please try to re-run the job.
💬 fanquake commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3242478751)
> @maflcko It should be fixed. Please try to re-run the job.
Thanks. Kicked the job.
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3242478751)
> @maflcko It should be fixed. Please try to re-run the job.
Thanks. Kicked the job.
💬 Sjors commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2314090104)
2cc7d585600e8f0760f877fecf4e7eaf9c5dc92b: here's a torrent magnet, with the same seeds as Bitcoin Core v29, except the binary seed: `magnet:?xt=urn:btih:7019437a2b1530624b100c0795cfc5f90b8322ca&dn=utxo-910000.dat&tr=udp%3A%2F%2Ftracker.openbittorrent.com%3A80&tr=udp%3A%2F%2Ftracker.opentrackr.org%3A1337%2Fannounce&tr=udp%3A%2F%2Ftracker.coppersurfer.tk%3A6969%2Fannounce&tr=udp%3A%2F%2Ftracker.leechers-paradise.org%3A6969%2Fannounce&tr=udp%3A%2F%2Fexplodie.org%3A6969%2Fannounce&tr=udp%3A%2F%2Ftra
...
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2314090104)
2cc7d585600e8f0760f877fecf4e7eaf9c5dc92b: here's a torrent magnet, with the same seeds as Bitcoin Core v29, except the binary seed: `magnet:?xt=urn:btih:7019437a2b1530624b100c0795cfc5f90b8322ca&dn=utxo-910000.dat&tr=udp%3A%2F%2Ftracker.openbittorrent.com%3A80&tr=udp%3A%2F%2Ftracker.opentrackr.org%3A1337%2Fannounce&tr=udp%3A%2F%2Ftracker.coppersurfer.tk%3A6969%2Fannounce&tr=udp%3A%2F%2Ftracker.leechers-paradise.org%3A6969%2Fannounce&tr=udp%3A%2F%2Fexplodie.org%3A6969%2Fannounce&tr=udp%3A%2F%2Ftra
...