Repository: bitcoin/bitcoin
File: src/validation.cpp (C++)

Code before:
                coins_file >> coin;
                if (coin.nHeight > base_height ||
                    outpoint.n >= std::numeric_limits<decltype(outpoint.n)>::max() // Avoid integer wrap-around in coinstats.cpp:ApplyHash
                ) {
                    LogPrintf("[snapshot] bad snapshot data after deserializing %d coins\n",
                              coins_count - coins_left);
                    return false;
                }
                if (!MoneyRange(coin.out.nValue)) {
                    LogPrintf("[snapshot] bad snapshot data after deserializing %d coins - bad tx out value\n",
                              coins_count - coins_left);
                    return false;
                }
                coins_cache.EmplaceCoinInternalDANGER(std::move(outpoint), std::move(coin));

                --coins_left;
                ++coins_processed;

                if (coins_processed % 1000000 == 0) {
                    LogPrintf("[snapshot] %d coins loaded (%.2f%%, %.2f MB)\n",
                        coins_processed,
                        static_cast<float>(coins_processed) * 100 / static_cast<float>(coins_count),
                        coins_cache.DynamicMemoryUsage() / (1000 * 1000));
                }

                // Batch write and flush (if we need to) every so often.
                //
                // If our average Coin size is roughly 41 bytes, checking every 120,000 coins
                // means <5MB of memory imprecision.
                if (coins_processed % 120000 == 0) {
                    if (m_interrupt) {
                        return false;
                    }

                    const auto snapshot_cache_state = WITH_LOCK(::cs_main,
                        return snapshot_chainstate.GetCoinsCacheSizeState());

                    if (snapshot_cache_state >= CoinsCacheSizeState::CRITICAL) {
                        // This is a hack - we don't know what the actual best block is, but that
                        // doesn't matter for the purposes of flushing the cache here. We'll set this
                        // to its correct value (`base_blockhash`) below after the coins are loaded.
                        coins_cache.SetBestBlock(GetRandHash());

                        // No need to acquire cs_main since this chainstate isn't being used yet.
                        FlushSnapshotToDisk(coins_cache, /*snapshot_loaded=*/false);
                    }
                }
            }
        } catch (const std::ios_base::failure&) {
            LogPrintf("[snapshot] bad snapshot format or truncated snapshot after deserializing %d coins\n",
                      coins_processed);

Review comment (suggestion):
In commit "rpc: Return precise loadtxoutset error messages" (fa91404c68b4d503e6568f49aaf74a5746887185)

This is ok, but IMO it would be better to change result type from `Result<void>` to `Result<InterruptResult>` and return `Interrupted{}` here. If a function exits early because the user requested a shutdown, it may or may not be an error, and bubbling up `InterruptResult` lets the top level caller decide what to do based on context.

For example, in this case because the call is being made by RPC it would probably be good for the RPC call to return an "Snapshot loading was aborted" error. But in the GUI case, if a user tries to load snapshot and then shuts down before it is loaded, it would probably be better to just shut down cleanly and maybe log something, than try to present an error message.

In either case, returning something like `return util::Error{Untranslated("Processing snapshot aborted")}` or `return Interrupted{}` would be better than returning an error with no message.

Code after:
                    return util::Error{strprintf(Untranslated("Bad snapshot data after deserializing %d coins"),
                              coins_count - coins_left)};
                }
                if (!MoneyRange(coin.out.nValue)) {
                    return util::Error{strprintf(Untranslated("Bad snapshot data after deserializing %d coins - bad tx out value"),
                              coins_count - coins_left)};
                }
                coins_cache.EmplaceCoinInternalDANGER(std::move(outpoint), std::move(coin));

                --coins_left;
                ++coins_processed;

                if (coins_processed % 1000000 == 0) {
                    LogPrintf("[snapshot] %d coins loaded (%.2f%%, %.2f MB)\n",
                        coins_processed,
                        static_cast<float>(coins_processed) * 100 / static_cast<float>(coins_count),
                        coins_cache.DynamicMemoryUsage() / (1000 * 1000));
                }

                // Batch write and flush (if we need to) every so often.
                //
                // If our average Coin size is roughly 41 bytes, checking every 120,000 coins
                // means <5MB of memory imprecision.
                if (coins_processed % 120000 == 0) {
                    if (m_interrupt) {
                        return util::Error{Untranslated("Aborting after an interrupt was requested")};
                    }

                    const auto snapshot_cache_state = WITH_LOCK(::cs_main,
                        return snapshot_chainstate.GetCoinsCacheSizeState());

                    if (snapshot_cache_state >= CoinsCacheSizeState::CRITICAL) {
                        // This is a hack - we don't know what the actual best block is, but that
                        // doesn't matter for the purposes of flushing the cache here. We'll set this
                        // to its correct value (`base_blockhash`) below after the coins are loaded.
                        coins_cache.SetBestBlock(GetRandHash());

                        // No need to acquire cs_main since this chainstate isn't being used yet.
                        FlushSnapshotToDisk(coins_cache, /*snapshot_loaded=*/false);
                    }
                }
            }
        } catch (const std::ios_base::failure&) {
            return util::Error{strprintf(Untranslated("Bad snapshot format or truncated snapshot after deserializing %d coins"),
                      coins_processed)};
        }
    }

    // Important that we set this. This and the coins_cache accesses above are
    // sort of a layer violation, but either we reach into the innards of
    // CCoinsViewCache here or we have to invert some of the Chainstate to