💬 Sjors commented on pull request "doc: add testnet4 section header for config file":
(https://github.com/bitcoin/bitcoin/pull/31007#issuecomment-2386113770)
ACK 61cdb1c9d83778b95f4f9596f34617b7a191d0a5

💬 hodlinator commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1782965796)
Exactly like that, thanks!

💬 hodlinator commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1782975091)
Ah, yeah, `mempool_util.py` looks like a better alternative than `blocktools.py`.

`test_node.py` feels more like the high-level P2P stuff to me at least + lower level dynamic RPC invocation.

💬 Sjors commented on pull request "refactor: move util/pcp and util/netif to common/":
(https://github.com/bitcoin/bitcoin/pull/31011#issuecomment-2386200566)
utACK fd38711217cafbd62e8abd22d2b43f85fede8cde

I suppose the alternative would be to move `netaddress.h` to the util library.

The `libraries.md` document is a bit vague on what belongs to `util` vs `common`. It just says "low level". Since util is also used by `libbitcoin_kernel` it could make sense to move things out of it that the kernel doesn't need, but #28690 introduces a whole new `kernel_util` for that purpose.

💬 hebasto commented on pull request "build, qt: Add Wayland support for Linux builds with depends":
(https://github.com/bitcoin/bitcoin/pull/22708#issuecomment-2386230968)
This PR might be reconsidered along with https://github.com/bitcoin/bitcoin/issues/29914.

👍 theuni approved a pull request: "depends: Print ready-to-use `--toolchain` option for CMake invocation"
(https://github.com/bitcoin/bitcoin/pull/31008#pullrequestreview-2340626823)
ACK 605926da0ab4ac7ae4e9aed55a059f37c31c15b5

💬 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)