💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249618312)
Thanks - moved into `LocationsIndex::CustomAppend()`.

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2249618367)
Thanks - removed.

> Was this supposed to be used to skip forward in the stream instead of deserializing the header?

Yes, but now it's not needed here.

💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3148142648)
Thanks for the review!
Applied the comments: [`1b928f5 -> b2a22ce`](https://github.com/bitcoin/bitcoin/compare/1b928f58fc78f4727cef988902075abc05c372b2..b2a22ce33dc69697181547fa1e83bd0ed3321565)

✅ fanquake closed an issue: "intermittent issue in wallet_sendall.py", line 440, in sendall_anti_fee_sniping assert_greater_than(tx_from_wallet["decoded"]["locktime"], tx_from_wallet["blockheight"]"
(https://github.com/bitcoin/bitcoin/issues/33114)

🚀 fanquake merged a pull request: "test: fix anti-fee-sniping off-by-one error"
(https://github.com/bitcoin/bitcoin/pull/33118)

⚠️ nervana21 opened an issue: "Crash when chain tip is missing from candidate set"
(https://github.com/bitcoin/bitcoin/issues/33129)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

`setBlockIndexCandidates` can become empty if the chain tip is not included in the set, which violates an internal invariant and leads to a crash during `FindMostWorkChain()`. This issue and potential solutions were originally surfaced by ataraxia009 in #33127.

### Expected behaviour

After pruning, the active tip must be present in the candidate set. No crash should occur in `FindMostWor
...

📝 nervana21 opened a pull request: "validation: Keep chain tip in candidate set"
(https://github.com/bitcoin/bitcoin/pull/33130)
After `PruneBlockIndexCandidates()` is called, the candidate set must include the current chain tip or a successor of it.

`PruneBlockIndexCandidates()` removes candidates with less work than the active tip. However, because the tip itself is never added to `setBlockIndexCandidates` via `TryAddBlockIndexCandidate()`, it may be absent from the set. Prior to this patch, if all lower-work candidates were pruned and the tip was not present, `setBlockIndexCandidates` could become empty—violating an
...

💬 nervana21 commented on pull request "validation: Keep chain tip in candidate set":
(https://github.com/bitcoin/bitcoin/pull/33130#issuecomment-3148457986)
cc: @Ataraxia009

📝 cedwies opened a pull request: "tests: cover abortrescan() in-flight True path with dynamic-tail retry"
(https://github.com/bitcoin/bitcoin/pull/33131)
This PR adds a test in wallet_transactiontime_rescan.py. Previously we only tested that abortrescan() returns False when no rescan was running. This change verifies that it returns True and actually interrupts a running scan.

How it works:
1. spawn a daemon background thread that rescans a tail of the wallet’s chain.
2. aggressively poll every 10 ms (for up to 5 s) until we see the node start scanning
3. Call 'abortrescan()' and assert to True
4. Threads are joined with short timeouts so
...

🤔 cedwies reviewed a pull request: "test: refactor mempool_accept_wtxid"
(https://github.com/bitcoin/bitcoin/pull/33067#pullrequestreview-3082223310)
Changes look great.

Nice to see the valid_witness_malleate_tx helper instead of the ValidWitnessMalleatedTx class. Makes the test's logic much clearer IMO.

ACK 0cbcdca

💬 ismaelsadeeq commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-3148513933)
It will be nice if this get in to v30, cc all previous reviewers @maflcko perhaps add the v30.0 milestone

💬 Ataraxia009 commented on issue "Crash when chain tip is missing from candidate set":
(https://github.com/bitcoin/bitcoin/issues/33129#issuecomment-3148515467)
The issue I was addressing has nothing to do with `FindMostWorkChain` or a crash in it.

The root issue that I witnessed was a crash on launch in `PruneBlockIndexCandidates` which is caused by the data dir reaching a bad state.

In this state, the block validity of the block indexes in the `setBlockIndexCandidates` was correct, i.e.: `BLOCK_VALID_TRANSACTIONS`,
but the block validity for the blocks that were used to to build the `CoinsView` and therefore the `M_Chain` did not have their require
...

💬 furszy commented on issue "Crash when chain tip is missing from candidate set":
(https://github.com/bitcoin/bitcoin/issues/33129#issuecomment-3148522739)
Which version of Bitcoin-Core are you running?

🤔 mzumsande reviewed a pull request: "validation: Keep chain tip in candidate set"
(https://github.com/bitcoin/bitcoin/pull/33130#pullrequestreview-3082372722)
> After PruneBlockIndexCandidates() is called, the candidate set must include the current chain tip or a successor of it.

`setBlockIndexCandidates` must at all times include all connectable blocks that have at least as much work as the tip, so the tip itself must at all times be in the set.

> However, because the tip itself is never added to setBlockIndexCandidates via TryAddBlockIndexCandidate(), it may be absent from the set.

This seems wrong / makes no sense to me. No block can ever
...

💬 mzumsande commented on issue "OpenBSD, NetBSD: `-reindex` is broken":
(https://github.com/bitcoin/bitcoin/issues/33128#issuecomment-3148775143)
Does this mean that `feature_reindex.py` still succeeds, so that reindex only fails if we delete the `index/` dir like in `feature_reindex_init.py`, but not if we just reindex without corrupting anything?!

💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2250201595)
Ok, I am working on a PR for this and I should have it ready by tomorrow but I think it's better to keep it separate from this one.

💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3148815473)
> which I think could be enabled for coinstatsindex relatively easily, FYI @furszy

Not sure if I maybe misunderstand that part but do you mean parallelization could be enabled easily for coinstatsindex? I haven't reviewed https://github.com/bitcoin/bitcoin/pull/26966 in depth yet but I don't think that's the case because the blocks have to be processed in order.

✅ nervana21 closed a pull request: "validation: Keep chain tip in candidate set"
(https://github.com/bitcoin/bitcoin/pull/33130)

💬 nervana21 commented on pull request "validation: Keep chain tip in candidate set":
(https://github.com/bitcoin/bitcoin/pull/33130#issuecomment-3148820941)
Hi mzumsande,
Thanks for the thorough feedback.

You're right, it is preferable to crash a node that is out of consensus. I believe that I did have a chain sequence construction that led to `setBlockIndexCandidates` without chain tip (now I'm less confident), but even so, I'm certain the chain construction would be out of consensus and therefore not worth salvaging. I wish I had considered before and I will try to consider in the future.

Thanks again!

📝 theStack opened a pull request: "fuzz: txgraph: fix `real_is_optimal` flag propagation in `CommitStaging`"
(https://github.com/bitcoin/bitcoin/pull/33132)
In the `txgraph` fuzz test, the `CommitStaging` step updates the `SimTxGraph` levels simply by erasing the front (=main) one in the `sims` vector, i.e. the staging level instance takes the place of the main level instance:

https://github.com/bitcoin/bitcoin/blob/83a2216f5278c1123ba740f1c8a055652e033ddd/src/test/fuzz/txgraph.cpp#L668-L672

This also includes the `real_is_optimal` flag (reflecting whether the corresponding real graph is known to be optimally linearized), without taking into a
...