🤔 hebasto reviewed a pull request: "build: Bump clang minimum supported version to 14"
(https://github.com/bitcoin/bitcoin/pull/29208#pullrequestreview-1819951236)
Post-merge ACK aaaace2fd1299939c755c281b787df0bbf1747a0.

🤔 hebasto reviewed a pull request: "ci: Rename tasks (previous releases, macOS cross)"
(https://github.com/bitcoin/bitcoin/pull/29218#pullrequestreview-1819952922)
Post-merge ACK fa0c594b33970e12d97e6879ab4ca57045453492.

@maflcko

Thank you for addressing https://github.com/bitcoin/bitcoin/pull/29203#discussion_r1446339387.

💬 hebasto commented on pull request "build: Drop `ALLOW_HOST_PACKAGES` support in depends":
(https://github.com/bitcoin/bitcoin/pull/29203#discussion_r1451423102)
Thanks for https://github.com/bitcoin/bitcoin/pull/29218 :)

💬 hebasto commented on pull request "build: Pass sanitize flags to instrument `libsecp256k1` code":
(https://github.com/bitcoin/bitcoin/pull/28875#issuecomment-1890400163)
> It might be better to reverse the order of the commits or squash them to avoid bisect failures in between.

Squashed.

👍 hebasto approved a pull request: "depends: Allow PATH with spaces in directory names."
(https://github.com/bitcoin/bitcoin/pull/29237#pullrequestreview-1819967328)
ACK 4756114e505cff8848fb6344ef9a48d8822066c1, successfully built depends on Ubuntu 22.04.

💬 hebasto commented on pull request "Fix typos":
(https://github.com/bitcoin-core/gui/pull/787#issuecomment-1890409751)
These changes are not GUI-specific. Therefore, they should be submitted to the main repository, i.e., https://github.com/bitcoin/bitcoin.

✅ hebasto closed a pull request: "Fix typos"
(https://github.com/bitcoin-core/gui/pull/787)

💬 TheCharlatan commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#issuecomment-1890414477)
Guix builds:
```
73062e70b4ec1fa814c0c25f12cbbcfbabfd3a5e82827a3bfeb2f29b30d0b84f guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/SHA256SUMS.part
0965ed91f84129a64b6e0873b5feb182d83cae0dac6d89a51667d5fbe46fc68c guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu-debug.tar.gz
87ee5bc2103fc10b8e7acbfa776430918538f05fccf024b624f224f73987f745 guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu.tar.gz
c9bad44733b9f9a90d2
...

💬 hebasto commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#issuecomment-1890414655)
Concept ACK.

👍 TheCharlatan approved a pull request: "depends: Allow PATH with spaces in directory names."
(https://github.com/bitcoin/bitcoin/pull/29237#pullrequestreview-1819985102)
ACK 4756114e505cff8848fb6344ef9a48d8822066c1

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1451496426)
> nit: I think constants are not supposed to be prefixed with g_

https://github.com/bitcoin/bitcoin/blob/3ba8de1b704d590fa4e1975620bd21d830d11666/doc/developer-notes.md?plain=1#L87-L93

> I think you could also just use the genesis block (Params().GenesisBlock()) for these two values.

Done.

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1451496443)
Done.

👍 hebasto approved a pull request: "build: remove `--enable-lto`"
(https://github.com/bitcoin/bitcoin/pull/29185#pullrequestreview-1820011320)
ACK 2d1b1c7daeeada3f737e62ceb2db7484cde5ff4e.

🤔 hebasto reviewed a pull request: "ci: move CMake into base packages"
(https://github.com/bitcoin/bitcoin/pull/29225#pullrequestreview-1820012896)
Post merge ACK f3ca6db8d349941f6bad6e8b328033222af8f063.

🤔 maflcko reviewed a pull request: "fuzz: make FuzzedDataProvider usage deterministic"
(https://github.com/bitcoin/bitcoin/pull/29043#pullrequestreview-1820019492)
I hacked together a quick clang-query, but it will catch too many false positives, so I guess hardcoding a list of class names and only checking those is a better alternative?


```
match expr(anyOf(cxxConstructExpr(unless(isListInitialization())).bind("ctor"),callExpr().bind("fun")),
allOf(hasDescendant(cxxMemberCallExpr(
unless(hasDeclaration(cxxMethodDecl(isConst()))),
callee(cxxMethodDecl(hasParent(recordDecl().bind("rd1"))))
).bind("mc1")),
hasDescendant(cxxMemberCallExpr(
...

💬 maflcko commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1451520124)
nit: Obviously this is fine here, but in a function call where a copy was previously elided and now is not, due to a missing `std::move`, this can make code perform worse.

💬 fjahr commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#issuecomment-1890480213)
> There's still some work in progress on fixing determinism, see [fjahr/asmap-data#6](https://github.com/fjahr/asmap-data/pull/6), but getting close!

This should be fixed now, the issue appears to have been the ordering of sets after combining/diffing them. The results are now explicitly sorted. On my system, I could replicate the issue by using different python versions and it was fixed with this change.

Credits to @sipa for pointing me in the right direction.

💬 hebasto commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#discussion_r1451554686)
nit: Using the underlying build system call directly makes CMake re-build all targets again. That could be avoided by using CMake's invocation like that:
```suggestion
cmake --install . --prefix $($(package)_staging_prefix_dir)
```

Could be verified by building sequentially the `miniupnpc_built` and `miniupnpc_staged` targets.

💬 hebasto commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#discussion_r1451560368)
nit: Maybe skip headers that are used only to build the package:
```suggestion
install ../natpmp.h ../natpmp_declspec.h $($(package)_staging_prefix_dir)/include &&\
```
?

👍 hebasto approved a pull request: "depends: remove dependency on Darwin libtool"
(https://github.com/bitcoin/bitcoin/pull/29232#pullrequestreview-1820037343)
ACK a28c6bbde0bcd064869cf861d92a6e23df4a7b73