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

💬 darosior commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2056631476)
Rebased on master.

💬 achow101 commented on pull request "test: Test that migration automatically repairs corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#issuecomment-2825129551)
> 1. The `listreceivedbyaddress` RPC doesn't show any entry for `addr` to which the 1 BTC was sent, but `listunspent` RPC does show.

This is expected since we didn't get `addr` through normal means, and it therefore is missing expected items for receiving addresses like an address book entry.

> 2. The `desc` field in the `bad_deriv_wallet.listunspent` and `bad_deriv_wallet_master.listunspent` calls for the same `addr` was different slightly as I see `'` was used in the former & `h` in the
...

💬 davidgumberg commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2056630902)
AFAICT the workaround is correct, and this test is already using `importdescriptors` on master.

This test succeeds as-is on this branch with this commit (https://github.com/bitcoin/bitcoin/pull/31250/commits/e6c900f9c0e1b143098663e1c5c80e1811946b2d) dropped.

💬 davidgumberg commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2056635528)
Nit: Extra "the".

💬 davidgumberg commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2054565214)
For other reviewers: Unlike for legacy wallets, watch only[^1] descriptor wallets do not need/have their ScriptPubKeyMans set up during wallet [creation](https://github.com/bitcoin/bitcoin/blob/e5a00b24972461f7a181bc184dd461cedcce6161/src/wallet/wallet.cpp#L3003).

https://github.com/bitcoin/bitcoin/blob/e5a00b24972461f7a181bc184dd461cedcce6161/src/wallet/wallet.cpp#L3073-L3080

---

Tangentially, it might be nice to refactor `SetupDescriptorsWallet()` to reuse `CWallet::Create()` rather
...

💬 achow101 commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2056733702)
I certain that this commit was required when writing this PR originally, however, I no longer recall the error that it fixed. Dropping this commit.

💬 achow101 commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2056734208)
I think the `%` is your command line prompt?

This was missing a newline, fixed.

💬 achow101 commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#discussion_r2056734975)
Fixed

💬 l0rinc commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2056768817)
We're continually updating this doc and for now we're adding more and more to the mac section - it's not getting any simpler for now, seems we still need the mac section for now. And when that changes, we'll update the doc.

💬 l0rinc commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#issuecomment-2825349060)
> it would be good to mention [Wiki: AddressSanitizerContainerOverflow (false positives) (google/sanitizers)](https://github.com/google/sanitizers/wiki/AddressSanitizerContainerOverflow#false-positives)

Added the links to the doc and added another workaround of setting the target architecture an Apple Silicon for errors like:
```bash
usr/include/malloc/_malloc_type.h:66:126: error: unknown type name 'size_t'; did you mean 'std::size_t'?
```

🤔 janb84 reviewed a pull request: "guix: Remove unused `file` package"
(https://github.com/bitcoin/bitcoin/pull/32242#pullrequestreview-2788550078)
Code review and Tested ACK [513e202](https://github.com/bitcoin/bitcoin/commit/513e2020a9acdd366d4933780b331f97bac85597)

- Did a code review
- Full GUIX build under nixos on macos via UTM, works !

💬 maflcko commented on issue "gui: translation spam?":
(https://github.com/bitcoin/bitcoin/issues/32295#issuecomment-2825373348)
I've pushed an update to check more broadly. The new issues (for just Polish) include some false positives around (change/reszta). Though, the others largely apply as far as I can see:

#### Erroneous translation:
<source>Warning: If you encrypt your wallet and lose your passphrase, you will &lt;b&gt;LOSE ALL OF YOUR BITCOINS&lt;/b&gt;!</source>
<translation>hasłoOstrzeżenie: Jeśli zaszyfrujesz swój portfel i zgubisz hasło - &lt;b&gt;STRACISZ WSZYSTKIE SWOJE BITCONY&lt;/b&gt;!</t
...