Merge #18637: coins: allow cache resize after init

f19fdd47a6371dcbe0760ef6f3c3c5adb31b1bb4 test: add test for CChainState::ResizeCoinsCaches() (James O'Beirne)
8ac3ef46999ed676ca3775f7b2f461d92f09a542 add ChainstateManager::MaybeRebalanceCaches() (James O'Beirne)
f36aaa6392fdbdac6891d92202d3efeff98754f4 Add CChainState::ResizeCoinsCaches (James O'Beirne)
b223111da2e0e9ceccef75df8a20252b0094b7bc txdb: add CCoinsViewDB::ChangeCacheSize (James O'Beirne)

Pull request description:

  This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):

  Parent PR: #15606
  Issue: #15605
  Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal

  ---

  In the assumeutxo implementation draft (#15056), once a UTXO snapshot is loaded, a new chainstate object is created after initialization. This means that we have to reclaim some of the cache that we've allocated to the original chainstate (per `dbcache=`) to repurpose for the snapshot chainstate.

  Furthermore, it makes sense to have different cache allocations depending on which chainstate is more active. While the snapshot chainstate is working to get to the network tip (and the background validation chainstate is idle), it makes sense that the snapshot chainstate should have the majority of cache allocation. And contrariwise once the snapshot has reached network tip, most of the cache should be given to the background validation chainstate.

  This set of changes (detailed in the commit messages) allows us to dynamically resize the various coins caches. None of the functionality introduced here is used at the moment, but will be in the next AU PR (which introduces `ActivateSnapshot`).

  `ChainstateManager::MaybeRebalanceCaches()` defines the (somewhat normative) cache allocations between the snapshot and background validation chainstates. I'd be interested in feedback if anyone has thoughts on the proportions I've set there.

ACKs for top commit:
  ajtowns:
    weak utACK f19fdd47a6371dcbe0760ef6f3c3c5adb31b1bb4 -- didn't find any major problems, but not super confident that I didn't miss anything
  fjahr:
    Code review ACK f19fdd4
  ryanofsky:
    Code review ACK f19fdd47a6371dcbe0760ef6f3c3c5adb31b1bb4. Only change since last review is constructor cleanup (no change in behavior). I think the suggestions here from ajtowns and others are good, but shouldn't delay merging the PR (and hold up assumeutxo)

Tree-SHA512: fffb7847fb6993dd4a1a41cf11179b211b0b20b7eb5f7cf6266442136bfe9d43b830bbefcafd475bfd4af273f5573500594aa41fff03e0ed5c2a1e8562ff9269

4 files changed, 137 insertions, 8 deletions

diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 24c0d6382b..0cd991f453 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -142,7 +142,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
     ::ChainstateActive().InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
     assert(!::ChainstateActive().CanFlushToDisk());
-    ::ChainstateActive().InitCoinsCache();
+    ::ChainstateActive().InitCoinsCache(1 << 23);
     assert(::ChainstateActive().CanFlushToDisk());
     if (!LoadGenesisBlock(chainparams)) {
         throw std::runtime_error("LoadGenesisBlock failed.");
diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp
new file mode 100644
index 0000000000..f531b393b8
--- /dev/null
+++ b/src/test/validation_chainstate_tests.cpp
@@ -0,0 +1,76 @@
+// Copyright (c) 2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+//
+#include <random.h>
+#include <uint256.h>
+#include <consensus/validation.h>
+#include <sync.h>
+#include <test/util/setup_common.h>
+#include <validation.h>
+
+#include <vector>
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_FIXTURE_TEST_SUITE(validation_chainstate_tests, TestingSetup)
+
+//! Test resizing coins-related CChainState caches during runtime.
+//!
+BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches)
+{
+    ChainstateManager manager;
+
+    //! Create and add a Coin with DynamicMemoryUsage of 80 bytes to the given view.
+    auto add_coin = [](CCoinsViewCache& coins_view) -> COutPoint {
+        Coin newcoin;
+        uint256 txid = InsecureRand256();
+        COutPoint outp{txid, 0};
+        newcoin.nHeight = 1;
+        newcoin.out.nValue = InsecureRand32();
+        newcoin.out.scriptPubKey.assign((uint32_t)56, 1);
+        coins_view.AddCoin(outp, std::move(newcoin), false);
+
+        return outp;
+    };
+
+    ENTER_CRITICAL_SECTION(cs_main);
+    CChainState& c1 = manager.InitializeChainstate();
+    LEAVE_CRITICAL_SECTION(cs_main);
+    c1.InitCoinsDB(
+        /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
+    WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23));
+
+    // Add a coin to the in-memory cache, upsize once, then downsize.
+    {
+        LOCK(::cs_main);
+        auto outpoint = add_coin(c1.CoinsTip());
+
+        // Set a meaningless bestblock value in the coinsview cache - otherwise we won't
+        // flush during ResizecoinsCaches() and will subsequently hit an assertion.
+        c1.CoinsTip().SetBestBlock(InsecureRand256());
+
+        BOOST_CHECK(c1.CoinsTip().HaveCoinInCache(outpoint));
+
+        c1.ResizeCoinsCaches(
+            1 << 24,  // upsizing the coinsview cache
+            1 << 22  // downsizing the coinsdb cache
+        );
+
+        // View should still have the coin cached, since we haven't destructed the cache on upsize.
+        BOOST_CHECK(c1.CoinsTip().HaveCoinInCache(outpoint));
+
+        c1.ResizeCoinsCaches(
+            1 << 22,  // downsizing the coinsview cache
+            1 << 23  // upsizing the coinsdb cache
+        );
+
+        // The view cache should be empty since we had to destruct to downsize.
+        BOOST_CHECK(!c1.CoinsTip().HaveCoinInCache(outpoint));
+    }
+
+    // Avoid triggering the address sanitizer.
+    WITH_LOCK(::cs_main, manager.Unload());
+}
+
+BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
index 0d149285ad..f99191b08f 100644
--- a/src/test/validation_chainstatemanager_tests.cpp
+++ b/src/test/validation_chainstatemanager_tests.cpp
@@ -28,13 +28,11 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
 
     // Create a legacy (IBD) chainstate.
     //
-    ENTER_CRITICAL_SECTION(cs_main);
-    CChainState& c1 = manager.InitializeChainstate();
-    LEAVE_CRITICAL_SECTION(cs_main);
+    CChainState& c1 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate());
     chainstates.push_back(&c1);
     c1.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
-    WITH_LOCK(::cs_main, c1.InitCoinsCache());
+    WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23));
 
     BOOST_CHECK(!manager.IsSnapshotActive());
     BOOST_CHECK(!manager.IsSnapshotValidated());
