💬 willcl-ark commented on issue "-fallbackfee should apply to estimatesmartfee":
(https://github.com/bitcoin/bitcoin/issues/27415#issuecomment-1498202324)
@kylezs Could you program your test to use the `--fallbackfee` if it recieves the error in reponse, or as Antoine reponded to you there fill up your test mempool so that `estimatesmartfee` works as intended?

Accessing `--fallbackfee` from within the fee estimation rpc involves importing a lot of wallet code which woudl be preferable to avoid...

🤔 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!