🤔 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"])
```

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

💬 pinheadmz commented on issue "contrib/signet/miner: miner won't consider --nbits parameter":
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2106397970)
@edilmedeiros this might be expected behavior although not well documented. If I recall, the signet miner generates the first 100 blocks as fast as possible with 100% CPU (to cross coinbase maturity) so you can start spending signet coins right away. It will *eventually* retarget to your configured difficulty. If you're able to, I'd let your process run at least 2,016 blocks and then check if its acting as expected.

💬 pinheadmz commented on issue "contrib/signet/miner: miner won't consider --nbits parameter":
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2106398197)
For the descriptor wallet question, I ran a custom signet for an educational program earlier this year and wrote a script that created a descriptor wallet in regtest, then spawned signet with that descriptor: https://github.com/chaincodelabs/signet-wallet-project/blob/main/signet-setup.py

🤔 furszy reviewed a pull request: "script/sign: avoid duplicated signature verification after signing (+introduce signing benchmarks)"
(https://github.com/bitcoin/bitcoin/pull/28923#pullrequestreview-2051468976)
utACK fe92c15f0c44

💬 edilmedeiros commented on issue "contrib/signet/miner: miner won't consider --nbits parameter":
(https://github.com/bitcoin/bitcoin/issues/30091#issuecomment-2106401030)
> @edilmedeiros this might be expected behavior although not well documented. If I recall, the signet miner generates the first 100 blocks as fast as possible with 100% CPU (to cross coinbase maturity) so you can start spending signet coins right away. It will _eventually_ retarget to your configured difficulty. If you're able to, I'd let your process run at least 2,016 blocks and then check if its acting as expected.

I was running this way after the 100 block threshold when I perceived this
...

✅ luke-jr closed a 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)

💬 luke-jr 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-2106408589)
Indeed, forgot about cs_main

💬 Saraeutsza commented on pull request "wallet: add coin selection parameter `add_excess_to_recipient_position` for changeless txs with excess that would be added to fees":
(https://github.com/bitcoin/bitcoin/pull/30080#issuecomment-2106429004)
[](https://github.com/bitcoin/bitcoin/actions/workflows/ci.yml)[](url)

💬 tdb3 commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#discussion_r1597779699)
Thanks @cbergqvist. Good observation.

Looks like rpc_bind.py uses `--ipv4` and `--ipv6` arguments to run the test exclusively with ipv4 or ipv6 (not both), which at first glance doesn't seem to be desirable for interface_bitcoin_cli.py (dependence on network type is a somewhat small part of the test). I could see there being value in refactoring interface_bitcoin_cli.py to include wider coverage of bitcoin-cli with ipv4 and ipv6, but I'm thinking this sort of refactor would be a good follow
...

💬 tdb3 commented on pull request "cli: Detect port errors in rpcconnect and rpcport":
(https://github.com/bitcoin/bitcoin/pull/29521#issuecomment-2106531828)
Seeing a CI error for arm-linux-gnueabihf-g++ (on a line unchanged in this latest push). Will look into this.

```
In function ‘UniValue CallRPC(BaseRequestHandler*, const std::string&, const std::vector<std::__cxx11::basic_string<char> >&, const std::optional<std::__cxx11::basic_string<char> >&)’,
inlined from ‘UniValue ConnectAndCallRPC(BaseRequestHandler*, const std::string&, const std::vector<std::__cxx11::basic_string<char> >&, const std::optional<std::__cxx11::basic_string<char> >
...