💬 sipa commented on pull request "rfc: Nuke getblocks message":
(https://github.com/bitcoin/bitcoin/pull/28045#issuecomment-1625436903)
Yes, not every P2P client uses headers-first sync.

💬 furszy commented on pull request "test: bugfix, synchronize indexes synchronously":
(https://github.com/bitcoin/bitcoin/pull/28026#issuecomment-1625445909)
> Why not just push it here, and re-use this PR (dropping any now-redundant changes) . Otherwise, why is this still open.

Closed the PR a minute before your comment.

> It got merged to fix the persistent (potentially confusing) CI issues. What you want to fix isn't currently an active issue.

Well, the previous CI failure was due a pretty clear timeout error. Now there is a possibility of tests hanging forever with no clear reason. So, this last one is practically more confusing than the
...

💬 theStack commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1255855804)
Changed to use the comparison operator. Note that the "can't go below zero-fee" comment from the above was removed, so I'm also not using it here.

💬 theStack commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1255855901)
Agree, done.

💬 theStack commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1255855976)
Nice idea, done.

💬 theStack commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1255856084)
Agree, done. Note that I had to trigger an UTXO set rescan for MiniWallet after restart, otherwise it would sometimes try to spend UTXOs from mempool transactions (which are now not available anymore).

💬 theStack commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1255856373)
You're right. I think the "disabled" string originated from sloppily copy/pasting the test loop structure from mempool_dust.py. Replaced by "zero" now.

💬 glozow commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1255860737)
Ah good thinking!

💬 theStack commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1255861498)
The idea was to check that in both RPC code-paths that result in assembling blocks (`generateto*` and `getblocktemplate`, AFAICT there aren't any others), the setting has the same effect. Happy to remove one of them though, if that's seen as redundant.

💬 glozow commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1255862835)
No need for this condition anymore
```suggestion
assert tx_below_min_feerate['txid'] not in block_template_txids
assert tx_below_min_feerate['txid'] not in block_txids
```

💬 theStack commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1255865677)
Oh right, done 👌

💬 glozow commented on pull request "test: miner: add coverage for `-blockmintxfee` setting":
(https://github.com/bitcoin/bitcoin/pull/27620#discussion_r1255867530)
No need to remove, I was just wondering

💬 furszy commented on pull request "test: indexes, fix on error infinite loop":
(https://github.com/bitcoin/bitcoin/pull/28044#discussion_r1255877236)
pushed.

💬 0xB10C commented on pull request "rfc: Nuke getblocks message":
(https://github.com/bitcoin/bitcoin/pull/28045#issuecomment-1625470129)
I have a `-peerbloomfilters=1` node that receives a few `getblocks` messages per minute. The peers have `/BitcoinKit:0.1.0/`, /WalletKit:0.1.0/`, `/bread:2.1/` as user-agents.

💬 MarcoFalke commented on pull request "test: indexes, fix on error infinite loop":
(https://github.com/bitcoin/bitcoin/pull/28044#issuecomment-1625471588)
Nice, thank you.

lgtm ACK 4b405318d4c476351cccc30588f9126edd94cc35

💬 dergoegge commented on pull request "rfc: Nuke getblocks message":
(https://github.com/bitcoin/bitcoin/pull/28045#issuecomment-1625478134)
> I have a -peerbloomfilters=1 node that receives a few getblocks messages per minute.

Alright closing.

✅ dergoegge closed a pull request: "rfc: Nuke getblocks message"
(https://github.com/bitcoin/bitcoin/pull/28045)

💬 theuni commented on pull request "wallet: don't include bdb files from our headers":
(https://github.com/bitcoin/bitcoin/pull/28039#discussion_r1255896389)
Nice catch. Fixed.

👍 hebasto approved a pull request: "wallet: don't include bdb files from our headers"
(https://github.com/bitcoin/bitcoin/pull/28039#pullrequestreview-1519034588)
ACK 8b5397c00e821d7eaab22f512e9d71a1a0392ebf

💬 MarcoFalke commented on pull request "rfc: Nuke getblocks message":
(https://github.com/bitcoin/bitcoin/pull/28045#issuecomment-1625488424)
It may be good to add basic tests for this feature, otherwise the risk is that it will break eventually and no one noticing for a long time, since there are only few users and existing nodes will continue to reply to messages for the time they are still running?

💬 fanquake commented on pull request "wallet: address book migration bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28038#issuecomment-1625494895)
@achow101 do you want these backported for 25.1?