💬 ryanofsky commented on pull request "kernel: De-globalize fReindex":
(https://github.com/bitcoin/bitcoin/pull/29817#discussion_r1624912618)
In commit "kernel: De-globalize fReindex" (b47bd959207e82555f07e028cc2246943d32d4c3)

It turns out there was a bug introduced here. Setting `chainman.m_blockman.m_reindexing = true` has no effect because chainman is destroyed and recreated on the next loop iteration on line 1537 above. So after this change, reindexing no longer occurs when the user answers "Yes" above. This is fixed in #30132

👍 ryanofsky approved a pull request: "indexes: Don't wipe indexes again when continuing a prior reindex"
(https://github.com/bitcoin/bitcoin/pull/30132#pullrequestreview-2094875458)
Code review ACK eeea0818c1a20adc5225b98b185953d386c033e0. All the changes here look good.

But I also think it was a mistake in #29817 to add the `BlockManagerOpts::reindex` option, and think it would be better to have a less confusing set of options. The following change built on top of this PR could provide a simpler alternative: 9c643e7ddd82523b84f65b614d3e6c1f640b23c7. Feel free to use any of these changes, or they could be a separate followup.

📝 achow101 opened a pull request: "wallet: Ensure best block matches wallet scan state"
(https://github.com/bitcoin/bitcoin/pull/30221)
Implements the idea discussed in https://github.com/bitcoin/bitcoin/pull/29652#issuecomment-2010579484

Currently, `m_last_block_processed` and `m_last_block_processed_height` are not guaranteed to match the block locator stored in the wallet, nor do either of those fields actually represent the last block that the wallet is synced up to. This is confusing and unintuitive.

This PR changes the those last block fields to actually be in sync with the record stored on disk. This requires adding
...

💬 achow101 commented on pull request "wallet: Avoid potentially writing incorrect best block locator":
(https://github.com/bitcoin/bitcoin/pull/29652#issuecomment-2146197896)
> I'm now wondering whether the wallet should even be doing anything on `ChainStateFlushed` since that doesn't seem like it should have any bearing on what the wallet knows about. [...]

I've implemented this idea in #30221

💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1625154834)
I followed IWYU here and added kernel/mempool_removal_reason.h to wallet.h

💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1625155299)
@josibake Deleting the nullptr constructor worked.

💬 Eunovo commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#issuecomment-2146288055)
Thanks for the reviews @josibake and @furszy I have implemented your suggested changes

📝 Eunovo converted_to_draft a pull request: "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict"
(https://github.com/bitcoin/bitcoin/pull/29680)
This PR implements a fix for the issue described in https://github.com/bitcoin/bitcoin/issues/29435.

The problem is that the wallet is unable to abandon transactions that have unrelated parent conflicts. The solution implemented here, augments the mempool transaction `REPLACED` signal with the double-spending transaction which the wallet stores and watches for in Block notifications. A map is added to the wallet to track conflicting tx ids and their child transactions. The entry is erased wh
...

🚀 fanquake merged a pull request: "depends: Update Boost download link"
(https://github.com/bitcoin/bitcoin/pull/30217)

✅ maflcko closed an issue: "Improve/simplify node sync for pruned nodes"
(https://github.com/bitcoin/bitcoin/issues/30220)

💬 maflcko commented on issue "Improve/simplify node sync for pruned nodes":
(https://github.com/bitcoin/bitcoin/issues/30220#issuecomment-2146978830)
This is possible to implement, but may be controversial, depending on how it is implemented. You can learn more about the topic by reading the various discussions around assumeutxo.

Usually the issue tracker is used to track technical issues related to the Bitcoin Core code base.

General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com) or the `#bitcoin` IRC channel on Libera Chat, or one of the Bitcoin subreddits,
...

💬 hebasto commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2147006298)
I've add a [benchmark](https://github.com/hebasto/bitcoin/commit/0f5abaf980122cce1bd0d405d17db3bfab273afa) for the `FeeRateCompare` function, which calls `Mul` twice.

Here are results on Windows 11, `Release` configuration, which implies `/O2 /Oi` compile flags:
- master branch @ 61de64df6790077857faba84796bb874b59c5d15:
```
> .\build_msvc\master\bench_bitcoin.exe -filter=FeefracMultipication

| ns/op | op/s | err% | total | benchmark
|---------------
...

💬 maflcko commented on pull request "guix: Use LTO to build releases":
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-2147129097)
> Is this safe to do in light of https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105469 ?

Also https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113359 (in combination with https://github.com/bitcoin/bitcoin/pull/29881)

Would be nice to extend the motivation (pull request description) a bit about the benefits and risks.

💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2147147524)
Re https://github.com/bitcoin/bitcoin/pull/30132#pullrequestreview-2094875458

> But I also think it was a mistake in https://github.com/bitcoin/bitcoin/pull/29817 to add the BlockManagerOpts::reindex option, and think it would be better to have a less confusing set of options. The following change built on top of this PR could provide a simpler alternative: https://github.com/bitcoin/bitcoin/commit/9c643e7ddd82523b84f65b614d3e6c1f640b23c7. Feel free to use any of these changes, or they could
...

📝 fanquake opened a pull request: "[27.x] rc2 or final"
(https://github.com/bitcoin/bitcoin/pull/30222)
Backports:
* https://github.com/bitcoin/bitcoin/pull/30216
* https://github.com/bitcoin/bitcoin/pull/30217

I don't think either of these changes warrants an `rc2` cycle.
Depending on what else is added, this could turn into final.

💬 fanquake commented on pull request "depends: Update Boost download link":
(https://github.com/bitcoin/bitcoin/pull/30217#issuecomment-2147211651)
Backported in #30222.

💬 fanquake commented on pull request "build: Fix building `fuzz` binary on on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/pull/30216#issuecomment-2147211876)
Backported to 27.x in #30222.

💬 hebasto commented on pull request "feefrac: 128-bit multiply support in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29758#discussion_r1625782590)
> on MSVC one needs to use the `_mul128` or `_mulh` intrinsics instead

From my research, it follows that [`__mulh`](https://learn.microsoft.com/en-us/cpp/intrinsics/mulh) seems more performant.

This code:
```suggestion
// On 64-bit MSVC, use __mulh intrinsic for wide multiplication.
return {__mulh(a, b), std::bit_cast<uint64_t>(a) * b};
```
gives the following numbers for the [benchmark](https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2147006298):
```
> bu
...

👍 maflcko approved a pull request: "Lint: Support running individual lint checks"
(https://github.com/bitcoin/bitcoin/pull/30219#pullrequestreview-2095977332)
lgtm

💬 maflcko commented on pull request "Lint: Support running individual lint checks":
(https://github.com/bitcoin/bitcoin/pull/30219#discussion_r1625752646)
```suggestion
( cd ./test/lint/test_runner/ && RUST_BACKTRACE=1 cargo run -- --help )
```