lightningd: simplify datastore internal db API.

The wallet_datastore_first() SELECT statement only iterates from the
given key (if any), relying on the caller to notice when the key no
longer applies.  (e.g. startkey = ["foo", "bar"] will return key
["foo", "bar"] then ["foo", "bar", "child" ], then ["foo", "baz"]).

The only caller (listdatastore) would notice the keychange and stop
looping, but reallly wallet_datastore_next() should do this.  When I
tried to use it for migrations, I got very confused!

Also, several places want a simple "wallet_datastore_get()" function,
so provide that.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/datastore.c

@@ -91,27 +91,6 @@ static struct command_result *param_list_or_string(struct command *cmd,
 	return NULL;
 }
 
-/* Does k1 match k2 as far as k2 goes? */
-static bool datastore_key_startswith(const char **k1, const char **k2)
-{
-	size_t k1len = tal_count(k1), k2len = tal_count(k2);
-
-	if (k2len > k1len)
-		return false;
-
-	for (size_t i = 0; i < k2len; i++) {
-		if (!streq(k1[i], k2[i]))
-			return false;
-	}
-	return true;
-}
-
-static bool datastore_key_eq(const char **k1, const char **k2)
-{
-	return tal_count(k1) == tal_count(k2)
-		&& datastore_key_startswith(k1, k2);
-}
-
 static char *datastore_key_fmt(const tal_t *ctx, const char **key)
 {
 	char *ret = tal_strdup(ctx, "[");
@@ -180,16 +159,14 @@ static struct command_result *json_datastore(struct command *cmd,
 		for (size_t i = 1; i < tal_count(key); i++) {
 			const char **parent;
 			parent = tal_dup_arr(cmd, const char *, key, i, 0);
-
+			if (wallet_datastore_get(cmd, cmd->ld->wallet, parent,
-			stmt = wallet_datastore_first(cmd, cmd->ld->wallet,
+						 NULL)) {
-						      parent, &k, NULL, NULL);
-			tal_free(stmt);
-			if (stmt && datastore_key_eq(k, parent))
 				return command_fail(cmd,
 						    DATASTORE_UPDATE_NO_CHILDREN,
 						    "Parent key %s exists",
 						    datastore_key_fmt(tmpctx,
 								      parent));
+			}
 		}
 	}
 
@@ -252,13 +229,13 @@ static struct command_result *json_listdatastore(struct command *cmd,
 	for (stmt = wallet_datastore_first(cmd, cmd->ld->wallet, key,
 					   &k, &data, &generation);
 	     stmt;
-	     stmt = wallet_datastore_next(cmd, cmd->ld->wallet,
+	     stmt = wallet_datastore_next(cmd, key,
 					  stmt, &k, &data,
 					  &generation)) {
 		log_debug(cmd->ld->log, "Got %s",
 			  datastore_key_fmt(tmpctx, k));
 
-		/* Don't list children, except implicitly */
+		/* Don't list sub-children, except as summary to show it exists. */
 		if (tal_count(k) > tal_count(key) + 1) {
 			log_debug(cmd->ld->log, "Too long");
 			if (!prev_k || !datastore_key_startswith(k, prev_k)) {
@@ -268,10 +245,6 @@ static struct command_result *json_listdatastore(struct command *cmd,
 				json_add_datastore(response, prev_k, NULL, 0);
 				json_object_end(response);
 			}
-		} else if (key && !datastore_key_startswith(k, key)) {
-			log_debug(cmd->ld->log, "Not interested");
-			tal_free(stmt);
-			break;
 		} else {
 			log_debug(cmd->ld->log, "Printing");
 			json_object_start(response, NULL);
@@ -289,11 +262,10 @@ static struct command_result *json_deldatastore(struct command *cmd,
 						 const jsmntok_t *params)
 {
 	struct json_stream *response;
-	const char **key, **k;
+	const char **key;
 	const u8 *data;
 	u64 *generation;
 	u64 actual_gen;
-	struct db_stmt *stmt;
 
 	if (!param(cmd, buffer, params,
 		   p_req("key", param_list_or_string, &key),
@@ -301,11 +273,8 @@ static struct command_result *json_deldatastore(struct command *cmd,
 		   NULL))
 		return command_param_failed();
 
-	stmt = wallet_datastore_first(cmd, cmd->ld->wallet, key,
+	data = wallet_datastore_get(cmd, cmd->ld->wallet, key, &actual_gen);
-				      &k, &data, &actual_gen);
+	if (!data) {
-	tal_free(stmt);
-
-	if (!stmt || !datastore_key_eq(k, key)) {
 		return command_fail(cmd, DATASTORE_DEL_DOES_NOT_EXIST,
 				    "Key does not exist");
 	}

wallet/wallet.c

@@ -5333,8 +5333,67 @@ void wallet_datastore_remove(struct wallet *w, const char **key)
 	db_datastore_remove(w->db, key);
 }
 
+/* Does k1 match k2 as far as k2 goes? */
+bool datastore_key_startswith(const char **k1, const char **k2)
+{
+	size_t k1len = tal_count(k1), k2len = tal_count(k2);
+
+	if (k2len > k1len)
+		return false;
+
+	for (size_t i = 0; i < k2len; i++) {
+		if (!streq(k1[i], k2[i]))
+			return false;
+	}
+	return true;
+}
+
+bool datastore_key_eq(const char **k1, const char **k2)
+{
+	return tal_count(k1) == tal_count(k2)
+		&& datastore_key_startswith(k1, k2);
+}
+
+static u8 *db_datastore_get(const tal_t *ctx,
+			    struct db *db,
+			    const char **key,
+			    u64 *generation)
+{
+	struct db_stmt *stmt;
+	u8 *ret;
+
+	stmt = db_prepare_v2(db,
+			     SQL("SELECT data, generation"
+				 " FROM datastore"
+				 " WHERE key = ?"));
+	db_bind_datastore_key(stmt, key);
+	db_query_prepared(stmt);
+
+	if (!db_step(stmt)) {
+		tal_free(stmt);
+		return NULL;
+	}
+
+	ret = db_col_arr(ctx, stmt, "data", u8);
+	if (generation)
+		*generation = db_col_u64(stmt, "generation");
+	else
+		db_col_ignore(stmt, "generation");
+	tal_free(stmt);
+	return ret;
+}
+
+u8 *wallet_datastore_get(const tal_t *ctx,
+			 struct wallet *w,
+			 const char **key,
+			 u64 *generation)
+{
+	return db_datastore_get(ctx, w->db, key, generation);
+}
+
 static struct db_stmt *db_datastore_next(const tal_t *ctx,
 					 struct db_stmt *stmt,
+					 const char **startkey,
 					 const char ***key,
 					 const u8 **data,
 					 u64 *generation)
@@ -5343,6 +5402,14 @@ static struct db_stmt *db_datastore_next(const tal_t *ctx,
 		return tal_free(stmt);
 
 	*key = db_col_datastore_key(ctx, stmt, "key");
+
+	/* We select from startkey onwards, so once we're past it, stop */
+	if (startkey && !datastore_key_startswith(*key, startkey)) {
+		db_col_ignore(stmt, "data");
+		db_col_ignore(stmt, "generation");
+		return tal_free(stmt);
+	}
+
 	if (data)
 		*data = db_col_arr(ctx, stmt, "data", u8);
 	else
@@ -5380,7 +5447,7 @@ static struct db_stmt *db_datastore_first(const tal_t *ctx,
 	}
 	db_query_prepared(stmt);
 
-	return db_datastore_next(ctx, stmt, key, data, generation);
+	return db_datastore_next(ctx, stmt, startkey, key, data, generation);
 }
 
 struct db_stmt *wallet_datastore_first(const tal_t *ctx,
@@ -5394,13 +5461,13 @@ struct db_stmt *wallet_datastore_first(const tal_t *ctx,
 }
 
 struct db_stmt *wallet_datastore_next(const tal_t *ctx,
-				      struct wallet *w,
+				      const char **startkey,
 				      struct db_stmt *stmt,
 				      const char ***key,
 				      const u8 **data,
 				      u64 *generation)
 {
-	return db_datastore_next(ctx, stmt, key, data, generation);
+	return db_datastore_next(ctx, stmt, startkey, key, data, generation);
 }
 
 /* We use a different query form if we only care about a single channel. */

wallet/wallet.h

@@ -1444,11 +1444,23 @@ void wallet_datastore_update(struct wallet *w,
  */
 void wallet_datastore_remove(struct wallet *w, const char **key);
 
+/**
+ * Get a single entry from the datastore
+ * @ctx: the tal ctx to allocate off
+ * @w: the wallet
+ * @key: the key
+ * @generation: the generation or NULL (set if returns non-NULL)
+ */
+u8 *wallet_datastore_get(const tal_t *ctx,
+			 struct wallet *w,
+			 const char **key,
+			 u64 *generation);
+
 /**
  * Iterate through the datastore.
  * @ctx: the tal ctx to allocate off
  * @w: the wallet
- * @startkey: NULL, or the first key to start with
+ * @startkey: NULL, or the subkey to iterate
  * @key: the first key (if returns non-NULL)
  * @data: the first data (if returns non-NULL)
  * @generation: the first generation (if returns non-NULL)
@@ -1466,7 +1478,7 @@ struct db_stmt *wallet_datastore_first(const tal_t *ctx,
 /**
  * Iterate through the datastore.
  * @ctx: the tal ctx to allocate off
- * @w: the wallet
+ * @startkey: NULL, or the subkey to iterate
  * @stmt: the previous statement.
  * @key: the key (if returns non-NULL)
  * @data: the data (if returns non-NULL)
@@ -1476,12 +1488,17 @@ struct db_stmt *wallet_datastore_first(const tal_t *ctx,
  * If you choose not to call wallet_datastore_next() you must free it!
  */
 struct db_stmt *wallet_datastore_next(const tal_t *ctx,
-				      struct wallet *w,
+				      const char **startkey,
 				      struct db_stmt *stmt,
 				      const char ***key,
 				      const u8 **data,
 				      u64 *generation);
 
+/* Does k1 match k2 as far as k2 goes? */
+bool datastore_key_startswith(const char **k1, const char **k2);
+/* Does k1 match k2? */
+bool datastore_key_eq(const char **k1, const char **k2);
+
 /**
  * Iterate through the htlcs table.
  * @w: the wallet