💬 ryanofsky commented on issue "Memory leak when using IPC mining interface":
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3571213239)
re: https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570541428
> do we have to explicitly call `destroy` or is it sufficient to drop the reference from memory on the client side?
Following up on this, it's good practice to call `destroy` methods on objects which have them, to guarantee the objects are destroyed right away instead of asynchronously. This is most important if objects have nontrivial destructors, or if it matters what order objects get destroyed.
Block templates usi
...
(https://github.com/bitcoin/bitcoin/issues/33940#issuecomment-3571213239)
re: https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3570541428
> do we have to explicitly call `destroy` or is it sufficient to drop the reference from memory on the client side?
Following up on this, it's good practice to call `destroy` methods on objects which have them, to guarantee the objects are destroyed right away instead of asynchronously. This is most important if objects have nontrivial destructors, or if it matters what order objects get destroyed.
Block templates usi
...
👍 fanquake approved a pull request: "ci: Use latest Xcode that the minimum macOS version allows"
(https://github.com/bitcoin/bitcoin/pull/33932#pullrequestreview-3500865738)
ACK https://github.com/bitcoin/bitcoin/pull/33932/commits/fa9537cde10120b12c96061cbc3f79a7680f9d64 - seems fine.
(https://github.com/bitcoin/bitcoin/pull/33932#pullrequestreview-3500865738)
ACK https://github.com/bitcoin/bitcoin/pull/33932/commits/fa9537cde10120b12c96061cbc3f79a7680f9d64 - seems fine.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2556652874)
Hmm, is this a general frowning against `size_t`?
Atomic is not necessary for `num_broadcasted`.
My thinking is that CPUs work best with word sized integers, so use that when the values will be small and there is no concern about overflow. 64 bit integers on 32 bit CPUs will bloat the program unnecessary and 32 bit integers on 64 bit CPUs may end up not saving space or be slower because of alignment.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2556652874)
Hmm, is this a general frowning against `size_t`?
Atomic is not necessary for `num_broadcasted`.
My thinking is that CPUs work best with word sized integers, so use that when the values will be small and there is no concern about overflow. 64 bit integers on 32 bit CPUs will bloat the program unnecessary and 32 bit integers on 64 bit CPUs may end up not saving space or be slower because of alignment.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2556657318)
It is correct either way. Performance is irrelevant here. Leaving it as it is for now unless somebody asks for a change. Lets move on to more important things.
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2556657318)
It is correct either way. Performance is irrelevant here. Leaving it as it is for now unless somebody asks for a change. Lets move on to more important things.
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3571252838)
`73ea6405da...7826148b12`: do https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2553247864
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-3571252838)
`73ea6405da...7826148b12`: do https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2553247864
💬 furszy commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2556684462)
As the index stores the tx position in disk, how is this correct?
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2556684462)
As the index stores the tx position in disk, how is this correct?
👍 darosior approved a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3500924127)
reACK c34bc01b2ff2fc91ed4020288c5fa15f0c5b075e
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3500924127)
reACK c34bc01b2ff2fc91ed4020288c5fa15f0c5b075e
💬 l0rinc commented on pull request "ci: Use latest Xcode that the minimum macOS version allows":
(https://github.com/bitcoin/bitcoin/pull/33932#issuecomment-3571308285)
This probably doesn't help https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550717279, the `<=>` is not fixed in [16.2](https://developer.apple.com/documentation/xcode-release-notes/xcode-16_2-release-notes) yet, only in [16.3](https://developer.apple.com/documentation/xcode-release-notes/xcode-16_3-release-notes).
And it seems this isn't the source of that error, we're [explicitly downloading](https://github.com/l0rinc/bitcoin/actions/runs/19633894569/job/56224901024?pr=57#step:9:126
...
(https://github.com/bitcoin/bitcoin/pull/33932#issuecomment-3571308285)
This probably doesn't help https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550717279, the `<=>` is not fixed in [16.2](https://developer.apple.com/documentation/xcode-release-notes/xcode-16_2-release-notes) yet, only in [16.3](https://developer.apple.com/documentation/xcode-release-notes/xcode-16_3-release-notes).
And it seems this isn't the source of that error, we're [explicitly downloading](https://github.com/l0rinc/bitcoin/actions/runs/19633894569/job/56224901024?pr=57#step:9:126
...
✅ hebasto closed an issue: "Qt6 version of Bitcoin Core (bitcoin-qt) flickers on Wayland"
(https://github.com/bitcoin-core/gui/issues/903)
(https://github.com/bitcoin-core/gui/issues/903)
🚀 hebasto merged a pull request: "Revert "gui, qt: brintToFront workaround for Wayland""
(https://github.com/bitcoin-core/gui/pull/914)
(https://github.com/bitcoin-core/gui/pull/914)
🤔 pablomartin4btc reviewed a pull request: "Revert "gui, qt: brintToFront workaround for Wayland""
(https://github.com/bitcoin-core/gui/pull/914#pullrequestreview-3500973605)
tested 0672e727bf1db5995562e9656d18b699aeba5fe0
(there's a typo on both the PR and commit titles.. brint -> bring)
<details>
<summary>The revert doesn't work for me on wayland (Qt v6.2.4 - Ubuntu 22.04.5 LTS), the window .</summary>
```
2025-11-24T15:08:07Z Bitcoin Core version v30.99.0-0672e727bf1d (release build)
2025-11-24T15:08:07Z Qt 6.2.4 (dynamic), plugin=wayland
2025-11-24T15:08:07Z No static plugins.
2025-11-24T15:08:07Z Style: fusion / QFusionStyle
2025-11-24T15:08:07Z S
...
(https://github.com/bitcoin-core/gui/pull/914#pullrequestreview-3500973605)
tested 0672e727bf1db5995562e9656d18b699aeba5fe0
(there's a typo on both the PR and commit titles.. brint -> bring)
<details>
<summary>The revert doesn't work for me on wayland (Qt v6.2.4 - Ubuntu 22.04.5 LTS), the window .</summary>
```
2025-11-24T15:08:07Z Bitcoin Core version v30.99.0-0672e727bf1d (release build)
2025-11-24T15:08:07Z Qt 6.2.4 (dynamic), plugin=wayland
2025-11-24T15:08:07Z No static plugins.
2025-11-24T15:08:07Z Style: fusion / QFusionStyle
2025-11-24T15:08:07Z S
...
💬 ryanofsky commented on pull request "mining: pass missing context to createNewBlock() and checkBlock()":
(https://github.com/bitcoin/bitcoin/pull/33936#issuecomment-3571368350)
> This is a breaking change
Could be nice to return an explicit error to clients that are too old with a change like the following from https://github.com/bitcoin/bitcoin/issues/33777#issuecomment-3528997008:
<details><summary>diff</summary>
<p>
```diff
--- a/src/ipc/capnp/init.capnp
+++ b/src/ipc/capnp/init.capnp
@@ -19,5 +19,8 @@ using Mining = import "mining.capnp";
interface Init $Proxy.wrap("interfaces::Init") {
construct @0 (threadMap: Proxy.ThreadMap) -> (threadMap :P
...
(https://github.com/bitcoin/bitcoin/pull/33936#issuecomment-3571368350)
> This is a breaking change
Could be nice to return an explicit error to clients that are too old with a change like the following from https://github.com/bitcoin/bitcoin/issues/33777#issuecomment-3528997008:
<details><summary>diff</summary>
<p>
```diff
--- a/src/ipc/capnp/init.capnp
+++ b/src/ipc/capnp/init.capnp
@@ -19,5 +19,8 @@ using Mining = import "mining.capnp";
interface Init $Proxy.wrap("interfaces::Init") {
construct @0 (threadMap: Proxy.ThreadMap) -> (threadMap :P
...
💬 willcl-ark commented on pull request "doc: Document compiler configuration for native depends packages":
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2556778530)
OK so having thought about this a bit more, my preference would be to not actually change the docs (as I have them in this PR), but to amend _depends/host/default.mk_ to better respect when `host_CC= host_CXX= ` have been set. I also posit that a setting of `host_CC` should override `CC`.
For depends, if I run `make build_CC=clang build_CXX=clang++ host_CC=clang host_CXX=clang++` I expect to be able to build all host and native packages.
I also believe that if I run `make build_CC=clang bu
...
(https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2556778530)
OK so having thought about this a bit more, my preference would be to not actually change the docs (as I have them in this PR), but to amend _depends/host/default.mk_ to better respect when `host_CC= host_CXX= ` have been set. I also posit that a setting of `host_CC` should override `CC`.
For depends, if I run `make build_CC=clang build_CXX=clang++ host_CC=clang host_CXX=clang++` I expect to be able to build all host and native packages.
I also believe that if I run `make build_CC=clang bu
...
💬 ryanofsky commented on pull request "depends: Fix native_capnp to respect build_CC and build_CXX":
(https://github.com/bitcoin/bitcoin/pull/33937#discussion_r2556822053)
In commit "depends: Fix native_capnp to respect build_CC and build_CXX" (eda7d38692ddb2a935ad0b2aefd2a3076c7aa599)
I'm surprised this wasn't happening previously. The depends system should already be setting CC and CXX values here:
https://github.com/bitcoin/bitcoin/blob/238c1c8933b1f7479a9bca2b7cb207d26151c39d/depends/funcs.mk#L215-L217
But I guess it is not setting them correctly? It would seem better for the depends system to set these correctly and not need package definitions to override
...
(https://github.com/bitcoin/bitcoin/pull/33937#discussion_r2556822053)
In commit "depends: Fix native_capnp to respect build_CC and build_CXX" (eda7d38692ddb2a935ad0b2aefd2a3076c7aa599)
I'm surprised this wasn't happening previously. The depends system should already be setting CC and CXX values here:
https://github.com/bitcoin/bitcoin/blob/238c1c8933b1f7479a9bca2b7cb207d26151c39d/depends/funcs.mk#L215-L217
But I guess it is not setting them correctly? It would seem better for the depends system to set these correctly and not need package definitions to override
...
💬 maflcko commented on pull request "Revert "gui, qt: brintToFront workaround for Wayland"":
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3571494725)
> The revert doesn't work for me on wayland (Qt v6.2.4 - Ubuntu 22.04.5 LTS), the window doesn't come back to front as it does when `plugin=xcb`.
The fix is in qt6.3, so this is expected. Your options are:
* Use a pre-compiled guix release
* Use depends yourself
* Upgrade your OS
* Install/compile a more recent qt yourself and use that
* ...?
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3571494725)
> The revert doesn't work for me on wayland (Qt v6.2.4 - Ubuntu 22.04.5 LTS), the window doesn't come back to front as it does when `plugin=xcb`.
The fix is in qt6.3, so this is expected. Your options are:
* Use a pre-compiled guix release
* Use depends yourself
* Upgrade your OS
* Install/compile a more recent qt yourself and use that
* ...?
💬 Sjors commented on pull request "mining: pass missing context to createNewBlock() and checkBlock()":
(https://github.com/bitcoin/bitcoin/pull/33936#issuecomment-3571507821)
Bumping the version number would also allow clients that wish to support older Bitcoin Core version to keep two capnp files around.
(https://github.com/bitcoin/bitcoin/pull/33936#issuecomment-3571507821)
Bumping the version number would also allow clients that wish to support older Bitcoin Core version to keep two capnp files around.
💬 willcl-ark commented on pull request "depends: sqlite 3.50.4; switch to autosetup":
(https://github.com/bitcoin/bitcoin/pull/32655#discussion_r2556842459)
3.51 was [released 04/11](https://sqlite.org/chronology.html) and claims to have some performance improvements:
> Performance enhancements:
>
> Use fewer CPU cycles to commit a read transaction.
> Early detection of joins that return no rows due to one or more of the tables containing no rows.
> Avoid evaluation of scalar subqueries if the result of the subquery does not change the result of the overall expression.
> Faster window function queries when using "BETWEEN :x
...
(https://github.com/bitcoin/bitcoin/pull/32655#discussion_r2556842459)
3.51 was [released 04/11](https://sqlite.org/chronology.html) and claims to have some performance improvements:
> Performance enhancements:
>
> Use fewer CPU cycles to commit a read transaction.
> Early detection of joins that return no rows due to one or more of the tables containing no rows.
> Avoid evaluation of scalar subqueries if the result of the subquery does not change the result of the overall expression.
> Faster window function queries when using "BETWEEN :x
...
💬 maflcko commented on pull request "ci: Use latest Xcode that the minimum macOS version allows":
(https://github.com/bitcoin/bitcoin/pull/33932#issuecomment-3571520280)
> This probably doesn't help [#29415 (comment)](https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550717279), the `<=>` is not fixed in [16.2](https://developer.apple.com/documentation/xcode-release-notes/xcode-16_2-release-notes) yet, only in [16.3](https://developer.apple.com/documentation/xcode-release-notes/xcode-16_3-release-notes).
Correct. This diff here does not change any behavior of the Bitcoin Core CI, as explained in the description.
> Do we have to keep those or can w
...
(https://github.com/bitcoin/bitcoin/pull/33932#issuecomment-3571520280)
> This probably doesn't help [#29415 (comment)](https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2550717279), the `<=>` is not fixed in [16.2](https://developer.apple.com/documentation/xcode-release-notes/xcode-16_2-release-notes) yet, only in [16.3](https://developer.apple.com/documentation/xcode-release-notes/xcode-16_3-release-notes).
Correct. This diff here does not change any behavior of the Bitcoin Core CI, as explained in the description.
> Do we have to keep those or can w
...
💬 fanquake commented on pull request "depends: sqlite 3.50.4; switch to autosetup":
(https://github.com/bitcoin/bitcoin/pull/32655#discussion_r2556857117)
I won't be doing that update here. We'd likely want to wait for a few more point releases before that update in any case.
(https://github.com/bitcoin/bitcoin/pull/32655#discussion_r2556857117)
I won't be doing that update here. We'd likely want to wait for a few more point releases before that update in any case.
👍 brunoerg approved a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3501159396)
ACK c34bc01b2ff2fc91ed4020288c5fa15f0c5b075e
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3501159396)
ACK c34bc01b2ff2fc91ed4020288c5fa15f0c5b075e