feat: only require reindexing when the index was off going to off

fix: init additional indexes in AppInitMain, adjust tests Update test/functional/feature_timestampindex.py don't need to reindex to disable in tests
dashpay · Oct 24, 2024 · c96f9e0 · c96f9e0
1 parent 90ced24
commit c96f9e0
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 46 deletions.
diff --git a/src/init.cpp b/src/init.cpp
@@ -1965,24 +1965,32 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                     return InitError(_("Incorrect or no devnet genesis block found. Wrong datadir for devnet specified?"));
                 }
 
-                // Check for changed -addressindex state
-                if (fAddressIndex != args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX)) {
-                    strLoadError = _("You need to rebuild the database using -reindex to change -addressindex");
-                    break;
-                }
+                if (!fReset && !fReindexChainState) {
+                    // Check for changed -addressindex state
+                    if (!fAddressIndex && fAddressIndex != args.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX)) {
+                        strLoadError = _("You need to rebuild the database using -reindex to enable -addressindex");
+                        break;
+                    }
 
-                // Check for changed -timestampindex state
-                if (fTimestampIndex != args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX)) {
-                    strLoadError = _("You need to rebuild the database using -reindex to change -timestampindex");
-                    break;
-                }
+                    // Check for changed -timestampindex state
+                    if (!fTimestampIndex && fTimestampIndex != args.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX)) {
+                        strLoadError = _("You need to rebuild the database using -reindex to enable -timestampindex");
+                        break;
+                    }
 
-                // Check for changed -spentindex state
-                if (fSpentIndex != args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX)) {
-                    strLoadError = _("You need to rebuild the database using -reindex to change -spentindex");
-                    break;
+                    // Check for changed -spentindex state
+                    if (!fSpentIndex && fSpentIndex != args.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX)) {
+                        strLoadError = _("You need to rebuild the database using -reindex to enable -spentindex");
+                        break;
+                    }
                 }
 
+                chainman.InitAdditionalIndexes();
+
+                LogPrintf("%s: address index %s\n", __func__, fAddressIndex ? "enabled" : "disabled");
+                LogPrintf("%s: timestamp index %s\n", __func__, fTimestampIndex ? "enabled" : "disabled");
+                LogPrintf("%s: spent index %s\n", __func__, fSpentIndex ? "enabled" : "disabled");
+
                 // Check for changed -prune state.  What we are concerned about is a user who has pruned blocks
                 // in the past, but is now trying to run unpruned.
                 if (chainman.m_blockman.m_have_pruned && !fPruneMode) {

diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
@@ -394,16 +394,10 @@ bool BlockManager::LoadBlockIndexDB()
 
     // Check whether we have an address index
     m_block_tree_db->ReadFlag("addressindex", fAddressIndex);
-    LogPrintf("%s: address index %s\n", __func__, fAddressIndex ? "enabled" : "disabled");
-
     // Check whether we have a timestamp index
     m_block_tree_db->ReadFlag("timestampindex", fTimestampIndex);
-    LogPrintf("%s: timestamp index %s\n", __func__, fTimestampIndex ? "enabled" : "disabled");
-
     // Check whether we have a spent index
     m_block_tree_db->ReadFlag("spentindex", fSpentIndex);
-    LogPrintf("%s: spent index %s\n", __func__, fSpentIndex ? "enabled" : "disabled");
-
     return true;
 }
 

diff --git a/src/validation.cpp b/src/validation.cpp
@@ -4535,20 +4535,25 @@ bool ChainstateManager::LoadBlockIndex()
         // needs_init.
 
         LogPrintf("Initializing databases...\n");
+        InitAdditionalIndexes();
+    }
+    return true;
+}
 
-        // Use the provided setting for -addressindex in the new database
-        fAddressIndex = gArgs.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX);
-        m_blockman.m_block_tree_db->WriteFlag("addressindex", fAddressIndex);
+void ChainstateManager::InitAdditionalIndexes()
+{
+    // Use the provided setting for -addressindex in the new database
+    fAddressIndex = gArgs.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX);
+    m_blockman.m_block_tree_db->WriteFlag("addressindex", fAddressIndex);
 
-        // Use the provided setting for -timestampindex in the new database
-        fTimestampIndex = gArgs.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX);
-        m_blockman.m_block_tree_db->WriteFlag("timestampindex", fTimestampIndex);
+    // Use the provided setting for -timestampindex in the new database
+    fTimestampIndex = gArgs.GetBoolArg("-timestampindex", DEFAULT_TIMESTAMPINDEX);
+    m_blockman.m_block_tree_db->WriteFlag("timestampindex", fTimestampIndex);
+
+    // Use the provided setting for -spentindex in the new database
+    fSpentIndex = gArgs.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX);
+    m_blockman.m_block_tree_db->WriteFlag("spentindex", fSpentIndex);
 
