🤔 pablomartin4btc reviewed a pull request: "wallet, tests: Expand and test when the blank wallet flag should be un/set"
(https://github.com/bitcoin/bitcoin/pull/25634)
re-ACK https://github.com/bitcoin/bitcoin/commit/c5fb9a4dd97b47db0e52c497a757dff943b255b1

📝 dimitaracev opened a pull request: "validation: Replicate MinBIP9WarningHeight with MinBIP9WarningStartTime"
(https://github.com/bitcoin/bitcoin/pull/27427)
This PR addresses [comment](https://github.com/bitcoin/bitcoin/pull/27357#issuecomment-1497009569). Replaces `CChainParams::MinBIP9WarningHeight` with `CChainParams::MinBIP9WarningStartTime`. It is then used in `WarningBitsConditionChecker::BeginTime` to only check block headers that are newer.
This helps reduce the block headers that are being scanned for warnings.

cc @ajtowns

👋 jonatack's pull request is ready for review: "test: move remaining rand code from util/setup_common to util/random"
(https://github.com/bitcoin/bitcoin/pull/27425)

👍 hebasto approved a pull request: "Fixes compile errors in MSVC build #27332"
(https://github.com/bitcoin/bitcoin/pull/27335)
re-ACK 6a9a4d13b2b22632e0acd4f86f7bac238294ba42

💬 josibake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1159666836)
confirmed this does fix the download selection filter

💬 MarcoFalke commented on pull request "ci: Run base install at most once":
(https://github.com/bitcoin/bitcoin/pull/27429#discussion_r1159667906)
Yes, `DANGER_RUN_CI_ON_HOST` is called that way to discourage people from trashing their system

💬 MarcoFalke commented on pull request "ci: Run base install at most once":
(https://github.com/bitcoin/bitcoin/pull/27429#discussion_r1159668508)
(This applies to all `CI_EXEC` lines, so I don't think anything is gained from mentioning it here)

🤔 TheCharlatan reviewed a pull request: "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet"
(https://github.com/bitcoin/bitcoin/pull/27279)
Approach ACK

💬 josibake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#discussion_r1159670307)
nit: it would be nice to have it also print out the destination, e.g "downloading {binary_filename} to {destination}"

💬 glozow commented on pull request "Deprecate and remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1499006887)
Concept ACK. I agree there should be a mailing list post.

💬 petertodd commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1499080469)
> > Anyway the data from [miningpool.observer/template-and-block](https://miningpool.observer/template-and-block) shows that blocks routinely include a dozen or so unexpected transactions. It's not clear if those are transactions not in miningpool.observer's mempoo, and thus relevant to compact blocksl; @0xB10C, can you clarify this point?
>
> Transactions listed under "Extra Transactions" are transactions that weren't in my mempool but the pool knew about. These are transactions my node woul
...

🤔 glozow reviewed a pull request: "wallet: when a block is disconnected, update transactions that are no longer conflicted"
(https://github.com/bitcoin/bitcoin/pull/27145)
Concept ACK

There is a missing dash in "Co-authored-by:" in the commit messages

💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159806134)
Sorry to bikeshed, but I think the naming is a bit hard to follow ("conflicted" vs "conflicting" don't immediately make sense to me, especially since both sets of transactions get conflicted at some point during the test). Might I suggest names that indicate which input(s) they spend, which node broadcasts them, and what "generation" they are? For example

`tx_AB_0_parent` instead of `conflicted`
`tx_A_1` instead of `conflicting_tx_A`
`tx_B_1` instead of `conflicting_tx_B`

`C` instead of
...

💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159787578)
nit in 77a2e522d845eda270b2c03fd7e0a16bc4c00fbb, I think this is clearer (same with the other arrays of inputs and outputs)

```suggestion
inputs_conflicted_tx = [{"txid": txid_conflict_from_1, "vout": nA}, {"txid": txid_conflict_from_2, "vout": nB}]
```

💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159765529)
in 77a2e522d845eda270b2c03fd7e0a16bc4c00fbb:

I think this comment is a little bit hard to follow. Something more descriptive might be
```suggestion
"""
Test that wallet correctly tracks transactions that have been conflicted by blocks, particularly during reorgs.
"""
```

💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159807735)
```suggestion
self.sync_blocks()
```

💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159814534)
in commit 49f8af533f2e6370f4ae5220f5c706e30b90d06c

no need for const reference to integer

💬 glozow commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#discussion_r1159784792)
We should also test who "wins" in this conflict contest and what the values are, i.e. conflicting A has 8 confirmations and conflicting B has 4 confirmations.

💬 pinheadmz commented on issue "Manual-pruning cursor rewind":
(https://github.com/bitcoin/bitcoin/issues/19807#issuecomment-1499130274)
> the only solution is to do a full resync and download all the way back to N-N (aka 0).

There is also `getblockfrompeer` https://github.com/bitcoin/bitcoin/pull/20295 I just tried this on my pruned node to inspect blocks below the prune depth, pretty neat!

🤔 furszy reviewed a pull request: "indexes: Read the locator's top block during init, allow interaction with reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/25193#pullrequestreview-1375021974)
Instead of the global flag that requires many manual sets at different locations and the indexes threads active wait, what if we move the indexes threads start after the loading process? e.g. https://github.com/furszy/bitcoin-core/commit/1e48179f70dafff8fad31b4048681103cca90b56.

It makes code shorter and more robust. Plus, it let us keep the pruning checks as well.