💬 fanquake commented on pull request "guix: Use LTO to build releases":
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-1812161592)
> Any idea where in6addr_any is coming from? Seems fine, just curious.

I can't remember exactly, but it looks like this is actually no-longer required. Did a new round of builds with this branch, and it dropped from the script.

🤔 BrandonOdiwuor reviewed a pull request: "wallet: propagete `checkChainLimits` error message to wallet"
(https://github.com/bitcoin/bitcoin/pull/28863#pullrequestreview-1731695630)
Concept ACK using the error message from `checkChainLimits(...)`

💬 hebasto commented on pull request "build: Windows SSP roundup":
(https://github.com/bitcoin/bitcoin/pull/28461#issuecomment-1812181560)
My Guix builds:
```
x86_64
c446041204b8b099c0b3e91f34bb783d11f57077ec685a141a766a9c44b917e0 guix-build-f95af98128f1/output/dist-archive/bitcoin-f95af98128f1.tar.gz
f929fd24af143247ca32a747ec29a7982b808c87d2221a47d257760efa1a216c guix-build-f95af98128f1/output/x86_64-w64-mingw32/SHA256SUMS.part
4287bb5bdc1ef7253fec176cd18ad1459dcb49705cc6b925aea41694fd5e390a guix-build-f95af98128f1/output/x86_64-w64-mingw32/bitcoin-f95af98128f1-win64-debug.zip
6109618130b5e240739da023a94d1b93ed05e619cee0
...

💬 ajtowns commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1812212359)
> I was thinking it would be nice to retain decimal but, due to ambiguity that could arise with CLTV and CSV, I'm not sure.

What ambiguity?

📝 fanquake opened a pull request: "build: switch to using LLVM 17.x for macOS builds"
(https://github.com/bitcoin/bitcoin/pull/28880)
This is the last step before #21778. We need LLVM 17.x so that lld has `-fixup_chains`. I did want to just get everything merged in that PR, without a bump, but I've split this out because that PR currently has an issue with Qt and lld, and, as a side-effect, this PR also unblocks #28622 (C++20), so this is similar to #28732 (cherry-picked the commit), but with the required Guix change, now that LLVM 17 is available there (#28580). If this is merged it would also close #28851, which is the Qt w
...

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1394020665)
This happens before the addition. Your suggestion would mean that the size could be 3001, which is not desired.

💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1812255751)
> The PR description looks outdated after the recent push :)

Updated the description, and rebased on #28880 instead, now that we can do that.

💬 hebasto commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1812264645)
Concept ACK.

💬 ajtowns commented on pull request "Improve peformance of CTransaction::HasWitness (28107 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/28766#discussion_r1394031188)
Really, we're already wasting 16 bytes due to `vin` and `vout` supporting `capacity() != size()` despite both being `const` and never needing to be resized.

💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#discussion_r1394051068)
The SDK version in a Guix built bitcoind is still being set to 11.0:
```bash
Load command 11
cmd LC_BUILD_VERSION
cmdsize 32
platform 1
minos 11.0
sdk 11.0
ntools 1
tool 3
version 711.0
```

👍 hebasto approved a pull request: "build: Windows SSP roundup"
(https://github.com/bitcoin/bitcoin/pull/28461#pullrequestreview-1731808847)
ACK f95af98128f17002bf137a48441167020f3ef9bb, I've verified binaries from `bitcoin-f95af98128f1-win64.zip` on Windows 11 Pro 23H2.

However, the penultimate commit works just fine as well. The `diffoscope` show minor changes in binaries.

So do we actually need the last commit at all?

💬 fanquake commented on pull request "build: Windows SSP roundup":
(https://github.com/bitcoin/bitcoin/pull/28461#issuecomment-1812317916)
> So do we actually need the last commit at all?

The point is to explicitly enable the hardening option. This is the same as we do for the Linux GCC.

💬 hebasto commented on pull request "build: Patch Qt to handle minimum macOS version properly":
(https://github.com/bitcoin/bitcoin/pull/28851#issuecomment-1812341926)
Could be closed in favour of https://github.com/bitcoin/bitcoin/pull/28880.

💬 hebasto commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1812354204)
> this PR also unblocks #28622 (C++20)

... due to [this](https://github.com/llvm/llvm-project/commit/c8e2dd8c6f490b68e41fe663b44535a8a21dfeab) commit that fixes Qt package build.

💬 hebasto commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1812374637)
From Guix build logs it follows that Clang 15.0.7 is used. Not Clang 17?

💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#discussion_r1394092853)
> Looks like we'll have to add a -Wl,-platform_version into ldflags here.

Should be resolved now.

💬 fanquake commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1812389164)
> From Guix build logs it follows that Clang 15.0.7 is used. Not Clang 17?

I don't think so? If that was the case the Guix builds in #28622 would be failing to build Qt? They are currently working with the most recent push, and addition of `-platform_version`.

💬 hebasto commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1812417405)
> > From Guix build logs it follows that Clang 15.0.7 is used. Not Clang 17?
>
> I don't think so?

Right. There was a wrong build setup on my side. Sorry for the noise.

💬 hebasto commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1812432364)
My Guix builds:
```
x86_64
34e5ff49486a0777be8c0cba514a0de0e010573ada67c4e7fc0d5f46b4489ac2 guix-build-1a9d3eb54ba0/output/arm64-apple-darwin/SHA256SUMS.part
c0b4a1b61a9b65c1765fbf33b1c0ea53b1f6aeb5efd5e3a2051f193f34f345fd guix-build-1a9d3eb54ba0/output/arm64-apple-darwin/bitcoin-1a9d3eb54ba0-arm64-apple-darwin-unsigned.tar.gz
911b8fa0cdc61195e96c479b5173b333f9efa647cbc71ef499284905ef3e1e1b guix-build-1a9d3eb54ba0/output/arm64-apple-darwin/bitcoin-1a9d3eb54ba0-arm64-apple-darwin-unsigned
...

💬 hebasto commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1812434474)
Guix hashes for `arm64-apple-darwin` differs across the build platforms, unfortunately.