-        // Use the provided setting for -spentindex in the new database
-        fSpentIndex = gArgs.GetBoolArg("-spentindex", DEFAULT_SPENTINDEX);
-        m_blockman.m_block_tree_db->WriteFlag("spentindex", fSpentIndex);
-    }
-    return true;
 }
 
 bool CChainState::AddGenesisBlock(const CBlock& block, BlockValidationState& state)

diff --git a/src/validation.h b/src/validation.h
@@ -1044,6 +1044,8 @@ class ChainstateManager
 
     //! Load the block tree and coins database from disk, initializing state if we're running with -reindex
     bool LoadBlockIndex() EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+    //! Initialize additional indexes and store their flags to disk
+    void InitAdditionalIndexes() EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
     //! Check to see if caches are out of balance and if so, call
     //! ResizeCoinsCaches() as needed.

diff --git a/test/functional/feature_addressindex.py b/test/functional/feature_addressindex.py
@@ -38,14 +38,13 @@ def setup_network(self):
         self.import_deterministic_coinbase_privkeys()
 
     def run_test(self):
-        self.log.info("Test that settings can't be changed without -reindex...")
-        self.stop_node(1)
-        self.nodes[1].assert_start_raises_init_error(["-addressindex=0"], "You need to rebuild the database using -reindex to change -addressindex", match=ErrorMatch.PARTIAL_REGEX)
-        self.start_node(1, ["-addressindex=0", "-reindex"])
+        self.log.info("Test that settings can be disabled without -reindex...")
+        self.restart_node(1, ["-addressindex=0"])
         self.connect_nodes(0, 1)
         self.sync_all()
+        self.log.info("Test that settings can't be enabled without -reindex...")
         self.stop_node(1)
-        self.nodes[1].assert_start_raises_init_error(["-addressindex"], "You need to rebuild the database using -reindex to change -addressindex", match=ErrorMatch.PARTIAL_REGEX)
+        self.nodes[1].assert_start_raises_init_error(["-addressindex"], "You need to rebuild the database using -reindex to enable -addressindex", match=ErrorMatch.PARTIAL_REGEX)
         self.start_node(1, ["-addressindex", "-reindex"])
         self.connect_nodes(0, 1)
         self.sync_all()

diff --git a/test/functional/feature_spentindex.py b/test/functional/feature_spentindex.py
@@ -40,14 +40,13 @@ def setup_network(self):
         self.import_deterministic_coinbase_privkeys()
 
     def run_test(self):
-        self.log.info("Test that settings can't be changed without -reindex...")
-        self.stop_node(1)
-        self.nodes[1].assert_start_raises_init_error(["-spentindex=0"], "You need to rebuild the database using -reindex to change -spentindex", match=ErrorMatch.PARTIAL_REGEX)
-        self.start_node(1, ["-spentindex=0", "-reindex"])
+        self.log.info("Test that settings can be disabled without -reindex...")
+        self.restart_node(1, ["-spentindex=0"])
         self.connect_nodes(0, 1)
         self.sync_all()
+        self.log.info("Test that settings can't be enabled without -reindex...")
         self.stop_node(1)
-        self.nodes[1].assert_start_raises_init_error(["-spentindex"], "You need to rebuild the database using -reindex to change -spentindex", match=ErrorMatch.PARTIAL_REGEX)
+        self.nodes[1].assert_start_raises_init_error(["-spentindex"], "You need to rebuild the database using -reindex to enable -spentindex", match=ErrorMatch.PARTIAL_REGEX)
         self.start_node(1, ["-spentindex", "-reindex"])
         self.connect_nodes(0, 1)
         self.sync_all()

diff --git a/test/functional/feature_timestampindex.py b/test/functional/feature_timestampindex.py
@@ -33,14 +33,13 @@ def setup_network(self):
         self.sync_all()
 
     def run_test(self):
-        self.log.info("Test that settings can't be changed without -reindex...")
-        self.stop_node(1)
-        self.nodes[1].assert_start_raises_init_error(["-timestampindex=0"], "You need to rebuild the database using -reindex to change -timestampindex", match=ErrorMatch.PARTIAL_REGEX)
-        self.start_node(1, ["-timestampindex=0", "-reindex"])
+        self.log.info("Test that settings can be disabled without -reindex...")
+        self.restart_node(1, ["-timestampindex=0"])
         self.connect_nodes(0, 1)
         self.sync_all()
+        self.log.info("Test that settings can't be enabled without -reindex...")
         self.stop_node(1)
-        self.nodes[1].assert_start_raises_init_error(["-timestampindex"], "You need to rebuild the database using -reindex to change -timestampindex", match=ErrorMatch.PARTIAL_REGEX)
+        self.nodes[1].assert_start_raises_init_error(["-timestampindex"], "You need to rebuild the database using -reindex to enable -timestampindex", match=ErrorMatch.PARTIAL_REGEX)
         self.start_node(1, ["-timestampindex", "-reindex"])
         self.connect_nodes(0, 1)
         self.sync_all()