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

💬 fanquake commented on pull request "randomenv: drop self define of 'environ'":
(https://github.com/bitcoin/bitcoin/pull/33675#issuecomment-3452288810)
Documented this in #33714.

👍 ryanofsky approved a pull request: "qa: Avoid knock-on exception in assert_start_raises_init_error"
(https://github.com/bitcoin/bitcoin/pull/32929#pullrequestreview-3384466555)
Code review ACK fde4d2f0e1cf1252cbb24aad475332d96f3e6e80. Very welcome change! I almost always get confused by "During handling of the above exception, another exception occurred" messages, so it is great to eliminate one and stop showing a spurious internal exception before the real error when `assert_start_raises_init_error` checks fail.

Implementation of this PR is also a little simpler that it may first appear. The actual fix is small and is in the first commit, and the later commits are co
...

💬 fanquake commented on pull request "contrib: Remove brittle, confusing and redundant UTF8 encoding from Python IO":
(https://github.com/bitcoin/bitcoin/pull/33702#issuecomment-3452308670)
cc @theStack

👍 hebasto approved a pull request: "random: clarify where the environ extern is needed"
(https://github.com/bitcoin/bitcoin/pull/33714#pullrequestreview-3384482126)
ACK f24ef2b4e01884ac1dea07ebbc7e7d0aa2c09541.

💬 brunoerg commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2466434070)
Yes, nice find!

⚠️ torkelrogstad opened an issue: "Failure to bind to ZMQ addresses is swallowed in debug logs"
(https://github.com/bitcoin/bitcoin/issues/33715)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

When failing to bind to specified ZMQ addresses due to them already being in use, the error is completely swallowed up when not running with debug logs.

### Expected behaviour

I'd expect one of two things to happen:

1. Bitcoin Core exiting with a non-zero status code and a clear message saying what happened (I'd prefer this)
2. An error message in the logs, that is visible without havi
...

💬 brunoerg commented on pull request "fuzz: wallet: add target for `MigrateToDescriptor`":
(https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2466463098)
Interesting how I didn't get it but I think this is related to https://github.com/bitcoin/bitcoin/pull/32624#discussion_r2466310562. The check was wrong so it was playing with not-fully-valid keys.