wallet: don't expose wallet_payment constructor, use wallet_add_payment.

Have it construct and return.  No need to expose details about dbid...

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

lightningd/pay.c

@@ -1100,8 +1100,8 @@ send_payment_core(struct lightningd *ld,
 					     partid);
 	}
 
-	payment = wallet_payment_new(cmd,
+	payment = wallet_add_payment(cmd,
-				     0, /* ID is not in db yet */
+				     ld->wallet,
 				     time_now().ts.tv_sec,
 				     NULL,
 				     rhash,
@@ -1122,9 +1122,6 @@ send_payment_core(struct lightningd *ld,
 				     NULL,
 				     local_invreq_id);
 
-	/* Save into db */
-	wallet_add_payment(ld->wallet, payment);
-
 	return json_sendpay_in_progress(cmd, payment);
 }
 
@@ -1412,8 +1409,8 @@ static struct command_result *self_payment(struct lightningd *ld,
 	u64 inv_dbid;
 	const char *err;
 
-	payment = wallet_payment_new(tmpctx,
+	payment = wallet_add_payment(tmpctx,
-				     0, /* ID is not in db yet */
+				     ld->wallet,
 				     time_now().ts.tv_sec,
 				     NULL,
 				     rhash,
@@ -1434,9 +1431,6 @@ static struct command_result *self_payment(struct lightningd *ld,
 				     NULL,
 				     local_invreq_id);
 
-	/* We write this into db immediately. */
-	wallet_add_payment(ld->wallet, payment);
-
 	/* Now, resolve the invoice */
 	inv = invoice_check_payment(tmpctx, ld, rhash, msat, payment_secret, &err);
 	if (!inv) {

wallet/test/run-wallet.c

@@ -2088,27 +2088,37 @@ static bool test_htlc_crud(struct lightningd *ld, const tal_t *ctx)
 
 static bool test_payment_crud(struct lightningd *ld, const tal_t *ctx)
 {
-	struct wallet_payment *t = tal(ctx, struct wallet_payment), *t2;
+	struct wallet_payment *t, *t2;
 	struct wallet *w = create_test_wallet(ld, ctx);
+	struct sha256 payment_hash;
+	struct node_id destination;
 
-	mempat(t, sizeof(*t));
+	memset(&payment_hash, 1, sizeof(payment_hash));
-	t->destination = tal(t, struct node_id);
+	memset(&destination, 2, sizeof(struct node_id));
-	memset(t->destination, 2, sizeof(struct node_id));
-
-	t->id = 0;
-	t->msatoshi = AMOUNT_MSAT(100);
-	t->msatoshi_sent = AMOUNT_MSAT(101);
-	t->total_msat = t->msatoshi;
-	t->status = PAYMENT_PENDING;
-	t->payment_preimage = NULL;
-	memset(&t->payment_hash, 1, sizeof(t->payment_hash));
-	t->partid = 0;
-	t->groupid = 12345;
 
 	db_begin_transaction(w->db);
 	load_indexes(w->db, ld->indexes);
-	t2 = tal_dup(NULL, struct wallet_payment, t);
+	t = wallet_add_payment(ctx,
-	wallet_add_payment(w, take(t2));
+			       w,
+			       1,
+			       NULL,
+			       &payment_hash,
+			       0, 12345,
+			       PAYMENT_PENDING,
+			       &destination,
+			       AMOUNT_MSAT(100),
+			       AMOUNT_MSAT(101),
+			       AMOUNT_MSAT(100),
+			       NULL,
+			       NULL,
+			       NULL,
+			       NULL,
+			       NULL,
+			       NULL,
+			       NULL,
+			       NULL,
+			       NULL);
+
 	t2 = wallet_payment_by_hash(ctx, w, &t->payment_hash, 0, t->groupid);
 	CHECK(t2 != NULL);
 	CHECK(t2->status == t->status);

wallet/wallet.c

@@ -3174,14 +3174,84 @@ void wallet_local_htlc_out_delete(struct wallet *wallet,
 	db_exec_prepared_v2(take(stmt));
 }
 
-void wallet_add_payment(struct wallet *wallet,
+/* FIXME: reorder! */
-			struct wallet_payment *payment)
+static
+struct wallet_payment *wallet_payment_new(const tal_t *ctx,
+					  u64 dbid,
+					  u32 timestamp,
+					  const u32 *completed_at,
+					  const struct sha256 *payment_hash,
+					  u64 partid,
+					  u64 groupid,
+					  enum payment_status status,
+					  /* The destination may not be known if we used `sendonion` */
+					  const struct node_id *destination,
+					  struct amount_msat msatoshi,
+					  struct amount_msat msatoshi_sent,
+					  struct amount_msat total_msat,
+					  /* If and only if PAYMENT_COMPLETE */
+					  const struct preimage *payment_preimage,
+					  const struct secret *path_secrets,
+					  const struct node_id *route_nodes,
+					  const struct short_channel_id *route_channels,
+					  const char *invstring,
+					  const char *label,
+					  const char *description,
+					  const u8 *failonion,
+					  const struct sha256 *local_invreq_id);
+
+struct wallet_payment *wallet_add_payment(const tal_t *ctx,
+					  struct wallet *wallet,
+					  u32 timestamp,
+					  const u32 *completed_at,
+					  const struct sha256 *payment_hash,
+					  u64 partid,
+					  u64 groupid,
+					  enum payment_status status,
+					  /* The destination may not be known if we used `sendonion` */
+					  const struct node_id *destination TAKES,
+					  struct amount_msat msatoshi,
+					  struct amount_msat msatoshi_sent,
+					  struct amount_msat total_msat,
+					  /* If and only if PAYMENT_COMPLETE */
+					  const struct preimage *payment_preimage TAKES,
+					  const struct secret *path_secrets TAKES,
+					  const struct node_id *route_nodes TAKES,
+					  const struct short_channel_id *route_channels TAKES,
+					  const char *invstring TAKES,
+					  const char *label TAKES,
+					  const char *description TAKES,
+					  const u8 *failonion TAKES,
+					  const struct sha256 *local_invreq_id)
 {
 	struct db_stmt *stmt;
+	struct wallet_payment *payment;
+	u64 id;
 
-        /* Don't attempt to add the same payment twice */
+	id = sendpay_index_created(wallet->ld,
-	assert(!payment->id);
+				   payment_hash,
+				   partid, groupid, status);
 
+	payment = wallet_payment_new(ctx, id,
+				     timestamp,
+				     completed_at,
+				     payment_hash,
+				     partid,
+				     groupid,
+				     status,
+				     destination,
+				     msatoshi,
+				     msatoshi_sent,
+				     total_msat,
+				     payment_preimage,
+				     path_secrets,
+				     route_nodes,
+				     route_channels,
+				     invstring,
+				     label,
+				     description,
+				     failonion,
+				     local_invreq_id);
 	stmt = db_prepare_v2(
 		wallet->db,
 		SQL("INSERT INTO payments ("
@@ -3204,11 +3274,6 @@ void wallet_add_payment(struct wallet *wallet,
 		    "  paydescription"
 		    ") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);"));
 
-	payment->id = sendpay_index_created(wallet->ld,
-					    &payment->payment_hash,
-					    payment->partid,
-					    payment->groupid,
-					    payment->status);
 	assert(payment->id > 0);
 
 	db_bind_u64(stmt, payment->id);
@@ -3267,8 +3332,7 @@ void wallet_add_payment(struct wallet *wallet,
 	db_exec_prepared_v2(stmt);
 	tal_free(stmt);
 
-	if (taken(payment))
+	return payment;
-		tal_free(payment);
 }
 
 u64 wallet_payment_get_groupid(struct wallet *wallet,
@@ -3323,6 +3387,7 @@ void wallet_payment_delete(struct wallet *wallet,
 	db_exec_prepared_v2(take(stmt));
 }
 
+static
 struct wallet_payment *wallet_payment_new(const tal_t *ctx,
 					  u64 dbid,
 					  u32 timestamp,

wallet/wallet.h

@@ -418,30 +418,6 @@ struct wallet_payment {
 	struct sha256 *local_invreq_id;
 };
 
-struct wallet_payment *wallet_payment_new(const tal_t *ctx,
-					  u64 dbid,
-					  u32 timestamp,
-					  const u32 *completed_at,
-					  const struct sha256 *payment_hash,
-					  u64 partid,
-					  u64 groupid,
-					  enum payment_status status,
-					  /* The destination may not be known if we used `sendonion` */
-					  const struct node_id *destination TAKES,
-					  struct amount_msat msatoshi,
-					  struct amount_msat msatoshi_sent,
-					  struct amount_msat total_msat,
-					  /* If and only if PAYMENT_COMPLETE */
-					  const struct preimage *payment_preimage TAKES,
-					  const struct secret *path_secrets TAKES,
-					  const struct node_id *route_nodes TAKES,
-					  const struct short_channel_id *route_channels TAKES,
-					  const char *invstring TAKES,
-					  const char *label TAKES,
-					  const char *description TAKES,
-					  const u8 *failonion TAKES,
-					  const struct sha256 *local_invreq_id);
-
 struct outpoint {
 	struct bitcoin_outpoint outpoint;
 	u32 blockheight;
@@ -921,11 +897,34 @@ struct htlc_stub *wallet_htlc_stubs(const tal_t *ctx, struct wallet *wallet,
 				    struct channel *chan, u64 commit_num);
 
 /**
- * wallet_add_payment - Store this payment in the db; sets payment->id
+ * wallet_add_payment - Store this payment in the db
+ * @ctx: context to allocate returned `struct wallet_payment` off.
  * @wallet: wallet we're going to store it in.
- * @payment: the payment for later committing.
+ * @...: the details
  */
-void wallet_add_payment(struct wallet *wallet, struct wallet_payment *payment TAKES);
+struct wallet_payment *wallet_add_payment(const tal_t *ctx,
+					  struct wallet *wallet,
+					  u32 timestamp,
+					  const u32 *completed_at,
+					  const struct sha256 *payment_hash,
+					  u64 partid,
+					  u64 groupid,
+					  enum payment_status status,
+					  /* The destination may not be known if we used `sendonion` */
+					  const struct node_id *destination TAKES,
+					  struct amount_msat msatoshi,
+					  struct amount_msat msatoshi_sent,
+					  struct amount_msat total_msat,
+					  /* If and only if PAYMENT_COMPLETE */
+					  const struct preimage *payment_preimage TAKES,
+					  const struct secret *path_secrets TAKES,
+					  const struct node_id *route_nodes TAKES,
+					  const struct short_channel_id *route_channels TAKES,
+					  const char *invstring TAKES,
+					  const char *label TAKES,
+					  const char *description TAKES,
+					  const u8 *failonion TAKES,
+					  const struct sha256 *local_invreq_id);
 
 /**
  * wallet_payment_delete - Remove a payment