💬 TheCharlatan commented on pull request "C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-2277481665)
Post-merge ACK 2925bd537cbd8c70594e23f6c4298b7101f7f73d

💬 maflcko commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1711088525)
(In the follow-up you can also adjust the error message to explain the new max-length check. Otherwise, this could be confusing:

`Error: Invalid non-hex (0x1234ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff) minimum chain work value specified`)

💬 fjahr commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#discussion_r1711119588)
Yeah, this fixed https://github.com/bitcoin/bitcoin/runs/28432306724

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711130542)
This definitely fixed it for me, how can I help with the other cases you've mentioned?

💬 ismaelsadeeq commented on issue "fuzz: `script`: Assertion `!extract_destination_ret' failed.":
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2277538703)
Yes I think it is, I've bisected and it starts crashing on 455fca86cfada1823aa28615b5683f9dc73dbb9a

IIUC I dont think we should `assert` that `TxoutType::ANCHOR` will return false after parsing a `CScript`.

It returns true.

https://github.com/bitcoin/bitcoin/blob/24ced5274438c0185963ae1296ec7dda97ea62f1/src/addresstype.cpp#L90-L92

So I think we should not `assert` for that?

<details>
<summary>diff</summary>

```diff
diff --git a/src/test/fuzz/script.cpp b/src/test/fuzz/script
...

🤔 ismaelsadeeq reviewed a pull request: "assumeutxo: Drop block height from metadata"
(https://github.com/bitcoin/bitcoin/pull/30598#pullrequestreview-2229782323)
Re-ACK 00618e8745192d209c23e3ae873c077e58168957

It looks like C.I failure is unrelated as this [diff](https://github.com/bitcoin/bitcoin/compare/6a020f019f709b595b68516bb3772b811a962e7c..00618e8745192d209c23e3ae873c077e58168957) is the last push.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711152030)
> and it doesn't explain why it doesn't happen for Autotools build

Autotools lack `-Wl,-fatal_warnings` in the following check: https://github.com/bitcoin/bitcoin/blob/24ced5274438c0185963ae1296ec7dda97ea62f1/configure.ac#L365-L375

💬 fanquake commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711157307)
Ok, but it's still not clear why a compiler we don't use / isn't installed, would be adding libraries to the link line.

🚀 fanquake merged a pull request: "guix: bump time-machine to 7bf1d7aeaffba15c4f680f93ae88fbef25427252"
(https://github.com/bitcoin/bitcoin/pull/30609)

💬 maflcko commented on pull request "lint: Find function calls in default arguments":
(https://github.com/bitcoin/bitcoin/pull/30553#issuecomment-2277582999)
Addressed all feedback. Should be easy to re-ack

💬 fjahr commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#discussion_r1711181802)
I will leave this unaddressed for now since either way seems ok and we have 3 acks.

💬 paplorinc commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711188855)
The proposed `-no_warn_duplicate_libraries` will only fix it for duplicate libs warnings (but any other warning would fail the build). Can we check if we're getting any other warning - or whether the above change fixes it?

💬 m3dwards commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711197957)
On Arm Mac I get the same configuration problem. In the logs I'm seeing:

```shell
ld: warning: ignoring duplicate libraries: '-lc++'
ld: fatal warning(s) induced error (-fatal_warnings)
```

It works with autotools (perhaps warnings aren't errors in the configure tests?). I've passed the full log to @hebasto.

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711200712)
@m3dwards

> It works with autotools (perhaps warnings aren't errors in the configure tests?).

See: https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711152030

💬 m3dwards commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711219004)
@hebasto I had a stale version of this page open and hadn't seen the latest comments 🤦

💬 paplorinc commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#discussion_r1711222223)
> blindly dereferencing it is not good practice

In C++23 we'll be able to chain them properly via something like `auto hex_value = sanitized_hex.and_then(uint256::FromHex);`
If there's a C++23 checklist somewhere (haven't seen any), we could just leave the dereference as is and fix it after we migrate to C++23 instead.

💬 maflcko commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2277655269)
Given that you can reproduce on 50% of your new action runs, does the following diff work around the bug?


```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index cf4dce3762..d2b6969370 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1873,10 +1873,13 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra

for (const COutPoint& hashTx : coins_to_uncache)
active_chainstate.CoinsTip().Uncache(hashTx);
+
...

💬 maflcko commented on pull request "refactor: Remove Span operator==, Use std::ranges::equal":
(https://github.com/bitcoin/bitcoin/pull/29071#discussion_r1711245423)
Fixed. Should be trivial to re-ack

💬 m3dwards commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711246805)
Similar problem: https://github.com/orgs/Homebrew/discussions/4794

💬 hebasto commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1711262045)
On my macOS Sonoma 14.6.1:
```
% cat test.cpp
int main() {}
% /opt/homebrew/opt/llvm/bin/clang++ --version
Homebrew clang version 18.1.8
Target: arm64-apple-darwin23.6.0
Thread model: posix
InstalledDir: /opt/homebrew/opt/llvm/bin
% /opt/homebrew/opt/llvm/bin/clang++ -fsanitize=fuzzer -o test test.cpp
ld: warning: ignoring duplicate libraries: '-lc++'
```