Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2386241193)
> MSAN says https://cirrus-ci.com/task/6248232848719872

Thanks! I created https://github.com/chaincodelabs/libmultiprocess/issues/115 with details about this, and I think I have a potential fix.
💬 ryanofsky commented on pull request "refactor: move util/pcp and util/netif to common/":
(https://github.com/bitcoin/bitcoin/pull/31011#issuecomment-2386313836)
> utACK [fd38711](https://github.com/bitcoin/bitcoin/commit/fd38711217cafbd62e8abd22d2b43f85fede8cde)
>
> I suppose the alternative would be to move `netaddress.h` to the util library.

I think that might work, but I didn't look into it because I don't know if netaddress.h depends on other things in common that would require more dependencies to be moved. Also the pcp/netif files were added recently in #30043, while netaddress.h is older so I assume it is more likely netaddress is in the pl
...
💬 petertodd commented on pull request "Disable RBF Rule 2":
(https://github.com/bitcoin/bitcoin/pull/30964#issuecomment-2386377588)
@arik-so FWIW I actually fix this loophole in Libre Relay by prohibiting replacements that add unconfirmed inputs to have multiple conflicts: https://github.com/petertodd/bitcoin/commit/b2b867f458388d5177aec378f95a874917fd6442

Libre Relay needs to do that because it implements replace-by-fee-rate, allowing transactions to be replaced if the fee-rate is increased. This loophole allows fee-rates to be decreased, which would allow an infinite replacement cycle.

Transactions violating this new
...
💬 theuni commented on pull request "Don't zero-after-free `DataStream`: ~25% faster IBD on some configurations":
(https://github.com/bitcoin/bitcoin/pull/30987#issuecomment-2386390637)
@davidgumberg Thanks for the very detailed description!

I suspect the speedup here is going to be very dependent on the architecture/environment, but it's not clear to me exactly what variables would matter most.

Would you be interested in working up a benchmark that illustrates the difference? If we knew exactly where the speedup was coming from, we could make a more informed decision about what/where to optimize.
👍 dergoegge approved a pull request: "fuzz: fix bug in p2p_headers_presync harness"
(https://github.com/bitcoin/bitcoin/pull/30980#pullrequestreview-2340806208)
utACK a7498cc7e26b4b3de976e111de2bd2d79b056b31
💬 ryanofsky commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2386452790)
re: https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2385186175

Thanks for the review! Fixed the problem with `-debug=ipc` now. This was missing a call to parse `-debug` arguments.

Updated 2227afac0372358287fb879f3b0bd07fd653f3f8 -> 2f9c2f4c6dcf8c45514e99d1018301aba61940fc ([`pr/mine.11`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.11) -> [`pr/mine.12`](https://github.com/ryanofsky/bitcoin/commits/pr/mine.12), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/min
...
💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2386473736)
ACK 2f9c2f4c6dcf8c45514e99d1018301aba61940fc

Logging works now.
💬 Sjors commented on pull request "refactor: move util/pcp and util/netif to common/":
(https://github.com/bitcoin/bitcoin/pull/31011#issuecomment-2386481778)
> I think it util is a good place for things that are general purpose and can be broadly useful, and common is a better place for things that are hacky or more specific to the bitcoin core codebase.

The PCP code is not Bitcoin specific. Some things in `netaddress.h` are though, but tearing that file apart doesn't seem worth the effort for now.
💬 ryanofsky commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2386491429)
Could you try cherry-picking following commit to fix the MSAN failure? commit 7363a3eb8a1771b862febf3bd7257ad093887485 ([branch](https://github.com/ryanofsky/bitcoin/commits/pr/downdep))
📝 brunoerg opened a pull request: "net: fuzz: bypass network magic and checksum validation"
(https://github.com/bitcoin/bitcoin/pull/31012)
Fixes #30957

We currently have some instructions on the documentation about how to fuzz the Bitcoin Core P2P layer using Honggfuzz NetDriver. It includes a patch to be applied to bypass network magic and checksum validation, that is outdated. However, we can change the code to bypass them using `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION`. It also makes the `p2p_transport_serialization` fuzz target simpler since we do not need to assist the network magic and checksum creation anymore.
💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2386500231)
cc: @dergoegge
💬 fanquake commented on issue "win64-cross CI timeout after 2h ":
(https://github.com/bitcoin/bitcoin/issues/30969#issuecomment-2386510723)
Guess I'm now seeing something similar locally when running the CI (fc642c33ef28829eda0119a0fe39fd9bc4b84051). Can't see why `univalue_object_test` would take three and a half minutes to run (included other known long-running tests for reference):
```bash
Test project /ci_container_base/ci/scratch/build-x86_64-w64-mingw32
Start 1: util_test_runner

3/136 Test #5: noverify_tests ....................... Passed 86.08 sec
5/136 Test #1: util_test_runner ..................
...
💬 dergoegge commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2386523622)
Concept ACK
💬 theStack commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1783207317)
considering that (rolling) bloom filters always have a certain false-positive rate, I'm wondering if these assertions could unintentionally fail on long fuzzing runs where they aren't reset for many iterations? (didn't look at any concrete math yet tho, maybe the set of of possible packages we create/pick from the generated data is so small that the likelihood of this happening is negligible, and everything is fine)
💬 sipa commented on pull request "[28.x] backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2386545831)
utACK 5de225f5c145368f70cb5f870933bcf9df6b92c8
💬 dergoegge commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#discussion_r1783229736)
`CPPFLAGS` will have to include `-DFUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION` when building bitcoind for fuzzing with the honggfuzz netdriver.
🚀 achow101 merged a pull request: "[28.x] backports and finalize"
(https://github.com/bitcoin/bitcoin/pull/30959)
💬 glozow commented on pull request "[28.x] backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2386592294)
post merge ACK
📝 laanwj opened a pull request: "depends: For mingw cross compile use -gcc-posix to prevent library conflict"
(https://github.com/bitcoin/bitcoin/pull/31013)
CMake parses some paths from the spec of the C compiler, assuming it will be the linker, resulting in the link to end up with `-L/usr/lib/gcc/x86_64-w64-mingw32/12-win32` on debian bookworm if both `-win32` and `-posix` variants are installed, and `-win32` is the default alternative.

This results in the wrong C++ library being linked, missing std::threads::hardware_concurrency and other threading functions.

To fix this, use the `-posix` variant of gcc as well when available. This fixes a
...