Merge 8fd522b223 into c5e44a0435

32d55e28af test: Use the correct node for doubled keypath test (Ava Chow)

Pull request description:

  #29124 had a silent merge conflict with #32350 which resulted in it using the wrong node. Fix the test to use the correct v22 node.

ACKs for top commit:
  maflcko:
    lgtm ACK 32d55e28af
  rkrux:
    ACK 32d55e28af
  BrandonOdiwuor:
    Code Review ACK 32d55e28af

Tree-SHA512: 1e0231985beb382b16e1d608c874750423d0502388db0c8ad450b22d17f9d96f5e16a6b44948ebda5efc750f62b60d0de8dd20131f449427426a36caf374af92

src/init.cpp

@@ -48,6 +48,7 @@
 #include <node/caches.h>
 #include <node/chainstate.h>
 #include <node/chainstatemanager_args.h>
+#include <node/coins_view_args.h>
 #include <node/context.h>
 #include <node/interface_ui.h>
 #include <node/kernel_notifications.h>
@@ -487,7 +488,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc)
     argsman.AddArg("-coinstatsindex", strprintf("Maintain coinstats index used by the gettxoutsetinfo RPC (default: %u)", DEFAULT_COINSTATSINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-conf=<file>", strprintf("Specify path to read-only configuration file. Relative paths will be prefixed by datadir location (only useable from command line, not configuration file) (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-datadir=<dir>", "Specify data directory", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS);
-    argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
+    argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: calculated from the `-dbcache` value or %u if not set)", node::GetDbBatchSize(DEFAULT_DB_CACHE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
     argsman.AddArg("-dbcache=<n>", strprintf("Maximum database cache size <n> MiB (minimum %d, default: %d). Make sure you have enough RAM. In addition, unused memory allocated to the mempool is shared with this cache (see -maxmempool).", MIN_DB_CACHE >> 20, DEFAULT_DB_CACHE >> 20), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-includeconf=<file>", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
     argsman.AddArg("-allowignoredconf", strprintf("For backwards compatibility, treat an unused %s file in the datadir as a warning, not an error.", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);

src/kernel/caches.h

@@ -16,6 +16,9 @@ static constexpr size_t MAX_BLOCK_DB_CACHE{2_MiB};
 //! Max memory allocated to coin DB specific cache (bytes)
 static constexpr size_t MAX_COINS_DB_CACHE{8_MiB};
 
+//! The batch size of DEFAULT_KERNEL_CACHE
+static constexpr size_t DEFAULT_DB_CACHE_BATCH{16_MiB};
+
 namespace kernel {
 struct CacheSizes {
     size_t block_tree_db;

src/node/coins_view_args.cpp

@@ -2,6 +2,8 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <kernel/caches.h>
+
 #include <node/coins_view_args.h>
 
 #include <common/args.h>
@@ -10,7 +12,9 @@
 namespace node {
 void ReadCoinsViewArgs(const ArgsManager& args, CoinsViewOptions& options)
 {
-    if (auto value = args.GetIntArg("-dbbatchsize")) options.batch_write_bytes = *value;
-    if (auto value = args.GetIntArg("-dbcrashratio")) options.simulate_crash_ratio = *value;
+    if (const auto value = args.GetIntArg("-dbbatchsize")) options.batch_write_bytes = *value;
+    else options.batch_write_bytes = GetDbBatchSize(args.GetIntArg("-dbcache", DEFAULT_KERNEL_CACHE));
+
+    if (const auto value = args.GetIntArg("-dbcrashratio")) options.simulate_crash_ratio = *value;
 }
 } // namespace node

src/node/coins_view_args.h

@@ -5,10 +5,22 @@
 #ifndef BITCOIN_NODE_COINS_VIEW_ARGS_H
 #define BITCOIN_NODE_COINS_VIEW_ARGS_H
 
+#include <kernel/caches.h>
+
 class ArgsManager;
 struct CoinsViewOptions;
 
-namespace node {
+namespace node
+{
+static constexpr size_t GetDbBatchSize(const size_t dbcache_bytes)
+{
+    return std::clamp(
+        (dbcache_bytes / DEFAULT_KERNEL_CACHE) * DEFAULT_DB_CACHE_BATCH,
+        /*lo=*/DEFAULT_DB_CACHE_BATCH,
+        /*hi=*/256_MiB
+    );
+}
+
 void ReadCoinsViewArgs(const ArgsManager& args, CoinsViewOptions& options);
 } // namespace node
 

src/test/coins_tests.cpp

@@ -20,6 +20,7 @@
 #include <vector>
 
 #include <boost/test/unit_test.hpp>
+#include <node/coins_view_args.h>
 
 using namespace util::hex_literals;
 
@@ -1074,4 +1075,26 @@ BOOST_AUTO_TEST_CASE(coins_resource_is_used)
     PoolResourceTester::CheckAllDataAccountedFor(resource);
 }
 
+BOOST_AUTO_TEST_CASE(db_batch_sizes)
+{
+    BOOST_REQUIRE_EQUAL(node::GetDbBatchSize(DEFAULT_KERNEL_CACHE), DEFAULT_DB_CACHE_BATCH);
+    BOOST_REQUIRE_EQUAL(node::GetDbBatchSize(0_MiB), DEFAULT_DB_CACHE_BATCH);
+
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(4_MiB), 16'777'216);
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(10_MiB), 16'777'216);
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(45_MiB), 16'777'216);
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(100_MiB), 16'777'216);
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(450_MiB), 16'777'216);
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(1000_MiB), 33'554'432);
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(2000_MiB), 67'108'864);
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(3000_MiB), 100'663'296);
+
+#if SIZE_MAX > UINT32_MAX
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(4500_MiB), 167'772'160);
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(7000_MiB), 251'658'240);
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(10000_MiB), 268'435'456);
+    BOOST_CHECK_EQUAL(node::GetDbBatchSize(45000_MiB), 268'435'456);
+#endif
+}
+
 BOOST_AUTO_TEST_SUITE_END()

src/txdb.h

@@ -8,6 +8,7 @@
 
 #include <coins.h>
 #include <dbwrapper.h>
+#include <kernel/caches.h>
 #include <kernel/cs_main.h>
 #include <sync.h>
 #include <util/fs.h>
@@ -21,16 +22,13 @@
 class COutPoint;
 class uint256;
 
-//! -dbbatchsize default (bytes)
-static const int64_t nDefaultDbBatchSize = 16 << 20;
-
 //! User-controlled performance and debug options.
 struct CoinsViewOptions {
     //! Maximum database write batch size in bytes.
-    size_t batch_write_bytes = nDefaultDbBatchSize;
+    size_t batch_write_bytes{DEFAULT_DB_CACHE_BATCH};
     //! If non-zero, randomly exit when the database is flushed with (1/ratio)
     //! probability.
-    int simulate_crash_ratio = 0;
+    int simulate_crash_ratio{0};
 };
 
 /** CCoinsView backed by the coin database (chainstate/) */

test/functional/wallet_backwards_compatibility.py

@@ -87,7 +87,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
         # 0.21.x and 22.x would both produce bad derivation paths when topping up an inactive hd chain
         # Make sure that this is being automatically cleaned up by migration
         node_master = self.nodes[1]
-        node_v22 = self.nodes[self.num_nodes - 5]
+        node_v22 = self.nodes[self.num_nodes - 3]
         wallet_name = "bad_deriv_path"
         node_v22.createwallet(wallet_name=wallet_name, descriptors=False)
         bad_deriv_wallet = node_v22.get_wallet_rpc(wallet_name)