💬 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.
💬 Sjors commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2314193410)
`magnet:?xt=urn:btih:7341b215b570e3bc69f5fbbe5e817b51b0b9b542&dn=utxo-testnet4-90000.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%2Ftracker.torrent.eu.org%3A451%2Fannounce&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2314193410)
`magnet:?xt=urn:btih:7341b215b570e3bc69f5fbbe5e817b51b0b9b542&dn=utxo-testnet4-90000.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%2Ftracker.torrent.eu.org%3A451%2Fannounce&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
📝 hebasto opened a pull request: "Release: 30.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/33275)
This PR follows our [Release Process](https://github.com/bitcoin/bitcoin/blob/53a996f122663e271efa52c45b173613b8ac635e/doc/release-process.md) and concludes the translation-specific efforts for this release cycle. It follows two previous translation-related PRs, https://github.com/bitcoin/bitcoin/pull/33152 and https://github.com/bitcoin/bitcoin/pull/33193.
It is one of the steps required _before_ branch-off, as scheduled in https://github.com/bitcoin/bitcoin/issues/32275.
A previous simil
...
(https://github.com/bitcoin/bitcoin/pull/33275)
This PR follows our [Release Process](https://github.com/bitcoin/bitcoin/blob/53a996f122663e271efa52c45b173613b8ac635e/doc/release-process.md) and concludes the translation-specific efforts for this release cycle. It follows two previous translation-related PRs, https://github.com/bitcoin/bitcoin/pull/33152 and https://github.com/bitcoin/bitcoin/pull/33193.
It is one of the steps required _before_ branch-off, as scheduled in https://github.com/bitcoin/bitcoin/issues/32275.
A previous simil
...
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3242712490)
I've manually removed from Transifex, before pulling into this branch, all malicious translations, such as Bitcoin addresses etc. Translation coordinators, both seasoned and newly assigned, did a great job on keeping their translations safe and consistent.
@maflcko
Could you please run your LLM-based checks on this branch?
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3242712490)
I've manually removed from Transifex, before pulling into this branch, all malicious translations, such as Bitcoin addresses etc. Translation coordinators, both seasoned and newly assigned, did a great job on keeping their translations safe and consistent.
@maflcko
Could you please run your LLM-based checks on this branch?
💬 Sjors commented on pull request "kernel: chainparams & headersync updates for 30.0":
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2314242221)
Ceterum censeo #31974 :-)
(https://github.com/bitcoin/bitcoin/pull/33274#discussion_r2314242221)
Ceterum censeo #31974 :-)
💬 fanquake commented on pull request "macdeploy: avoid use of `Bitcoin Core` in Linux cross build":
(https://github.com/bitcoin/bitcoin/pull/33158#issuecomment-3242773819)
> I find it weird that we would produce files named differently for the same target when the hosts are different
> Agreed that different names depending on originating platform might be a bit confusing.
I don't mind much, but I don't think the different names are that weird, given the two outputs are different. One is a release-like zip, with an immediately usable binary, the other is a zip containing a binary that wont run, which is immediately renamed, and fed into the codesigning process.
...
(https://github.com/bitcoin/bitcoin/pull/33158#issuecomment-3242773819)
> I find it weird that we would produce files named differently for the same target when the hosts are different
> Agreed that different names depending on originating platform might be a bit confusing.
I don't mind much, but I don't think the different names are that weird, given the two outputs are different. One is a release-like zip, with an immediately usable binary, the other is a zip containing a binary that wont run, which is immediately renamed, and fed into the codesigning process.
...
💬 151henry151 commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3242805794)
Is there anything further required here? I understand that review for pull requests can take a long time (especially for non-critical things such as this one) but just wanted to check in and see if there's further action needed from me, or anything about this pull request that I could improve.
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3242805794)
Is there anything further required here? I understand that review for pull requests can take a long time (especially for non-critical things such as this one) but just wanted to check in and see if there's further action needed from me, or anything about this pull request that I could improve.