db-fix: resolve crash on fundchannel

Fixes error introduced by 1dbdc74bc where a new fundchannel
can cause a crash after start if the max dbid is for a closed
channel.

lightningd/peer_control.c

@@ -1425,8 +1425,8 @@ void load_channels_from_wallet(struct lightningd *ld)
 {
 	struct peer *peer;
 
-	/* Load peers from database */
-	if (!wallet_channels_load_active(ld->wallet))
+	/* Load channels from database */
+	if (!wallet_init_channels(ld->wallet))
 		fatal("Could not load channels from the database");
 
 	/* This is a poor-man's db join :( */

lightningd/test/run-invoice-select-inchan.c

@@ -477,9 +477,9 @@ void wallet_channel_close(struct wallet *w UNNEEDED, u64 wallet_id UNNEEDED)
 /* Generated stub for wallet_channel_save */
 void wallet_channel_save(struct wallet *w UNNEEDED, struct channel *chan UNNEEDED)
 { fprintf(stderr, "wallet_channel_save called!\n"); abort(); }
-/* Generated stub for wallet_channels_load_active */
-bool wallet_channels_load_active(struct wallet *w UNNEEDED)
-{ fprintf(stderr, "wallet_channels_load_active called!\n"); abort(); }
+/* Generated stub for wallet_init_channels */
+bool wallet_init_channels(struct wallet *w UNNEEDED)
+{ fprintf(stderr, "wallet_init_channels called!\n"); abort(); }
 /* Generated stub for wallet_channel_stats_load */
 void wallet_channel_stats_load(struct wallet *w UNNEEDED, u64 cdbid UNNEEDED, struct channel_stats *stats UNNEEDED)
 { fprintf(stderr, "wallet_channel_stats_load called!\n"); abort(); }

tests/test_db.py

@@ -1,5 +1,5 @@
 from fixtures import *  # noqa: F401,F403
-from utils import wait_for
+from utils import wait_for, sync_blockheight
 
 
 def test_db_dangling_peer_fix(node_factory):
@@ -77,3 +77,37 @@ def test_block_backfill(node_factory, bitcoind):
     l1.rpc.close(l2.info['id'])
     bitcoind.generate_block(1)
     wait_for(lambda: len(l3.rpc.listchannels()['channels']) == 0)
+
+
+# Test that the max-channel-id is set correctly between
+# restarts (with forgotten channel)
+def test_max_channel_id(node_factory, bitcoind):
+    # Create a channel between two peers.
+    # Close the channel and have 100 blocks happen (forget channel)
+    # Restart node, create channel again. Should succeed.
+    l1, l2 = node_factory.line_graph(2, fundchannel=True, wait_for_announce=True)
+    sync_blockheight(bitcoind, [l1, l2])
+
+    # Now shutdown cleanly.
+    l1.rpc.close(l2.info['id'], 0)
+
+    l1.daemon.wait_for_log(' to CLOSINGD_COMPLETE')
+    l2.daemon.wait_for_log(' to CLOSINGD_COMPLETE')
+
+    # And should put closing into mempool.
+    l1.wait_for_channel_onchain(l2.info['id'])
+    l2.wait_for_channel_onchain(l1.info['id'])
+
+    bitcoind.generate_block(101)
+    wait_for(lambda: l1.rpc.listpeers()['peers'] == [])
+    wait_for(lambda: l2.rpc.listpeers()['peers'] == [])
+
+    # Stop l2, and restart
+    l2.stop()
+    l2.start()
+
+    # Reconnect
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+
+    # Fundchannel again, should succeed.
+    l1.rpc.fundchannel(l2.info['id'], 10**5)

wallet/test/run-wallet.c

@@ -937,7 +937,7 @@ static struct channel *wallet_channel_load(struct wallet *w, const u64 dbid)
 	struct channel *channel;
 
 	/* We expect only one peer, but reuse same code */
-	if (!wallet_channels_load_active(w))
+	if (!wallet_init_channels(w))
 		return NULL;
 	peer = list_top(&w->ld->peers, struct peer, list);
 	CHECK(peer);

wallet/wallet.c

@@ -847,14 +847,28 @@ static const char *channel_fields =
     /*41*/ "last_sent_commit, "
     /*42*/ "feerate_base, feerate_ppm, remote_upfront_shutdown_script";
 
-bool wallet_channels_load_active(struct wallet *w)
+static void set_max_channel_dbid(struct wallet *w)
+{
+	sqlite3_stmt *stmt;
+	int result;
+
+	stmt = db_select(w->db, "id FROM channels ORDER BY id DESC LIMIT 1;");
+	w->max_channel_dbid = 0;
+
+	result = sqlite3_step(stmt);
+	if (result == SQLITE_ROW)
+		w->max_channel_dbid = sqlite3_column_int64(stmt, 0);
+
+	db_stmt_done(stmt);
+}
+
+static bool wallet_channels_load_active(struct wallet *w)
 {
 	bool ok = true;
 	sqlite3_stmt *stmt;
 
-	/* We load all channels */
+	/* We load all non-closed channels */
 	stmt = db_select(w->db, "%s FROM channels WHERE state < %d;", channel_fields, CLOSED);
-	w->max_channel_dbid = 0;
 
 	int count = 0;
 	while (db_select_step(w->db, stmt)) {
@@ -864,14 +878,21 @@ bool wallet_channels_load_active(struct wallet *w)
 			db_stmt_done(stmt);
 			break;
 		}
-		if (c->dbid > w->max_channel_dbid)
-			w->max_channel_dbid = c->dbid;
 		count++;
 	}
 	log_debug(w->log, "Loaded %d channels from DB", count);
+
 	return ok;
 }
 
+bool wallet_init_channels(struct wallet *w)
+{
+	/* We set the max channel database id separately */
+	set_max_channel_dbid(w);
+	return wallet_channels_load_active(w);
+}
+
+
 static
 void wallet_channel_stats_incr_x(struct wallet *w,
 				 char const *dir,

wallet/wallet.h

@@ -471,14 +471,15 @@ bool wallet_channel_config_load(struct wallet *w, const u64 id,
 				struct channel_config *cc);
 
 /**
- * wlalet_channels_load_active -- Load persisted active channels into the peers
+ * wallet_init_channels -- Loads active channels into peers
+ *    and inits the dbid counter for next channel.
  *
  *    @w: wallet to load from
  *
  * Be sure to call this only once on startup since it'll append peers
  * loaded from the database to the list without checking.
  */
-bool wallet_channels_load_active(struct wallet *w);
+bool wallet_init_channels(struct wallet *w);
 
 /**
  * wallet_channel_stats_incr_* - Increase channel statistics.