💬 fanquake commented on pull request "ci: Temporarily disable bpfcc-tools":
(https://github.com/bitcoin/bitcoin/pull/29788#issuecomment-2039984016)
Pulled this into 27.x.

💬 TheCharlatan commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2039994022)
I am building depends on macos 12.7.4 and am not seeing any patch related errors or warning. Seems weird that this suddenly pops up now after having worked for a long time? I checked my `brew list` and don't have any entries for "patch" or "gpatch" and when I run `which patch`, it is in /usr/bin, so seems like it is just the default program?

💬 fanquake commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2039996663)
Yea. I'm also sure that my macOS 13.6.6 system is using the default apple patch (will double check), and I don't see any issues.

💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2040012852)
Rebased after #29419.

💬 maflcko commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2040023180)
Without knowing the cause, there is little that can be done. Given that WSL-built binaries didn't regress, it hints at some build flags inside of guix. My suggestion would be to bisect while doing guix builds. An alternative would be to try to match the guix compile flags and compiler version in WSL.

Also, instead of IBD, the benchmarks can be used, if they differ enough. Though, they'll need to be enabled:
https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2034314366

💬 maflcko commented on pull request "ci: Drop duplicated compiler flags":
(https://github.com/bitcoin/bitcoin/pull/29800#issuecomment-2040027759)
re-ACK a3485af67da4949c72c45acc608f8746ed0e0848

💬 fjahr commented on pull request "clang-tidy: Enable misc-no-recursion":
(https://github.com/bitcoin/bitcoin/pull/29690#discussion_r1553824794)
That framing makes a lot more sense to me 👍

💬 vostrnad commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2040045243)
@maflcko Just to clarify, what didn't regress was the release binary for Linux (which I assume is built with guix, not WSL) running in WSL.

💬 fanquake commented on issue "IBD performance regression in 27.0rc1 on Windows":
(https://github.com/bitcoin/bitcoin/issues/29785#issuecomment-2040086861)
> which I assume is built with guix

All of the release binaries are built using Guix. Windows is produced in Guix using GCC+Mingw-w64. We don't produce any release binaries using WSL or MSVC.

💬 fanquake commented on pull request "clang-tidy: Enable misc-no-recursion":
(https://github.com/bitcoin/bitcoin/pull/29690#issuecomment-2040102948)
Concept ACK. I think tracking where any recursion currently is, is useful, as well as being alerted to new usage in the future.

👍 fanquake approved a pull request: "refactor: Remove gmtime*"
(https://github.com/bitcoin/bitcoin/pull/29081#pullrequestreview-1983608799)
ACK fa9f36babaceba6ab2f88e64bc4bc2956f58871f - more std lib & even less stuff to port.

🚀 fanquake merged a pull request: "refactor: Remove gmtime*"
(https://github.com/bitcoin/bitcoin/pull/29081)

💬 ismaelsadeeq commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#issuecomment-2040171293)
> Any chance to turn the first commit into an scripted-diff?

The commit message was misleading It's not just renaming `max_weight`, it also updates the input parameter docstring to match match the new name.
And also the fuzz test renames has more info like `low_max_selection_weight` and `high_max_selection_weight`.

I've updated the commit message of the first commit https://github.com/bitcoin/bitcoin/pull/29523/commits/270639324a6637e0025e1458221ab488a8a09554

👍 fanquake approved a pull request: "ci: Drop duplicated compiler flags"
(https://github.com/bitcoin/bitcoin/pull/29800#pullrequestreview-1983625214)
ACK a3485af67da4949c72c45acc608f8746ed0e0848 - no-longer a change in behaviour.

💬 fanquake commented on pull request "[WIP] ci: test secp256k1 MSAN asm annotations":
(https://github.com/bitcoin/bitcoin/pull/29742#discussion_r1553941342)
Will be done via #29800.

💬 Sjors commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2040173278)
> Seems weird that this suddenly pops up now after having worked for a long time?

I agree. It's worked fine for me too on this machine back in the day, say 2017-2019. Replacing the built-in patch with GNU patch immediately fixed it, so it does seem _related_.

💬 fanquake commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2040178090)
> say 2017-2019.

So it stopped working in 2019, but you only reported it in 2024?

Given that there are 3 machines here, 2 where it always has worked, and continues to work, and one where it used to work, and now no-longer does, it would seem more likely that something has changed on your machine?

📝 theuni opened a pull request: "crypto: chacha20: always use our fallback timingsafe_bcmp rather than libc's"
(https://github.com/bitcoin/bitcoin/pull/29815)
Looking at libc sources, apple and openbsd implementations match our naive fallback. Only FreeBSD (and only x86_64) seems to [implement an optimized version](https://github.com/freebsd/freebsd-src/blob/main/lib/libc/amd64/string/timingsafe_bcmp.S).

It's not worth the hassle of using a platform-specific function for such little gain.

Additionally, as mentioned below, this is the only case outside of sha2 that requires an autoconf check, and I have upcoming PRs to remove the sha2 ones.



...

💬 Sjors commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2040180002)
> So it stopped working in 2019, but you only reported it in 2024?

No, I bought a faster machine in 2019 which I've been using for most compilation work. I may have run a depends build on the old machine since then, but I know when the last time was.

💬 theuni commented on pull request "crypto: chacha20: always use our fallback timingsafe_bcmp rather than libc's":
(https://github.com/bitcoin/bitcoin/pull/29815#issuecomment-2040180394)
Ping @sipa for a quick concept ACK.