💬 Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621965610)
I'm now able to build depends with `make MULTIPROCES=1` on macOS without using the gnu versions.

However when using the result cmake complains about OpenSSL missing:

```
% cmake -B build --toolchain /Users/sjors/dev/bitcoin/depends/aarch64-apple-darwin24.3.0/toolchain.cmake
-- The CXX compiler identification is AppleClang 16.0.0.16000026
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Dev
...

💬 sipa commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934107086)
Should there perhaps be a preference for outbound peers here then, like in tx requesting?

💬 hebasto commented on issue "multiprocess: build failure on Alpine with depends & `DEBUG=1`":
(https://github.com/bitcoin/bitcoin/issues/31455#issuecomment-2621983844)
> Does configuring with `-DCMAKE_BUILD_TYPE=Debug`, which pulls debug-specific macros, fix the issue?

It seems to [work](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/13023723653/job/36329211464).

💬 mzumsande commented on pull request "test: fix intermittent timeout in p2p_1p1c_network.py":
(https://github.com/bitcoin/bitcoin/pull/31751#discussion_r1934137159)
done

👍 instagibbs approved a pull request: "test: fix intermittent timeout in p2p_1p1c_network.py"
(https://github.com/bitcoin/bitcoin/pull/31751#pullrequestreview-2581480055)
reACK 152a2dcdefa6ec744db5b106d5c0a8c5b8aca416

💬 fjahr commented on pull request "rpc: have getblocktemplate mintime account for timewarp":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2622034950)
I didn't have time to write yesterday but I've been leaning towards having `mintime` honor the timewarp rule since it's more intuitive and it's inconsistent to say we want to prevent potential bugs but also maybe not all the bugs.

Did we have any blocks on mainnet in the last couple of years that would have violated the new `mintime`? If not, that should be a stronger argument that the `mintime` change is relatively safe. And if there are some maybe a compromise would be a deprecation cycle f
...

🤔 sipa reviewed a pull request: "Embed default ASMap as binary dump header file"
(https://github.com/bitcoin/bitcoin/pull/28792#pullrequestreview-2581421447)
Concept ACK.

I'd recommend integrating the last commit into the rest of the PR, making the `std::vector<bool>` -> `std::span<std::byte>` for the interpreter change first, and then introducing the embedded asmap data afterwards.

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r1934116502)
This could be a *lot* smaller by using the `""_hex` user-defined literal (if you include `util/strencodings.h`); it'd create an `std::array<std::byte, N>` instead of a C-array, but the `std::span` constructor below just work just as well on it.

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r1934103723)
This seems to be the prefix it's trying to match itself, not code for matching it.

I suggest: `static const auto = "fb03ec0fb03fc0fe00fb03ec0fb03fc0fe00fb03ec0fb0fffffeff"_hex_v;`

💬 sipa commented on pull request "Embed default ASMap as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r1934122382)
This new `gen_header` command doesn't seem to be used by the build system; what is its point?

💬 ariard commented on pull request "Add NODE_TXRELAY_V2.":
(https://github.com/bitcoin/bitcoin/pull/30837#issuecomment-2622044669)
@sipa Yeah now it’s post-embargo, I’ll update the 2 bips with more context, fix the code at minima for the 2 PRs, and prepare a list of questions on the trade-offs or comparison of different approaches, and then re-open a PR or ask on the ML for conceptual feedback. Should do it in the coming weeks.

🤔 marcofleon reviewed a pull request: "func test: Expand tx download preference tests"
(https://github.com/bitcoin/bitcoin/pull/31437#pullrequestreview-2581509816)
lgtm ACK 846a1387280fa584f70ccb1f5d0198339b065528

I commented out the delay addition for a non-preferred peer in `AddTxAnnouncement` and the inbound connection check in `test_preferred_inv` failed as expected.

Also did a simple change making all peers preferred when calling `ReceivedInv`. Removing that distinction causes `test_preferred_tiebreaker_inv` to fail (most of the time) because there's only a 1/11 chance `pref_peer` gets selected first.

💬 ariard commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2622053406)
Working on a re-write of the 2 bips for this PR + #30837 and updating PRs now more context on the rational of the change is available: https://bitcoinops.org/en/newsletters/2024/12/06/

I should give an update in the coming weeks.

💬 hebasto commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r1934168830)
> I guess the check could be limited to exclude the msvc build?

Unfortunately, I can't find a simple way to detect whether binaries were built with MSVC without adding extra complexity to the Python code.

🤔 BrandonOdiwuor reviewed a pull request: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741#pullrequestreview-2581532869)
Concept ACK

💬 instagibbs commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934173722)
The attack scenario here would be:
1) honest node gives you orphan_A
2) adversary (sybil) announce orphan_A
3) someone (could be adversary) gives parent_A, enters into mempool
4) node adds to adversary's workset
5) adversary disconnects
6) we don't re-evaluate the orphan

without this change, we may be doing the re-evaluation work up to num_inbound times erroneously?

💬 hebasto commented on pull request "cmake: Fix `-pthread` flags in summary":
(https://github.com/bitcoin/bitcoin/pull/31724#discussion_r1934186962)
> if an upstream change in CMake ever sets different `COMPILE_OPTIONS` for `Threads::Threads` (probably unlikely)...

I would argue the opposite. It’s possible that more cases could be added where the `-pthread` flag should be filtered out using additional generator expressions. The last [modification](https://gitlab.kitware.com/cmake/cmake/-/commit/d7963aa9ee38bf26ab31433f2e7bfaff7ddf6c57) was made two years ago.

💬 hebasto commented on pull request "ci: Allow build dir on CI host":
(https://github.com/bitcoin/bitcoin/pull/31428#issuecomment-2622101449)
> Should be RFM with two acks?

💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2622111651)
> However when using the result cmake complains about OpenSSL missing:

Openssh issue you are seeing is puzzling because I don't know what change here could cause it, and I wonder if you see the same error without this PR? When you are building with depends capnproto is built without ssl support should should not be looking for ssl at all.

If you are not seeing this issue happen before this PR, but are seeing this issue happen after this PR, I think it would probably most likely be the chan
...

💬 vasild commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-2622122439)
> Something else I see using Sockman is in the `mempool_limit` functional test, we call `getrawtransaction` and expect around 540,000 bytes back. The call to `send()` returns the total amount of bytes immediately ...

Hmm, what I observed is that the send call sends less bytes than requested.

_(Lets move the discussion away from the main thread of this PR into the below link)_

Some a more elaborate explanation plus patch that fixes the problem in https://github.com/pinheadmz/bitcoin/comm
...