💬 MarcoFalke commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172737216)
Won't this need mempool.cs to ensure an atomic result? Otherwise you may get nonsense back on a data race, like a parent missing from the mempool, when the child is in the mempool, no?

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739437)
Added some comments. Please have a look.

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739546)
Done.

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739678)
Done.

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739938)
Done. Also renamed to just `SIZE`.

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172741182)
Your math is correct. And indeed, it's such a low probability event that it's not worth adding code to deal with it (even outside of a benchmark, I'd say). Still, I've added an assert to make it clear to the reader this is always supposed to be the case.

💬 glozow commented on pull request "mempool: keep CPFP'd transactions when loading from mempool.dat":
(https://github.com/bitcoin/bitcoin/pull/27476#issuecomment-1516508803)
> Maybe move most of the text in the PR description into a comment? "Review this first" and "Alternatives considered" stuff is useful here, but doesn't seem useful for the merge commit message?

Moved "alternatives considered." I prefer putting "review this first" at the top so reviewers see it as soon as possible. It will go away if/when I rebase, so definitely before any merging happens.

💬 instagibbs commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172742351)
significantly clearer, thanks

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172747649)
Yes good point. Will add a lock-already-held `exists` and put this all under `LOCK(mempool.cs)`

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172748986)
Oh, or would it be better to have `CTxMemPool::GetPrioritisationMap` take the lock and create a map from txid to pair<fee, bool>?

💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1172760920)
Correct. Do you want me to add it to the doc?

💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1172763708)
Nice, pushed your test

🤔 stickies-v reviewed a pull request: "blockstorage: do not flush block to disk if it is already there"
(https://github.com/bitcoin/bitcoin/pull/27039#pullrequestreview-1289406384)
Approach ACK 93c70287a6434c6c665a211dc4dfbbd9c3db4083 (modulo tests)

Conceptually, this seems straightforward: we don't want to update block files during reindex, so avoiding the flushing altogether is desireable.

I think the only potential for unexpected behaviour change is if there are places other than `BlockManager::SaveBlockToDisk()` where block files are modified (and potentially not flushed). Looking at the callsites of `FlatFileSeq::Open()` and all of its parent callsites, it seems
...

💬 stickies-v commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1162797652)
nit: some inconsistent usage of `BOOST_CHECK` and `BOOST_CHECK_EQUAL`, I think using `BOOST_CHECK_EQUAL` here and in a few other places makes sense

💬 stickies-v commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1171578190)
nit: unintuitive name, it's not a block
```suggestion
FlatFilePos last_block_pos;
```

💬 stickies-v commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1164077010)
Is this a reliable way to check for a FAT filesystem? WIN32 could also use other filesystems I think (e.g. NTFS), and likewise - FAT could still be used without WIN32, I think?

I think I agree with [LarryRuane](https://github.com/bitcoin/bitcoin/pull/27039/files#r1122400769) that comparing files is probably more robust and portable, which I think is reasonable with these small file sizes? But perhaps you've found this is too much of a performance hit?

You could also check the filesystem di
...

💬 glozow commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1172766671)
Sorry for being pedantic, yes please. Something like "fee delta priority from the file is added on top of any priority already present in the mempool"

💬 achow101 commented on pull request "p2p: update hardcoded mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1516558561)
ACK 31b1798d2c3fa3c479eb2d1896240e0b7fad600b

I think we should reconsider how we do the tor, i2p, and cjdns seeds for the next release.

💬 mzumsande commented on pull request "p2p: update hardcoded mainnet seeds for 25.x":
(https://github.com/bitcoin/bitcoin/pull/27488#issuecomment-1516561832)
reACK 31b1798d2c3fa3c479eb2d1896240e0b7fad600b

After running the (now bugfixed) script based on sipa's seeder data locally, I have fewer differences compared to my ACK from yesterday for IPv4/6.

💬 MarcoFalke commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1516564787)
.test.ignore.

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172792521)
Done