💬 stickies-v commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#discussion_r1631416825)
I think this has to be the inverse, but just "reverting back" to `!do_reindex` is probably more clear:
```suggestion
if (!do_reindex && !do_reindex_chainstate && chain_active_height <= 1) {
```

💬 achow101 commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-2155189032)
Rebased

> Seems like this is a remaining mention of `CTransaction::nVersion`?

Good catch, fixed.

👍 tdb3 approved a pull request: "test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16"
(https://github.com/bitcoin/bitcoin/pull/28312#pullrequestreview-2105015222)
re ACK for 5cf0a1f230389ef37e0ff65de5fc98394f32f60c
Re-ran functional tests (passed) and induced a failure by hard coding 1 as the k value.

💬 tdb3 commented on pull request "test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16":
(https://github.com/bitcoin/bitcoin/pull/28312#discussion_r1631478208)
nit: In case this file is touched, it might be clearer (as rkrux mentioned) to add a comment explicitly explaining the math, although this is just a nice-to-have since the addition breaks it out such that the reader is provided a hint (typical multisig script).
Author's preference.

```diff
diff --git a/test/functional/test_framework/script_util.py b/test/functional/test_framework/script_util.py
index 855f3b8cf5..69be490a67 100755
--- a/test/functional/test_framework/script_util.py
+++ b/
...

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631480159)
I haven't benchmarked or investigated compiled code, but I want to give the compiler every opportunity to use the simpler operation here (the conditional `Set` is more complex, and inlining will hopefully strength-reduce it, but I prefer not counting on that).

👍 tdb3 approved a pull request: "json-rpc 2.0 followups: docs, tests, cli"
(https://github.com/bitcoin/bitcoin/pull/30238#pullrequestreview-2105025972)
ACK for 1f6ab1215bbb1f8a5f1743c3c413b95ad08090df
Great work and thanks for following up with the comments.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631489780)
Done.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631489851)
Done.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631490140)
Added coverage for the `swap` function in the fuzz test.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631490229)
Done.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631490300)
Done.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631490491)
Same reasoning as [here](https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631480159)

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631490602)
Done.

💬 sipa commented on pull request "util: add BitSet":
(https://github.com/bitcoin/bitcoin/pull/30160#discussion_r1631490667)
Done.

💬 katesalazar commented on pull request "wallet: Improve error log color in the console":
(https://github.com/bitcoin-core/gui/pull/823#issuecomment-2155231832)
~

✅ murchandamus closed a pull request: "Avoid changeless input sets when SFFO is active"
(https://github.com/bitcoin/bitcoin/pull/28985)

💬 murchandamus commented on pull request "Avoid changeless input sets when SFFO is active":
(https://github.com/bitcoin/bitcoin/pull/28985#issuecomment-2155232793)
Closed in favor of #29532 and #28366

💬 theuni commented on pull request "Enable clang-tidy checks for self-assignment":
(https://github.com/bitcoin/bitcoin/pull/30234#issuecomment-2155239303)
Added a commit to work around in leveldb.

@fanquake Would you be ok with carrying this in our subtree? I'm not sure of a better solution.

💬 stickies-v commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#discussion_r1631503948)
I actually don't understand why we also need to check for `!do_reindex_chainstate` here, when `CChain::vChain` is populated by the blockindex building process, so my intuition tells me `chain_active_height` should only be affected by `-reindex` (but lldb tells me otherwise, so it does indeed seem necessary).

💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2155258493)
Thanks for the review @stickies-v,

Updated f68cba29b3be0dec7877022b18a193a3b78c1099 -> f68cba29b3be0dec7877022b18a193a3b78c1099 ([preserveIndexOnRestart_5](https://github.com/TheCharlatan/bitcoin/tree/preserveIndexOnRestart_5) -> [preserveIndexOnRestart_6](https://github.com/TheCharlatan/bitcoin/tree/preserveIndexOnRestart_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/preserveIndexOnRestart_5..preserveIndexOnRestart_6))

* Addressed @stickies-v's [comment](https://github.com
...