@@ -57,12 +55,13 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
     // Create a snapshot-based chainstate.
     //
     ENTER_CRITICAL_SECTION(cs_main);
-    CChainState& c2 = manager.InitializeChainstate(GetRandHash());
+    CChainState& c2 = *WITH_LOCK(::cs_main,
+        return &manager.InitializeChainstate(GetRandHash()));
     LEAVE_CRITICAL_SECTION(cs_main);
     chainstates.push_back(&c2);
     c2.InitCoinsDB(
         /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
-    WITH_LOCK(::cs_main, c2.InitCoinsCache());
+    WITH_LOCK(::cs_main, c2.InitCoinsCache(1 << 23));
     // Unlike c1, which doesn't have any blocks. Gets us different tip, height.
     c2.LoadGenesisBlock(chainparams);
     BlockValidationState _;
@@ -104,4 +103,58 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
     WITH_LOCK(::cs_main, manager.Unload());
 }
 
+//! Test rebalancing the caches associated with each chainstate.
+BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches)
+{
+    ChainstateManager manager;
+    size_t max_cache = 10000;
+    manager.m_total_coinsdb_cache = max_cache;
+    manager.m_total_coinstip_cache = max_cache;
+
+    std::vector<CChainState*> chainstates;
+
+    // Create a legacy (IBD) chainstate.
+    //
+    ENTER_CRITICAL_SECTION(cs_main);
+    CChainState& c1 = manager.InitializeChainstate();
+    LEAVE_CRITICAL_SECTION(cs_main);
+    chainstates.push_back(&c1);
+    c1.InitCoinsDB(
+        /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
+
+    {
+        LOCK(::cs_main);
+        c1.InitCoinsCache(1 << 23);
+        c1.CoinsTip().SetBestBlock(InsecureRand256());
+        manager.MaybeRebalanceCaches();
+    }
+
+    BOOST_CHECK_EQUAL(c1.m_coinstip_cache_size_bytes, max_cache);
+    BOOST_CHECK_EQUAL(c1.m_coinsdb_cache_size_bytes, max_cache);
+
+    // Create a snapshot-based chainstate.
+    //
+    ENTER_CRITICAL_SECTION(cs_main);
+    CChainState& c2 = manager.InitializeChainstate(GetRandHash());
+    LEAVE_CRITICAL_SECTION(cs_main);
+    chainstates.push_back(&c2);
+    c2.InitCoinsDB(
+        /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
+
+    {
+        LOCK(::cs_main);
+        c2.InitCoinsCache(1 << 23);
+        c2.CoinsTip().SetBestBlock(InsecureRand256());
+        manager.MaybeRebalanceCaches();
+    }
+
+    // Since both chainstates are considered to be in initial block download,
+    // the snapshot chainstate should take priority.
+    BOOST_CHECK_CLOSE(c1.m_coinstip_cache_size_bytes, max_cache * 0.05, 1);
+    BOOST_CHECK_CLOSE(c1.m_coinsdb_cache_size_bytes, max_cache * 0.05, 1);
+    BOOST_CHECK_CLOSE(c2.m_coinstip_cache_size_bytes, max_cache * 0.95, 1);
+    BOOST_CHECK_CLOSE(c2.m_coinsdb_cache_size_bytes, max_cache * 0.95, 1);
+
+}
+
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp
index a863e3a4d5..7111fc3380 100644
--- a/src/test/validation_flush_tests.cpp
+++ b/src/test/validation_flush_tests.cpp
@@ -21,7 +21,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
     BlockManager blockman{};
     CChainState chainstate{blockman};
     chainstate.InitCoinsDB(/*cache_size_bytes*/ 1 << 10, /*in_memory*/ true, /*should_wipe*/ false);
-    WITH_LOCK(::cs_main, chainstate.InitCoinsCache());
+    WITH_LOCK(::cs_main, chainstate.InitCoinsCache(1 << 10));
     CTxMemPool tx_pool{};
 
     constexpr bool is_64_bit = sizeof(void*) == 8;