💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621793644)
Rebased 93ba80b3d78fa1c4e7f614df6517b9cb492d05d7 -> ab041937289f4939758c1896f5cb57f556ae0615 ([`pr/subtree.3`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.3) -> [`pr/subtree.4`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.3-rebase..pr/subtree.4)) after base pr was merged, also updated documentation as suggested, and fixed compatibility with gnu make 3.81 and bsd tar.
---
re: https://github.com/bit
...
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621793644)
Rebased 93ba80b3d78fa1c4e7f614df6517b9cb492d05d7 -> ab041937289f4939758c1896f5cb57f556ae0615 ([`pr/subtree.3`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.3) -> [`pr/subtree.4`](https://github.com/ryanofsky/bitcoin/commits/pr/subtree.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/subtree.3-rebase..pr/subtree.4)) after base pr was merged, also updated documentation as suggested, and fixed compatibility with gnu make 3.81 and bsd tar.
---
re: https://github.com/bit
...
💬 fanquake commented on issue "build: depends cros-compile using Clang fails":
(https://github.com/bitcoin/bitcoin/issues/31748#issuecomment-2621838436)
Not sure I follow. Depends was configured to cross-compile for aarch64-linux using Clang as the compiler. This has worked fine and produced aarch64 code and libraries. We then pass the toolchain file from depends to CMake (which should contain everything CMake needs to know produce aarch64 linux binaries using the just compiled aarch64 libraries). CMake has configured using Clang as the compiler, and think it's cross-compiling for `aarch64`, according to it's own output:
> Cross compiling ......
...
(https://github.com/bitcoin/bitcoin/issues/31748#issuecomment-2621838436)
Not sure I follow. Depends was configured to cross-compile for aarch64-linux using Clang as the compiler. This has worked fine and produced aarch64 code and libraries. We then pass the toolchain file from depends to CMake (which should contain everything CMake needs to know produce aarch64 linux binaries using the just compiled aarch64 libraries). CMake has configured using Clang as the compiler, and think it's cross-compiling for `aarch64`, according to it's own output:
> Cross compiling ......
...
👍 instagibbs approved a pull request: "test: fix intermittent timeout in p2p_1p1c_network.py"
(https://github.com/bitcoin/bitcoin/pull/31751#pullrequestreview-2581288524)
ACK 215d8b0a2ccf3a7275ea483339c58da5e0085e83
I suggested a comment change to leave more breadcrumbs for readers, but the fix seems correct
(https://github.com/bitcoin/bitcoin/pull/31751#pullrequestreview-2581288524)
ACK 215d8b0a2ccf3a7275ea483339c58da5e0085e83
I suggested a comment change to leave more breadcrumbs for readers, but the fix seems correct
💬 instagibbs commented on pull request "test: fix intermittent timeout in p2p_1p1c_network.py":
(https://github.com/bitcoin/bitcoin/pull/31751#discussion_r1934024853)
```suggestion
# Disconnect python peers to clear outstanding orphan requests with them, avoiding timeouts.
# We are only interested in the syncing behavior between real nodes.
```
(https://github.com/bitcoin/bitcoin/pull/31751#discussion_r1934024853)
```suggestion
# Disconnect python peers to clear outstanding orphan requests with them, avoiding timeouts.
# We are only interested in the syncing behavior between real nodes.
```
💬 theuni commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621866270)
> With some additional work implementing Cory's EXTERNAL_MPGEN option, add_subdirectory could be used instead of depends to build the library (not the code generator). This way cmake could be used to control libmultiprocess compile flags even in depends builds, if we want that.
I have this hacked up to work, though I'm still investigating some subdirectory leakage (ugh) that I don't quite understand. I pretty strongly prefer this approach as it means everyone's compiling in-tree. I'll try to
...
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621866270)
> With some additional work implementing Cory's EXTERNAL_MPGEN option, add_subdirectory could be used instead of depends to build the library (not the code generator). This way cmake could be used to control libmultiprocess compile flags even in depends builds, if we want that.
I have this hacked up to work, though I'm still investigating some subdirectory leakage (ugh) that I don't quite understand. I pretty strongly prefer this approach as it means everyone's compiling in-tree. I'll try to
...
💬 ryanofsky commented on issue "build: depends cross-compile using Clang fails":
(https://github.com/bitcoin/bitcoin/issues/31748#issuecomment-2621888989)
CC=clang CXX=clang++ specify paths to specific compiler binaries, and compiler binaries usually have a default platform that they target. On an x86_64 system they will usually produce x86_64 binaries unless passed additional flags which I don't think the depends system is using.
The host string aarch64-linux-gnu you are specifying will be used to find the right compiler binaries for that host by default, but if you are overriding the compiler, the compiler you specified will take precedence.
I
...
(https://github.com/bitcoin/bitcoin/issues/31748#issuecomment-2621888989)
CC=clang CXX=clang++ specify paths to specific compiler binaries, and compiler binaries usually have a default platform that they target. On an x86_64 system they will usually produce x86_64 binaries unless passed additional flags which I don't think the depends system is using.
The host string aarch64-linux-gnu you are specifying will be used to find the right compiler binaries for that host by default, but if you are overriding the compiler, the compiler you specified will take precedence.
I
...
💬 Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621909172)
> I feel like CI change should be a separate PR because there is already a lot to review and discuss here, but let me know if you think this would be important to include.
I'll be testing this with #30975 so we'll get some CI runs regardless.
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621909172)
> I feel like CI change should be a separate PR because there is already a lot to review and discuss here, but let me know if you think this would be important to include.
I'll be testing this with #30975 so we'll get some CI runs regardless.
🤔 Countrymom71 reviewed a pull request: "qa: Fix `wallet_multiwallet.py`"
(https://github.com/bitcoin/bitcoin/pull/31410#pullrequestreview-2581348466)
I need help on Cloudflare seeing my wallet. I have tried setting up all the domains and things to keep it secure and reroute things and until I get it right I can't see my account with crypto. I only want to use to to keep my crypto secure and send and receive. I have no interest in anything else on the computer I am old and don't understand all the computer lingo I just need some simple easy way to get to my account and use it pay my fee and that's it. The other stuff on here makes me want
...
(https://github.com/bitcoin/bitcoin/pull/31410#pullrequestreview-2581348466)
I need help on Cloudflare seeing my wallet. I have tried setting up all the domains and things to keep it secure and reroute things and until I get it right I can't see my account with crypto. I only want to use to to keep my crypto secure and send and receive. I have no interest in anything else on the computer I am old and don't understand all the computer lingo I just need some simple easy way to get to my account and use it pay my fee and that's it. The other stuff on here makes me want
...
💬 ryanofsky commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621916864)
re: https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621866270
@theuni can the change your are working on be a followup? It doesn't seem like this change will have any effect for developers unless they are using depends for development which I think few do normally. And even then the main effect is that the library is built with cmake flags instead of depends flags. The change would also increase complexity (would only increase not decrease changes in this PR) and would be easier t
...
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621916864)
re: https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621866270
@theuni can the change your are working on be a followup? It doesn't seem like this change will have any effect for developers unless they are using depends for development which I think few do normally. And even then the main effect is that the library is built with cmake flags instead of depends flags. The change would also increase complexity (would only increase not decrease changes in this PR) and would be easier t
...
💬 fanquake commented on issue "build: depends cross-compile using Clang fails":
(https://github.com/bitcoin/bitcoin/issues/31748#issuecomment-2621918625)
> You could also try CC=aarch64-linux-gnu-clang etc or whatever the aarch64 cross-compiler path is
You don't really need to do this with, clang, it is natively a cross-compiler. You can (generally) just pass `--target=whatever`, and generate code for that target, without needing a specially compiled `clang` binary. In this case, by specifying `HOST=aarch64-linux-gnu`, I end up with compiler invocations like `clang --target=aarch64-linux-gnu`, and everything works as expected in depends. The iss
...
(https://github.com/bitcoin/bitcoin/issues/31748#issuecomment-2621918625)
> You could also try CC=aarch64-linux-gnu-clang etc or whatever the aarch64 cross-compiler path is
You don't really need to do this with, clang, it is natively a cross-compiler. You can (generally) just pass `--target=whatever`, and generate code for that target, without needing a specially compiled `clang` binary. In this case, by specifying `HOST=aarch64-linux-gnu`, I end up with compiler invocations like `clang --target=aarch64-linux-gnu`, and everything works as expected in depends. The iss
...
💬 sipa commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934061466)
In commit "[fuzz] GetCandidatePeers"
This tests that all values returned by `GetCandidatePeers` are in the set of expected results, but not that nothing is missing from the `GetCandidatePeers` result (so if `GetCandidatePeers()` never modified `candidate_peers`, no issue would be detected).
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934061466)
In commit "[fuzz] GetCandidatePeers"
This tests that all values returned by `GetCandidatePeers` are in the set of expected results, but not that nothing is missing from the `GetCandidatePeers` result (so if `GetCandidatePeers()` never modified `candidate_peers`, no issue would be detected).
💬 sipa commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934078679)
In commit "[p2p] assign just 1 random announcer in AddChildrenToWorkSet"
How is this prevented? Just by virtue of picking a random one, so it's unlikely to be an attacker?
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934078679)
In commit "[p2p] assign just 1 random announcer in AddChildrenToWorkSet"
How is this prevented? Just by virtue of picking a random one, so it's unlikely to be an attacker?
💬 sipa commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934063157)
In commit "[refactor] helper function for adding orphan parents to txrequest".
µnit: period at the end of a sentence?
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934063157)
In commit "[refactor] helper function for adding orphan parents to txrequest".
µnit: period at the end of a sentence?
💬 ariard commented on pull request "Add NODE_TXRELAY_V2.":
(https://github.com/bitcoin/bitcoin/pull/30837#issuecomment-2621950357)
@glozow Thanks for the comment.
Have a look on the original PR on #21224, when all the existing tests at the time were fixed, before a real conversation on the conceptual trade-offs that one can evaluate at high-level was effectively done. Sorry, not sorry, it’s not rational to fix CI if it’s to waste time again, before there is _conceptual convergence_.
As a contributor yourself, before to be a maintainer, and I think someone able to conceptually review PRs yourself, if there are technica
...
(https://github.com/bitcoin/bitcoin/pull/30837#issuecomment-2621950357)
@glozow Thanks for the comment.
Have a look on the original PR on #21224, when all the existing tests at the time were fixed, before a real conversation on the conceptual trade-offs that one can evaluate at high-level was effectively done. Sorry, not sorry, it’s not rational to fix CI if it’s to waste time again, before there is _conceptual convergence_.
As a contributor yourself, before to be a maintainer, and I think someone able to conceptually review PRs yourself, if there are technica
...
💬 glozow commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934088717)
Indeed not really prevented, just that it isn't trivially gameable.
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934088717)
Indeed not really prevented, just that it isn't trivially gameable.
💬 sipa commented on pull request "Add NODE_TXRELAY_V2.":
(https://github.com/bitcoin/bitcoin/pull/30837#issuecomment-2621959885)
@ariard Asking for conceptual feedback is reasonable, but here I don't even see any concept being proposed, neither in the PR nor in the associated BIP draft. It is *just* introducing a new flag, without any specification regarding what should be changed in the new protocol, nor even any discussion about what problems it is trying to address.
(https://github.com/bitcoin/bitcoin/pull/30837#issuecomment-2621959885)
@ariard Asking for conceptual feedback is reasonable, but here I don't even see any concept being proposed, neither in the PR nor in the associated BIP draft. It is *just* introducing a new flag, without any specification regarding what should be changed in the new protocol, nor even any discussion about what problems it is trying to address.
💬 ryanofsky commented on issue "build: depends cross-compile using Clang fails":
(https://github.com/bitcoin/bitcoin/issues/31748#issuecomment-2621962267)
> > You could also try CC=aarch64-linux-gnu-clang etc or whatever the aarch64 cross-compiler path is
>
> You don't really need to do this with, clang, it is natively a cross-compiler.
Yes that is what I meant by "compiler binaries usually have a default platform that they target".
Maybe I misunderstood context for your post and thought you were reporting a bug and looking for a workaround, not just reporting a bug. I think I agree that dropping CC=clang or specifying CC=aarch64-linux-gnu-clan
...
(https://github.com/bitcoin/bitcoin/issues/31748#issuecomment-2621962267)
> > You could also try CC=aarch64-linux-gnu-clang etc or whatever the aarch64 cross-compiler path is
>
> You don't really need to do this with, clang, it is natively a cross-compiler.
Yes that is what I meant by "compiler binaries usually have a default platform that they target".
Maybe I misunderstood context for your post and thought you were reporting a bug and looking for a workaround, not just reporting a bug. I think I agree that dropping CC=clang or specifying CC=aarch64-linux-gnu-clan
...
💬 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
...
(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?
(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).
(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
(https://github.com/bitcoin/bitcoin/pull/31751#discussion_r1934137159)
done