💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2593602568)
> Proposed alternative to this PR: https://github.com/theuni/bitcoin/commits/cmake-trycompile-exe/
> What do you think?

Thank you. It Looks great!

I also pushed an updated branch, which has a cleaner resulting code.

However, I'm happy to switch to your branch, if you prefer it.

📝 hebasto converted_to_draft a pull request: "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally"
(https://github.com/bitcoin/bitcoin/pull/31662)
This was requested in https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2515287092.

From https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2511246212:
> (Almost?) every CMake check internally uses the [`try_compile()`](https://cmake.org/cmake/help/latest/command/try_compile.html) command, whose behaviour, in turn, depends on the [`CMAKE_TRY_COMPILE_TARGET_TYPE`](https://cmake.org/cmake/help/latest/variable/CMAKE_TRY_COMPILE_TARGET_TYPE.html) variable:
>
> 1. The defau
...

👋 hebasto's pull request is ready for review: "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally"
(https://github.com/bitcoin/bitcoin/pull/31662)

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2593680026)
A typo has been fixed (

⚠️ darosior opened an issue: "Enable PCP by default?"
(https://github.com/bitcoin/bitcoin/issues/31663)
Centralize discussion around turning the `natpmp` setting to on by default.

UPnP used to be turned on by default. It was turned off by default following numerous vulnerabilities found in miniupnpc. We recently got rid of the miniupnpc dependency by dropping the UPnP feature (https://github.com/bitcoin/bitcoin/pull/31130). In addition we implemented PCP with NAT-PMP fallback in house (https://github.com/bitcoin/bitcoin/pull/30043), safer protocols enabling the same NAT traversal feature to end u
...

💬 fjahr commented on pull request "test: avoid internet traffic":
(https://github.com/bitcoin/bitcoin/pull/31646#issuecomment-2593757745)
Post-merge tACK 2ed161c5ce648cb66ec3d2941b02d68b6ca4c390

💬 darosior commented on issue "Enable PCP by default?":
(https://github.com/bitcoin/bitcoin/issues/31663#issuecomment-2593761021)
To be clear i'm not suggesting we turn this option on by default in 29, this merely opens the discussion more formally than yesterday's chat on IRC. And i started writing this mainly to centralize results of tests against various routers.

💬 theuni commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1917235927)
Could you add a `LINK_LIBRARIES` option to `check_cxx_source_compiles_with_flags` and use that here instead?

💬 theuni commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1917234974)
Could you add an LDFLAGS param to the function as `LINK_OPTIONS` and use that here instead?

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1917262536)
`LDFLAGS` might make more sense because, for example, in the OSS-Fuzz environment, the `SANITIZER_LDFLAGS` variable can be set to `-fsanitize=fuzzer` or `/usr/lib/libFuzzingEngine.a`.

💬 davidgumberg commented on pull request "ci: Supply `--platform` argument to `docker build`.":
(https://github.com/bitcoin/bitcoin/pull/31657#discussion_r1917272201)
> Does this env var work for podman as well?

The "podman" cli frontend doesn't use this environment variable[^1], but I tested locally (on Fedora 40) that this PR works for the build phase when using docker cli to interact with podman container host:

```console
$ sudo dnf install podman-machine
$ podman machine init
$ podman machine start
Starting machine "podman-machine-default"
API forwarding listening on: /run/user/1000/podman/podman-machine-default-api.sock
$ env -i HOME="$HOME"
...

💬 brunoerg commented on pull request "descriptor: check whitespace in pubkeys within fragments":
(https://github.com/bitcoin/bitcoin/pull/31603#issuecomment-2593855664)
Force-pushed simplifying the approach. Now it just checks the whitespace in the `ParsePubkeyInner` function.

💬 pablomartin4btc commented on pull request "Bugfix on TransactionsView - Disable if privacy mode is set during wallet selection":
(https://github.com/bitcoin-core/gui/pull/815#issuecomment-2593858698)
Updates:
- Rebased (fixed previous CI failure - _ASan + LSan + UBSan..._ ).

💬 ryanofsky commented on pull request "wallet: Translate [default wallet] string in progress messages":
(https://github.com/bitcoin/bitcoin/pull/31296#issuecomment-2593868257)
Rebased ebb77ab77df4081f27ce55e8b28b0d35990caac8 -> a46ec1dece8d8a1b16ff1fc3d2932bca130bf67f ([`pr/dtran.2`](https://github.com/ryanofsky/bitcoin/commits/pr/dtran.2) -> [`pr/dtran.3`](https://github.com/ryanofsky/bitcoin/commits/pr/dtran.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/dtran.2-rebase..pr/dtran.3)) due to conflict with #31061

💬 pablomartin4btc commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-2593886226)
Updates:
- Rebased (fixed previous CI failure - _previous releases, depends DEBUG..._ ).

👍 ryanofsky approved a pull request: "kernel: Move kernel-related cache constants to kernel cache"
(https://github.com/bitcoin/bitcoin/pull/31483#pullrequestreview-2553888604)
Code review ACK 2a92702bafca5c78b270a9502a22cb9deac02cfc. Changes since last review are fixing size options to use size_t instead of int64_t again, simplifying CheckedLeftShift more, and making other minor suggested cleanups

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2593891189)
@theuni

Thank you for your review! Your comments have been addressed.

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1917307456)
Thanks! [Done](https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2593891189).

💬 hebasto commented on pull request "cmake: Do not modify `CMAKE_TRY_COMPILE_TARGET_TYPE` globally":
(https://github.com/bitcoin/bitcoin/pull/31662#discussion_r1917309058)
The `LDFLAGS` option has been [added](https://github.com/bitcoin/bitcoin/pull/31662#issuecomment-2593891189).

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2593925274)
Thanks for the suggestions @laanwj,

Rebased f157b0cbc7d90075858a6522d13a7bc4f0b25a5f -> f25616bec485ee6a70e4b797758d4987d25a7c25 ([kernelApi_14](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_14) -> [kernelApi_15](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_15), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_14..kernelApi_15))

* Fixed conflict with #31061

Updated f25616bec485ee6a70e4b797758d4987d25a7c25 -> 4dde75858a3b08f84d71176c7be14bae62020b1f
...