💬 rkrux 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_r1938961976)
`provided`

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1938960467)
nit:
```suggestion
// Helper to check if the tip has changed, and also update tip_changed.
```

💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1938970989)
`wait_until()` will first call the predicate, so `check_tip_changed() || ` is not needed. The above code is equivalent to:

```cpp
notifications().m_tip_block_cv.wait_until(lock, std::min(now + tick, deadline), [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) {
return check_tip_changed() || chainman().m_interrupt;
});
```

✅ hebasto closed a pull request: "MacOS updated Bitcoin-Core gui icon in accordance with Apple design docs"
(https://github.com/bitcoin/bitcoin/pull/31780)

💬 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.