Merge pull request #5827 from ethereum/minorversion

Write database minor version to disk and fix rebuild on minor version mismatch bug

CHANGELOG.md

@@ -14,6 +14,7 @@
 - Fixed: [#5811](https://github.com/ethereum/aleth/pull/5811) RPC methods querying transactions (`eth_getTransactionByHash`, `eth_getBlockByNumber`) return correct `v` value.
 - Fixed: [#5821](https://github.com/ethereum/aleth/pull/5821) `test_setChainParams` correctly initializes custom configuration of precompiled contracts.
 - Fixed: [#5826](https://github.com/ethereum/aleth/pull/5826) Fix blocking bug in database rebuild functionality - users can now rebuild their databases via Aleth's '-R' switch.
+- Fixed: [#5827](https://github.com/ethereum/aleth/pull/5827) Detect database upgrades and automatically rebuild the database when they occur.
 
 ## [1.7.0] - 2019-11-14
 

libethcore/Common.cpp

@@ -23,11 +23,11 @@ namespace eth
 
 const unsigned c_protocolVersion = 63;
 #if ETH_FATDB
-const unsigned c_minorProtocolVersion = 3;
+const unsigned c_databaseMinorVersion = 3;
 const unsigned c_databaseBaseVersion = 9;
 const unsigned c_databaseVersionModifier = 1;
 #else
-const unsigned c_minorProtocolVersion = 2;
+const unsigned c_databaseMinorVersion = 2;
 const unsigned c_databaseBaseVersion = 9;
 const unsigned c_databaseVersionModifier = 0;
 #endif

libethcore/Common.h

@@ -23,8 +23,8 @@ namespace eth
 /// Current protocol version.
 extern const unsigned c_protocolVersion;
 
-/// Current minor protocol version.
-extern const unsigned c_minorProtocolVersion;
+/// Current minor database version (for the extras database).
+extern const unsigned c_databaseMinorVersion;
 
 /// Current database version.
 extern const unsigned c_databaseVersion;

libethereum/BlockChain.cpp

@@ -192,42 +192,50 @@ void BlockChain::init(ChainParams const& _p)
     genesis();
 }
 
-unsigned BlockChain::open(fs::path const& _path, WithExisting _we)
+bool BlockChain::open(fs::path const& _path, WithExisting _we)
 {
     auto const path = _path.empty() ? db::databasePath() : _path;
     auto const chainPath = path / fs::path(toHex(m_genesisHash.ref().cropped(0, 4)));
     auto const chainSubPathBlocks = chainPath / fs::path("blocks");
     auto const extrasPath = chainPath / fs::path(toString(c_databaseVersion));
     auto const extrasSubPathExtras = extrasPath / fs::path("extras");
-    unsigned lastMinor = c_minorProtocolVersion;
+    unsigned lastMinor = c_databaseMinorVersion;
+    bool rebuildNeeded = false;
 
     if (db::isDiskDatabase())
     {
-        fs::create_directories(extrasPath);
-        DEV_IGNORE_EXCEPTIONS(fs::permissions(extrasPath, fs::owner_all));
-
-        auto const extrasSubPathMinor = extrasPath / fs::path("minor");
-        bytes const status = contents(extrasSubPathMinor);
-        if (!status.empty())
-            DEV_IGNORE_EXCEPTIONS(lastMinor = (unsigned)RLP(status));
-        if (c_minorProtocolVersion != lastMinor)
-        {
-            cnote << "Killing extras database " << extrasPath << " (DB minor version:" << lastMinor
-                  << " != our minor version: " << c_minorProtocolVersion << ").";
-            DEV_IGNORE_EXCEPTIONS(fs::remove_all(extrasPath / fs::path("details.old")));
-            fs::rename(extrasSubPathExtras, extrasPath / fs::path("extras.old"));
-            fs::remove_all(extrasPath / fs::path("state"));
-            writeFile(extrasSubPathMinor, rlp(c_minorProtocolVersion));
-            lastMinor = (unsigned)RLP(status);
-        }
-
         if (_we == WithExisting::Kill)
         {
             cnote << "Killing blockchain (" << chainSubPathBlocks << ") & extras ("
                   << extrasSubPathExtras << ") databases (WithExisting::Kill).";
             fs::remove_all(chainSubPathBlocks);
             fs::remove_all(extrasSubPathExtras);
         }
+
+        fs::create_directories(extrasPath);
+        DEV_IGNORE_EXCEPTIONS(fs::permissions(extrasPath, fs::owner_all));
+
+        auto const extrasSubPathMinor = extrasPath / fs::path("minor");
+        bytes const minorVersionBytes = contents(extrasSubPathMinor);
+        bool writeMinorVersion = false;
+        if (!minorVersionBytes.empty())
+        {
+            DEV_IGNORE_EXCEPTIONS(lastMinor = (unsigned)RLP(minorVersionBytes));
+            if (c_databaseMinorVersion != lastMinor)
+            {
+                rebuildNeeded = true;
+                LOG(m_loggerInfo) << "Database minor version change detected, the extras and state "
+                                     "databases will be rebuilt.";
+                LOG(m_loggerInfo) << "Version from " << extrasSubPathMinor << " (" << lastMinor
+                                  << ") != Aleth's version (" << c_databaseMinorVersion << ")";
+                writeMinorVersion = true;
+                lastMinor = (unsigned)RLP(minorVersionBytes);
+            }
+        }
+        else
+            writeMinorVersion = true;
+        if (writeMinorVersion)
+            writeFile(extrasSubPathMinor, rlp(c_databaseMinorVersion));
     }
 
     try
@@ -268,7 +276,7 @@ unsigned BlockChain::open(fs::path const& _path, WithExisting _we)
         throw;
     }
 
-    if (_we != WithExisting::Verify && !details(m_genesisHash))
+    if (_we != WithExisting::Verify && !rebuildNeeded && !details(m_genesisHash))
     {
         BlockHeader gb(m_params.genesisBlock());
         // Insert details of genesis block.
@@ -284,13 +292,14 @@ unsigned BlockChain::open(fs::path const& _path, WithExisting _we)
 
     m_lastBlockNumber = number(m_lastBlockHash);
 
-    ctrace << "Opened blockchain DB. Latest: " << currentHash() << (lastMinor == c_minorProtocolVersion ? "(rebuild not needed)" : "*** REBUILD NEEDED ***");
-    return lastMinor;
+    ctrace << "Opened blockchain DB. Latest: " << currentHash()
+           << (!rebuildNeeded ? "(rebuild not needed)" : "*** REBUILD NEEDED ***");
+    return rebuildNeeded;
 }
 
 void BlockChain::open(fs::path const& _path, WithExisting _we, ProgressCallback const& _pc)
 {
-    if (open(_path, _we) != c_minorProtocolVersion || _we == WithExisting::Verify)
+    if (open(_path, _we) || _we == WithExisting::Verify)
         rebuild(_path, _pc);
 }
 

libethereum/BlockChain.h

@@ -314,8 +314,8 @@ class BlockChain
 
     /// Initialise everything and ready for openning the database.
     void init(ChainParams const& _p);
-    /// Open the database.
-    unsigned open(boost::filesystem::path const& _path, WithExisting _we);
+    /// Open the database. Returns whether or not the database needs to be rebuilt.
+    bool open(boost::filesystem::path const& _path, WithExisting _we);
     /// Open the database, rebuilding if necessary.
     void open(boost::filesystem::path const& _path, WithExisting _we, ProgressCallback const& _pc);
     /// Finalise everything and close the database.