json_db: enforce order of 'load_transactions' and 'upgrade'

fixes #5331

electrum/json_db.py

@@ -59,11 +59,12 @@ class JsonDB(Logger):
         self.data = {}
         self._modified = False
         self.manual_upgrades = manual_upgrades
-        if raw:
+        self._called_load_transactions = False
+        if raw:  # loading existing db
             self.load_data(raw)
-        else:
+        else:  # creating new db
             self.put('seed_version', FINAL_SEED_VERSION)
-        self.load_transactions()
+            self.load_transactions()
 
     def set_modified(self, b):
         with self.lock:
@@ -145,11 +146,13 @@ class JsonDB(Logger):
         if not isinstance(self.data, dict):
             raise WalletFileException("Malformed wallet file (not dict)")
 
-        if not self.manual_upgrades:
+        if not self.manual_upgrades and self.requires_split():
-            if self.requires_split():
+            raise WalletFileException("This wallet has multiple accounts and must be split")
-                raise WalletFileException("This wallet has multiple accounts and must be split")
+
-            if self.requires_upgrade():
+        self.load_transactions()
-                self.upgrade()
+
+        if not self.manual_upgrades and self.requires_upgrade():
+            self.upgrade()
 
     def requires_split(self):
         d = self.get('accounts', {})
@@ -201,6 +204,11 @@ class JsonDB(Logger):
     @profiler
     def upgrade(self):
         self.logger.info('upgrading wallet format')
+        if not self._called_load_transactions:
+            # note: not sure if this is how we should go about this...
+            # alternatively, we could make sure load_transactions is always called after upgrade
+            # still, we need strict ordering between the two.
+            raise Exception("'load_transactions' must be called before 'upgrade'")
         self._convert_imported()
         self._convert_wallet_type()
         self._convert_account()
@@ -738,6 +746,7 @@ class JsonDB(Logger):
 
     @profiler
     def load_transactions(self):
+        self._called_load_transactions = True
         # references in self.data
         self.txi = self.get_data_ref('txi')  # txid -> address -> list of (prev_outpoint, value)
         self.txo = self.get_data_ref('txo')  # txid -> address -> list of (output_index, value, is_coinbase)

electrum/tests/test_storage_upgrade.py

@@ -1,5 +1,6 @@
 import shutil
 import tempfile
+import os
 
 from electrum.storage import WalletStorage
 from electrum.wallet import Wallet
@@ -290,14 +291,25 @@ class TestStorageUpgrade(WalletTestCase):
         shutil.rmtree(cls.electrum_path)
 
     def _upgrade_storage(self, wallet_json, accounts=1):
-        storage = self._load_storage_from_json_string(wallet_json, manual_upgrades=True)
-
         if accounts == 1:
+            # test manual upgrades
+            storage = self._load_storage_from_json_string(wallet_json=wallet_json,
+                                                          path=self.wallet_path,
+                                                          manual_upgrades=True)
             self.assertFalse(storage.requires_split())
             if storage.requires_upgrade():
                 storage.upgrade()
                 self._sanity_check_upgraded_storage(storage)
+            # test automatic upgrades
+            path2 = os.path.join(self.user_dir, "somewallet2")
+            storage2 = self._load_storage_from_json_string(wallet_json=wallet_json,
+                                                           path=path2,
+                                                           manual_upgrades=False)
+            self._sanity_check_upgraded_storage(storage2)
         else:
+            storage = self._load_storage_from_json_string(wallet_json=wallet_json,
+                                                          path=self.wallet_path,
+                                                          manual_upgrades=True)
             self.assertTrue(storage.requires_split())
             new_paths = storage.split_accounts()
             self.assertEqual(accounts, len(new_paths))
@@ -310,8 +322,9 @@ class TestStorageUpgrade(WalletTestCase):
         self.assertFalse(storage.requires_upgrade())
         w = Wallet(storage)
 
-    def _load_storage_from_json_string(self, wallet_json, manual_upgrades=True):
+    @staticmethod
-        with open(self.wallet_path, "w") as f:
+    def _load_storage_from_json_string(*, wallet_json, path, manual_upgrades):
+        with open(path, "w") as f:
             f.write(wallet_json)
-        storage = WalletStorage(self.wallet_path, manual_upgrades=manual_upgrades)
+        storage = WalletStorage(path, manual_upgrades=manual_upgrades)
         return storage