backup files on write (#542)

* backup files

* fix tests

pyinstaller/hwibridge.py

@@ -10,7 +10,7 @@ if __name__ == "__main__":
             "version": 1,
             "formatters": {
                 "default": {
-                    "format": "[%(asctime)s] %(levelname)s in %(module)s: %(message)s",
+                    "format": "[%(asctime)s] %(levelname)s in %(module)s: %(message)s"
                 }
             },
             "handlers": {

src/cryptoadvance/specter/cli.py

@@ -98,11 +98,7 @@ def server(daemon, stop, restart, force, port, host, cert, key, debug, tor, hwib
         key = os.getenv("KEY", None)
 
     protocol = "http"
-    kwargs = {
-        "host": host,
-        "port": port,
-        "extra_files": extra_files,
-    }
+    kwargs = {"host": host, "port": port, "extra_files": extra_files}
     if cert is not None and key is not None:
         cert = os.path.abspath(cert)
         key = os.path.abspath(key)
@@ -270,7 +266,7 @@ if __name__ == "__main__":
             "version": 1,
             "formatters": {
                 "default": {
-                    "format": "[%(asctime)s] %(levelname)s in %(module)s: %(message)s",
+                    "format": "[%(asctime)s] %(levelname)s in %(module)s: %(message)s"
                 }
             },
             "handlers": {

src/cryptoadvance/specter/controller.py

@@ -33,7 +33,6 @@ from .helpers import (
     get_txid,
     generate_mnemonic,
     get_startblock_by_chain,
-    fslock,
     to_ascii20,
 )
 from .util.shell import run_shell
@@ -325,12 +324,7 @@ def register():
             return redirect("register?otp={}".format(otp))
         if app.specter.burn_new_user_otp(otp):
             config = {
-                "explorers": {
-                    "main": "",
-                    "test": "",
-                    "regtest": "",
-                    "signet": "",
-                },
+                "explorers": {"main": "", "test": "", "regtest": "", "signet": ""},
                 "hwi_bridge_url": "/hwi/api/",
             }
             user = User(user_id, username, password, config)

src/cryptoadvance/specter/device.py

@@ -1,6 +1,5 @@
 import json
 from .key import Key
-from .helpers import fslock
 from .persistence import read_json_file, write_json_file
 import logging
 
@@ -60,8 +59,7 @@ class Device:
         }
 
     def _update_keys(self):
-        with fslock:
-            write_json_file(self.json, self.fullpath)
+        write_json_file(self.json, self.fullpath)
         self.manager.update()
 
     def remove_key(self, key):
@@ -77,8 +75,7 @@ class Device:
     def rename(self, new_name):
         logger.info("Renaming {}".format(self.alias))
         self.name = new_name
-        with fslock:
-            write_json_file(self.json, self.fullpath)
+        write_json_file(self.json, self.fullpath)
         self.manager.update()
 
     def wallets(self, wallet_manager):

src/cryptoadvance/specter/device_manager.py

@@ -1,7 +1,7 @@
 import os
 import json
 import logging
-from .helpers import alias, load_jsons, fslock
+from .helpers import alias, load_jsons
 from .rpc import get_default_datadir
 
 from .devices import __all__ as device_classes

src/cryptoadvance/specter/devices/electrum.py

@@ -17,8 +17,5 @@ class Electrum(Device):
     def create_psbts(self, base64_psbt, wallet):
         # remove non_witness utxo for QR code
         updated_psbt = wallet.fill_psbt(base64_psbt, non_witness=False, xpubs=False)
-        psbts = {
-            "qrcode": b43_encode(a2b_base64(updated_psbt)),
-            "sdcard": base64_psbt,
-        }
+        psbts = {"qrcode": b43_encode(a2b_base64(updated_psbt)), "sdcard": base64_psbt}
         return psbts

src/cryptoadvance/specter/devices/generic.py

@@ -12,8 +12,5 @@ class GenericDevice(Device):
         super().__init__(name, alias, keys, fullpath, manager)
 
     def create_psbts(self, base64_psbt, wallet):
-        psbts = {
-            "qrcode": base64_psbt,
-            "sdcard": base64_psbt,
-        }
+        psbts = {"qrcode": base64_psbt, "sdcard": base64_psbt}
         return psbts

src/cryptoadvance/specter/devices/hwi/trezor.py

@@ -31,11 +31,7 @@ from hwilib.devices.trezorlib.ui import (
     PIN_MATRIX_DESCRIPTION,
     prompt,
 )
-from hwilib.devices.trezorlib import (
-    tools,
-    btc,
-    device,
-)
+from hwilib.devices.trezorlib import tools, btc, device
 from hwilib.devices.trezorlib import messages as proto
 from hwilib.base58 import (
     encode as base58_encode,
@@ -62,7 +58,9 @@ import logging
 import sys
 import struct
 
-py_enumerate = enumerate  # Need to use the enumerate built-in but there's another function already named that
+# Need to use the enumerate built-in
+# but there's another function already named that
+py_enumerate = enumerate
 
 # Only handles up to 15 of 15
 def parse_multisig(script):
@@ -748,9 +746,9 @@ def enumerate(password=""):
                 )
             if client.client.features.initialized:
                 d_data["fingerprint"] = client.get_master_fingerprint_hex()
-                d_data[
-                    "needs_passphrase_sent"
-                ] = False  # Passphrase is always needed for the above to have worked, so it's already sent
+                # Passphrase is always needed for the above to have worked,
+                # so it's already sent
+                d_data["needs_passphrase_sent"] = False
             else:
                 d_data["error"] = "Not initialized"
                 d_data["code"] = DEVICE_NOT_INITIALIZED

src/cryptoadvance/specter/helpers.py

@@ -12,7 +12,7 @@ import sys
 from collections import OrderedDict
 from mnemonic import Mnemonic
 from hwilib.serializations import PSBT, CTransaction
-from .persistence import read_json_file
+from .persistence import read_json_file, write_json_file
 from .util.descriptor import AddChecksum
 from .util.bcur import bcur_decode
 import threading
@@ -21,11 +21,11 @@ import re
 
 logger = logging.getLogger(__name__)
 
-# use this for all fs operations
-fslock = threading.Lock()
+# default lock for @locked()
+defaultlock = threading.Lock()
 
 
-def locked(customlock=fslock):
+def locked(customlock=defaultlock):
     """
     @locked(lock) decorator.
     Make sure you are not calling
@@ -113,12 +113,9 @@ def hwi_get_config(specter):
     config = {"whitelisted_domains": "http://127.0.0.1:25441/"}
     # if hwi_bridge_config.json file exists - load from it
     if os.path.isfile(os.path.join(specter.data_folder, "hwi_bridge_config.json")):
-        with fslock:
-            with open(
-                os.path.join(specter.data_folder, "hwi_bridge_config.json"), "r"
-            ) as f:
-                file_config = json.load(f)
-                deep_update(config, file_config)
+        fname = os.path.join(specter.data_folder, "hwi_bridge_config.json")
+        file_config = read_json_file(fname)
+        deep_update(config, file_config)
     # otherwise - create one and assign unique id
     else:
         save_hwi_bridge_config(specter, config)
@@ -134,11 +131,8 @@ def save_hwi_bridge_config(specter, config):
                 url += "/"
             whitelisted_domains += url.strip() + "\n"
         config["whitelisted_domains"] = whitelisted_domains
-    with fslock:
-        with open(
-            os.path.join(specter.data_folder, "hwi_bridge_config.json"), "w"
-        ) as f:
-            json.dump(config, f, indent=4)
+    fname = os.path.join(specter.data_folder, "hwi_bridge_config.json")
+    write_json_file(config, fname)
 
 
 def der_to_bytes(derivation):

src/cryptoadvance/specter/persistence.py

@@ -21,22 +21,67 @@ def read_json_file(path):
     """read_json_file from the .specter-directory. Don't use it for
     something else"""
     with fslock:
-        with open(path, "r") as f:
-            content = json.load(f)
+        bkp = path + ".bkp"
+        # try reading file
+        try:
+            with open(path, "r") as f:
+                content = json.load(f)
+        # if failed - try reading from the backup
+        except Exception as e:
+            # if no backup exists - raise
+            if not os.path.isfile(bkp):
+                raise e
+            # try loading from backup
+            with open(bkp, "r") as f:
+                content = json.load(f)
+            # recover from backup
+            if os.path.isfile(path):
+                os.remove(path)
+            os.rename(bkp, path)
+            logger.error(f"failed to open {path}, recovered from backup.")
         return content
 
 
+def _delete_folder(path):
+    """ Internal method which won't trigger the callback """
+    with fslock:
+        if os.path.exists(path):
+            shutil.rmtree(path)
+
+
+def _write_json_file(content, path, lock=None):
+    """ Internal method which won't trigger the callback """
+    with fslock:
+        # backup file
+        bkp = path + ".bkp"
+        # check if file exists
+        if os.path.isfile(path):
+            # check if backup exists
+            if os.path.isfile(bkp):
+                # remove backup file
+                os.remove(bkp)
+            # move file to backup
+            os.rename(path, bkp)
+        with open(path, "w") as f:
+            json.dump(content, f, indent=4)
+        # check if write was sucessfull
+        try:
+            with open(path, "r") as f:
+                c = json.load(f)
+        # if not - move back backup
+        except:
+            # remove damaged file
+            if os.path.isfile(path):
+                os.remove(path)
+            os.rename(bkp, path)
+            raise RuntimeError(f"Failed to write to file {path}")
+
+
 def write_json_file(content, path, lock=None):
     _write_json_file(content, path, lock)
     storage_callback()
 
 
-def _write_json_file(content, path, lock=None):
-    """ Internal method which won't trigger the callback """
-    with open(path, "w") as f:
-        json.dump(content, f, indent=4)
-
-
 def delete_json_file(path):
     if os.path.exists(path):
         os.remove(path)
@@ -45,15 +90,13 @@ def delete_json_file(path):
 
 def write_devices(devices_json):
     """ interpret a json as a list of devices and write them in the devices subfolder inside the specter-folder """
-    with fslock:
-        for device_json in devices_json:
-            _write_json_file(
-                device_json,
-                os.path.join(
-                    app.specter.device_manager.data_folder,
-                    "%s.json" % device_json["alias"],
-                ),
-            )
+    for device_json in devices_json:
+        _write_json_file(
+            device_json,
+            os.path.join(
+                app.specter.device_manager.data_folder, "%s.json" % device_json["alias"]
+            ),
+        )
     storage_callback()
 
 
@@ -61,28 +104,18 @@ def write_wallet(wallet_json):
     """interpret a json as wallet and writes it in the wallets subfolder inside the specter-folder.
     overwrites it if existing.
     """
-    with fslock:
-        with open(
-            os.path.join(
-                app.specter.wallet_manager.working_folder,
-                "%s.json" % wallet_json["alias"],
-            ),
-            "w",
-        ) as file:
-            file.write(json.dumps(wallet_json, indent=4))
+    fpath = os.path.join(
+        app.specter.wallet_manager.working_folder, "%s.json" % wallet_json["alias"]
+    )
+    _write_json_file(wallet_json, fpath)
     storage_callback()
 
 
 def write_device(device, fullpath):
-    _write_device(device, fullpath)
+    _write_json_file(device.json, fullpath)
     storage_callback()
 
 
-def _write_device(device, fullpath):
-    with open(fullpath, "w") as file:
-        file.write(json.dumps(device.json, indent=4))
-
-
 def delete_folder(path):
     _delete_folder(path)
     storage_callback()
@@ -94,11 +127,6 @@ def delete_folders(paths):
     storage_callback()
 
 
-def _delete_folder(path):
-    if os.path.exists(path):
-        shutil.rmtree(path)
-
-
 def storage_callback():
     if os.getenv("SPECTER_PERSISTENCE_CALLBACK"):
         result = run_shell(os.getenv("SPECTER_PERSISTENCE_CALLBACK").split(" "))

src/cryptoadvance/specter/rpc.py

@@ -57,12 +57,7 @@ def get_rpcconfig(datadir=get_default_datadir()):
                         current[k.strip()] = v.strip()
         except Exception:
             print("Can't open %s file" % bitcoin_conf_file)
-    folders = {
-        "main": "",
-        "test": "testnet3",
-        "regtest": "regtest",
-        "signet": "signet",
-    }
+    folders = {"main": "", "test": "testnet3", "regtest": "regtest", "signet": "signet"}
     for chain in folders:
         fname = os.path.join(datadir, folders[chain], ".cookie")
         if os.path.exists(fname):

src/cryptoadvance/specter/user.py

@@ -5,7 +5,6 @@ import binascii
 import json
 from flask_login import UserMixin
 from .specter_error import SpecterError
-from .helpers import fslock
 from .persistence import read_json_file, write_json_file, delete_folder
 
 

src/cryptoadvance/specter/util/shell.py

@@ -48,11 +48,7 @@ def run_shell(cmd):
     Returns: dict({"code": returncode, "out": stdout, "err": stderr})
     """
     try:
-        proc = subprocess.Popen(
-            cmd,
-            stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE,
-        )
+        proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
         stdout, stderr = proc.communicate()
         return {"code": proc.returncode, "out": stdout, "err": stderr}
     except Exception as e:

src/cryptoadvance/specter/util/version.py

@@ -32,11 +32,7 @@ class VersionChecker:
 
     @property
     def info(self):
-        return {
-            "current": self.current,
-            "latest": self.latest,
-            "upgrade": self.upgrade,
-        }
+        return {"current": self.current, "latest": self.latest, "upgrade": self.upgrade}
 
     def loop(self, dt=3600):
         """Checks for updates once per hour"""

src/cryptoadvance/specter/wallet.py

@@ -4,7 +4,7 @@ from hwilib.descriptor import AddChecksum
 from .device import Device
 from .key import Key
 from .util.merkleblock import is_valid_merkle_proof
-from .helpers import der_to_bytes, sort_descriptor, fslock, parse_utxo
+from .helpers import der_to_bytes, sort_descriptor, parse_utxo
 from .util.base58 import decode_base58
 from .util.descriptor import Descriptor
 from .util.xpub import get_xpub_fingerprint
@@ -696,9 +696,7 @@ class Wallet:
             # Find corresponding wallet key
             slip132_keys.append(LOOKUP_TABLE[desc_key])
 
-        to_return = {
-            "wallet_type": "{}of{}".format(self.sigs_required, len(self.keys)),
-        }
+        to_return = {"wallet_type": "{}of{}".format(self.sigs_required, len(self.keys))}
         for cnt, slip132_key in enumerate(slip132_keys):
             to_return["x{}/".format(cnt + 1)] = {
                 "derivation": slip132_key.derivation.replace("h", "'"),

tests/test_persistence.py

@@ -4,8 +4,14 @@ from cryptoadvance.specter.key import Key
 import json
 
 # count files
-def count_files_in(path):
-    return len([f for f in os.listdir(path) if os.path.isfile(os.path.join(path, f))])
+def count_files_in(path, extension=".json"):
+    return len(
+        [
+            f
+            for f in os.listdir(path)
+            if f.endswith(extension) and os.path.isfile(os.path.join(path, f))
+        ]
+    )
 
 
 def test_write_devices(app, monkeypatch, caplog):