Merge #15588: Log the actual wallet file version and no longer publicly expose the "version" record

35e60e790f2cd602d1bdd0be835d27f0ba37efa9 Remove ReadVersion and WriteVersion (Andrew Chow)
b3d4f6c9619142948ab3d53551b4f3c0d7d73bde Log the actual wallet file version (Andrew Chow)
c88e87c3b2be3f97b712107e04285d06dfef3878 Remove nFileVersion from CWalletScanState (Andrew Chow)

Pull request description:

  The wallet file version is stored in the "minversion" record, not the "version" record. However "version" is no longer used anywhere except to record the highest versioned client which has opened a wallet file (which is currently only used to check whether this was most recently opened by a 0.4.0 or 0.5.0rc1 client which had a broken wallet encryption implementation). Furthermore, "version" was logged to the debug.log which is confusing because it is not the actual wallet file version.

  This PR changes it so that this confusion largely no longer exists. The wallet file version logging is changed to use "minversion" and reading and writing the "version" record is no longer publicly exposed to prevent potential confusion about whether the actual file version is being read or written. Lastly, in the one place it is actually used, the variable name is changed from nFileVersion to last_client to better reflect what that record actually represents.

ACKs for top commit:
  jb55:
    ACK 35e60e7, I compiled locally as a quick sanity check.
  ryanofsky:
    utACK 35e60e790f2cd602d1bdd0be835d27f0ba37efa9. This code still pretty confusing, but a little simpler now. And the previous log statement was really misleading and useless compared to the new one here.
  meshcollider:
    Looks good, thanks! utACK 35e60e790f2cd602d1bdd0be835d27f0ba37efa9

Tree-SHA512: f782b2f215d07fbc9b806322bda8085445b81c02b65ca674a8c6a3e1de505a0abd050669afe0ead4778816144a1c18462e13930071cedb7227a058aeb39493f7

4 files changed, 11 insertions, 38 deletions

diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp
index 852b194386..26aeb754ad 100644
--- a/src/wallet/db.cpp
+++ b/src/wallet/db.cpp
@@ -585,7 +585,7 @@ BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode, bo
             if (fCreate && !Exists(std::string("version"))) {
                 bool fTmp = fReadOnly;
                 fReadOnly = false;
-                WriteVersion(CLIENT_VERSION);
+                Write(std::string("version"), CLIENT_VERSION);
                 fReadOnly = fTmp;
             }
         }
diff --git a/src/wallet/db.h b/src/wallet/db.h
index b3856fbaf9..94f41eaf16 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -399,17 +399,6 @@ public:
         return (ret == 0);
     }
 
-    bool ReadVersion(int& nVersion)
-    {
-        nVersion = 0;
-        return Read(std::string("version"), nVersion);
-    }
-
-    bool WriteVersion(int nVersion)
-    {
-        return Write(std::string("version"), nVersion);
-    }
-
     bool static Rewrite(BerkeleyDatabase& database, const char* pszSkip = nullptr);
 };
 
diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp
index ece97e2a75..43dd28b675 100644
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -165,7 +165,6 @@ public:
     unsigned int m_unknown_records{0};
     bool fIsEncrypted{false};
     bool fAnyUnordered{false};
-    int nFileVersion{0};
     std::vector<uint256> vWalletUpgrade;
 
     CWalletScanState() {
@@ -376,12 +375,6 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
 
             pwallet->LoadKeyPool(nIndex, keypool);
         }
-        else if (strType == "version")
-        {
-            ssValue >> wss.nFileVersion;
-            if (wss.nFileVersion == 10300)
-                wss.nFileVersion = 300;
-        }
         else if (strType == "cscript")
         {
             uint160 hash;
@@ -419,7 +412,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
                 return false;
             }
         } else if (strType != "bestblock" && strType != "bestblock_nomerkle" &&
-                strType != "minversion" && strType != "acentry") {
+                strType != "minversion" && strType != "acentry" && strType != "version") {
             wss.m_unknown_records++;
         }
     } catch (const std::exception& e) {
@@ -519,7 +512,12 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
     if (result != DBErrors::LOAD_OK)
         return result;
 
-    pwallet->WalletLogPrintf("nFileVersion = %d\n", wss.nFileVersion);
+    // Last client version to open this wallet, was previously the file version number
+    int last_client = CLIENT_VERSION;
+    m_batch.Read(std::string("version"), last_client);
+
+    int wallet_version = pwallet->GetVersion();
+    pwallet->WalletLogPrintf("Wallet File Version = %d\n", wallet_version > 0 ? wallet_version : last_client);
 
     pwallet->WalletLogPrintf("Keys: %u plaintext, %u encrypted, %u w/ metadata, %u total. Unknown wallet records: %u\n",
            wss.nKeys, wss.nCKeys, wss.nKeyMeta, wss.nKeys + wss.nCKeys, wss.m_unknown_records);
@@ -532,11 +530,11 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
         WriteTx(pwallet->mapWallet.at(hash));
 
     // Rewrite encrypted wallets of versions 0.4.0 and 0.5.0rc:
-    if (wss.fIsEncrypted && (wss.nFileVersion == 40000 || wss.nFileVersion == 50000))
+    if (wss.fIsEncrypted && (last_client == 40000 || last_client == 50000))
         return DBErrors::NEED_REWRITE;
 
-    if (wss.nFileVersion < CLIENT_VERSION) // Update
-        WriteVersion(CLIENT_VERSION);
+    if (last_client < CLIENT_VERSION) // Update
+        m_batch.Write(std::string("version"), CLIENT_VERSION);
 
     if (wss.fAnyUnordered)
         result = pwallet->ReorderTransactions();
@@ -779,13 +777,3 @@ bool WalletBatch::TxnAbort()
 {
     return m_batch.TxnAbort();
 }
-
-bool WalletBatch::ReadVersion(int& nVersion)
-{
-    return m_batch.ReadVersion(nVersion);
-}
-
-bool WalletBatch::WriteVersion(int nVersion)
-{
-    return m_batch.WriteVersion(nVersion);
-}
diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h
index d4a3bba97a..90692317ea 100644
--- a/src/wallet/walletdb.h
+++ b/src/wallet/walletdb.h
@@ -249,10 +249,6 @@ public:
     bool TxnCommit();
     //! Abort current transaction
     bool TxnAbort();
-    //! Read wallet version
-    bool ReadVersion(int& nVersion);
-    //! Write wallet version
-    bool WriteVersion(int nVersion);
 private:
     BerkeleyBatch m_batch;
     WalletDatabase& m_database;