✅ fanquake closed a pull request: "DO NOT MERGE: CCoinsCacheEntry move constructor Sonar fix"
(https://github.com/bitcoin/bitcoin/pull/30641)

💬 paplorinc commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2284890283)
> Because of that, there is no reason to perform a wiping flush in case the contents of the cache is still useful.

I assumed (without diving into the code) that this meant an optimization.
<details>
<summary>Did 2x5 reindex checks until 500k blocks - 0% change.</summary>

Time (mean ± σ): 15396.778 s ± 31.337 s [User: 81183.230 s, System: 364.223 s]
Range (min … max): 15363.334 s … 15427.586 s 5 runs

Time (mean ± σ): 15410.168 s ± 20.832 s [User: 81268.923 s,
...

💬 agroce commented on issue "fuzz: `script`: Assertion `!extract_destination_ret' failed.":
(https://github.com/bitcoin/bitcoin/issues/30615#issuecomment-2284891365)
@maflcko:

> This is trivial to find in libFuzzer with a small enough max_len and value profile ( for example, -use_value_profile=1 -max_len=24).

How trivial is trivial? I have a set up that can definitely find it, and that setting is taking quite some time...

💬 sipa commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2284893423)
@paplorinc Yes, it is an optimization, but only affects the `scantxoutset` and `gettxoutsetinfo` RPCs. Normal synchronization should be unaffected.

💬 mzumsande commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2284894942)
Maybe copying the string into a static or global variable would help rule out a lifetime issue?

```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 9126d8e928..683d13e0e5 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -1863,6 +1863,8 @@ MempoolAcceptResult AcceptToMemoryPool(Chainstate& active_chainstate, const CTra
CTxMemPool& pool{*active_chainstate.GetMempool()};

std::vector<COutPoint> coins_to_uncache;
+ static char string_buffer[120];

...

📝 theStack opened a pull request: "doc: add missing "testnet4" network string in RPC/init help texts"
(https://github.com/bitcoin/bitcoin/pull/30642)
The following bitcoind parameters / RPC calls still missed the "testnet4" network string:
- `-chain=` parameter
- `getblockchaininfo` RPC, `"chain"` result
- `getmininginfo` RPC, `"chain"` result

The occurences were found via `$ git grep \".*main.*test.*\"`.

📝 paplorinc opened a pull request: "coins: Add move operations to CCoinsCacheEntry"
(https://github.com/bitcoin/bitcoin/pull/30643)
Related to https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1703187118

Added defaulted move constructor and move assignment operator to `CCoinsCacheEntry`.
And to make sure the mentioned Sonar warnings aren't triggered by CI anymore, I've modernized the call-site minimally.

💬 paplorinc commented on pull request "DO NOT MERGE: CCoinsCacheEntry move constructor Sonar fix":
(https://github.com/bitcoin/bitcoin/pull/30641#issuecomment-2284920490)
I'm a bit surprised at these reactions, but opened https://github.com/bitcoin/bitcoin/pull/30643 with the proposed fix.

💬 furszy commented on pull request "wallet: fix blank legacy detection":
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714380473)
done

💬 furszy commented on pull request "wallet: fix blank legacy detection":
(https://github.com/bitcoin/bitcoin/pull/30621#discussion_r1714380532)
done

💬 andrewtoth commented on pull request "coins: Add move operations to CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/30643#issuecomment-2284929509)
Don't we also need to add move constructor to `Coin`? Its prevector is the only thing that really benefits from being moved I believe.

🤔 stickies-v reviewed a pull request: "refactor: Remove Span operator==, Use std::ranges::equal"
(https://github.com/bitcoin/bitcoin/pull/29071#pullrequestreview-2232822586)
Approach ACK

💬 stickies-v commented on pull request "refactor: Remove Span operator==, Use std::ranges::equal":
(https://github.com/bitcoin/bitcoin/pull/29071#discussion_r1713639323)
nit: no more need for `#include <algorithm>`

<details>
<summary>iwyu</summary>

```
span.h should add these lines:
#include <utility> // for declval, forward

span.h should remove these lines:
- #include <algorithm> // lines 8-8
- #include <cassert> // lines 9-9

The full include-list for span.h:
#include <cstddef> // for size_t, byte, nullptr_t
#include <span> // for span
#include <type_traits> // for remove_pointer, is_convertible, enable_if, enable_if_t,
...

👍 AngusP approved a pull request: "test: update satoshi_round function"
(https://github.com/bitcoin/bitcoin/pull/29566#pullrequestreview-2234072206)
ACK ec317bc44b0cb089419d809b5fef38ecb9423051

🤔 pablomartin4btc reviewed a pull request: "build: Introduce CMake-based build system"
(https://github.com/bitcoin/bitcoin/pull/30454#pullrequestreview-2234121687)
tACK 67b1e236334f38ec4e4d2251dbdfb790f20ed88b

Built with `GUI` on macOS 14.4 following the instructions in `/doc/build-osx.md` (part of this PR) and run `./build/qt/bitcoin-qt` successfully.

A few observations:

- Regarding the configuration: in the documentation says:
"_If `berkeley-db@4` is installed, then legacy wallet support will be built_."
But I have to use `-DWITH_BDB=ON` in order to get that functionality.
- After I compile with `cmake` I get this warning:
`ld: warning
...

🤔 tdb3 reviewed a pull request: "wallet: fix blank legacy detection"
(https://github.com/bitcoin/bitcoin/pull/30621#pullrequestreview-2234159075)
Approach ACK

Nice simplifications.

Retested (https://github.com/bitcoin/bitcoin/pull/30621#pullrequestreview-2231909317) with same result (migration allowed).

Saw some `may be used uninitialized` warnings when building, e.g.
```
inlined from ‘bool wallet::CWallet::IsSpentKey(const CScript&) const’ at wallet/wallet.cpp:1046:37:
./prevector.h:175:37: warning: ‘*(const prevector<28, unsigned char, unsigned int, int>*)((char*)&<unnamed> + offsetof(const std::CTxDestination, std::varian
...

👍 ryanofsky approved a pull request: "logging: use bitset for categories"
(https://github.com/bitcoin/bitcoin/pull/26697#pullrequestreview-2234186346)
Code review ACK d8c637a0ad704c30dc128d0c66215d5ac220f85d

I wouldn't have a problem just switching the code to use uint64_t but I think this approach is a nicer and more type safe. I like that this gets rid of all the bitwise operations and (1 << 28) cruft and stops hardcoding uint32_t various places. The atomic bitset class seems pretty simple to me and has a nice interface.

💬 ryanofsky commented on pull request "logging: use bitset for categories":
(https://github.com/bitcoin/bitcoin/pull/26697#discussion_r1714473985)
This is a good catch and I think it's worth commenting on, but I don't think it is a bug. Would suggest adding a comment like: "Checks if any bits are set. Note: Bits are checked independently, so result reflects the state of all bits, but not at a single instant in time."

💬 ryanofsky commented on pull request "logging: use bitset for categories":
(https://github.com/bitcoin/bitcoin/pull/26697#discussion_r1714475980)
In commit "logging: use bitset class for categories" (d8c637a0ad704c30dc128d0c66215d5ac220f85d)

Would be nice to rename `bits` member to `m_bits` for clarity I think

👍 ryanofsky approved a pull request: "fix: increase consistency of rpcauth parsing"
(https://github.com/bitcoin/bitcoin/pull/30401#pullrequestreview-2234212279)
Code review ACK f4b0b69a3e7df58430cb887028b13265834d8c7e.

This PR is now just a one line fix accompanied by various tests.