lightningd: test that hsm_secret is as expected, at startup.

If you get the wrong hsm_secret, your node_id will change, and
peers won't know who you are, bitcoind will reject your transaction
signatures, and other madness.

Catch this as soon as it happens, by storing our node_id in the db.

Suggested-by: @cdecker, @fiatjaf
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Config: `lightningd` will refuse to start with the wrong node_id (i.e. hsm_secret changes).

lightningd/lightningd.c

@@ -1048,9 +1048,12 @@ int main(int argc, char *argv[])
 
 	/*~ Our default names, eg. for the database file, are not dependent on
 	 * the network.  Instead, the db knows what chain it belongs to, and we
-	 * simple barf here if it's wrong. */
+	 * simple barf here if it's wrong.
-	if (!wallet_network_check(ld->wallet))
+	 *
-		errx(1, "Wallet network check failed.");
+	 * We also check that our node_id is what we expect: otherwise a change
+	 * in hsm_secret will have strange consequences! */
+	if (!wallet_sanity_check(ld->wallet))
+		errx(1, "Wallet sanity check failed.");
 
 	/*~ Initialize the transaction filter with our pubkeys. */
 	init_txfilter(ld->wallet, ld->owned_txfilter);

lightningd/test/run-find_my_abspath.c

@@ -223,13 +223,13 @@ void waitblockheight_notify_new_block(struct lightningd *ld UNNEEDED,
 /* Generated stub for wallet_blocks_heights */
 void wallet_blocks_heights(struct wallet *w UNNEEDED, u32 def UNNEEDED, u32 *min UNNEEDED, u32 *max UNNEEDED)
 { fprintf(stderr, "wallet_blocks_heights called!\n"); abort(); }
-/* Generated stub for wallet_network_check */
-bool wallet_network_check(struct wallet *w UNNEEDED)
-{ fprintf(stderr, "wallet_network_check called!\n"); abort(); }
 /* Generated stub for wallet_new */
 struct wallet *wallet_new(struct lightningd *ld UNNEEDED, struct timers *timers UNNEEDED,
 			  struct ext_key *bip32_base UNNEEDED)
 { fprintf(stderr, "wallet_new called!\n"); abort(); }
+/* Generated stub for wallet_sanity_check */
+bool wallet_sanity_check(struct wallet *w UNNEEDED)
+{ fprintf(stderr, "wallet_sanity_check called!\n"); abort(); }
 /* AUTOGENERATED MOCKS END */
 
 struct log *crashlog;

tests/test_db.py

@@ -2,7 +2,7 @@ from decimal import Decimal
 from fixtures import *  # noqa: F401,F403
 from fixtures import TEST_NETWORK
 from pyln.client import RpcError
-from utils import wait_for, sync_blockheight, COMPAT, VALGRIND, DEVELOPER, only_one
+from utils import wait_for, sync_blockheight, COMPAT, VALGRIND, DEVELOPER, TIMEOUT, only_one
 
 import base64
 import os
@@ -413,3 +413,30 @@ def test_sqlite3_builtin_backup(bitcoind, node_factory):
 
     # Should still see the funds.
     assert(len(l1.rpc.listfunds()['outputs']) == 1)
+
+
+@unittest.skipIf(os.getenv('TEST_DB_PROVIDER', 'sqlite3') != 'sqlite3', "Don't know how to swap dbs in Postgres")
+def test_db_sanity_checks(bitcoind, node_factory):
+    l1, l2 = node_factory.get_nodes(2, opts=[{'allow_broken_log': True,
+                                              'may_fail': True}, {}])
+
+    l1.stop()
+    l2.stop()
+
+    # Provide the --wallet option and start with wrong db
+    l1.daemon.opts['wallet'] = "sqlite3://" + l2.db.path
+    l1.daemon.start(wait_for_initialized=False)
+    l1.daemon.wait_for_log(r'\*\*BROKEN\*\* wallet: Wallet node_id does not match HSM')
+    # Will have exited with non-zero status.
+    assert l1.daemon.proc.wait(TIMEOUT) != 0
+    assert l1.daemon.is_in_stderr('Wallet sanity check failed')
+
+    # Now try wrong network,
+    l1.daemon.opts['wallet'] = "sqlite3://" + l1.db.path
+    l1.daemon.opts['network'] = "bitcoin"
+
+    l1.daemon.start(wait_for_initialized=False)
+    l1.daemon.wait_for_log(r'\*\*BROKEN\*\* wallet: Wallet blockchain hash does not match network blockchain hash')
+    # Will have exited with non-zero status.
+    assert l1.daemon.proc.wait(TIMEOUT) != 0
+    assert l1.daemon.is_in_stderr('Wallet sanity check failed')

tests/test_wallet.py

@@ -1220,8 +1220,7 @@ def test_hsmtool_dump_descriptors(node_factory, bitcoind):
 
 @unittest.skipIf(VALGRIND, "It does not play well with prompt and key derivation.")
 def test_hsmtool_generatehsm(node_factory):
-    l1 = node_factory.get_node()
+    l1 = node_factory.get_node(start=False)
-    l1.stop()
     hsm_path = os.path.join(l1.daemon.lightning_dir, TEST_NETWORK,
                             "hsm_secret")
 
@@ -1248,6 +1247,7 @@ def test_hsmtool_generatehsm(node_factory):
 
     # We can start the node with this hsm_secret
     l1.start()
+    assert l1.info['id'] == '02244b73339edd004bc6dfbb953a87984c88e9e7c02ca14ef6ec593ca6be622ba7'
 
 
 # this test does a 'listtransactions' on a yet unconfirmed channel

wallet/wallet.c

@@ -3643,7 +3643,7 @@ void wallet_htlc_sigs_save(struct wallet *w, u64 channel_id,
 	}
 }
 
