💬 TheCharlatan commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621465199)
As an alternative to reaching into depends or committing generated files, could it be feasible to set native compilers in the toolchain file instead for compiling the code generators?
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621465199)
As an alternative to reaching into depends or committing generated files, could it be feasible to set native compilers in the toolchain file instead for compiling the code generators?
🚀 fanquake merged a pull request: "depends: Update libmultiprocess library before converting to subtree"
(https://github.com/bitcoin/bitcoin/pull/31740)
(https://github.com/bitcoin/bitcoin/pull/31740)
🤔 glozow reviewed a pull request: "test: fix intermittent timeout in p2p_1p1c_network.py"
(https://github.com/bitcoin/bitcoin/pull/31751#pullrequestreview-2580850613)
utACK 215d8b0a2ccf3a7275ea483339c58da5e0085e83, the explanation makes sense to me and agree the p2ps don't need to be connected after this point.
(https://github.com/bitcoin/bitcoin/pull/31751#pullrequestreview-2580850613)
utACK 215d8b0a2ccf3a7275ea483339c58da5e0085e83, the explanation makes sense to me and agree the p2ps don't need to be connected after this point.
💬 hebasto commented on issue "build: `cmake --install` fails if only select targets are built":
(https://github.com/bitcoin/bitcoin/issues/31745#issuecomment-2621498472)
> I think what you're really looking for is a way to only build/install certain binaries. For that, the workaround would be to name each one as a separate component. That would allow for:
>
> cmake -B build
> cmake --build build --target bitcoind bitcoin-cli
> cmake --install build --component bitcoind bitcoin-cli
I agree. We have already [done](https://github.com/bitcoin/bitcoin/pull/30835) it for the `bitcoinkernel` target.
On the other hand, the [`CMAKE_SKIP_INSTALL_ALL_DEPENDENCY`](https:
...
(https://github.com/bitcoin/bitcoin/issues/31745#issuecomment-2621498472)
> I think what you're really looking for is a way to only build/install certain binaries. For that, the workaround would be to name each one as a separate component. That would allow for:
>
> cmake -B build
> cmake --build build --target bitcoind bitcoin-cli
> cmake --install build --component bitcoind bitcoin-cli
I agree. We have already [done](https://github.com/bitcoin/bitcoin/pull/30835) it for the `bitcoinkernel` target.
On the other hand, the [`CMAKE_SKIP_INSTALL_ALL_DEPENDENCY`](https:
...
🤔 hebasto reviewed a pull request: "doc: update links in ci.yml"
(https://github.com/bitcoin/bitcoin/pull/31736#pullrequestreview-2580887381)
Post-merge ACK 1681c08d42c302088586ca9b36b278217723f66a.
(https://github.com/bitcoin/bitcoin/pull/31736#pullrequestreview-2580887381)
Post-merge ACK 1681c08d42c302088586ca9b36b278217723f66a.
💬 hebasto commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621519226)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2621519226)
Concept ACK.
💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2621698276)
Rebased after the test change in #31740.
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2621698276)
Rebased after the test change in #31740.
👋 ryanofsky's pull request is ready for review: "multiprocess: Add libmultiprocess git subtree"
(https://github.com/bitcoin/bitcoin/pull/31741)
(https://github.com/bitcoin/bitcoin/pull/31741)
💬 hodlinator commented on pull request "miniscript: convert non-critical asserts to CHECK_NONFATAL":
(https://github.com/bitcoin/bitcoin/pull/31727#issuecomment-2621725687)
> Same reason as [here](https://github.com/bitcoin/bitcoin/pull/31727#discussion_r1929121645). I kept asserts where breaking the invariant would lead to UB. In the examples you point out it would read out of the `sats` vector bounds.
Okay, and there is no way *user input* from RPCs etc would currently be able to trigger the asserts?
> Actually this should be a strict check, not an inferior or equal to!
Ah, didn't realize, good catch indeed, `=` should be dropped.
(https://github.com/bitcoin/bitcoin/pull/31727#issuecomment-2621725687)
> Same reason as [here](https://github.com/bitcoin/bitcoin/pull/31727#discussion_r1929121645). I kept asserts where breaking the invariant would lead to UB. In the examples you point out it would read out of the `sats` vector bounds.
Okay, and there is no way *user input* from RPCs etc would currently be able to trigger the asserts?
> Actually this should be a strict check, not an inferior or equal to!
Ah, didn't realize, good catch indeed, `=` should be dropped.
💬 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).