💬 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.
(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.
(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
(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)`
(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>?
(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?
(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
(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
...
(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
(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;
```
(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
...
(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"
(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.
(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.
(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.
(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
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172792521)
Done
💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172799613)
Fixed
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172799613)
Fixed
💬 ariard commented on issue "Document CoreDev organization":
(https://github.com/bitcoin/bitcoin/issues/27497#issuecomment-1516584111)
@fanquake Without a desire to make more noise here, just to clarify your position you think it's not a discussion we should have as a project or it's more the unclarity of the outcome ?
For the place, of course there is https://github.com/coredev-tech/coredev-tech.github.io which might a better place. Just no contributors are looking on this repository.
For the outcome, we could have a `COREDEV.md`, detailing the process:
- reach out to past organisers to check if there is someone already
...
(https://github.com/bitcoin/bitcoin/issues/27497#issuecomment-1516584111)
@fanquake Without a desire to make more noise here, just to clarify your position you think it's not a discussion we should have as a project or it's more the unclarity of the outcome ?
For the place, of course there is https://github.com/coredev-tech/coredev-tech.github.io which might a better place. Just no contributors are looking on this repository.
For the outcome, we could have a `COREDEV.md`, detailing the process:
- reach out to past organisers to check if there is someone already
...
💬 ajtowns commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1516588784)
> @ajtowns I don't think I am properly communicating my usecase correctly,
When I asked what [you were trying to solve](https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1502605806), you [didn't respond](https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1503777153) to that question at all...
The example listed on [the mutinynet blog entry](https://blog.mutinywallet.com/mutinynet/) is "The most obvious win here is we can have a channel "confirmed" much more quickly." -- b
...
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1516588784)
> @ajtowns I don't think I am properly communicating my usecase correctly,
When I asked what [you were trying to solve](https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1502605806), you [didn't respond](https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1503777153) to that question at all...
The example listed on [the mutinynet blog entry](https://blog.mutinywallet.com/mutinynet/) is "The most obvious win here is we can have a channel "confirmed" much more quickly." -- b
...
⚠️ dergoegge opened an issue: "meta: Isolated fuzzing of net processing"
(https://github.com/bitcoin/bitcoin/issues/27502)
Efficient isolated fuzzing of our message processing code (net processing) would be very valuable. However, to make that deterministic, fast and fuzzer friendly it appears that extensive refactoring is required. There are three main blockers: module separation (net/net processing/validation split), determinism, performance.
I am gonna use this issue to track and motivate the work that needs to be done. It would be great if we can achieve the same outcome with less refactoring but I don't see
...
(https://github.com/bitcoin/bitcoin/issues/27502)
Efficient isolated fuzzing of our message processing code (net processing) would be very valuable. However, to make that deterministic, fast and fuzzer friendly it appears that extensive refactoring is required. There are three main blockers: module separation (net/net processing/validation split), determinism, performance.
I am gonna use this issue to track and motivate the work that needs to be done. It would be great if we can achieve the same outcome with less refactoring but I don't see
...