👍 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)
🚀 fanquake merged a pull request: "test: reduce runtime of p2p_opportunistic_1p1c.py"
(https://github.com/bitcoin/bitcoin/pull/33048)
(https://github.com/bitcoin/bitcoin/pull/33048)
💬 furszy commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3144915011)
> > > Hi @furszy , sorry for the delay, I've picked your commit of the test, which I missed earlier.
> >
> >
> > No problem. This needs a rebase.
>
> Hi @furszy , seems the test causes some data race, would be great if you could update it. I tried to fix it but I'm not familiar with the index logic.
It seems thread-related. Wait for the background thread to finish before the test ends: https://github.com/furszy/bitcoin-core/commit/30e0b2929da53a1519e282504720fac43b180ea0 (same code, j
...
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3144915011)
> > > Hi @furszy , sorry for the delay, I've picked your commit of the test, which I missed earlier.
> >
> >
> > No problem. This needs a rebase.
>
> Hi @furszy , seems the test causes some data race, would be great if you could update it. I tried to fix it but I'm not familiar with the index logic.
It seems thread-related. Wait for the background thread to finish before the test ends: https://github.com/furszy/bitcoin-core/commit/30e0b2929da53a1519e282504720fac43b180ea0 (same code, j
...
💬 mzumsande commented on issue "test: spurious failure in p2p_leak.py --v1transport":
(https://github.com/bitcoin/bitcoin/issues/33090#issuecomment-3144942332)
FYI the failing test is `p2p_leak_tx.py` (not `p2p_leak.py` as in title).
(https://github.com/bitcoin/bitcoin/issues/33090#issuecomment-3144942332)
FYI the failing test is `p2p_leak_tx.py` (not `p2p_leak.py` as in title).