💬 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

💬 willcl-ark commented on issue "contrib/signet/miner: miner won't consider --nbits parameter":
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2106383337)
Thanks for the detailed report @edilmedeiros !

I can confirm that I see the same behaviour as you running a custom signet, where even after (allegedly) calibrating to an nbits targetting `--seconds=600`, I get 2m30s blocks:

```log
2024-05-12 21:59:49 INFO Mined block at height 100; next in -2m58s (mine)
2024-05-12 21:59:50 INFO Mined block at height 101; next in -29s (mine)
2024-05-12 21:59:50 INFO Mined block at height 102; next in 2m0s (mine)
2024-05-12 22:01:51 INFO Mined block at h
...

💬 edilmedeiros commented on issue "contrib/signet/miner: miner won't consider --nbits parameter":
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2106388477)
> Failing that, it could perhaps be useful to in any case include information on creating a custom signet using descriptor wallets as a new section in `contrib/signet/README.md`, so that approach may also make sense ?

I'm already documenting so in my site, I can post it here when done. If there's consensus, I can also open a PR for extending this piece of documentation.

🤔 theStack reviewed a pull request: "p2p: index TxOrphanage by wtxid, allow entries with same txid"
(https://github.com/bitcoin/bitcoin/pull/30000#pullrequestreview-2051463502)
Concept ACK

Code changes look good at first glance, still thinking about the implications of 9d2e9054134cedbf688368445eb6d2de14e2a8bd (w.r.t. discussion in https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1586713579). Verified that both the unit tests and each of the newly introduced functional sub-tests fail without the main commit 4206ba3a36b329d1cc41df93cf59b685912d6d94, as expected.

💬 theStack commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1597723673)
nit:
```suggestion
assert_equal(node.getmempoolentry(tx_child["txid"])["wtxid"], tx_child["wtxid"])
```