💬 fanquake commented on pull request "contrib: drop use of `PermissionsStartOnly` & `Group=`":
(https://github.com/bitcoin/bitcoin/pull/33044#discussion_r2248031122)
Thanks, fixed the commit message.
(https://github.com/bitcoin/bitcoin/pull/33044#discussion_r2248031122)
Thanks, fixed the commit message.
📝 marcofleon opened a pull request: "refactor: Convert uint256 to Txid"
(https://github.com/bitcoin/bitcoin/pull/33116)
This is the final leg of the [type safety refactor](https://github.com/bitcoin/bitcoin/pull/32189).
All of these changes are straightforward `uint256` --> `Txid` along with any necessary explicit conversions. Also, `transaction_identifier.h` is moved to primitives in the last commit, as `Txid` and `Wtxid` become fundamental types after this PR.
(https://github.com/bitcoin/bitcoin/pull/33116)
This is the final leg of the [type safety refactor](https://github.com/bitcoin/bitcoin/pull/32189).
All of these changes are straightforward `uint256` --> `Txid` along with any necessary explicit conversions. Also, `transaction_identifier.h` is moved to primitives in the last commit, as `Txid` and `Wtxid` become fundamental types after this PR.
💬 willcl-ark commented on pull request "contrib: drop use of `PermissionsStartOnly` & `Group=`":
(https://github.com/bitcoin/bitcoin/pull/33044#issuecomment-3144655781)
> Separately , I am wondering if we should move these files out to [bitcoin-core/packaging](https://github.com/bitcoin-core/packaging/?rgh-link-date=2025-07-23T13%3A35%3A02Z).
A web search for the systemd service file shows barely any direct results, most notably some [Stacks.co docs](https://docs.stacks.co/guides-and-tutorials/nodes-and-miners/run-a-bitcoin-node)
Fewer still for the openrc and init files.
So, I suspect that moving these to the packaging repo would not have much systema
...
(https://github.com/bitcoin/bitcoin/pull/33044#issuecomment-3144655781)
> Separately , I am wondering if we should move these files out to [bitcoin-core/packaging](https://github.com/bitcoin-core/packaging/?rgh-link-date=2025-07-23T13%3A35%3A02Z).
A web search for the systemd service file shows barely any direct results, most notably some [Stacks.co docs](https://docs.stacks.co/guides-and-tutorials/nodes-and-miners/run-a-bitcoin-node)
Fewer still for the openrc and init files.
So, I suspect that moving these to the packaging repo would not have much systema
...
👍 willcl-ark approved a pull request: "contrib: drop use of `PermissionsStartOnly` & `Group=`"
(https://github.com/bitcoin/bitcoin/pull/33044#pullrequestreview-3079321239)
reACK 18d1071dd1244cf3252d687bb46f88d65f652e4d
based on: git range-diff b392a513f02...18d1071dd12
<details>
<summary>Details</summary>
```bash
1: b12a63e71aa = 91: 1caaf650436 init: remove Group= as it will default to the user's default group
2: b392a513f02 ! 92: 18d1071dd12 init: replace deprecated PermissionsStartOnly systemd directive
@@ Commit message
PermissionsStartOnly is deprecated [1]. This removes the directives
and instead we prefixes the v
...
(https://github.com/bitcoin/bitcoin/pull/33044#pullrequestreview-3079321239)
reACK 18d1071dd1244cf3252d687bb46f88d65f652e4d
based on: git range-diff b392a513f02...18d1071dd12
<details>
<summary>Details</summary>
```bash
1: b12a63e71aa = 91: 1caaf650436 init: remove Group= as it will default to the user's default group
2: b392a513f02 ! 92: 18d1071dd12 init: replace deprecated PermissionsStartOnly systemd directive
@@ Commit message
PermissionsStartOnly is deprecated [1]. This removes the directives
and instead we prefixes the v
...
✅ marcofleon closed a pull request: "refactor: Txid type safety (parent PR)"
(https://github.com/bitcoin/bitcoin/pull/32189)
(https://github.com/bitcoin/bitcoin/pull/32189)
💬 marcofleon commented on pull request "refactor: Txid type safety (parent PR)":
(https://github.com/bitcoin/bitcoin/pull/32189#issuecomment-3144688621)
Closing this, as the final complete refactor can be seen in https://github.com/bitcoin/bitcoin/pull/32238, https://github.com/bitcoin/bitcoin/pull/32631, https://github.com/bitcoin/bitcoin/pull/33005, and https://github.com/bitcoin/bitcoin/pull/33116. Glad the warning here was heeded 🚨
(https://github.com/bitcoin/bitcoin/pull/32189#issuecomment-3144688621)
Closing this, as the final complete refactor can be seen in https://github.com/bitcoin/bitcoin/pull/32238, https://github.com/bitcoin/bitcoin/pull/32631, https://github.com/bitcoin/bitcoin/pull/33005, and https://github.com/bitcoin/bitcoin/pull/33116. Glad the warning here was heeded 🚨
🤔 janb84 reviewed a pull request: "build: note that sysctl was removed from Linux"
(https://github.com/bitcoin/bitcoin/pull/33111#pullrequestreview-3079402674)
ACK a78dfd7cee4d25a455bd35c5ef2bd7d5cd2072b0
PR changes the error message of `sysctl.h` when used on linux to reflect current state (from deprecated to removed)
(https://github.com/bitcoin/bitcoin/pull/33111#pullrequestreview-3079402674)
ACK a78dfd7cee4d25a455bd35c5ef2bd7d5cd2072b0
PR changes the error message of `sysctl.h` when used on linux to reflect current state (from deprecated to removed)
💬 glozow commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#discussion_r2248057197)
Sharp, thanks!
(https://github.com/bitcoin/bitcoin/pull/33106#discussion_r2248057197)
Sharp, thanks!
💬 glozow commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#discussion_r2248092864)
I've added a few words about it changing more gradually now that incremental relay feerate is lower
(https://github.com/bitcoin/bitcoin/pull/33106#discussion_r2248092864)
I've added a few words about it changing more gradually now that incremental relay feerate is lower
💬 glozow commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#discussion_r2248094200)
I've elaborated in the comments on both `BulkTransaction`s
(https://github.com/bitcoin/bitcoin/pull/33106#discussion_r2248094200)
I've elaborated in the comments on both `BulkTransaction`s
💬 glozow commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#discussion_r2248093280)
added to commit message
(https://github.com/bitcoin/bitcoin/pull/33106#discussion_r2248093280)
added to commit message
🤔 ismaelsadeeq reviewed a pull request: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-3079333077)
Code review ACK 082b416a1bcbfbbdcafb3a3eb1ae800c93385203
I've tested this on MacOS
1. By building the depends for my OS
```terminal
cd depends
brew install cmake make ninja
...
gmake
...
cd ..
cmake -B build_deps --toolchain depends/aarch64-apple-darwin23.6.0/toolchain.cmake
...
cmake --build build_deps -j 5
...
ls build_deps/bin/
bitcoin* bitcoin-cli* bitcoin-gui* bitcoin-node* bitcoin-qt* bitcoin-tx* bitcoin-util* bitcoin-wallet* bitcoind*
...
(https://github.com/bitcoin/bitcoin/pull/31802#pullrequestreview-3079333077)
Code review ACK 082b416a1bcbfbbdcafb3a3eb1ae800c93385203
I've tested this on MacOS
1. By building the depends for my OS
```terminal
cd depends
brew install cmake make ninja
...
gmake
...
cd ..
cmake -B build_deps --toolchain depends/aarch64-apple-darwin23.6.0/toolchain.cmake
...
cmake --build build_deps -j 5
...
ls build_deps/bin/
bitcoin* bitcoin-cli* bitcoin-gui* bitcoin-node* bitcoin-qt* bitcoin-tx* bitcoin-util* bitcoin-wallet* bitcoind*
...
💬 ismaelsadeeq commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2248060554)
Nit: perhaps just mention that we have two new binaries and then state that.
Tiny note mining interface addition is not done in this PR however still mentioning it in this release is okay.
It might be helpful to clarify what we mean by "experimental" in this context. i.e I will prefer us being explicit for example, we could mention that the binary has undergone rounds of review and testing by a group of core contributors, but since this is its first release, users should still proceed with c
...
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2248060554)
Nit: perhaps just mention that we have two new binaries and then state that.
Tiny note mining interface addition is not done in this PR however still mentioning it in this release is okay.
It might be helpful to clarify what we mean by "experimental" in this context. i.e I will prefer us being explicit for example, we could mention that the binary has undergone rounds of review and testing by a group of core contributors, but since this is its first release, users should still proceed with c
...
💬 ismaelsadeeq commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2248049503)
nitty-nit: when you click the link to see the multiprocess docs on github on "display rich diff" it is broken, however this is trivial and will go away once merged.
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2248049503)
nitty-nit: when you click the link to see the multiprocess docs on github on "display rich diff" it is broken, however this is trivial and will go away once merged.
🤔 ismaelsadeeq reviewed a pull request: "ipc: Handle unclean shutdowns better"
(https://github.com/bitcoin/bitcoin/pull/32345#pullrequestreview-3079436916)
Concept ACK
The unclean shutdown I noticed while looking at #30437 is gone after I rebase the PR on top of this.
(https://github.com/bitcoin/bitcoin/pull/32345#pullrequestreview-3079436916)
Concept ACK
The unclean shutdown I noticed while looking at #30437 is gone after I rebase the PR on top of this.
💬 glozow commented on pull request "policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3144779698)
> dust limits need to be taken into account by second layer protocols that build on Bitcoin, so we don't have the same option to try decreasing them and then, if it turns out to be a bad idea, to re-increase them.
I've edited the OP to include this as the main reason for marking dust feerate out of scope in this PR.
> avoiding the p2p network providing attackers with a cheaper way of DoSing itself than commercial providers.
Also reworded the pricing section with this framing.
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3144779698)
> dust limits need to be taken into account by second layer protocols that build on Bitcoin, so we don't have the same option to try decreasing them and then, if it turns out to be a bad idea, to re-increase them.
I've edited the OP to include this as the main reason for marking dust feerate out of scope in this PR.
> avoiding the p2p network providing attackers with a cheaper way of DoSing itself than commercial providers.
Also reworded the pricing section with this framing.
💬 sr-gi commented on issue "intermittent issue in wallet_sendall.py", line 440, in sendall_anti_fee_sniping assert_greater_than(tx_from_wallet["decoded"]["locktime"], tx_from_wallet["blockheight"]":
(https://github.com/bitcoin/bitcoin/issues/33114#issuecomment-3144786536)
Got the same here: https://cirrus-ci.com/task/5816988552921088?logs=ci#L2099
(https://github.com/bitcoin/bitcoin/issues/33114#issuecomment-3144786536)
Got the same here: https://cirrus-ci.com/task/5816988552921088?logs=ci#L2099
📝 hebasto converted_to_draft a pull request: "cmake: Switch to generated `ts_files.cmake` file"
(https://github.com/bitcoin/bitcoin/pull/33115)
This PR:
1. Moves handling of Qt TS files into the `locale` directory.
2. Switches from inferior globbing to the explicit file list generated by the [`bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) script.
Closes #32653.
(https://github.com/bitcoin/bitcoin/pull/33115)
This PR:
1. Moves handling of Qt TS files into the `locale` directory.
2. Switches from inferior globbing to the explicit file list generated by the [`bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) script.
Closes #32653.
🚀 fanquake merged a pull request: "package validation: relax the package-not-child-with-unconfirmed-parents rule"
(https://github.com/bitcoin/bitcoin/pull/31385)
(https://github.com/bitcoin/bitcoin/pull/31385)
👋 hebasto's pull request is ready for review: "cmake: Switch to generated `ts_files.cmake` file"
(https://github.com/bitcoin/bitcoin/pull/33115)
(https://github.com/bitcoin/bitcoin/pull/33115)