backup: Rely solely on backup.lock and do not cache writes

Caching writes was causing us some issues if the startup of
`lightningd` didn't complete, i.e., the writes would happen on the
live DB, but we'd forget about them. Since we have the backup.lock
file in same directory as the plugin is running in, we no longer have
to defer the writes. This hugely simplifies our logic.

Fixes #155

Changelog-Fixed: backup: The plugin doesn't lose sync anymore if the startup is interrupted

backup/backup.py

@@ -289,52 +289,14 @@ def check_first_write(plugin, data_version):
 @plugin.hook('db_write')
 def on_db_write(writes, data_version, plugin, **kwargs):
     change = Change(data_version, None, writes)
-    if not hasattr(plugin, 'backend'):
-        plugin.early_writes.append(change)
-        return {"result": "continue"}
-    else:
-        return apply_write(plugin, change)
-
-
-def apply_write(plugin, change):
     if not plugin.initialized:
         assert(check_first_write(plugin, change.version))
         plugin.initialized = True
 
     if plugin.backend.add_change(change):
         return {"result": "continue"}
-
+    else:
-
+        kill("Could not append DB change to the backup. Need to shutdown!")
-@plugin.init()
-def on_init(options: Mapping[str, str], plugin: Plugin, **kwargs):
-    # Reach into the DB and
-    configs = plugin.rpc.listconfigs()
-    plugin.db_path = configs['wallet']
-    destination = options['backup-destination']
-
-    # Ensure that we don't inadventently switch the destination
-    if not os.path.exists("backup.lock"):
-        print("Files in the current directory {}".format(", ".join(os.listdir("."))))
-        kill("Could not find backup.lock in the lightning-dir, have you initialized using the backup-cli utility?")
-
-    d = json.load(open("backup.lock", 'r'))
-    if destination is None or destination == 'null':
-        destination = d['backend_url']
-    elif destination != d['backend_url']:
-        kill(
-            "The destination specified as option does not match the one "
-            "specified in backup.lock. Please check your settings"
-        )
-
-    if not plugin.db_path.startswith('sqlite3'):
-        kill("The backup plugin only works with the sqlite3 database.")
-
-    plugin.backend = get_backend(destination, require_init=True)
-
-    for c in plugin.early_writes:
-        apply_write(plugin, c)
-
-
 
 
 def kill(message: str):
@@ -356,11 +318,6 @@ def kill(message: str):
         time.sleep(30)
 
 
-def preflight(plugin: Plugin):
-    """Perform some preflight checks.
-    """
-    if not os.path.exists("backup.lock"):
-        kill("Could not find backup.lock in the lightning-dir")
 
 plugin.add_option(
     'backup-destination', None,
@@ -369,8 +326,12 @@ plugin.add_option(
 
 
 if __name__ == "__main__":
-    preflight(plugin)
+    # Did we perform the first write check?
-    # Did we perform the version check of backend versus the first write?
     plugin.initialized = False
-    plugin.early_writes = []
+    if not os.path.exists("backup.lock"):
+        kill("Could not find backup.lock in the lightning-dir")
+
+    d = json.load(open("backup.lock", 'r'))
+    destination = d['backend_url']
+    plugin.backend = get_backend(destination, require_init=True)
     plugin.run()

backup/test_backup.py

@@ -11,6 +11,8 @@ plugin_dir = os.path.dirname(__file__)
 plugin_path = os.path.join(plugin_dir, "backup.py")
 cli_path = os.path.join(os.path.dirname(__file__), "backup-cli")
 
+# For the transition period we require deprecated_apis to be true
+deprecated_apis = True
 
 def test_start(node_factory, directory):
     bpath = os.path.join(directory, 'lightning-1', 'regtest')
@@ -19,16 +21,17 @@ def test_start(node_factory, directory):
     subprocess.check_call([cli_path, "init", bpath, bdest])
     opts = {
         'plugin': plugin_path,
-        'backup-destination': bdest,
+        'allow-deprecated-apis': deprecated_apis,
         }
     l1 = node_factory.get_node(options=opts, cleandir=False)
-
+    plugins = [os.path.basename(p['name']) for p in l1.rpc.plugin("list")['plugins']]
-    l1.daemon.wait_for_log(r'backup.py')
+    assert("backup.py" in plugins)
 
     # Restart the node a couple of times, to check that we can resume normally
     for i in range(5):
         l1.restart()
-        l1.daemon.wait_for_log(r'Versions match up')
+        plugins = [os.path.basename(p['name']) for p in l1.rpc.plugin("list")['plugins']]
+        assert("backup.py" in plugins)
 
 
 def test_start_no_init(node_factory, directory):
@@ -39,7 +42,6 @@ def test_start_no_init(node_factory, directory):
     os.makedirs(bpath)
     opts = {
         'plugin': plugin_path,
-        'backup-destination': bdest,
     }
     l1 = node_factory.get_node(
         options=opts, cleandir=False, may_fail=True, start=False
@@ -70,8 +72,9 @@ def test_init_not_empty(node_factory, directory):
 
     # Now restart and add the plugin
     l1.daemon.opts['plugin'] = plugin_path
+    l1.daemon.opts['allow-deprecated-apis'] = deprecated_apis
     l1.start()
-    l1.daemon.wait_for_log(r'plugin-backup.py: Versions match up')
+    assert(l1.daemon.is_in_log(r'plugin-backup.py: Versions match up'))
 
 
 def test_tx_abort(node_factory, directory):
@@ -90,7 +93,7 @@ def test_tx_abort(node_factory, directory):
     subprocess.check_call([cli_path, "init", bpath, bdest])
     opts = {
         'plugin': plugin_path,
-        'backup-destination': bdest,
+        'allow-deprecated-apis': deprecated_apis,
         }
     l1 = node_factory.get_node(options=opts, cleandir=False)
     l1.stop()
@@ -103,7 +106,7 @@ def test_tx_abort(node_factory, directory):
     print(l1.db.query("SELECT * FROM vars;"))
 
     l1.restart()
-    l1.daemon.wait_for_log(r'Last changes not applied')
+    assert(l1.daemon.is_in_log(r'Last changes not applied'))
 
 
 @flaky
@@ -120,7 +123,7 @@ def test_failing_restore(node_factory, directory):
     subprocess.check_call([cli_path, "init", bpath, bdest])
     opts = {
         'plugin': plugin_path,
-        'backup-destination': bdest,
+        'allow-deprecated-apis': deprecated_apis,
         }
 
     def section(comment):
@@ -153,13 +156,12 @@ def test_intermittent_backup(node_factory, directory):
     subprocess.check_call([cli_path, "init", bpath, bdest])
     opts = {
         'plugin': plugin_path,
-        'backup-destination': bdest,
+        'allow-deprecated-apis': deprecated_apis,
         }
     l1 = node_factory.get_node(options=opts, cleandir=False, may_fail=True)
 
     # Now start without the plugin. This should work fine.
     del l1.daemon.opts['plugin']
-    del l1.daemon.opts['backup-destination']
     l1.restart()
 
     # Now restart adding the plugin again, and it should fail due to gaps in
@@ -180,7 +182,7 @@ def test_restore(node_factory, directory):
     subprocess.check_call([cli_path, "init", bpath, bdest])
     opts = {
         'plugin': plugin_path,
-        'backup-destination': bdest,
+        'allow-deprecated-apis': deprecated_apis,
         }
     l1 = node_factory.get_node(options=opts, cleandir=False)
     l1.stop()