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

🤔 tdb3 reviewed a pull request: "test: Validate oversized transactions or without inputs"
(https://github.com/bitcoin/bitcoin/pull/29862#pullrequestreview-2051424070)
re ACK for eae4aeb17503ce51ef34c60c94341e00fafc12c8

Range diff showed only metadata updates (e.g. email). Re-ran unit tests (passed).

💬 tdb3 commented on pull request "doc: removed help text saying that peers may not connect automatically":
(https://github.com/bitcoin/bitcoin/pull/29994#issuecomment-2106334086)
Additional data. Two Tor-only nodes (v27.0):

```
$ bitcoin-cli getnodeaddresses 0 | jq 'length'
16551
$ bitcoin-cli getnodeaddresses 0 |jq 'map(select(.port != 8333)) |map(select(.network != "i2p")) |length'
18
$ bitcoin-cli getnodeaddresses 0 |jq 'map(select(.port != 8333)) |map(select(.network != "i2p")) |.[].port' |sort |uniq -c |sort -r
6 8334
1 9333
1 9185
1 9180
1 9050
1 8433
1 8335
1 8332
1 8330
1 58333

...

⚠️ edilmedeiros opened an issue: "contrib/signet/miner: miner won't consider --nbits parameter"
(https://github.com/bitcoin/bitcoin/issues/30091)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

`contrib/signet/miner` won't respect the `--nbits` param. It will keep using an interval of 2m29s.

```bash
❯ ./contrib/signet/miner --cli "bitcoin-cli" generate --address $MINER_ADDR --grind-cmd "bitcoin-util grind" --nbits $NBITS --ongoing
2024-05-12 16:09:20 INFO Mined block at height 362; next in 3m30s (mine)
2024-05-12 16:12:51 INFO Mined block at height 363; next in 2m29s (mine)
...

💬 TheCharlatan commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1597710487)
Nit: Can you clean up the includes?

💬 TheCharlatan commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1597710496)
How about:
```diff
diff --git a/src/test/fuzz/block_index.cpp b/src/test/fuzz/block_index.cpp
index 57fc59e88b..ea3af83bd2 100644
--- a/src/test/fuzz/block_index.cpp
+++ b/src/test/fuzz/block_index.cpp
@@ -6 +6,2 @@
-#include <chainparams.h>
+#include <kernel/chainparams.h>
+#include <logging.h>
@@ -17 +18 @@ namespace {
-const BasicTestingSetup* g_setup;
+static const std::unique_ptr<const CChainParams> g_params{CChainParams::Main()};
@@ -20 +21 @@ const BasicTestingSetup* g_setup;
...

🤔 TheCharlatan reviewed a pull request: "fuzz: a target for the block index database"
(https://github.com/bitcoin/bitcoin/pull/28209#pullrequestreview-2051451161)
Concept ACK