🤔 furszy reviewed a pull request: "index: fix wrong assert of current_tip == m_best_block_index"
(https://github.com/bitcoin/bitcoin/pull/32878#pullrequestreview-3003004554)
@HowHsu, check this test https://github.com/furszy/bitcoin-core/commit/f2b8a06060e2a313f35101454c7669d46e0edc38.
It triggers the reorg assertion failure without needing to deduplicate the index synchronization code. The test fails without your fix commit and passes with it. Feel free to cherry-pick it.

> The Sync in the test is same as in bitcoind, expect the thread synchronization code which is to trigger the reorg. The only cons is we have to keep it updated when the logic of Sync() change
...

💬 achow101 commented on pull request "wallet: Track no-longer-spendable TXOs separately":
(https://github.com/bitcoin/bitcoin/pull/27865#discussion_r2195947572)
Using a default value has the potential very problematic because its value is expected to be correct. By using an optional, we can detect when there is a programming error that did not set the value.

💬 sipa commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#discussion_r2195958660)
Fixed.

💬 sipa commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#issuecomment-3054002498)
@darosior I did it slightly differently, but added a commit that makes the use of the sighash cache optional. I avoided using default values as they can result in unexpected/missed call sites.

💬 davidgumberg commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-3054117201)
Post-merge ACK https://github.com/bitcoin/bitcoin/commit/4c772cbd83e502a1339e8993d192ea6416ecd45c

Lightly reviewed code, tested by running IBD with `debug=all` and `-nodebug`, no messages suppressed, ran logging related benchmarks and measured performance impact is minimal.

One more really minor and feel-free-to-disregard nit for the follow-up pile:

The `LOG_TIME*` macros incorrectly report source locations from `logging/timer.h` rather than where the macros are invoked. There are curr
...

🚀 achow101 merged a pull request: "wallet, test: best block locator matches scan state follow-ups"
(https://github.com/bitcoin/bitcoin/pull/32580)

📝 hodlinator opened a pull request: "qa: Avoid knock-on exception in assert_start_raises_init_error"
(https://github.com/bitcoin/bitcoin/pull/32929)
Raising a new exception from within a Python `except`-block causes the interpreter to generate extra error output which is unnecessary in this case.

Found while testing #32835 using the suggested method (https://github.com/bitcoin/bitcoin/pull/32835#issue-3188748624) which triggered expected timeouts, but with the extra error noise.

<details><summary>Example output before & after this PR</summary>

Before:
```
2025-07-08T20:05:48.407001Z TestFramework (ERROR): Assertion failed
Traceba
...

💬 mzumsande commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3054182296)
General question - should we remove the assert, or would it maybe be better to keep it, but commit the best block index before calling `Rewind()` in `Sync()`?

💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2196069916)
Thanks for the follow-up. The current fix is wrong. Looking at the original `getdescriptoractivity` [implementation](https://github.com/bitcoin/bitcoin/pull/30708) PR samples and tests, also checking the logic in the RPC, the second array, descriptors, has to be provided. Now, the first one, blockhashes, could be empty and activities may be returned in result if there are any in the mempool (3rd parameter `include_mempool` is true by default, but if the user sets it to false, the RPC is still us
...

💬 pablomartin4btc commented on pull request "rpc, test: Fix JSON parsing errors in unloadwallet and getdescriptoractivity RPCs":
(https://github.com/bitcoin/bitcoin/pull/32845#discussion_r2196177876)
> just want to prevent .get_array() from throwing

Yes, plus we need to validate both arrays... we could make blockhashes not optional but we can't do the same with descriptors (`scan_objects_arg_desc`) as it's used in different places.

💬 achow101 commented on pull request "validation: Add eligible ancestors of reconsidered block to setBlockIndexCandidates":
(https://github.com/bitcoin/bitcoin/pull/30479#issuecomment-3054466669)
ACK 8cc3ac6c2328873e57c31f237d194c5f22b6748a

💬 ryanofsky commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2196208483)
In commit "rpc: Handle -named argument parsing where Base64 encoding is used" (bc2241ab3d01a38f19588cb85134e5330b77e3f4)

Thanks for the updates!

Looking at more closely at this solution, some things don't make sense to me. This code from lines 513-515 is checking to see if a command line argument that parses as NAME=VALUE has a NAME that is listed in the table above as a known string parameter. But if it is instead of adding the parameter to `params` and passing the argument by name, it is
...

✅ achow101 closed an issue: "Assertion `setBlockIndexCandidates.count(pindex)' failed"
(https://github.com/bitcoin/bitcoin/issues/16444)

🚀 achow101 merged a pull request: "validation: Add eligible ancestors of reconsidered block to setBlockIndexCandidates"
(https://github.com/bitcoin/bitcoin/pull/30479)

🤔 w0xlt reviewed a pull request: "test: less ambiguous error if bitcoind is missing"
(https://github.com/bitcoin/bitcoin/pull/32921#pullrequestreview-3003460655)
ACK https://github.com/bitcoin/bitcoin/pull/32921/commits/83bb41455715a9e05320ba791987204031626c10

🤔 w0xlt reviewed a pull request: "test: Turn rpcauth.py test into functional test"
(https://github.com/bitcoin/bitcoin/pull/32881#pullrequestreview-3003468022)
ACK https://github.com/bitcoin/bitcoin/pull/32881/commits/fa4d68cf97b6d77dbb559facbc425376e2c51f62

🤔 w0xlt reviewed a pull request: "wallet: Remove isminetypes"
(https://github.com/bitcoin/bitcoin/pull/32523#pullrequestreview-3003496887)
ACK https://github.com/bitcoin/bitcoin/pull/32523/commits/d9ca23fb6f3ce6d4863583226f5ae967a7c49497

💬 pstratem commented on pull request "Cache m_cached_finished_ibd where SetTip is called.":
(https://github.com/bitcoin/bitcoin/pull/32885#issuecomment-3055626490)
> Concept ACK, but I'm not convinced this implementation is safe as-is. If we want to maintain the current behaviour, it's not sufficient to update only when the tip changes. We also need to re-check when importing/reindexing completes, and schedule an update timer if max_tip_age is the final cause of not exiting IBD.

This made me revisit the function and consider what we're trying to achieve.

The function is only interesting when it can latch to the IBD finished state.

That's only poss
...

💬 achow101 commented on issue "guix: Windows build is non-deterministic across build architectures":
(https://github.com/bitcoin/bitcoin/issues/32923#issuecomment-3055693691)
#32597 being the cause of this non-determinism doesn't make any sense to me.

The function with the non-determinsm is `WalletModel::prepareTransaction` which is a GUI only function that was not modified by #32597. That PR didn't change anything remotely near that, so it being the root cause makes no sense.

addr2line indicates that the problematic line corresponds to https://github.com/bitcoin/bitcoin/blob/a40e9536588c366886de4f4b9d67b8665a509929/src/prevector.h#L174 After unwrapping all of the
...

📝 achow101 opened a pull request: "Resolve guix non-determinism with emplace_back instead of push_back"
(https://github.com/bitcoin/bitcoin/pull/32930)
For some reason, building x86_64-w64-mingw32 on x86_64 and aarch64 results in a single instruction difference which can be traced down to prevector.h:174. The ultimate caller of this is the copy constructor for a prevector that ends up being called by std::vector::push_back in walletmodel.cpp:183. By replacing the push_back with an emplace_back, somehow this non-determinism goes away.

Closes #32923