💬 fanquake commented on issue "29.x Having qt(@6) breaks build for qt@5 on macOS 15.0 and 13.7":
(https://github.com/bitcoin/bitcoin/issues/31009#issuecomment-3451579595)
Was closed with #32810.

💬 hMsats commented on issue "Seemingly second (very long) validation at the same height":
(https://github.com/bitcoin/bitcoin/issues/33687#issuecomment-3451724835)
After more than 48 hours, not a single timeout for a division by 10 (based on 1 minute or 600/10=60 seconds) and a cold start. For height, verification time, time and date and peer see [here](https://bitcoinserver.nl/deltv_days.txt).

🤔 mzumsande reviewed a pull request: "net: option to disallow v1 connection on ipv4 and ipv6 peers"
(https://github.com/bitcoin/bitcoin/pull/30951#pullrequestreview-3384015944)
A year later, adoption is now almost at ~70% according to https://21.ninja/reachable-nodes/node-service-share/ - could update the OP with that info.

💬 hodlinator commented on pull request "qa: Avoid knock-on exception in assert_start_raises_init_error":
(https://github.com/bitcoin/bitcoin/pull/32929#issuecomment-3451806957)
> No objections, but in this case, it seems beneficial (or at least harmless) to list the init args, which could help debug why the init error was not raised?

Good point, that information from the initial exception was no longer included. Added back in latest push & updated PR description example output.

👍 pinheadmz approved a pull request: "test: add functional test for `TestShell` (matching doc example)"
(https://github.com/bitcoin/bitcoin/pull/33546#pullrequestreview-3384132863)
ACK 57f7c68821d96cc096db624cb06b7a252d038300

Modulo @stratospher comment about adding to `TEST_FRAMEWORK_UNIT_TESTS`.

Reviewed changes in both commits, followed the example from the doc manually and tried a variety of setup options and testshell commands. Confirmed the changes to the doc (wallet name and noshutdown). Built and ran test on macos/arm64

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 57f7c68821d96cc096db624cb06b7a2
...

💬 pinheadmz commented on pull request "test: add functional test for `TestShell` (matching doc example)":
(https://github.com/bitcoin/bitcoin/pull/33546#discussion_r2466128505)
I think this counts as unit test? Could just go in `TEST_FRAMEWORK_UNIT_TESTS` IMO

🤔 Eunovo reviewed a pull request: "interfaces: enable cancelling running `waitNext` calls"
(https://github.com/bitcoin/bitcoin/pull/33676#pullrequestreview-3384019091)
Concept ACK https://github.com/bitcoin/bitcoin/pull/33676/commits/f4e63cc807f8c34a780604bd3802739c37209b75

I took a first look and left some nits. I'll take a second look.

💬 Eunovo commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2466053998)
Consider using `m_interrupt_wait` instead.

💬 Eunovo commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2466059370)
consider `bool& interrupt_wait`

💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3451978821)
Rebased on master

💬 0xB10C commented on pull request "p2p: reduce false-positives in addr rate-limiting":
(https://github.com/bitcoin/bitcoin/pull/33699#issuecomment-3451984555)
> I wonder what's going on with those peers above and to the left of the staircase?

It can happen that a (Bitcoin Core) peer sends us an addr message with more than X entries by random chance. Observed from the logs I looked at:

```
chance >= 1 entries 100.00%
chance >= 2 entries 37.62%
chance >= 3 entries 19.14%
chance >= 4 entries 10.16%
chance >= 5 entries 5.66%
chance >= 6 entries 3.28%
chance >= 7 entries 1.98%
chance >= 8 entries 1.25%
chance >= 9 entries
...

👍 brunoerg approved a pull request: "test: Use same rpc timeout for authproxy and cli"
(https://github.com/bitcoin/bitcoin/pull/33698#pullrequestreview-3384258159)
ACK 66667d6512294fd5dd02161b7c68c19af0865865

-----

> This can be tested by introducing a timeout error and checking it happens with and without --usecli after the exact same time.

I tested it with the provided timeout error example and worked as expected. With this PR, timeout is quite the same with and without --use-cli. With master, they're very different and I ended up by interrupting the run after some long seconds without `--usecli` just in case.


This PR:

```sh
time ./buil
...

💬 mzumsande commented on issue "Seemingly second (very long) validation at the same height":
(https://github.com/bitcoin/bitcoin/issues/33687#issuecomment-3452107390)
One reason why we haven't simply lowered the timeout from 10 minutes is to not change the minimum required bandwidth to keep up with the network. In principle, a synced `-blocksonly` node with a bandwith so slow that they can only download ~2-4MB every 10minutes should be able to keep up with the tip today. If we changed the timeout, the minimum required bandwidth would change.
I also encountered this argument in #29664 (which is a slightly different scenario than the reorg here).

In my opinion
...

👍 ryanofsky approved a pull request: "interfaces: enable cancelling running `waitNext` calls"
(https://github.com/bitcoin/bitcoin/pull/33676#pullrequestreview-3384352720)
Code review ACK f4e63cc807f8c34a780604bd3802739c37209b75. Thanks for the PR! This should be very helpful and close a significant gap in the mining interface, so clients are not forced to use shorter timeouts or keep around extra waitNext threads they don't actually need.

I do think there is a minor race condition that would be good to address here, but it could also be addressed in a followup:

In current implementation if a client makes a `waitNext` call followed by an `interruptWait` call
...

🚀 glozow merged a pull request: "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)"
(https://github.com/bitcoin/bitcoin/pull/29640)

💬 marcofleon commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2466295904)
<details>
<summary>immediate crash on this assertion here</summary>

```bash
fuzz: ../../../../src/wallet/test/fuzz/scriptpubkeyman.cpp:335: void wallet::(anonymous namespace)::spkm_migration_fuzz_target(FuzzBufferType): Assertion `result->desc_spkms.size() == added_size' failed.
==1736884== ERROR: libFuzzer: deadly signal
#0 0x5650eb34b2a4 in __sanitizer_print_stack_trace (/root/bitcoin/fuzzbuild/bin/fuzz+0x9fd2a4) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
#1 0x5650eb3211d8 in
...

💬 marcofleon commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2466310562)
Shouldn't this be `||`? Wouldn't want to dereference if `pub_key` is null.

✅ maflcko closed a pull request: "fuzz: wallet: add target for `MigrateToDescriptor`"
(https://github.com/bitcoin/bitcoin/pull/32624)

📝 maflcko reopened a pull request: "fuzz: wallet: add target for `MigrateToDescriptor`"
(https://github.com/bitcoin/bitcoin/pull/32624)
This PR adds fuzz coverage for the scriptpubkeyman migration (`MigrateToDescriptor`). Note that it's a test for the migration of the scriptpubkey manager, not for the whole migration process as tried in #29694, because:
1) The wallet migration deals with DBs which is expensive for fuzzing (was getting around 3 exec/s);
2) Mocking would require lots of refactors.

This target loads keys, HDChain (even inactive ones), watch only and might add tons of different scripts, then calls `MigrateToDes
...

📝 fanquake opened a pull request: "random: clarify where the environ extern is needed"
(https://github.com/bitcoin/bitcoin/pull/33714)
We know this is needed on at least macOS (#33675), so be explicit.