💬 hebasto commented on issue "cmake: (release) version handling is broken":
(https://github.com/bitcoin/bitcoin/issues/31898#issuecomment-2706310874)
It seems that the first thing to decide is the source of the version information:
- the `CLIENT_VERSION_*` variables set by the build system, or
- a GitHub tag
(https://github.com/bitcoin/bitcoin/issues/31898#issuecomment-2706310874)
It seems that the first thing to decide is the source of the version information:
- the `CLIENT_VERSION_*` variables set by the build system, or
- a GitHub tag
👍 hebasto approved a pull request: "doc: add note to Windows build about stripping bins"
(https://github.com/bitcoin/bitcoin/pull/32002#pullrequestreview-2667104012)
ACK 416aeeaae9abb322389ec55bd9b281290cb390c7.
(https://github.com/bitcoin/bitcoin/pull/32002#pullrequestreview-2667104012)
ACK 416aeeaae9abb322389ec55bd9b281290cb390c7.
👍 hebasto approved a pull request: "Update minisketch subtree to d1e6bb8bbf8ef104b9dd002cab14a71b91061177"
(https://github.com/bitcoin/bitcoin/pull/32000#pullrequestreview-2667130467)
ACK 4fde88bc469dc1c827591f764bd635038ccaf852, I've updated the subtree locally and got zero diff with this PR.
(https://github.com/bitcoin/bitcoin/pull/32000#pullrequestreview-2667130467)
ACK 4fde88bc469dc1c827591f764bd635038ccaf852, I've updated the subtree locally and got zero diff with this PR.
🚀 hebasto merged a pull request: "Update minisketch subtree to d1e6bb8bbf8ef104b9dd002cab14a71b91061177"
(https://github.com/bitcoin/bitcoin/pull/32000)
(https://github.com/bitcoin/bitcoin/pull/32000)
🤔 BrandonOdiwuor reviewed a pull request: "wallet: Replace "non-0" with "non-zero" in translatable error message"
(https://github.com/bitcoin/bitcoin/pull/31987#pullrequestreview-2667196940)
Code Review ACK 18e83534ace7aa2d26bc7dfa521b1d591b66edfa
(https://github.com/bitcoin/bitcoin/pull/31987#pullrequestreview-2667196940)
Code Review ACK 18e83534ace7aa2d26bc7dfa521b1d591b66edfa
👍 pablomartin4btc approved a pull request: "qt: 29.0 translations update"
(https://github.com/bitcoin/bitcoin/pull/32004#pullrequestreview-2667220755)
re-ACK 86a28b6a3368acd2e6183fcfc9bbb13ee306c9fb
(comparing `master` sync with this PR shows a minor diff on `bitcoin_gl_ES.ts` - Galician Spanish - done today while this PR was created 2 days ago)
(https://github.com/bitcoin/bitcoin/pull/32004#pullrequestreview-2667220755)
re-ACK 86a28b6a3368acd2e6183fcfc9bbb13ee306c9fb
(comparing `master` sync with this PR shows a minor diff on `bitcoin_gl_ES.ts` - Galician Spanish - done today while this PR was created 2 days ago)
💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2706446575)
`2747f135be...e35a382388`: fix the CI failure, thanks!
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2706446575)
`2747f135be...e35a382388`: fix the CI failure, thanks!
💬 dergoegge commented on issue "test: `p2p_message_capture.py` fails with GCC 14 & undefined sanitizer":
(https://github.com/bitcoin/bitcoin/issues/32016#issuecomment-2706454955)
Maybe related: https://github.com/bitcoin/bitcoin/issues/28761
(https://github.com/bitcoin/bitcoin/issues/32016#issuecomment-2706454955)
Maybe related: https://github.com/bitcoin/bitcoin/issues/28761
💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1985062506)
The idea is that it does not matter which thread will destroy the `CNode` objects.
> there are no guarantees (afaict) that `disconnected_nodes` will actually be the last owner.
That is right, but there is no need for such a guarantee. `disconnected_nodes` is to guarantee that the objects will not be destroyed while holding `m_nodes_mutex`.
(https://github.com/bitcoin/bitcoin/pull/32015#discussion_r1985062506)
The idea is that it does not matter which thread will destroy the `CNode` objects.
> there are no guarantees (afaict) that `disconnected_nodes` will actually be the last owner.
That is right, but there is no need for such a guarantee. `disconnected_nodes` is to guarantee that the objects will not be destroyed while holding `m_nodes_mutex`.
🤔 rkrux reviewed a pull request: "Docs: fix typos in documentation files"
(https://github.com/bitcoin/bitcoin/pull/32011#pullrequestreview-2667264633)
crACK 8ff8af1
(https://github.com/bitcoin/bitcoin/pull/32011#pullrequestreview-2667264633)
crACK 8ff8af1
👍 dergoegge approved a pull request: "ci: Do not try to install for fuzz builds"
(https://github.com/bitcoin/bitcoin/pull/32014#pullrequestreview-2667265203)
utACK a3c3f37e71efc1ad13fcad49b1ac651e5843b26b
This fixes the build issue seen earlier on https://github.com/bitcoin-core/qa-assets/pull/219
(https://github.com/bitcoin/bitcoin/pull/32014#pullrequestreview-2667265203)
utACK a3c3f37e71efc1ad13fcad49b1ac651e5843b26b
This fixes the build issue seen earlier on https://github.com/bitcoin-core/qa-assets/pull/219
🤔 rkrux reviewed a pull request: "docs: fix typos"
(https://github.com/bitcoin/bitcoin/pull/32006#pullrequestreview-2667266682)
crACK a695c9f517d5b1c8a5b8b33abaa59c38b19c012b
(https://github.com/bitcoin/bitcoin/pull/32006#pullrequestreview-2667266682)
crACK a695c9f517d5b1c8a5b8b33abaa59c38b19c012b
💬 hebasto commented on issue "Having qt(@6) breaks build for qt@5 on macOS 15.0 and 13.7":
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2706511666)
> I'm not sure when this started happening. E.g. the commit [338bc2c](https://github.com/bitcoin/bitcoin/commit/338bc2cd261ba3daf7fb494f8cb4a534762e292c) which introduced cmake has the same issue.
I can confirm that this is a regression after migrating to CMake.
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2706511666)
> I'm not sure when this started happening. E.g. the commit [338bc2c](https://github.com/bitcoin/bitcoin/commit/338bc2cd261ba3daf7fb494f8cb4a534762e292c) which introduced cmake has the same issue.
I can confirm that this is a regression after migrating to CMake.
✅ fanquake closed an issue: "ci: `native_fuzz_with_*` jobs are broken"
(https://github.com/bitcoin/bitcoin/issues/32001)
(https://github.com/bitcoin/bitcoin/issues/32001)
🚀 fanquake merged a pull request: "ci: Do not try to install for fuzz builds"
(https://github.com/bitcoin/bitcoin/pull/32014)
(https://github.com/bitcoin/bitcoin/pull/32014)
💬 hebasto commented on issue "Having qt(@6) breaks build for qt@5 on macOS 15.0 and 13.7":
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2706655187)
> I assume this is related to [`AUTOMOC`](https://cmake.org/cmake/help/latest/manual/cmake-qt.7.html#automoc), and CMake getting confused and picking the wrong tooling to use.
From my perspective, Qt messes with included directories when compiling `qt/bitcoinqt_autogen/mocs_compilation.cpp`.
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2706655187)
> I assume this is related to [`AUTOMOC`](https://cmake.org/cmake/help/latest/manual/cmake-qt.7.html#automoc), and CMake getting confused and picking the wrong tooling to use.
From my perspective, Qt messes with included directories when compiling `qt/bitcoinqt_autogen/mocs_compilation.cpp`.
👍 hebasto approved a pull request: "doc: warn against having qt6 installed on macOS"
(https://github.com/bitcoin/bitcoin/pull/32017#pullrequestreview-2667531271)
ACK d79dab0fa999002a0c5b70c1688240e2a5032ce1.
It seems OK as a temporary measure until the [regression](https://github.com/bitcoin/bitcoin/issues/31009) is fixed or we switch to Qt 6.
(https://github.com/bitcoin/bitcoin/pull/32017#pullrequestreview-2667531271)
ACK d79dab0fa999002a0c5b70c1688240e2a5032ce1.
It seems OK as a temporary measure until the [regression](https://github.com/bitcoin/bitcoin/issues/31009) is fixed or we switch to Qt 6.
💬 hebasto commented on issue "Having qt(@6) breaks build for qt@5 on macOS 15.0 and 13.7":
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2706679598)
> I opened [#32017](https://github.com/bitcoin/bitcoin/pull/32017) to document this behavior. I suggest we add that PR to the milestone and remove this issue from it. The actual problem can be solved later.
In that case, this issue shouldn't be a blocker for the v29.0 release.
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-2706679598)
> I opened [#32017](https://github.com/bitcoin/bitcoin/pull/32017) to document this behavior. I suggest we add that PR to the milestone and remove this issue from it. The actual problem can be solved later.
In that case, this issue shouldn't be a blocker for the v29.0 release.
💬 tnndbtc commented on pull request "miniscript: fixes #29098 by only use first k valid signatures":
(https://github.com/bitcoin/bitcoin/pull/31719#discussion_r1985228860)
@hodlinator Ah, sorry, overlooked the comment part. Thank you for the example and I have made the change accordingly. Please have a look.
(https://github.com/bitcoin/bitcoin/pull/31719#discussion_r1985228860)
@hodlinator Ah, sorry, overlooked the comment part. Thank you for the example and I have made the change accordingly. Please have a look.
💬 sipa commented on pull request "miniscript: fixes #29098 by only use first k valid signatures":
(https://github.com/bitcoin/bitcoin/pull/31719#issuecomment-2706695834)
@tnndbtc Have you considered my [suggestion](https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-1860568169) to instead of just picking the first `k` signatures, keep using the existing algorithm, but stop trying things as soon as the optimal size is reached? I suspect that that will in practice have the same performance as first-k, but won't lose the optimality property the current algorithm has.
(https://github.com/bitcoin/bitcoin/pull/31719#issuecomment-2706695834)
@tnndbtc Have you considered my [suggestion](https://github.com/bitcoin/bitcoin/issues/29098#issuecomment-1860568169) to instead of just picking the first `k` signatures, keep using the existing algorithm, but stop trying things as soon as the optimal size is reached? I suspect that that will in practice have the same performance as first-k, but won't lose the optimality property the current algorithm has.