💬 hebasto commented on issue "cmake: GUI build broken with `-stdlib=libc++`":
(https://github.com/bitcoin/bitcoin/issues/32331#issuecomment-2824399437)
Is it supposed to work without using depends built with `-stdlib=libc++`?

💬 Sjors commented on pull request "refactor: inline `UndoWriteToDisk` and `WriteBlockToDisk` to reduce serialization calls":
(https://github.com/bitcoin/bitcoin/pull/31490#issuecomment-2824428654)
@l0rinc I was rebasing a branch that used the old name and didn't know what it was renamed too, but otherwise no problem.

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056077020)
That's better. But I still think it's redundant as the code block now ends with
```C++
m_obfuscation = obfuscate_key_vector;
```
and all code paths lead there.

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056150490)
```suggestion
const bool all_zero = !obfuscation || std::ranges::all_of(std::span{key_bytes.begin(), write_size}, [](auto& b) { return b == std::byte{0}; });
```
```suggestion
const bool all_zero = !obfuscation || std::all_of(key_bytes.begin(), key_bytes.begin() + write_size, [](auto& b) { return b == std::byte{0}; });
```

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056093869)
Seems you forgot to remove "The `CreateObfuscateKey` method and its private helper were also removed."?
I think saying CreateObfuscateKey was inlined is sufficient and no private helper was removed as stated above in (1.

🤔 stickies-v reviewed a pull request: "Add rpcestimateconservativefees"
(https://github.com/bitcoin/bitcoin/pull/32329#pullrequestreview-2787516215)
Concept NACK, for reasons outlined by @glozow and @ismaelsadeeq - this seems like way too niche of a use case to add the maintenance burden and UX confusion of a new RPC/startup option into Bitcoin Core.

I think you can easily solve this by running a small JSON-RPC proxy that listens on the usual RPC port, injects a `estimate_mode` argument if none is provided and forwards that to the `bitcoind` instance? Then your clients won't need to make any changes for as long as you're willing to mainta
...

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056174501)
Yeah, can you tell me what's wrong with it? If you have a suggestion that passes ci (and local IBD for some blocks), let me know

💬 hodlinator commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2056177529)
I had that one coming. :)

💬 furszy commented on pull request "Add rpcestimateconservativefees":
(https://github.com/bitcoin/bitcoin/pull/32329#issuecomment-2824481962)
> I think you can easily solve this by running a small JSON-RPC proxy that listens on the usual RPC port, injects a estimate_mode argument if none is provided and forwards that to the bitcoind instance? Then your clients won't need to make any changes for as long as you're willing to maintain backwards compatibility.

Or.. while the clients adapt, simpler to just downgrade or compile the repository yourself.

🤔 rkrux reviewed a pull request: "test: Test that migration automatically repairs corrupted metadata with doubled derivation path"
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2787605306)
LGTM to me at bfcf5c5ab1fdba46d492aa0872c3d5f9a4a87633, ACK - it's just one functional test now.

Couple of observations/questions after rotating the seed and unloading the wallet that I found noteworthy enough to share here.

1. The `listreceivedbyaddress` RPC doesn't show any entry for `addr` to which the 1 BTC was sent, but `listunspent` RPC does show.

2. The `desc` field in the `bad_deriv_wallet.listunspent` and `bad_deriv_wallet_master.listunspent` calls for the same `addr` was diff
...

💬 maflcko commented on issue "cmake: GUI build broken with `-stdlib=libc++`":
(https://github.com/bitcoin/bitcoin/issues/32331#issuecomment-2824604630)
IIRC it used to work with qt5, but it is unclear if that was intentional or by accident. I guess in any case this is an upstream issue that would need to be fixed in the distro or in qt6, if it is an issue at all?

💬 fanquake commented on pull request "Add rpcestimateconservativefees":
(https://github.com/bitcoin/bitcoin/pull/32329#issuecomment-2824608069)
Agree with the other comments here. Closing for now.

✅ fanquake closed a pull request: "Add rpcestimateconservativefees"
(https://github.com/bitcoin/bitcoin/pull/32329)

💬 juanmigdr commented on pull request "Add rpcestimateconservativefees":
(https://github.com/bitcoin/bitcoin/pull/32329#issuecomment-2824637123)
You're right, @ismaelsadeeq, @glozow, @stickies-v, @furszy, and @fanquake. Thanks for the feedback and suggestions. I see that adding this config option isn't the best solution, as this change is quite specific to my situation, and the cost of maintaining it outweighs the benefits. I'll consider alternative ways to handle this. Appreciate the help!

💬 hebasto commented on issue "PIE+LTO causes Bitcoin-Qt to segfault at startup":
(https://github.com/bitcoin-core/gui/issues/867#issuecomment-2824676516)
@whitslack @eli-schwartz @thesamesam

Setting the required conditions aside, can you confirm whether the following patch resolves the issue on Gentoo:

```diff
--- a/src/qt/CMakeLists.txt
+++ b/src/qt/CMakeLists.txt
@@ -250,6 +250,8 @@ if(qt_lib_type STREQUAL "STATIC_LIBRARY")
)
endif()

+list(TRANSFORM CMAKE_CXX_LINK_OPTIONS_PIE REPLACE "${CMAKE_CXX_COMPILE_OPTIONS_PIE}" "${CMAKE_CXX_COMPILE_OPTIONS_PIC}")
+
add_executable(bitcoin-qt
main.cpp
../init/bitcoin-qt.cpp
```
?

💬 hebasto commented on issue "PIE+LTO causes Bitcoin-Qt to segfault at startup":
(https://github.com/bitcoin-core/gui/issues/867#issuecomment-2824841846)
> Interesting! It should happen since https://gitlab.archlinux.org/archlinux/packaging/packages/qt6-base/-/commit/5cd71aed56de955b182e20263a50cd91bf7b6aaa.

This approach appears suitable for Gentoo's Qt packages as well, since, according to [this](https://github.com/qt/qtbase/commit/19b7f854a274812d9c95fc7aaf134a12530c105f) commit message, the `-Bsymbolic` and `-Bsymbolic-functions` functionality—underlying the `-reduce-relocations` option—is broken.

💬 w0xlt commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2056499667)
`FindNode()` is not bad, but I personally like `ConnectedToAddrPort()`. This name makes the purpose of the function very clear.
The parameter type (`CService` and `str`) makes the difference clear. It is not necessary for the function name to describe the parameter.

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2824999837)
re-ACK 1c16944a4aff71e6560703916b11b2a544ea71ca

Diff from 2b34857ad54aebbf0a9271e742c2caee85577bc8 is trivial.

💬 l0rinc commented on pull request "[IBD] flush UTXO set in batches proportional to `dbcache` size":
(https://github.com/bitcoin/bitcoin/pull/31645#discussion_r2056598543)
Makes sense, will do next time I push

💬 darosior commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2056629920)
Done. Removed the `height > 0` condition, corrected `height - 1` for `height` and added a comment that the `height` variable does not actually correspond to the block height as it mind be confusing to readers. Verified correctness by rebasing my Consensus Cleanup PR on top of this one.