From 7b899da801491d891c1aef926b15cebccb0bcca7 Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Fri, 26 Jun 2020 17:35:57 +0200
Subject: [PATCH] db: Retrieve peer ID if it exists or create the peer if not

We were assuming `wallet_channel_insert` that there cannot be a matching peer
if our in-memory representation isn't bound to it (`dbid == 0`). If we then
attempt to create the peer, and we already had one it'd cause a unique
constraint violation. As far as I can tell this could end up happening if we
have an uncommitted channel, and then exited without cleanup (`tal_destructor`
on the uncommitted channel not running). This could then leave the peer in the
DB. This is because the constraint that every peer has at least one channel is
not enforce at DB level, but rather in destructors that may or may not run.

Changelog-Fixed: Fixed a failing assertion if we reconnect to a peer that we had a channel with before, and then attempt to insert the peer into the DB twice.
---
 wallet/wallet.c | 43 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/wallet/wallet.c b/wallet/wallet.c
index 4db76a1e6..865531af3 100644
--- a/wallet/wallet.c
+++ b/wallet/wallet.c
@@ -1549,22 +1549,47 @@ void wallet_channel_save(struct wallet *w, struct channel *chan)
 	tal_free(stmt);
 }
 
-void wallet_channel_insert(struct wallet *w, struct channel *chan)
+static void wallet_peer_save(struct wallet *w, struct peer *peer)
 {
-	struct db_stmt *stmt;
+	const char *addr =
+	    type_to_string(tmpctx, struct wireaddr_internal, &peer->addr);
+	struct db_stmt *stmt =
+	    db_prepare_v2(w->db, SQL("SELECT id FROM peers WHERE node_id = ?"));
+
+	db_bind_node_id(stmt, 0, &peer->id);
+	db_query_prepared(stmt);
 
-	if (chan->peer->dbid == 0) {
-		/* Need to create the peer first */
+	if (db_step(stmt)) {
+		/* So we already knew this peer, just return its dbid */
+		peer->dbid = db_column_u64(stmt, 0);
+		tal_free(stmt);
+
+		/* Since we're at it update the wireaddr */
+		stmt = db_prepare_v2(
+		    w->db, SQL("UPDATE peers SET address = ? WHERE id = ?"));
+		db_bind_text(stmt, 0, addr);
+		db_bind_u64(stmt, 1, peer->dbid);
+		db_exec_prepared_v2(take(stmt));
+
+	} else {
+		/* Unknown peer, create it from scratch */
+		tal_free(stmt);
 		stmt = db_prepare_v2(w->db,
 				     SQL("INSERT INTO peers (node_id, address) VALUES (?, ?);")
 			);
-		db_bind_node_id(stmt, 0, &chan->peer->id);
-		db_bind_text(stmt, 1,
-			     type_to_string(tmpctx, struct wireaddr_internal,
-					    &chan->peer->addr));
+		db_bind_node_id(stmt, 0, &peer->id);
+		db_bind_text(stmt, 1,addr);
 		db_exec_prepared_v2(stmt);
-		chan->peer->dbid = db_last_insert_id_v2(take(stmt));
+		peer->dbid = db_last_insert_id_v2(take(stmt));
 	}
+}
+
+void wallet_channel_insert(struct wallet *w, struct channel *chan)
+{
+	struct db_stmt *stmt;
+
+	if (chan->peer->dbid == 0)
+		wallet_peer_save(w, chan->peer);
 
 	/* Insert a stub, that we update, unifies INSERT and UPDATE paths */
 	stmt = db_prepare_v2(