💬 hodlinator commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1751902511)
nit: Possibly slightly more optimal to encourage compile time length calculation for the string literal on the right. But maybe compilers already do the `const char[]` -> `string_view` conversion for the literal on the right at compile time to later match `string_view::operator==` at runtime.
```suggestion
auto check_mix{[](const ErrType& str) { return str == std::string_view{"Format specifiers must be all positional or all non-positional!"}; }};
```

👍 ryanofsky approved a pull request: "build: Improve `ccache` performance for different build directories"
(https://github.com/bitcoin/bitcoin/pull/30861#pullrequestreview-2292625730)
Code review ACK 66b73bbf375ff96bf4a77539c9cf3fe52fbc15d5. This seems like a really helpful improvement, but did have some questions about it below.

💬 ryanofsky commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752076990)
In commit "build: Improve `ccache` performance for different build directories" (66b73bbf375ff96bf4a77539c9cf3fe52fbc15d5)

Reading https://ccache.dev/manual/latest.html#_configuration_options it seems like it would make sense in most cases to set base_dir to CMAKE_SOURCE_DIR not CMAKE_BINARY_DIR.

For example if CMAKE_SOURCE_DIR is /home/bitcoin and CMAKE_BINARY_DIR is /home/bitcoin/build, it sounds like ccmake will *not* rewrite the include path `-I/home/bitcoin/src` to `-I../src` for grea
...

💬 ryanofsky commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752039301)
In commit "build: Propagate `-fdebug-prefix-map` flags to secp256k1 subtree" (b9fb3d1d99ae0b178b31c654b03a3b4f4a449fe7)

Is there some reason why only secp256k1 is specified here, not other subprojects like leveldb and crc32c? Would be good to expand comment to say whether secp256k1, or if this also makes sense to apply other projects.

💬 ryanofsky commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752055586)
In commit "build: Improve `ccache` performance for different build directories" (66b73bbf375ff96bf4a77539c9cf3fe52fbc15d5)

Would be good to add a comment explaining `if(CMAKE_GENERATOR MATCHES "Ninja|Makefiles")` saying that COMPILER_LAUNCHER variable only works for these backends not other backends. Otherwise the check seems completely arbitrary and doesn't make sense.

💬 mzumsande commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2340998016)
I managed to get my windows installation compiled (Windows 10 Pro 22H2), didn't observe a notable difference in the benchmarks.
I'm still in the process of trying to test the sync speed, but if it's due to the undo data, then a reindex-chainstate should be sufficient to see the difference?! Did anyone try that already?

💬 hebasto commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752106537)
For now, only the `secp256k1` subtree uses its own build system. All other subtrees' targets consume the `core_interface`, which propagates these flags to them.

💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1752114277)
Thanks @maflcko.
@dergoegge, would a `CallOneOf` based solution satisfy your needs?

💬 hebasto commented on issue "build: reproducibility issue with macOS Guix builds":
(https://github.com/bitcoin/bitcoin/issues/30815#issuecomment-2341060674)
> Any idea why this happens?
>
> Is it something else from the context outside GUIX that leaks in? Umasks? File system differences?

I can confirm that the problem is caused by different `umask` values for the Linux users running the `./contrib/guix/guix-build` script`.

> We can likely work around this at the cmake level but ideally the guix build shell would prevent this from happening in the first place.
>
> Edit: oh i think i see, guix mounts the git directory into its build enviro
...

💬 ryanofsky commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752126150)
re: https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752106537

> For now, only the `secp256k1` subtree uses its own build system. All other subtrees' targets consume the `core_interface`, which propagates these flags to them.

Thanks! Might be good to add "because secp256k1, unlike other subprojects, does not use core_interface"

💬 fanquake commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752127638)
I don't think "which apply to both C++ and C, " is relevant here. At least, mentioning C++ isn't relevant for secp256k1.

👋 hebasto's pull request is ready for review: "build: Use CMake's default permissions in macOS `deploy` target"
(https://github.com/bitcoin/bitcoin/pull/30838)

👍 ryanofsky approved a pull request: "[RFC] Switch and/or align debugging flags (back) to `-Og`"
(https://github.com/bitcoin/bitcoin/pull/29796#pullrequestreview-2292765754)
Code review ACK a4350695d9b5551a7720766a8af99c4ad5667e23. I don't know all of the tradeoffs between flags, but this change makes sense and seems to do what is described.

I did notice this is changing darwin, linux, and windows flags from O1 to Og but not freebsd/netbsd/openbsd flags so was curious if that was intentional.

💬 fanquake commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1752135299)
In 21bfce58c1cad31801e045c606bd8fb16a00e11d: Just generally, I'd say it's a bit less ideal, having secp256k1 flags/config getting split up into multiple (arbitrary) places. Especially if in future, we'll have to duplicate all this logic in all places, for all subtrees (when using their own cmake build systems).

💬 fanquake commented on pull request "[RFC] Switch and/or align debugging flags (back) to `-Og`":
(https://github.com/bitcoin/bitcoin/pull/29796#issuecomment-2341105921)
> I did notice this is changing darwin, linux, and windows flags from O1 to Og but not freebsd/netbsd/openbsd flags so was curious if that was intentional.

It was not. Have fixed that now. Will take this out of draft.

👋 fanquake's pull request is ready for review: "[RFC] Switch and/or align debugging flags (back) to `-Og`"
(https://github.com/bitcoin/bitcoin/pull/29796)

💬 brunoerg commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1752157540)
Nevermind, I just force-pushed a better solution. No need to shuffle anymore.

💬 brunoerg commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#issuecomment-2341166441)
Force-pushed with a small improvement on fuzz target (https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1752007850)

💬 hebasto commented on issue "28.0rc1 synchronizes much slower on Windows":
(https://github.com/bitcoin/bitcoin/issues/30833#issuecomment-2341174848)
> I managed to get bitcoind compiled on my windows installation (Windows 10 Pro 22H2, MSVC 2022), didn't observe a notable difference in the benchmarks.

I hope everyone measuring performance on Windows has disabled all antivirus software, at least for the data directory. Right?

💬 fjahr commented on issue "28.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/30854#issuecomment-2341197816)
How about adding an instruction for testing assumeutxo mainnet parameters? Unless I am misremembering I think we left assumeutxo out of the testing guide previously because it was not available on mainnet so it would be good to include it now. For some people it may be taking too long or be too resource intensive so I would suggest to put it at the end as a bonus or something like that.

Here is a high-level description of what I think of for a full assumeutxo test: https://github.com/bitcoin/
...