💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2093530014)
It doesn't actually matter, the parameter does nothing as you can't do hardened derivation from the musig aggregate key.

💬 l0rinc commented on pull request "[IBD] precalculate SipHash constant salt calculations":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-2887450663)
@laanwj, @sipa, I've addressed the concerns you've raised, A re-review would be really appreciated here

💬 sipa commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#issuecomment-2887460978)
> Why would a 64 bit system care about the 32 bit limits?

It wouldn't. This PR only affects 32-bit systems.

💬 maflcko commented on pull request "restrict std::cerr to errors; use std::cout for warnings and info":
(https://github.com/bitcoin/bitcoin/pull/32538#issuecomment-2887470962)
> Warning messages were previously sent to `stderr`, which could cause them to
> be misinterpreted as actual errors.

Not sure.

* When something literally starts with `Warning: ...`, I fail to see how it can be misinterpreted.
* This change means that warnings may be missed in tests/CI or elsewhere (see the failing CI).
* Sending warnings to stderr is normal and expected. See for example `python -c 'print(b"\p")'`

🤔 pablomartin4btc reviewed a pull request: "restrict std::cerr to errors; use std::cout for warnings and info"
(https://github.com/bitcoin/bitcoin/pull/32538#pullrequestreview-2847419694)
Concept ACK

I think some tests using `stop_node` with the warning in the args would fail as in `is_node_stopped` checks for the warning in `stderr` (eg `wallet_multiwallet.py`), it needs to be fixed too.

💬 pablomartin4btc commented on pull request "restrict std::cerr to errors; use std::cout for warnings and info":
(https://github.com/bitcoin/bitcoin/pull/32538#issuecomment-2887476146)
> This change means that warnings may be missed in tests/CI or elsewhere (see the failing CI).

That's my concern with this approach.

💬 purpleKarrot commented on issue "cmake: Cannot find Qt 6 on SunOS / illumos (OpenIndiana Distribution)":
(https://github.com/bitcoin/bitcoin/issues/32536#issuecomment-2887477698)
It sure is annoying when system packages are installed to a location where they cannot be found by default. On the other hand, package maintainers may have their reasons for doing that. There are probably multiple packages which depend on Qt6 and are built with CMake. It is very likely that package maintainers have their build environment set up for that.

Trying to work around that "issue" creates a maintenance burden for us and potentially for package maintainers too, because our workaround ma
...

🤔 l0rinc reviewed a pull request: "node: cap `-maxmempool` and `-dbcache` values for 32-bit"
(https://github.com/bitcoin/bitcoin/pull/32530#pullrequestreview-2847428745)
Looking at the code, I see that I misunderstood the description, I though we're capping the values in every circumstance, but now I see that the architectures are handles independently.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2093554238)
Done, added to musig.h

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2093554514)
Updated the comments.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2093554645)
Done

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2093554764)
Done

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2093554945)
"is"

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2093555270)
Added comments.

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2093555374)
Done

💬 achow101 commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2093555457)
any that we have.

💬 maflcko commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#issuecomment-2887503071)
No objection, but there is no hand-holding on 64-bit systems regarding OOM footguns, so doing it on 32-bit systems (the edge case ) seems fine, but a bit unexpected, given that OOM reports are generally on 64-bit systems? Again, no objection, just wondering if there is more background to it.

✅ furszy closed a pull request: "restrict std::cerr to errors; use std::cout for warnings and info"
(https://github.com/bitcoin/bitcoin/pull/32538)

💬 furszy commented on pull request "restrict std::cerr to errors; use std::cout for warnings and info":
(https://github.com/bitcoin/bitcoin/pull/32538#issuecomment-2887513237)
> When something literally starts with Warning: ..., I fail to see how it can be misinterpreted.

It is actually being misinterpreted by our own test framework. The framework does not look
at the stderr content, it only checks whether there is something inside stderr or not during
shutdown, failing when finds something there.

> This change means that warnings may be missed in tests/CI or elsewhere (see the failing CI).

That's ok. That's what the PR is proposing. To redirect them to std
...

💬 furszy commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#discussion_r2093584558)
> why the `.rpc.`?

because my subconscious still thinks about the legacy wallet proxy methods and we don't have them anymore.