💬 maflcko commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2559477612)
> `[[unlikely]]`

Will this actually help in a large function without a loop, where a single if is annotated?

Generally, I have the impression that `[[likely]]`, and `[[unlikely]]` should not be used. Possibly in rare cases, where there is a really hot loop, and all release compilers agree that the attribute will help.

In other cases, if the attributes are used too liberal, it seems an accidental pessimization is plausible: It is unclear what the meaning of the attributes are, if nested scopes
...

🚀 fanquake merged a pull request: "ci: Use latest Xcode that the minimum macOS version allows"
(https://github.com/bitcoin/bitcoin/pull/33932)

💬 maflcko commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2559487359)
nit: Could use named args consistently, like below? `/*part_offset=*/0, /*part_size=*/std::nullopt);`

💬 l0rinc commented on pull request "[IBD] specialize CheckBlock's input & coinbase checks":
(https://github.com/bitcoin/bitcoin/pull/31682#issuecomment-3574992541)
Rebased after #33629, removed the conflicting [`1783fbe` (#31682)](https://github.com/bitcoin/bitcoin/pull/31682/commits/1783fbe04dd4938300dadf2553a9bb20accf2ccc) for simplicity

🚀 fanquake merged a pull request: "depends: sqlite 3.50.4; switch to autosetup"
(https://github.com/bitcoin/bitcoin/pull/32655)

💬 dergoegge commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2559558868)
I'm not going to add the steps back in. It's too much of a game of whack-a-mole which invites bike-shedding (what works for one person doesn't work for someone else, it depends on macOS version, brew version etc.) and it looks like people are perfectly capable of figuring out what works for them on their own.

💬 l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2559577489)
> it depends on macOS version, brew version etc

What if we documented a solution that would work with latest xcode and clang and whatever else is needed?
That shouldn't be too difficult and shouldn't involve "bike shedding" (I don't think it's the correct term here since it's not a "minor, easily understandable issues" at the expense of "neglecting more complex, important matters").

As mentioned, I strongly think we should have guidance for at least basic fuzzing support for Mac as well!
...

💬 l0rinc commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#issuecomment-3575153013)
Concept ACK, I agree that we should clarify usage of fuzzing on a Max.

However, approach NACK, as described in https://github.com/bitcoin/bitcoin/pull/33921#discussion_r2559577489 (which was marked as resolved for some reason, so reposting here for visibility)

💬 diegoviola commented on pull request "Revert "gui, qt: brintToFront workaround for Wayland"":
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3575174383)
> Instead of linking to a bunch of threads, it could make sense to just say that the issue has been fixed upstream in Qt6, so that the workaround is no longer needed?

The `Qt::WindowStaysOnTopHint` was never going to work on Wayland. Clients don't get to dictate whether they stay on top or not. The compositor does, and you (the user) are supposed to control the compositor through configuration. This is a fundamental difference that a lot of people don't seem to understand.

It seems like th
...

✅ maflcko closed an issue: "RFC: Replacing `tinyformat` with `{fmt}`"
(https://github.com/bitcoin/bitcoin/issues/33942)

💬 maflcko commented on issue "RFC: Replacing `tinyformat` with `{fmt}`":
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575193582)
> In addition to all of that, I think that will be a good step toward migrating to C++20 in the future, and we will be able to depend on the standard format library itself.

C++20 is already used, so I'll go ahead and close this for now.


I don't think there is value in switching `tinyformat` to `{fmt}` and then again to `std::format`, with all the churn.

If the switch is done, it should be straight to `std::format`.

So I'll close this for now, but the discussion can continue.

💬 hebasto commented on pull request "depends: Fix native_capnp to respect build_CC and build_CXX":
(https://github.com/bitcoin/bitcoin/pull/33937#issuecomment-3575194223)
> Fixes #33859

I’m not convinced it does.

On the master branch @ 0690514d4f72aac251ee0b876cded9187d42c63e:
```
$ gmake --no-print-directory -C depends print-native_capnp_cxx CXX=clang++
native_capnp_cxx=g++
```

With this PR, the behaviour is unchanged:
```
$ gmake --no-print-directory -C depends print-native_capnp_cxx CXX=clang++
native_capnp_cxx=g++
```

💬 diegoviola commented on pull request "Revert "gui, qt: brintToFront workaround for Wayland"":
(https://github.com/bitcoin-core/gui/pull/914#issuecomment-3575197345)
> Instead of linking to a bunch of threads, it could make sense to just say that the issue has been fixed upstream in Qt6, so that the workaround is no longer needed?

The `Qt::WindowStaysOnTopHint` workaround was never going to work on Wayland. Clients don't get to dictate whether they stay on top or not. The compositor does, and you (the user) are supposed to control the compositor through configuration. This is a fundamental difference that a lot of people don't seem to understand.

It se
...

💬 maflcko commented on issue "RFC: Replacing `tinyformat` with `{fmt}`":
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575205474)
For further context: The current minimum required GCC version is 12.1 (https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md#compiler), however `std::format` only comes with GCC libstdc++ 13.1

💬 maflcko commented on pull request "depends: Fix native_capnp to respect build_CC and build_CXX":
(https://github.com/bitcoin/bitcoin/pull/33937#issuecomment-3575210686)
@psam21 Is this LLM generated? Have you tested this yourself?

💬 hebasto commented on pull request "depends: Fix native_capnp to respect build_CC and build_CXX":
(https://github.com/bitcoin/bitcoin/pull/33937#issuecomment-3575216394)
The title says "depends: Fix native_capnp to respect build_CC and build_CXX", but it seems working as expected on the master:
```
$ gmake --no-print-directory -C depends print-native_capnp_cxx build_CXX=clang++
native_capnp_cxx=clang++
```

💬 fanquake commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2559669334)
Some of these are folders, but this will also work.

💬 fanquake commented on pull request "contrib: turn off compression of macOS SDK to fix determinism (across distros)":
(https://github.com/bitcoin/bitcoin/pull/32009#discussion_r2559669730)
Done.

💬 hulxv commented on issue "RFC: Replacing `tinyformat` with `{fmt}`":
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575220797)
> For further context: The current minimum required GCC version is 12.1 (https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md#compiler), however `std::format` only comes with GCC libstdc++ 13.1

For that, I thought about switching first to `{fmt}` and after that, we can switch to `std::format` easily when we bump the minimum required GCC version

💬 maflcko commented on issue "RFC: Replacing `tinyformat` with `{fmt}`":
(https://github.com/bitcoin/bitcoin/issues/33942#issuecomment-3575238806)
Yes, I understand, but there are no meaningful user-visible benefits:

* Performance does not matter, because formatting isn't done in hot loops
* A few KiB of binary size don't matter, because the binary is several MiB in size