🤔 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
...

💬 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 ?

💬 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.

💬 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 ==
...

💬 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");
}

...

💬 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?

💬 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]".

💬 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.

💬 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`

💬 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?

📝 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`.

💬 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.

💬 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.

💬 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
...

💬 maflcko commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2314092127)
self.log.info("Test that estimatesmartfee return sub 1s/vb fee rate estimate") -> self.log.info("Test that estimatesmartfee returns a sub 1s/vb fee rate estimate") [subject-verb agreement and article: "estimatesmartfee return" should be "estimatesmartfee returns" and include "a" for clarity.]

💬 maflcko commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2314091146)


with sufficient data, it's average value will be returned as the fee rate estimate. -> its average value will be returned as the fee rate estimate. ["it's" (contraction of "it is") is incorrect here; the possessive "its" is required.]

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2314115440)
Done

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2314140232)
This just serves as a floor value for `-maxtxfee`.

Since this is a configurable knob, there is a "far-fetched edge case where " a user could start the node with a value so low that it would prevent creating transactions.

Because while creating transaction we ensure the fee is not above `-maxtxfee`.

A more accurate check would use the actual transaction size, but since size varies per transaction, 1000 is chosen arbitrarily. The idea is to calculate the fee rate when the fee is `max_t
...

💬 ismaelsadeeq commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2314141299)
Done

💬 maflcko commented on pull request "wallet, rpc: remove settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/32138#discussion_r2314142415)
this should also be removed, or moved to the only place where it used (with docs on what it is for)