💬 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
...
💬 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.]
(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.]
(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
(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
...
(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
(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)
(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)
💬 maflcko commented on pull request "wallet, rpc: remove settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/32138#issuecomment-3242623608)
> Will squash after review, think it's easier this way.
I'd say to either squash now, or create meaningful separate commits. E.g:
* First, remove from rpc (settxfee, paytxfee result)
* Then, remove from cli (-paytxfee, tests only)
* Then, remove from cli (-paytxfee, code, and remaining leftovers)
(https://github.com/bitcoin/bitcoin/pull/32138#issuecomment-3242623608)
> Will squash after review, think it's easier this way.
I'd say to either squash now, or create meaningful separate commits. E.g:
* First, remove from rpc (settxfee, paytxfee result)
* Then, remove from cli (-paytxfee, tests only)
* Then, remove from cli (-paytxfee, code, and remaining leftovers)
💬 maflcko commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#issuecomment-3242635933)
re-ACK 5ded99a7f007b142f6b0ec89e0c71ef281b42684 🎯
<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: re-ACK 5ded99a7f007b142f6b0
...
(https://github.com/bitcoin/bitcoin/pull/33210#issuecomment-3242635933)
re-ACK 5ded99a7f007b142f6b0ec89e0c71ef281b42684 🎯
<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: re-ACK 5ded99a7f007b142f6b0
...
🤔 ismaelsadeeq reviewed a pull request: "fuzz: enhance wallet_fees by mocking mempool stuff"
(https://github.com/bitcoin/bitcoin/pull/33210#pullrequestreview-3173960912)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33210#pullrequestreview-3173960912)
Concept ACK
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3242681908)
Looks like Cirrus switched from Neoverse N1 to Apple (with virtual Linux aarch64), which doesn't support 32-bit mode, so the CI here fails expectedly.
Maybe you can use GHA `ubuntu-24.04-arm` for now, which has:
```
VM Image
- OS: Linux (arm64)
- Source: Partner
- Name: Ubuntu 24.04 by Arm Limited
- Version: 20250728.24.1
- Included Software: https://github.com/actions/partner-runner-images/blob/main/images/arm-ubuntu-24-image.md
Run lscpu
Architecture:
...
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3242681908)
Looks like Cirrus switched from Neoverse N1 to Apple (with virtual Linux aarch64), which doesn't support 32-bit mode, so the CI here fails expectedly.
Maybe you can use GHA `ubuntu-24.04-arm` for now, which has:
```
VM Image
- OS: Linux (arm64)
- Source: Partner
- Name: Ubuntu 24.04 by Arm Limited
- Version: 20250728.24.1
- Included Software: https://github.com/actions/partner-runner-images/blob/main/images/arm-ubuntu-24-image.md
Run lscpu
Architecture:
...
✅ fanquake closed a pull request: "[29.x] depends: remove xinerama extension from libxcb"
(https://github.com/bitcoin/bitcoin/pull/33238)
(https://github.com/bitcoin/bitcoin/pull/33238)
💬 fanquake commented on pull request "[29.x] depends: remove xinerama extension from libxcb":
(https://github.com/bitcoin/bitcoin/pull/33238#issuecomment-3242682102)
Don't think I'm going to try and backport this into QT.
> then https://github.com/bitcoin/bitcoin/issues/32097 should be addressed for all branches.
Put this onto 30.0, so there is some resolution.
(https://github.com/bitcoin/bitcoin/pull/33238#issuecomment-3242682102)
Don't think I'm going to try and backport this into QT.
> then https://github.com/bitcoin/bitcoin/issues/32097 should be addressed for all branches.
Put this onto 30.0, so there is some resolution.
💬 fanquake commented on issue "Linux download needs installation instructions":
(https://github.com/bitcoin/bitcoin/issues/32097#issuecomment-3242683980)
Added this to `30.0`, but this seems to need addressing for all release branches, in some way.
(https://github.com/bitcoin/bitcoin/issues/32097#issuecomment-3242683980)
Added this to `30.0`, but this seems to need addressing for all release branches, in some way.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2314191993)
Done.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2314191993)
Done.