Simplify semantics of ChainStateFlushed callback

Previously, ChainStateFlushed would fire either if a full flush
completed (which can happen due to memory limits, forced flush, or
on its own DATABASE_WRITE_INTERVAL timer) *or* on a
ChainStateFlushed-specific DATABASE_WRITE_INTERVAL timer. This is
both less clear for clients (as there are no guarantees about a
flush having actually happened prior to the call), and reults in
extra flushes not clearly intended by the code. We drop the second
case, providing a strong guarantee without removing the periodit
timer-based flushing.

src/validation.cpp

@@ -2066,13 +2066,12 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState &
     LOCK(cs_main);
     static int64_t nLastWrite = 0;
     static int64_t nLastFlush = 0;
-    static int64_t nLastSetChain = 0;
     std::set<int> setFilesToPrune;
-    bool fFlushForPrune = false;
-    bool fDoFullFlush = false;
-    int64_t nNow = 0;
+    bool full_flush_completed = false;
     try {
     {
+        bool fFlushForPrune = false;
+        bool fDoFullFlush = false;
         LOCK(cs_LastBlockFile);
         if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) {
             if (nManualPruneHeight > 0) {
@@ -2089,7 +2088,7 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState &
                 }
             }
         }
-        nNow = GetTimeMicros();
+        int64_t nNow = GetTimeMicros();
         // Avoid writing/flushing immediately after startup.
         if (nLastWrite == 0) {
             nLastWrite = nNow;
@@ -2097,9 +2096,6 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState &
         if (nLastFlush == 0) {
             nLastFlush = nNow;
         }
-        if (nLastSetChain == 0) {
-            nLastSetChain = nNow;
-        }
         int64_t nMempoolSizeMax = gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000;
         int64_t cacheSize = pcoinsTip->DynamicMemoryUsage();
         int64_t nTotalSpace = nCoinCacheUsage + std::max<int64_t>(nMempoolSizeMax - nMempoolUsage, 0);
@@ -2156,12 +2152,12 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState &
             if (!pcoinsTip->Flush())
                 return AbortNode(state, "Failed to write to coin database");
             nLastFlush = nNow;
+            full_flush_completed = true;
         }
     }
-    if (fDoFullFlush || ((mode == FlushStateMode::ALWAYS || mode == FlushStateMode::PERIODIC) && nNow > nLastSetChain + (int64_t)DATABASE_WRITE_INTERVAL * 1000000)) {
+    if (full_flush_completed) {
         // Update best block in wallet (so we can detect restored wallets).
         GetMainSignals().ChainStateFlushed(chainActive.GetLocator());
-        nLastSetChain = nNow;
     }
     } catch (const std::runtime_error& e) {
         return AbortNode(state, std::string("System error while flushing: ") + e.what());

src/validationinterface.h

@@ -99,6 +99,17 @@ protected:
     /**
      * Notifies listeners of the new active block chain on-disk.
      *
+     * Prior to this callback, any updates are not guaranteed to persist on disk
+     * (ie clients need to handle shutdown/restart safety by being able to
+     * understand when some updates were lost due to unclean shutdown).
+     *
+     * When this callback is invoked, the validation changes done by any prior
+     * callback are guaranteed to exist on disk and survive a restart, including
+     * an unclean shutdown.
+     *
+     * Provides a locator describing the best chain, which is likely useful for
+     * storing current state on disk in client DBs.
+     *
      * Called on a background thread.
      */
     virtual void ChainStateFlushed(const CBlockLocator &locator) {}