-bool wallet_network_check(struct wallet *w)
+bool wallet_sanity_check(struct wallet *w)
 {
 	struct bitcoin_blkid chainhash;
 	struct db_stmt *stmt = db_prepare_v2(
@@ -3676,6 +3676,34 @@ bool wallet_network_check(struct wallet *w)
 		db_bind_sha256d(stmt, 0, &chainparams->genesis_blockhash.shad);
 		db_exec_prepared_v2(take(stmt));
 	}
+
+	stmt = db_prepare_v2(w->db,
+			     SQL("SELECT blobval FROM vars WHERE name='node_id'"));
+	db_query_prepared(stmt);
+
+	if (db_step(stmt)) {
+		struct node_id id;
+		db_col_node_id(stmt, "blobval", &id);
+		tal_free(stmt);
+
+		if (!node_id_eq(&id, &w->ld->id)) {
+			log_broken(w->log, "Wallet node_id does not "
+					   "match HSM: %s "
+					   "!= %s. "
+					   "Did your hsm_secret change?",
+				   type_to_string(tmpctx, struct node_id, &id),
+				   type_to_string(tmpctx, struct node_id,
+						  &w->ld->id));
+			return false;
+		}
+	} else {
+		tal_free(stmt);
+		/* Still a pristine wallet, claim it for the node_id we are now */
+		stmt = db_prepare_v2(w->db, SQL("INSERT INTO vars (name, blobval) "
+						"VALUES ('node_id', ?);"));
+		db_bind_node_id(stmt, 0, &w->ld->id);
+		db_exec_prepared_v2(take(stmt));
+	}
 	return true;
 }
 

wallet/wallet.h

@@ -1198,13 +1198,15 @@ void wallet_htlc_sigs_save(struct wallet *w, u64 channel_id,
 			   const struct bitcoin_signature *htlc_sigs);
 
 /**
- * wallet_network_check - Check that the wallet is setup for this chain
+ * wallet_sanity_check - Check that the wallet is setup for this node_id and chain
  *
  * Ensure that the genesis_hash from the chainparams matches the
- * genesis_hash with which the DB was initialized. Returns false if
+ * genesis_hash with which the DB was initialized, and that the HSM
- * the check failed, i.e., if the genesis hashes do not match.
+ * gave us the same node_id as the one is the db.
+ *
+ * Returns false if the checks failed.
  */
-bool wallet_network_check(struct wallet *w);
+bool wallet_sanity_check(struct wallet *w);
 
 /**
  * wallet_block_add - Add a block to the blockchain tracked by this wallet