💬 TheCharlatan commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2106231470)
There is an error in commit fd342cb40b63ca00d23946743a038847673f8726, which can be fixed with:
```diff
diff --git a/src/warnings.cpp b/src/warnings.cpp
index 5b6ace63dd..40043cd8e7 100644
--- a/src/warnings.cpp
+++ b/src/warnings.cpp
@@ -11,0 +12 @@
+#include <util/string.h>
```

💬 Saraeutsza commented on pull request "wallet: Target a pre-defined utxo set composition by adjusting change outputs":
(https://github.com/bitcoin/bitcoin/pull/29442#issuecomment-2106232151)
remyers:2024-05-bnb-excess

💬 paplorinc commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1597631104)
would it make sense to preallocate the vouts before pushing?
```C++
txNew.vout.reserve(txNew.vout.size() + vecSend.size());
```

💬 paplorinc commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1597631354)
nit: found it a but confusing that we have two different style comments for the same line

💬 TheCharlatan commented on pull request "kernel: Streamline util library":
(https://github.com/bitcoin/bitcoin/pull/29015#issuecomment-2106237892)
The check script is very useful, could you add it as the first commit? I think it could make review a bit easier.

💬 hebasto commented on pull request "Correct tooltip wording for watch-only wallets":
(https://github.com/bitcoin-core/gui/pull/792#issuecomment-2106238212)
@hernanmarino

Are you still working on this?

👍 hebasto approved a pull request: "Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one"
(https://github.com/bitcoin/bitcoin/pull/29658#pullrequestreview-2051376920)
ACK d1ed09a7643b567e021b2ecb756bc925c48fc708, tested on Ubuntu 24.04.

This change indeed aligns the behavior of `bitcoin-qt` with that of `bitcoind` and other Bitcoin Core binaries.

💬 sipa commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1597636692)
I'd prefer to just call this BECH32, and come up with another name for variants that have other limits.

🚀 hebasto merged a pull request: "Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one"
(https://github.com/bitcoin/bitcoin/pull/29658)

💬 hebasto commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#issuecomment-2106255970)
While introducing a new `wallet/util_spend.h` header, it seems reasonable to address all related IWYU warnings, no?

💬 hebasto commented on pull request "refactor: interfaces, make 'createTransaction' less error-prone ":
(https://github.com/bitcoin-core/gui/pull/807#issuecomment-2106256091)
cc @achow101 @ryanofsky

💬 cbergqvist commented on pull request "Bugfix: RPC/Mining: getblocktemplate: Delay updating nTransactionsUpdatedLast and time_start until after the new template is cached":
(https://github.com/bitcoin/bitcoin/pull/30088#issuecomment-2106258218)
A unique lock on `cs_main` is taken on line 605. It is temporarily released `if (!lpval.isNull())`, but re-locked again before exiting that if-block, well before the block with the static variables around line 740 and the rest of the function body. So only one thread should be executing that section at a time.

That would mean having a null `pindexPrev` should be enough for the null `pblocktemplate` to be recalculated after an OOM. But maybe something less obvious is happening here?

💬 jlopp commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2106262769)
TACK 06c2c713c52b60231efc3e00d2c5eb0bf9e345f9

👍 hebasto approved a pull request: "Fix create unsigned transaction fee bump"
(https://github.com/bitcoin-core/gui/pull/812#pullrequestreview-2051388406)
ACK 671b7a32516d62e1e79393ded4b45910bd08010a, tested on Ubuntu 24.04.

✅ hebasto closed an issue: "Create unsigned when increasing fee asks for the passphrase"
(https://github.com/bitcoin-core/gui/issues/810)

🚀 hebasto merged a pull request: "Fix create unsigned transaction fee bump"
(https://github.com/bitcoin-core/gui/pull/812)

💬 hebasto commented on pull request "Add used balance to overview page":
(https://github.com/bitcoin-core/gui/pull/775#issuecomment-2106268602)
@BrandonOdiwuor

> **Second part:** Fixes #769
>
> Add used balance to the overview page for wallets with the avoid_reuse flag enabled
> ### Prerequsite:
>
> * **Part one (should be merged first)**: [Wallet: Functions to enable adding used balance to GUI overview page bitcoin/bitcoin#28776](https://github.com/bitcoin/bitcoin/pull/28776)

If you are still working on this PR, please address https://github.com/bitcoin/bitcoin/pull/28776#discussion_r1557805199.

👍 TheCharlatan approved a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29888#pullrequestreview-2051393527)
ACK bd5860bc7a892c6bcffe313246dd6b81b973b9c6

💬 tdb3 commented on pull request "Add option dbfilesize to control LevelDB target ("max") file size":
(https://github.com/bitcoin/bitcoin/pull/30059#discussion_r1597668634)
Yeah, redundant checking of range (beyond LevelDB) seems a bit much for a debug option (which is used by a more niche userbase). The range in the help message seems to be more of an example of a usable range rather than a recommendation of range or a strictly enforced range. The default value would be the defacto recommendation.

If we're set on continuing to have LevelDB handle the range rather than Core, it seems like it would make sense to inform the user in the help message that the range
...

💬 theStack commented on pull request "script/sign: avoid duplicated signature verification after signing (+introduce signing benchmarks)":
(https://github.com/bitcoin/bitcoin/pull/28923#issuecomment-2106301489)
Rebased on master, which was necessary due to the removal of `ECC_{Start,Stop}` from the key header in #29252.
The `ECC_Context` RAII wrapper is used now instead (like done in commit 28905c1a64a87a56f16aea8a4d23dea7eec9ca59). Re-review should be trivial.