Merge bitcoin/bitcoin#26999: A few follow-ups to #17487 (coins write without cache drop)

2e16054a66 Add assertions that BatchWrite(erase=true) erases (Pieter Wuille)
941feb6ca2 Avoid unclear {it = ++it;} (Pieter Wuille)
98db35c2f8 Follow coding style for named arguments (Pieter Wuille)
bb00357add Make test/fuzz/coins_view exercise CCoinsViewCache::Sync() (Pieter Wuille)

Pull request description:

  This addresses a few nits left open in #17487.

ACKs for top commit:
  jonatack:
    ACK 2e16054a66
  Sjors:
    utACK 2e16054a66
  achow101:
    ACK 2e16054a66
  jamesob:
    ACK 2e16054a66 ([`jamesob/ackr/26999.1.sipa.a_few_follow_ups_to_1748`](https://github.com/jamesob/bitcoin/tree/ackr/26999.1.sipa.a_few_follow_ups_to_1748))

Tree-SHA512: 5e64807b850fa12ce858e87470420555ad081f2f63d53649d9904034ed5311592005eb979a8a4df2b70ecb56cf022db9750cd6999e5480bc8226184d4be22ce4

src/coins.cpp

@@ -249,15 +249,18 @@ bool CCoinsViewCache::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlockIn
 }
 
 bool CCoinsViewCache::Flush() {
-    bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/ true);
-    cacheCoins.clear();
+    bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/true);
+    if (fOk && !cacheCoins.empty()) {
+        /* BatchWrite must erase all cacheCoins elements when erase=true. */
+        throw std::logic_error("Not all cached coins were erased");
+    }
     cachedCoinsUsage = 0;
     return fOk;
 }
 
 bool CCoinsViewCache::Sync()
 {
-    bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/ false);
+    bool fOk = base->BatchWrite(cacheCoins, hashBlock, /*erase=*/false);
     // Instead of clearing `cacheCoins` as we would in Flush(), just clear the
     // FRESH/DIRTY flags of any coin that isn't spent.
     for (auto it = cacheCoins.begin(); it != cacheCoins.end(); ) {

src/test/coins_tests.cpp

@@ -55,7 +55,7 @@ public:
 
     bool BatchWrite(CCoinsMap& mapCoins, const uint256& hashBlock, bool erase = true) override
     {
-        for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); it = erase ? mapCoins.erase(it) : ++it) {
+        for (CCoinsMap::iterator it = mapCoins.begin(); it != mapCoins.end(); it = erase ? mapCoins.erase(it) : std::next(it)) {
             if (it->second.flags & CCoinsCacheEntry::DIRTY) {
                 // Same optimization used in CCoinsViewDB is to only write dirty entries.
                 map_[it->first] = it->second.coin;

src/test/fuzz/coins_view.cpp

@@ -74,6 +74,9 @@ FUZZ_TARGET_INIT(coins_view, initialize_coins_view)
             [&] {
                 (void)coins_view_cache.Flush();
             },
+            [&] {
+                (void)coins_view_cache.Sync();
+            },
             [&] {
                 coins_view_cache.SetBestBlock(ConsumeUInt256(fuzzed_data_provider));
             },