💬 1ma commented on pull request "[29.x] 33106 backport and final changes for rc2":
(https://github.com/bitcoin/bitcoin/pull/33226#issuecomment-3206599181)
What is the criteria for backporting PRs to point releases? Sub 1 s/vB standardization is certainly not a bugfix.
(https://github.com/bitcoin/bitcoin/pull/33226#issuecomment-3206599181)
What is the criteria for backporting PRs to point releases? Sub 1 s/vB standardization is certainly not a bugfix.
✅ pinheadmz closed an issue: "RPC sendmany first (dummy, empty string) argument is not optional"
(https://github.com/bitcoin/bitcoin/issues/33182)
(https://github.com/bitcoin/bitcoin/issues/33182)
💬 pinheadmz commented on issue "RPC sendmany first (dummy, empty string) argument is not optional":
(https://github.com/bitcoin/bitcoin/issues/33182#issuecomment-3206632314)
ah using `-named=1` is my answer thanks.
(https://github.com/bitcoin/bitcoin/issues/33182#issuecomment-3206632314)
ah using `-named=1` is my answer thanks.
💬 janb84 commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206654682)
> Please take a step back and just think like a user. v29 shipped with `bitcoind`. They'll now see `bitcoin`, `bitcoind`, and `bitcoin-node`. Even if their behavior doesn't have to change at all, anyone could be forgiven for being confused by that.
>
@theuni could it be that the context you are describing is the `<<buid-dir>>/bin` directory and not the result of `cmake --install ` result that splits binaries in `${CMAKE_PREFIX_PATH}/bin` and `${CMAKE_PREFIX_PATH}/libexec` (as per #31679)?
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206654682)
> Please take a step back and just think like a user. v29 shipped with `bitcoind`. They'll now see `bitcoin`, `bitcoind`, and `bitcoin-node`. Even if their behavior doesn't have to change at all, anyone could be forgiven for being confused by that.
>
@theuni could it be that the context you are describing is the `<<buid-dir>>/bin` directory and not the result of `cmake --install ` result that splits binaries in `${CMAKE_PREFIX_PATH}/bin` and `${CMAKE_PREFIX_PATH}/libexec` (as per #31679)?
...
⚠️ fanquake opened an issue: "tracing: issue running `contrib/tracing/log_utxos.bt`"
(https://github.com/bitcoin/bitcoin/issues/33227)
Using master (9cf7b3d90c76fd299e156d9615e42b938ee884b2) and running `bpftrace contrib/tracing/log_utxos.bt -v`:
```bash
# Linux fedora-32gb-hel1-3 6.17.0-0.rc1.250812g53e760d89498.18.fc44.aarch64 #1 SMP PREEMPT_DYNAMIC Tue Aug 12 19:11:45 UTC 2025 aarch64 GNU/Linux
bpftrace contrib/tracing/log_utxos.bt -v
Attaching 4 probes...
ERROR: Error loading BPF program for BEGIN_1.
Kernel error log:
processed 103 insns (limit 1000000) max_states_per_insn 0 total_states 2 peak_states 2 mark_read 1
ERRO
...
(https://github.com/bitcoin/bitcoin/issues/33227)
Using master (9cf7b3d90c76fd299e156d9615e42b938ee884b2) and running `bpftrace contrib/tracing/log_utxos.bt -v`:
```bash
# Linux fedora-32gb-hel1-3 6.17.0-0.rc1.250812g53e760d89498.18.fc44.aarch64 #1 SMP PREEMPT_DYNAMIC Tue Aug 12 19:11:45 UTC 2025 aarch64 GNU/Linux
bpftrace contrib/tracing/log_utxos.bt -v
Attaching 4 probes...
ERROR: Error loading BPF program for BEGIN_1.
Kernel error log:
processed 103 insns (limit 1000000) max_states_per_insn 0 total_states 2 peak_states 2 mark_read 1
ERRO
...
💬 theuni commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206701996)
> @theuni Thanks for stating your preference. If I am following, you would like to:
It seems (as usual :p) that we're talking past each other.
Above, I described what would've been, in my opinion, the ideal features for v30. That would've been a bitcoind with build-time opt-out for IPC. That build-time opt-out would've added `-ipcbind` to bitcoind. It would've been default-on, so that all devs would've encountered IPC as part of their build process, or turned it off manually. By default, t
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206701996)
> @theuni Thanks for stating your preference. If I am following, you would like to:
It seems (as usual :p) that we're talking past each other.
Above, I described what would've been, in my opinion, the ideal features for v30. That would've been a bitcoind with build-time opt-out for IPC. That build-time opt-out would've added `-ipcbind` to bitcoind. It would've been default-on, so that all devs would've encountered IPC as part of their build process, or turned it off manually. By default, t
...
🤔 maflcko reviewed a pull request: "test: use local `CBlockIndex` in block read hash mismatch check"
(https://github.com/bitcoin/bitcoin/pull/33154#pullrequestreview-3136954211)
lgtm ACK cb173b8e939d63821a966d0d76b299f20742c619
(https://github.com/bitcoin/bitcoin/pull/33154#pullrequestreview-3136954211)
lgtm ACK cb173b8e939d63821a966d0d76b299f20742c619
💬 maflcko commented on pull request "test: use local `CBlockIndex` in block read hash mismatch check":
(https://github.com/bitcoin/bitcoin/pull/33154#discussion_r2288403957)
nit: Could use the `m_node.chainman->GetMutex()` alias for new code.
(https://github.com/bitcoin/bitcoin/pull/33154#discussion_r2288403957)
nit: Could use the `m_node.chainman->GetMutex()` alias for new code.
💬 0xB10C commented on issue "tracing: issue running `contrib/tracing/log_utxos.bt`":
(https://github.com/bitcoin/bitcoin/issues/33227#issuecomment-3206722277)
To confirm, this is on `aarch64`, correct? I'll see if I can get a machine to test this.
(https://github.com/bitcoin/bitcoin/issues/33227#issuecomment-3206722277)
To confirm, this is on `aarch64`, correct? I'll see if I can get a machine to test this.
💬 fanquake commented on issue "tracing: issue running `contrib/tracing/log_utxos.bt`":
(https://github.com/bitcoin/bitcoin/issues/33227#issuecomment-3206727240)
> To confirm, this is on aarch64, correct?
Yes, aarch64.
(https://github.com/bitcoin/bitcoin/issues/33227#issuecomment-3206727240)
> To confirm, this is on aarch64, correct?
Yes, aarch64.
👍 dergoegge approved a pull request: "[29.x] 33106 backport and final changes for rc2"
(https://github.com/bitcoin/bitcoin/pull/33226#pullrequestreview-3137001892)
utACK 0034dcfba9dc599449e7569ed1b30e58d4f4434f
(pending CI)
(https://github.com/bitcoin/bitcoin/pull/33226#pullrequestreview-3137001892)
utACK 0034dcfba9dc599449e7569ed1b30e58d4f4434f
(pending CI)
💬 theuni commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206751392)
> > Please take a step back and just think like a user. v29 shipped with `bitcoind`. They'll now see `bitcoin`, `bitcoind`, and `bitcoin-node`. Even if their behavior doesn't have to change at all, anyone could be forgiven for being confused by that.
>
> @theuni could it be that the context you are describing is the `<<buid-dir>>/bin` directory and not the result of `cmake --install ` result that splits binaries in `${CMAKE_PREFIX_PATH}/bin` and `${CMAKE_PREFIX_PATH}/libexec` (as per #31679)?
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206751392)
> > Please take a step back and just think like a user. v29 shipped with `bitcoind`. They'll now see `bitcoin`, `bitcoind`, and `bitcoin-node`. Even if their behavior doesn't have to change at all, anyone could be forgiven for being confused by that.
>
> @theuni could it be that the context you are describing is the `<<buid-dir>>/bin` directory and not the result of `cmake --install ` result that splits binaries in `${CMAKE_PREFIX_PATH}/bin` and `${CMAKE_PREFIX_PATH}/libexec` (as per #31679)?
...
💬 janb84 commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206789051)
>
> Heh, I promise I understand the `libexec/` distinction. Really. I understand its use as a path for subcommand execution (ala git). Really.
>
> I only mentioned it because I was picturing a user downloading from bitcoincore.org (it's the binary release that we're all arguing about, right?), unpacking it, and seeing new files.
>
Thank you for this insight, did not think of this while reviewing the conversation / PR.
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206789051)
>
> Heh, I promise I understand the `libexec/` distinction. Really. I understand its use as a path for subcommand execution (ala git). Really.
>
> I only mentioned it because I was picturing a user downloading from bitcoincore.org (it's the binary release that we're all arguing about, right?), unpacking it, and seeing new files.
>
Thank you for this insight, did not think of this while reviewing the conversation / PR.
💬 maflcko commented on issue "Crash on launch in PruneBlockIndexCandidates":
(https://github.com/bitcoin/bitcoin/issues/33129#issuecomment-3206789552)
With the datadir deleted (https://github.com/bitcoin/bitcoin/pull/33127#issuecomment-3177718602), this makes it harder to debug without any debug log or state to investigate. Maybe this can be closed for now?
(https://github.com/bitcoin/bitcoin/issues/33129#issuecomment-3206789552)
With the datadir deleted (https://github.com/bitcoin/bitcoin/pull/33127#issuecomment-3177718602), this makes it harder to debug without any debug log or state to investigate. Maybe this can be closed for now?
💬 marcofleon commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206842147)
@TheBlueMatt I agree that having the experience be seamless for miners is the goal. I more meant for the purposes of SRI contributors and maybe a couple of brave miners who are willing to help with testing of Core and SV2, so we can avoid buggy code in releases.
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206842147)
@TheBlueMatt I agree that having the experience be seamless for miners is the goal. I more meant for the purposes of SRI contributors and maybe a couple of brave miners who are willing to help with testing of Core and SV2, so we can avoid buggy code in releases.
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206864744)
re: https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206701996
> > I'm in favor of multiprocess happening when it's ready. We should have a point in time where we decide, as a project, to start moving everything to multiprocess. At that point, there will inevitably be a mental cost to users, but at least it can be explained then as "this gives you the benefit of process separation, being able to run wallets elsewhere, stop/start gui independently from node, ...", and it'll be used
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206864744)
re: https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206701996
> > I'm in favor of multiprocess happening when it's ready. We should have a point in time where we decide, as a project, to start moving everything to multiprocess. At that point, there will inevitably be a mental cost to users, but at least it can be explained then as "this gives you the benefit of process separation, being able to run wallets elsewhere, stop/start gui independently from node, ...", and it'll be used
...
💬 maflcko commented on pull request "CI: silent merge check":
(https://github.com/bitcoin/bitcoin/pull/33145#discussion_r2288552077)
Yeah, merging this with the main CI file is certainly better, if possible. However, it still leaves the issue of label spam. Also, it needs to be confirmed to be working correctly (to send the failed-notification to the right person)
(https://github.com/bitcoin/bitcoin/pull/33145#discussion_r2288552077)
Yeah, merging this with the main CI file is certainly better, if possible. However, it still leaves the issue of label spam. Also, it needs to be confirmed to be working correctly (to send the failed-notification to the right person)
📝 luke-jr opened a pull request: "Bugfix: AllocateFileRange: Address various issues"
(https://github.com/bitcoin/bitcoin/pull/33228)
Not a real fix for #33128, but a workaround for the issue.
Potentially might fix #28552 too, but I don't have a way to reproduce that. (After digging into Apple's XNU code, it looks too buggy to try to use `F_PREALLOCATE` at all IMO; for reference: [XNU kernel code](https://github.com/apple-oss-distributions/xnu/blob/e3723e1f17661b24996789d8afc084c0c3303b26/bsd/kern/kern_descrip.c#L3289) and [HFS driver code](https://github.com/apple-oss-distributions/hfs/blob/a4bba3ecca6d79d997caf27c9a388eb1
...
(https://github.com/bitcoin/bitcoin/pull/33228)
Not a real fix for #33128, but a workaround for the issue.
Potentially might fix #28552 too, but I don't have a way to reproduce that. (After digging into Apple's XNU code, it looks too buggy to try to use `F_PREALLOCATE` at all IMO; for reference: [XNU kernel code](https://github.com/apple-oss-distributions/xnu/blob/e3723e1f17661b24996789d8afc084c0c3303b26/bsd/kern/kern_descrip.c#L3289) and [HFS driver code](https://github.com/apple-oss-distributions/hfs/blob/a4bba3ecca6d79d997caf27c9a388eb1
...
💬 purpleKarrot commented on pull request "kernel: improve BlockChecked ownership semantics":
(https://github.com/bitcoin/bitcoin/pull/33078#issuecomment-3206928002)
> The alternative would be to raise the level of abstraction and use `std::shared_ptr<T const>` as an implementation detail.
> I understand that this may conflict with your approach of giving access to both the pointer and the pointee through the public API. I would prefer to delay this PR until we reached consensus on the ownership approach for the API.
It seams like we reached consensus that we will **not** provide access to the pointee through the public api. That means we can replace `st
...
(https://github.com/bitcoin/bitcoin/pull/33078#issuecomment-3206928002)
> The alternative would be to raise the level of abstraction and use `std::shared_ptr<T const>` as an implementation detail.
> I understand that this may conflict with your approach of giving access to both the pointer and the pointee through the public API. I would prefer to delay this PR until we reached consensus on the ownership approach for the API.
It seams like we reached consensus that we will **not** provide access to the pointee through the public api. That means we can replace `st
...
💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206935621)
re: https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206751392
> They'll encounter the old `bitcoind` binary, a new `bitcoin` binary, and a new `bitcoin-node` binary in a weird directory that seems to do the same thing as `bitcoind` if they try to run it.
This also seems like actionable feedback. Note there is also a `test_bitcoin` binary in the `libexec/` directory and a [readme](https://github.com/bitcoin/bitcoin/blob/master/doc/README.md) file in the root directory to help or
...
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206935621)
re: https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3206751392
> They'll encounter the old `bitcoind` binary, a new `bitcoin` binary, and a new `bitcoin-node` binary in a weird directory that seems to do the same thing as `bitcoind` if they try to run it.
This also seems like actionable feedback. Note there is also a `test_bitcoin` binary in the `libexec/` directory and a [readme](https://github.com/bitcoin/bitcoin/blob/master/doc/README.md) file in the root directory to help or
...
🤔 mzumsande reviewed a pull request: "index: Fix coinstats overflow"
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3137273070)
I like the approach, though it's a little bit weird that
`m_total_amount` and `m_total_unspendable_amount` are first removed in e54317bf7c1b93d790e169759f64a4564a96ea5f, and after that reintroduced in 053ac55eb569be6b49c5249a7d8c0eeaa149a18b. Happy to ACK once @achow101's point about `m_total_unspendables_bip30` is addressed (which I also tripped over).
> somehow we ever have the situation where we are appending a block that doesn't build on the index's current block hash.
I kinda view
...
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3137273070)
I like the approach, though it's a little bit weird that
`m_total_amount` and `m_total_unspendable_amount` are first removed in e54317bf7c1b93d790e169759f64a4564a96ea5f, and after that reintroduced in 053ac55eb569be6b49c5249a7d8c0eeaa149a18b. Happy to ACK once @achow101's point about `m_total_unspendables_bip30` is addressed (which I also tripped over).
> somehow we ever have the situation where we are appending a block that doesn't build on the index's current block hash.
I kinda view
...