π¬ theDavidCoen commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3327813332)
> > This is the 3rd PR since #32406's merge discussing what the config options' docs should say.
>
> Thank you to all who have reviewed and provided thoughtful feedback on this PR.
>
> > I believe this PR tries to align the documentation with what users can expect. Usage of these options can cause the node to reject transactions that are likely to be mined, so the docs should discourage their use (to me, that means deprecated). However, it seems that removal in the near future is (1) unlik
...
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3327813332)
> > This is the 3rd PR since #32406's merge discussing what the config options' docs should say.
>
> Thank you to all who have reviewed and provided thoughtful feedback on this PR.
>
> > I believe this PR tries to align the documentation with what users can expect. Usage of these options can cause the node to reject transactions that are likely to be mined, so the docs should discourage their use (to me, that means deprecated). However, it seems that removal in the near future is (1) unlik
...
π¬ janb84 commented on pull request "guix: documented shasum gathering command":
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3327869054)
Addressed all the feedback (including the linter's spelling sugestion )
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3327869054)
Addressed all the feedback (including the linter's spelling sugestion )
π¬ ryanofsky commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3327870842)
> Tangibly, that would mean closing this PR
FWIW, I like the current PR more than these other ideas. I see these options as basically the same as other options. Not particularly dangerous, not particularly interesting, likely to be kept if used and there are contributors willing to maintain them, and likely to be removed if not used or they impose significant costs. I think having a warning which draws special attention to these options just amps up partisans on both sides and does not help t
...
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3327870842)
> Tangibly, that would mean closing this PR
FWIW, I like the current PR more than these other ideas. I see these options as basically the same as other options. Not particularly dangerous, not particularly interesting, likely to be kept if used and there are contributors willing to maintain them, and likely to be removed if not used or they impose significant costs. I think having a warning which draws special attention to these options just amps up partisans on both sides and does not help t
...
π¬ trevarj commented on pull request "guix: documented shasum gathering command":
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3327942653)
ACK [d29ab99](https://github.com/bitcoin/bitcoin/commit/d29ab9946f3c1916032f41e7a365dfcb26af2c46)
Thanks for this
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3327942653)
ACK [d29ab99](https://github.com/bitcoin/bitcoin/commit/d29ab9946f3c1916032f41e7a365dfcb26af2c46)
Thanks for this
π¬ trevarj commented on pull request "contrib: Add zsh completion scripts":
(https://github.com/bitcoin/bitcoin/pull/33402#issuecomment-3327956176)
@fanquake this seems useful and I think the bot made a mistake. Could it be reopened?
(https://github.com/bitcoin/bitcoin/pull/33402#issuecomment-3327956176)
@fanquake this seems useful and I think the bot made a mistake. Could it be reopened?
π€ maflcko reviewed a pull request: "contrib: Add zsh completion scripts"
(https://github.com/bitcoin/bitcoin/pull/33402#pullrequestreview-3262619549)
You'd have to update the line in the docs? https://github.com/bitcoin/bitcoin/blob/3b5e6121f58f0ca4663deabe793a26be38b0f535/contrib/README.md?plain=1#L45
(https://github.com/bitcoin/bitcoin/pull/33402#pullrequestreview-3262619549)
You'd have to update the line in the docs? https://github.com/bitcoin/bitcoin/blob/3b5e6121f58f0ca4663deabe793a26be38b0f535/contrib/README.md?plain=1#L45
π¬ maflcko commented on pull request "contrib: Add zsh completion scripts":
(https://github.com/bitcoin/bitcoin/pull/33402#discussion_r2375595433)
nit: for new files, you can drop the year range, or use `2025-present`, to avoid having to touch it again in the future.
(https://github.com/bitcoin/bitcoin/pull/33402#discussion_r2375595433)
nit: for new files, you can drop the year range, or use `2025-present`, to avoid having to touch it again in the future.
π TheCharlatan approved a pull request: "rpc: fix getblock(header) returns target for tip"
(https://github.com/bitcoin/bitcoin/pull/33446#pullrequestreview-3262639879)
ACK bf7996cbc3becf329d8b1cd2f1007fec9b3a3188
(https://github.com/bitcoin/bitcoin/pull/33446#pullrequestreview-3262639879)
ACK bf7996cbc3becf329d8b1cd2f1007fec9b3a3188
π¬ vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-3328166580)
`5d7b70161d...e4e13ce4c0`: rebase due to conflicts
Part of this PR was merged via https://github.com/bitcoin/bitcoin/pull/33388, so drop that bit from here. Thanks!
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-3328166580)
`5d7b70161d...e4e13ce4c0`: rebase due to conflicts
Part of this PR was merged via https://github.com/bitcoin/bitcoin/pull/33388, so drop that bit from here. Thanks!
π€ ismaelsadeeq reviewed a pull request: "rpc: fix getblock(header) returns target for tip"
(https://github.com/bitcoin/bitcoin/pull/33446#pullrequestreview-3262538477)
Code review ACK bf7996cbc3becf329d8b1cd2f1007fec9b3a3188
Comments are on tests and doc
(https://github.com/bitcoin/bitcoin/pull/33446#pullrequestreview-3262538477)
Code review ACK bf7996cbc3becf329d8b1cd2f1007fec9b3a3188
Comments are on tests and doc
π¬ ismaelsadeeq commented on pull request "rpc: fix getblock(header) returns target for tip":
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375540370)
In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd
nit: commit message typo
s/retarger_period/retarget_period/g
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375540370)
In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd
nit: commit message typo
s/retarger_period/retarget_period/g
π¬ ismaelsadeeq commented on pull request "rpc: fix getblock(header) returns target for tip":
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375610799)
In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd
This is a bit confusing @Sjors you updated the name to `halving_period` but still use the regtest retaget period constant.
The constant should be updated `REGTEST_SUBSIDY_HALVING_INTERVAL` for regtest and also define the interval for mainet too, it can be reused in other places.
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375610799)
In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd
This is a bit confusing @Sjors you updated the name to `halving_period` but still use the regtest retaget period constant.
The constant should be updated `REGTEST_SUBSIDY_HALVING_INTERVAL` for regtest and also define the interval for mainet too, it can be reused in other places.
π¬ ismaelsadeeq commented on pull request "rpc: fix getblock(header) returns target for tip":
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375621220)
In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd
Is this okay to be here? Shouldn't it be in an issue to discuss the tradeoff and why those checks are added in the first place and why it should be removed?
(https://github.com/bitcoin/bitcoin/pull/33446#discussion_r2375621220)
In "test: add block 2016 to mock mainnet" 4c3c1f42cf705e039751395799240da33ca969bd
Is this okay to be here? Shouldn't it be in an issue to discuss the tradeoff and why those checks are added in the first place and why it should be removed?
π¬ hebasto commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3328210405)
@davidgumberg
> Trying to do a guix build on this branch I get the following error: ...
What's the architecture of your build platform?
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3328210405)
@davidgumberg
> Trying to do a guix build on this branch I get the following error: ...
What's the architecture of your build platform?
π¬ m3dwards commented on pull request "Backport Cirrus runners to 29.x":
(https://github.com/bitcoin/bitcoin/pull/33403#issuecomment-3328222723)
ACK 5750355139eb7fc2bd11124adf46bf053be6b690
So here are all the (innocent) changed that I've found compared with PR https://github.com/bitcoin/bitcoin/pull/32989
- Windows job uses cmd rather than powershell
- i686 job file is different in 29.x
- Macos test script is different in 29.x
- Adds decreasing runner sizes
- Adds checking latest merged pulls
- Adds ci: link against -lstdc++
Tools used to compare: [Will's backport fish function](https://github.com/willcl-ark/nixos-config/b
...
(https://github.com/bitcoin/bitcoin/pull/33403#issuecomment-3328222723)
ACK 5750355139eb7fc2bd11124adf46bf053be6b690
So here are all the (innocent) changed that I've found compared with PR https://github.com/bitcoin/bitcoin/pull/32989
- Windows job uses cmd rather than powershell
- i686 job file is different in 29.x
- Macos test script is different in 29.x
- Adds decreasing runner sizes
- Adds checking latest merged pulls
- Adds ci: link against -lstdc++
Tools used to compare: [Will's backport fish function](https://github.com/willcl-ark/nixos-config/b
...
π¬ hebasto commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3328228978)
> This library is no-longer present on modern Ubuntu.
Specifically, the Ubuntu 24.04.3 LTS image from https://ubuntu.com/download/desktop does not install `libxcb-cursor0` when using the "Default selection" install option.
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3328228978)
> This library is no-longer present on modern Ubuntu.
Specifically, the Ubuntu 24.04.3 LTS image from https://ubuntu.com/download/desktop does not install `libxcb-cursor0` when using the "Default selection" install option.
π¬ janb84 commented on pull request "guix: documented shasum gathering command":
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3328233182)
Guix Build Output
```shell
9e3e3372c1bb063a575e9f6037c53a666537cf6d297116601e2f2de8092cfe61 guix-build-d29ab9946f3c/output/aarch64-linux-gnu/SHA256SUMS.part
6b1183039d07aa9ba16b50980d10d28f3c5598ad8412bb9088d3c9e66f059f43 guix-build-d29ab9946f3c/output/aarch64-linux-gnu/bitcoin-d29ab9946f3c-aarch64-linux-gnu-debug.tar.gz
d7abff7f58f2bf1813afb76226a96646b057c56f1ff8f4b5a45e309063c9c23e guix-build-d29ab9946f3c/output/aarch64-linux-gnu/bitcoin-d29ab9946f3c-aarch64-linux-gnu.tar.gz
724
...
(https://github.com/bitcoin/bitcoin/pull/33472#issuecomment-3328233182)
Guix Build Output
```shell
9e3e3372c1bb063a575e9f6037c53a666537cf6d297116601e2f2de8092cfe61 guix-build-d29ab9946f3c/output/aarch64-linux-gnu/SHA256SUMS.part
6b1183039d07aa9ba16b50980d10d28f3c5598ad8412bb9088d3c9e66f059f43 guix-build-d29ab9946f3c/output/aarch64-linux-gnu/bitcoin-d29ab9946f3c-aarch64-linux-gnu-debug.tar.gz
d7abff7f58f2bf1813afb76226a96646b057c56f1ff8f4b5a45e309063c9c23e guix-build-d29ab9946f3c/output/aarch64-linux-gnu/bitcoin-d29ab9946f3c-aarch64-linux-gnu.tar.gz
724
...
π¬ TheCharlatan commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3328236725)
I tried rebasing this PR, but I'm getting a bunch of compilation errors. Can you rebase it @ryanofsky?
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3328236725)
I tried rebasing this PR, but I'm getting a bunch of compilation errors. Can you rebase it @ryanofsky?
π¬ plebhash commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3328278574)
> Most useful thing to debug would be a stack trace, which you could get by running the process under GDB and running `thread apply all bt`.
@ryanofsky I'm on macOS, would a stack trace from LLDB be equally useful?
> Other things to try might be running with `-debug=ipc` to log IPC calls
here's logs running with `-debug=ipc`, trying to kill it with ctrl+c, not being able to, and then actually killing via `sudo kill -9`:
```
./build/bin/bitcoin-node -regtest -ipcbind=unix -debug=ipc
...
2025-0
...
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3328278574)
> Most useful thing to debug would be a stack trace, which you could get by running the process under GDB and running `thread apply all bt`.
@ryanofsky I'm on macOS, would a stack trace from LLDB be equally useful?
> Other things to try might be running with `-debug=ipc` to log IPC calls
here's logs running with `-debug=ipc`, trying to kill it with ctrl+c, not being able to, and then actually killing via `sudo kill -9`:
```
./build/bin/bitcoin-node -regtest -ipcbind=unix -debug=ipc
...
2025-0
...
π¬ bitschmidty commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3328286118)
> "deprecated but not currently scheduled for removal" makes no sense to me from a user point of view
I can understand where youre coming from. Confusion around this is partly due to different definitions of deprecated. Most include 'discouraged for use', yet some also add something like 'slated for future removal'. Two separate considerations in one single flag. So can end up to some as a sort of SchrΓΆdinger's deprecation.
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3328286118)
> "deprecated but not currently scheduled for removal" makes no sense to me from a user point of view
I can understand where youre coming from. Confusion around this is partly due to different definitions of deprecated. Most include 'discouraged for use', yet some also add something like 'slated for future removal'. Two separate considerations in one single flag. So can end up to some as a sort of SchrΓΆdinger's deprecation.