Bitcoin Core Github
42 subscribers
124K links
Download Telegram
💬 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
💬 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
💬 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
...
💬 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
...
⚠️ 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
...
💬 hebasto commented on pull request "depends: reuse _config_opts for CMake options":
(https://github.com/bitcoin/bitcoin/pull/27496#issuecomment-1516598966)
My Guix builds:
```
45d96de0248ec8b604cc5c980f1abbc780862d497f195a795c6088087d646c73 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/SHA256SUMS.part
ebdd087cbb45f1fa337b717c0d7bd53c8b7d0bc78d4e9693de2430a1ab9e03d9 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu-debug.tar.gz
e09206e6a70068036ea52731e0f8122665525a132e85f2798662ff9d7644dd8b guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu.tar.gz
20e71234d2e90386
...
💬 dergoegge commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#issuecomment-1516600484)
Maybe add a short description for what `mapDeltas` is to the PR description under "motivation / background"? afaict `mapDeltas` also doesn't have any code documentation.
💬 ajtowns commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1516600820)
> If we want to allow for more configurable signet consensus rules, then I think we need to do it in a more robust way

I think the easy, mostly backwards compatible way of doing this would just be to have the signetchallenge contain the data directly, eg making it be the script `OP_RETURN PUSHDATA<params> PUSHDATA<actual challenge>`. Then you're still just passing around a hex "challenge" string that completely defines the signet consensus behaviour, and will get different magic when there a
...
👍 MarcoFalke approved a pull request: "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1394327087)
lgtm ACK. Left two nits.
💬 MarcoFalke commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172806194)
clang-format new code (only if you retouch)
💬 MarcoFalke commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172814908)
```suggestion
result.try_emplace(txid, delta, in_mempool);
```

nit: `try_emplace` should avoid duplicate and unneeded (move) constructor calls? Also, it is shorter.