💬 vasild commented on pull request "net_processing: rename RelayTransaction to better describe what it does":
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2434804167)
I will address if I have to re-touch, thanks!
(https://github.com/bitcoin/bitcoin/pull/33565#discussion_r2434804167)
I will address if I have to re-touch, thanks!
📝 diegoviola opened a pull request: "qt: fix Wayland visual glitches"
(https://github.com/bitcoin-core/gui/pull/904)
Qt::WindowStaysOnTopHint is ineffective on Wayland and causes bitcoin-qt to flicker (see #903 and #33432).
Wayland's window behavior is controlled by the compositor, not the client. This change skips applying hints when running natively under Wayland, resolving the flickering issue.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened agains
...
(https://github.com/bitcoin-core/gui/pull/904)
Qt::WindowStaysOnTopHint is ineffective on Wayland and causes bitcoin-qt to flicker (see #903 and #33432).
Wayland's window behavior is controlled by the compositor, not the client. This change skips applying hints when running natively under Wayland, resolving the flickering issue.
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened agains
...
💬 Sjors commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-3409544132)
Needs rebase.
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-3409544132)
Needs rebase.
💬 maflcko commented on pull request "CI: silent merge check":
(https://github.com/bitcoin/bitcoin/pull/33145#issuecomment-3409554826)
Any updates here? Just asking, because it is a bit tedious to manually search for all silent merge conflicts. Example ref: https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-3409544132
(https://github.com/bitcoin/bitcoin/pull/33145#issuecomment-3409554826)
Any updates here? Just asking, because it is a bit tedious to manually search for all silent merge conflicts. Example ref: https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-3409544132
💬 maflcko commented on issue "Slow unit tests delay functional tests and leave CPU unused":
(https://github.com/bitcoin/bitcoin/issues/32770#issuecomment-3409566092)
On the general topic, there was commit https://github.com/bitcoin/bitcoin/commit/636b611781154f32c8d7ae8ea301d12b1464c151, so someone could some time in the future check if it helps.
(https://github.com/bitcoin/bitcoin/issues/32770#issuecomment-3409566092)
On the general topic, there was commit https://github.com/bitcoin/bitcoin/commit/636b611781154f32c8d7ae8ea301d12b1464c151, so someone could some time in the future check if it helps.
✅ maflcko closed an issue: "Revisiting us self-hosting parts of our CI"
(https://github.com/bitcoin/bitcoin/issues/31965)
(https://github.com/bitcoin/bitcoin/issues/31965)
💬 maflcko commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3409569349)
Closing for now. I think this is fixed. There are some possible follow-ups (https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3241685779), but they all have their own issue or pull request already.
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3409569349)
Closing for now. I think this is fixed. There are some possible follow-ups (https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3241685779), but they all have their own issue or pull request already.
💬 diegoviola commented on issue "Qt6 version of Bitcoin Core (bitcoin-qt) flickers on Wayland":
(https://github.com/bitcoin-core/gui/issues/903#issuecomment-3409599121)
> I'll take a look...
Appreciate you planning to look into that, I managed to track down the cause and submitted a fix in #904. Let me know if you have any questions during the review. Thanks!
(https://github.com/bitcoin-core/gui/issues/903#issuecomment-3409599121)
> I'll take a look...
Appreciate you planning to look into that, I managed to track down the cause and submitted a fix in #904. Let me know if you have any questions during the review. Thanks!
📝 vastonus opened a pull request: "chore: remove repetitive word in src/leveldb/README.md"
(https://github.com/bitcoin/bitcoin/pull/33638)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/33638)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
💬 yuvicc commented on pull request "rpc, test: allow multiple data outputs in `createrawtransaction` & `createpsbt`":
(https://github.com/bitcoin/bitcoin/pull/32790#issuecomment-3409691430)
@mikekelly see the [comment](https://github.com/bitcoin/bitcoin/issues/32478#issuecomment-2876030352), this changes will come in the next release 31.0. Will update this one soon and mark it for review.
(https://github.com/bitcoin/bitcoin/pull/32790#issuecomment-3409691430)
@mikekelly see the [comment](https://github.com/bitcoin/bitcoin/issues/32478#issuecomment-2876030352), this changes will come in the next release 31.0. Will update this one soon and mark it for review.
👍 ryanofsky approved a pull request: "doc: document capnproto and libmultiprocess deps in 29.x"
(https://github.com/bitcoin/bitcoin/pull/33623#pullrequestreview-3343442293)
Code review ACK bf236d7a8af0826efc39a04113b79f96fe05f0c7. Looks good but left some more suggestions below. I think I might also adjust PR description not to say documented version is the latest version that will ever work (see below). I think it would be more accurate just to say that later versions are known not to work.
(https://github.com/bitcoin/bitcoin/pull/33623#pullrequestreview-3343442293)
Code review ACK bf236d7a8af0826efc39a04113b79f96fe05f0c7. Looks good but left some more suggestions below. I think I might also adjust PR description not to say documented version is the latest version that will ever work (see below). I think it would be more accurate just to say that later versions are known not to work.
💬 ryanofsky commented on pull request "doc: document capnproto and libmultiprocess deps in 29.x":
(https://github.com/bitcoin/bitcoin/pull/33623#discussion_r2434900293)
In commit "doc: document capnproto and libmultiprocess deps" (bf236d7a8af0826efc39a04113b79f96fe05f0c7)
I think I'd suggest dropping this paragraph because we can't really say 5.0 is the latest version that will work. There is a [v5](https://github.com/bitcoin-core/libmultiprocess/commits/v5) branch, so in theory if there were fixes or features we wanted to backport it would be possible to create 5.1, 5.2, etc versions that should be compatible with 5.0.
Libmultiprocess doesn't seem different
...
(https://github.com/bitcoin/bitcoin/pull/33623#discussion_r2434900293)
In commit "doc: document capnproto and libmultiprocess deps" (bf236d7a8af0826efc39a04113b79f96fe05f0c7)
I think I'd suggest dropping this paragraph because we can't really say 5.0 is the latest version that will work. There is a [v5](https://github.com/bitcoin-core/libmultiprocess/commits/v5) branch, so in theory if there were fixes or features we wanted to backport it would be possible to create 5.1, 5.2, etc versions that should be compatible with 5.0.
Libmultiprocess doesn't seem different
...
💬 ryanofsky commented on pull request "doc: document capnproto and libmultiprocess deps in 29.x":
(https://github.com/bitcoin/bitcoin/pull/33623#discussion_r2434877805)
In commit "doc: document capnproto and libmultiprocess deps" (bf236d7a8af0826efc39a04113b79f96fe05f0c7)
I think this could list 0.7.0 as minimum required version of Cap'n Proto linking to https://github.com/bitcoin-core/libmultiprocess/pull/88 (which dropped support for previous Cap'n Proto versions) and list 5.0 as minimum required version of libmultiprocess linking to https://github.com/bitcoin/bitcoin/pull/31740 (which added a dependency on the `mp/type-*.h` files that were introduced in v
...
(https://github.com/bitcoin/bitcoin/pull/33623#discussion_r2434877805)
In commit "doc: document capnproto and libmultiprocess deps" (bf236d7a8af0826efc39a04113b79f96fe05f0c7)
I think this could list 0.7.0 as minimum required version of Cap'n Proto linking to https://github.com/bitcoin-core/libmultiprocess/pull/88 (which dropped support for previous Cap'n Proto versions) and list 5.0 as minimum required version of libmultiprocess linking to https://github.com/bitcoin/bitcoin/pull/31740 (which added a dependency on the `mp/type-*.h` files that were introduced in v
...
✅ fanquake closed a pull request: "chore: remove repetitive word in src/leveldb/README.md"
(https://github.com/bitcoin/bitcoin/pull/33638)
(https://github.com/bitcoin/bitcoin/pull/33638)
💬 fanquake commented on pull request "chore: remove repetitive word in src/leveldb/README.md":
(https://github.com/bitcoin/bitcoin/pull/33638#issuecomment-3409791142)
Leveldb changes need to go to the upstream repo.
(https://github.com/bitcoin/bitcoin/pull/33638#issuecomment-3409791142)
Leveldb changes need to go to the upstream repo.
💬 fanquake commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3409804641)
Yea, I have seen the same, on one of my Fedora boxes, which would be a blocker, given it breaks running the CI locally.
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3409804641)
Yea, I have seen the same, on one of my Fedora boxes, which would be a blocker, given it breaks running the CI locally.
💬 vasild commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#discussion_r2435059938)
Such an `Assume()` would make for stricter requirements, even in `master`, or more complex reasoning to confirm that it does not make stricter requirements. I mean, can `lNodesAnnouncingHeaderAndIDs` have e.g. 5 elements when this code snippet starts? It is not obvious by just looking at this code, so one has to assume that it can or study the code elsewhere to confirm that it cannot have 4 or more.
So, both in `master` and in this PR such an `Assume()` would be triggered if `lNodesAnnouncing
...
(https://github.com/bitcoin/bitcoin/pull/32394#discussion_r2435059938)
Such an `Assume()` would make for stricter requirements, even in `master`, or more complex reasoning to confirm that it does not make stricter requirements. I mean, can `lNodesAnnouncingHeaderAndIDs` have e.g. 5 elements when this code snippet starts? It is not obvious by just looking at this code, so one has to assume that it can or study the code elsewhere to confirm that it cannot have 4 or more.
So, both in `master` and in this PR such an `Assume()` would be triggered if `lNodesAnnouncing
...
💬 vasild commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#discussion_r2435076628)
Yes, good catch! Done.
(https://github.com/bitcoin/bitcoin/pull/32394#discussion_r2435076628)
Yes, good catch! Done.
💬 vasild commented on pull request "net: make m_nodes_mutex non-recursive":
(https://github.com/bitcoin/bitcoin/pull/32394#issuecomment-3409829097)
`4aad3714d6...4b3a2c2360`: do https://github.com/bitcoin/bitcoin/pull/32394#discussion_r2433600255
(https://github.com/bitcoin/bitcoin/pull/32394#issuecomment-3409829097)
`4aad3714d6...4b3a2c2360`: do https://github.com/bitcoin/bitcoin/pull/32394#discussion_r2433600255
🚀 fanquake merged a pull request: "Update libmultiprocess subtree in 30.x branch"
(https://github.com/bitcoin/bitcoin/pull/33519)
(https://github.com/bitcoin/bitcoin/pull/33519)