💬 hebasto commented on pull request "MacOS updated Bitcoin-Core gui icon in accordance with Apple design docs":
(https://github.com/bitcoin/bitcoin/pull/31780#issuecomment-2630308019)
GUI-related proposal should be opened in https://github.com/bitcoin-core/gui.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939003712)
Ah oops, we're going in circles, I forgot about this as well. Will add a comment.

✅ hebasto closed an issue: "Organize the address types on the "Receiving addresses" tab"
(https://github.com/bitcoin-core/gui/issues/646)

💬 hebasto commented on issue "Organize the address types on the "Receiving addresses" tab":
(https://github.com/bitcoin-core/gui/issues/646#issuecomment-2630333317)
Given the developments in https://github.com/bitcoin-core/gui-qml, it doesn’t seem worthwhile to add a new feature solely based on the rationale that "it would be nicer".

Feel free to reopen if anyone disagrees.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939023678)
@ryanofsky pointed out the same mistake in a different (now closed) PR: https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1848409060

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939026787)
Actually, I forgot that `wait_until` already checks the predicate once before waiting. See https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1938970989

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2630362917)
Dropped unnecessary `check_tip_changed()` check before `wait_until`..

💬 Sjors commented on pull request "psbt: add non-default sighash types to PSBTs and unify sighash type match checking":
(https://github.com/bitcoin/bitcoin/pull/31622#discussion_r1939034855)
> at the same time

You mean a transaction with both a taproot and non-taproot input?

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1939042762)
It's easier to read in doxygen html, and can always be expanded.

💬 hebasto commented on pull request "depends: Fix compiling `libevent` package on NetBSD":
(https://github.com/bitcoin/bitcoin/pull/31500#issuecomment-2630389701)
> > Can you link to the upstream issue / PR.
>
> > LGTM, but yeah, it'd be helpful to upstream this.
>
> Sure thing!
>
> Here is an upstream PR: [libevent/libevent#1768](https://github.com/libevent/libevent/pull/1768).

The upstream changes have been merged. So this branch was rebased, and the upstream commit was mentioned.

💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1939050902)
(Resolved by now merged #31433).

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939056704)
> then there's no wait to test its internals

Should that be "no way"?

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1939058572)
IIUC the difference between release and close is only relevant in edge cases. The main purpose of this function it to close them. I would keep the name, but maybe document the function to point out the edge case.

💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2630417340)
I suggest assigning the "29.0" milestone to this PR, as there is currently no CI job that tests binaries similar to our Windows release ones.

💬 Sjors commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#discussion_r1939064794)
According to developer notes, `LogError` is for:

> severe problems that require the node (or a subsystem) to shut down
entirely

And IIUC we indeed shut down here.

💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939070752)
Will push fix if I need to change anything else.

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939075014)
Consider the following. It avoids the hackish part and achieves the same purpose:

```diff
diff --git i/src/node/interfaces.cpp w/src/node/interfaces.cpp
index 9fe4984cd0..8022b47b5e 100644
--- i/src/node/interfaces.cpp
+++ w/src/node/interfaces.cpp
@@ -960,13 +960,13 @@ public:
// could have been generated before a tip exists.
Assume(tip_block);
tip_changed = tip_block != m_block_template->block.hashPrevBlock;
return tip_changed;

...

💬 Sjors commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#discussion_r1939076473)
Agreed.

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1939077245)
This is marked as resolved, but the code is not changed.

💬 Sjors commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#discussion_r1939079280)
Thanks for clarifying. So in that case both too many and too few bytes are prevented.