Merge bitcoin/bitcoin#32553: wallet: Fix logging of wallet version

4b2cd0b test: check that creating a wallet does not log version info (Ava Chow)
39a483c test: Check that the correct versions are logged on wallet load (Ava Chow)
359ecd3 walletdb: Log the wallet version after it has been read from disk (Ava Chow)

Pull request description:

  The wallet's version (in the minversion record) needs to be logged only after we have read it from disk. Otherwise, we always log the lowest version number of 10500 which is incorrect. Furthermore, it doesn't make sense to log the last client version number if the record didn't exist. This is a regression caused by #26021.

  The wallet file version logging is moved inside of `LoadMinVersion` so that it is logged after the record is read. It will also log unconditionally if a version is read so that the version number is reported even when there is an error. The last client logging is split into its own log line that will only occur if a last client record is read. The only situation where we expect no version numbers to be logged is when a wallet is being created.

  A test is added in the second commit to check that the version number is correctly logged on loading. This commit can be cherrypicked to master to verify that it fails there. The last commit adds an additional check that creating a new wallet does not log any version info at all.

ACKs for top commit:
  laanwj:
    Code review ACK 4b2cd0b
  janb84:
    ACK bitcoin/bitcoin@4b2cd0b
  furszy:
    ACK 4b2cd0b
  rkrux:
    ACK 4b2cd0b

Tree-SHA512: b30c76f414d87be6c14b42d2d3c8794a91a7e8601501f4c24641d51ff2b5c5144776563baf41ca1c38415844740b760b19a3e5791f78013b39984dfedd3b1de7

Author: fanquake

src/wallet/walletdb.cpp

@@ -447,6 +447,7 @@ static DBErrors LoadMinVersion(CWallet* pwallet, DatabaseBatch& batch) EXCLUSIVE
     AssertLockHeld(pwallet->cs_wallet);
     int nMinVersion = 0;
     if (batch.Read(DBKeys::MINVERSION, nMinVersion)) {
+        pwallet->WalletLogPrintf("Wallet file version = %d\n", nMinVersion);
         if (nMinVersion > FEATURE_LATEST)
             return DBErrors::TOO_NEW;
         pwallet->LoadMinVersion(nMinVersion);
@@ -1156,7 +1157,7 @@ DBErrors WalletBatch::LoadWallet(CWallet* pwallet)
     // Last client version to open this wallet
     int last_client = CLIENT_VERSION;
     bool has_last_client = m_batch->Read(DBKeys::VERSION, last_client);
-    pwallet->WalletLogPrintf("Wallet file version = %d, last client version = %d\n", pwallet->GetVersion(), last_client);
+    if (has_last_client) pwallet->WalletLogPrintf("Last client version = %d\n", last_client);
 
     try {
         if ((result = LoadMinVersion(pwallet, *m_batch)) != DBErrors::LOAD_OK) return result;

test/functional/wallet_createwallet.py

@@ -165,6 +165,19 @@ def run_test(self):
         self.log.info("Test that legacy wallets cannot be created")
         assert_raises_rpc_error(-4, 'descriptors argument must be set to "true"; it is no longer possible to create a legacy wallet.', self.nodes[0].createwallet, wallet_name="legacy", descriptors=False)
 
+        self.log.info("Check that the version number is being logged correctly")
+        with node.assert_debug_log(expected_msgs=[], unexpected_msgs=["Last client version = ", "Wallet file version = "]):
+            node.createwallet("version_check")
+        wallet = node.get_wallet_rpc("version_check")
+        wallet_version = wallet.getwalletinfo()["walletversion"]
+        client_version = node.getnetworkinfo()["version"]
+        wallet.unloadwallet()
+        with node.assert_debug_log(
+            expected_msgs=[f"Last client version = {client_version}", f"Wallet file version = {wallet_version}"],
+            unexpected_msgs=["Wallet file version = 10500"]
+        ):
+            node.loadwallet("version_check")
+
 
 if __name__ == '__main__':
     CreateWalletTest(__file__).main()