👍 ryanofsky approved a pull request: "depends: Build the `native_capnp` and `capnp` packages with CMake"
(https://github.com/bitcoin/bitcoin/pull/28856#pullrequestreview-1762769796)
Code review ACK 11d797e3a078b8f5f0039a1073047d3f0a8c6cdc. Since last review arm64-apple-darwin platform is now mentioned in the commit message, and the change to `depends/packages/libmultiprocess.mk` in d1604d4b1d1ee8df279a1776303e167cc3d06193 which was unrelated (but probably still a good optimization) was reverted.

👍 ryanofsky approved a pull request: "depends: fix libmultiprocess build on aarch64"
(https://github.com/bitcoin/bitcoin/pull/28846#pullrequestreview-1762817862)
Thanks for reopening, code review ACK 68823aa3c071b49754fe3396be49f5531aa33fd4 for just the third commit. This PR could be a draft since it is based on another PR.

💬 ryanofsky commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1414128811)
In commit "depends: always install libmultiprocess to /lib" (68823aa3c071b49754fe3396be49f5531aa33fd4)

I still think it would be nice to add some code comment here, maybe like the one I suggested earlier:

> \# Hardcode library install path to "lib" to match the PKG_CONFIG_PATH setting in depends/config.site.in which also hardcodes "lib". Without this setting, cmake by default would use the OS library directory, which might be "lib64" or something else, not "lib", on multiarch systems.

I
...

💬 fanquake commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1414135724)
Sure, pushed up your comment and added co-author.

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1414142645)
I actually did it in this way per achow request during coredev. My initial version involved returning the coin selection information in a separate struct https://github.com/furszy/bitcoin-core/commit/a1b2c53d1feddf748d7d91a884fe52e50998f61e in the tx creation result.

I do prefer my previous approach because it would allows us to retrieve this information at the RPC level as well. However, I am also fine going with this other option as well. It's not something widely used nowadays anyway. We c
...

👍 ryanofsky approved a pull request: "depends: fix libmultiprocess build on aarch64"
(https://github.com/bitcoin/bitcoin/pull/28846#pullrequestreview-1762850175)
Code review ACK 1a90ac42e2591b5105ac846a5f6d07ec4b95fa1a, just adding comment since last review (thanks!)

💬 dergoegge commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1414154588)
> My initial version involved returning the coin selection information in a separate struct https://github.com/furszy/bitcoin-core/commit/a1b2c53d1feddf748d7d91a884fe52e50998f61e in the tx creation result.

Your initial version seems better to me. Looks like about the same amount of code or less for the better outcome. Checking for log messages in tests is a bit brittle and usually results in more churn down the line when log messages change.

💬 furszy commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1414156191)
No, it is not used. IIRC, I did it in this way to get a mechanism to enable/disable the logs watcher on the fly, without requiring to create a new one per watched message.
Can migrate it to use a bool too. Was just considering potential future utilities.

💬 maflcko commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1414161933)
nit from tidy CI:


```
tainer_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/wallet/spend.cpp
wallet/spend.cpp:1266:28: error: Unterminated format string used with LogPrintf [bitcoin-unterminated-logprintf,-warnings-as-errors]
1266 | wallet.WalletLogPrintf("Coin Selection: Algorithm:%s, Waste:%d", GetAlgorithmName(result.GetAlgo()), result.GetWaste());
| ^
|
...

💬 dergoegge commented on pull request "Replace Boost.Process with cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/28981#issuecomment-1839024177)
Concept ACK

💬 dergoegge commented on issue "fuzz, brainstorm: Individual binaries per harness":
(https://github.com/bitcoin/bitcoin/issues/28971#issuecomment-1839044094)
> including a full re-compilation, which I think is similar to what you are asking for here? I guess you want the patch to be maintained inside this repo and not outside?

Yes but the patch I'm proposing is quite different, as it completely removes the map because I don't think simply search and replacing `std::getenv("FUZZ")` let's LTO prune the other targets code.

I'll code up a PR for this so it's more clear what I have in mind.

🤔 murchandamus reviewed a pull request: "wallet: skip BnB when SFFO is enabled"
(https://github.com/bitcoin/bitcoin/pull/28994#pullrequestreview-1762920527)
Approach ACK: 233a07ade88d13154771417cf7700736ffce5211

💬 murchandamus commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#discussion_r1414190927)
Nit: This test was already a bit weird to me, because it is categorized as belonging to "BnB", but the composition of preset inputs and the selection of additional inputs is a `SelectCoins`-level functionality that has little to do with specific coin selection algorithms. Composability should be tested at `SelectCoins` level (which it is in `spend_tests` and `wallet_fundrawtransaction.py`, although it could probably be more extensive). Since we already tested whether we can select two UTXOs that
...

💬 theuni commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1839051585)
> c0c2e7bb92c1fee0c4e9f3a485e4530786732d6c6dd9e9f418c282aa6892f55d Xcode-15.0-15A240d-extracted-SDK-with-libcxx-headers.tar.gz

Has been generated and is ready for use now by c-i.

💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1839054583)
> Has been generated and is ready for use now by c-i.

Force pushed for a CI run.

🤔 Sjors reviewed a pull request: "build: use macOS 14 SDK (Xcode 15.0)"
(https://github.com/bitcoin/bitcoin/pull/28622#pullrequestreview-1762543137)
Guix hashes:

```
fedbab3012043b4277a19fbd0a1abdb5d8e895446bba470981cef361064cdba6 guix-build-984cbe4f84a6/output/arm64-apple-darwin/SHA256SUMS.part
e40c88780560f4aa1e15657373cf6e6149878c90c1acb2980a6015c368378485 guix-build-984cbe4f84a6/output/arm64-apple-darwin/bitcoin-984cbe4f84a6-arm64-apple-darwin-unsigned.tar.gz
795221e68c86cb46684dcb80e729e6dd887402930bc8f911b97840f563c4fab8 guix-build-984cbe4f84a6/output/arm64-apple-darwin/bitcoin-984cbe4f84a6-arm64-apple-darwin-unsigned.zip
079
...

💬 Sjors commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#discussion_r1414183497)
This matches for me, as of 984cbe4f84a6eb32153f7fc46c220a0ccb2746f9

💬 theuni commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#discussion_r1414202624)
Does this obsolete/interact with mlinker-version at all?

🤔 theuni reviewed a pull request: "build: use macOS 14 SDK (Xcode 15.0)"
(https://github.com/bitcoin/bitcoin/pull/28622#pullrequestreview-1762939462)
Concept ACK. My generated tarball hash matches @fanquake's.

💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1839077604)
Guix build (aarch64):
```bash
fedbab3012043b4277a19fbd0a1abdb5d8e895446bba470981cef361064cdba6 guix-build-984cbe4f84a6/output/arm64-apple-darwin/SHA256SUMS.part
e40c88780560f4aa1e15657373cf6e6149878c90c1acb2980a6015c368378485 guix-build-984cbe4f84a6/output/arm64-apple-darwin/bitcoin-984cbe4f84a6-arm64-apple-darwin-unsigned.tar.gz
795221e68c86cb46684dcb80e729e6dd887402930bc8f911b97840f563c4fab8 guix-build-984cbe4f84a6/output/arm64-apple-darwin/bitcoin-984cbe4f84a6-arm64-apple-darwin-unsign
...