pay: Invert ownership of wallet_payment

`wallet_payment_store` would free the `wallet_payment` instance which would
then cause us to reload it from the DB. Instead of doing the store->free->load
dance we now tell `wallet_payment_store` whether it should take ownership and
leave it alone if not.

Passing the payment around instead of referencing it through payment_hash and
partid is a nice side-effect.

lightningd/pay.c

@@ -487,26 +487,26 @@ remote_routing_failure(const tal_t *ctx,
 	return routing_failure;
 }
 
-void payment_store(struct lightningd *ld,
-		   const struct sha256 *payment_hash, u64 partid)
+void payment_store(struct lightningd *ld, struct wallet_payment *payment TAKES)
 {
 	struct sendpay_command *pc;
 	struct sendpay_command *next;
-	const struct wallet_payment *payment;
+	/* Need to remember here otherwise wallet_payment_store will free us. */
+	bool ptaken = taken(payment);
 
-	wallet_payment_store(ld->wallet, payment_hash, partid);
-	payment = wallet_payment_by_hash(tmpctx, ld->wallet,
-					 payment_hash, partid);
-	assert(payment);
+	wallet_payment_store(ld->wallet, payment);
 
 	/* Trigger any sendpay commands waiting for the store to occur. */
 	list_for_each_safe(&ld->sendpay_commands, pc, next, list) {
-		if (!sha256_eq(payment_hash, &pc->payment_hash))
+		if (!sha256_eq(&payment->payment_hash, &pc->payment_hash))
 			continue;
 
 		/* Deletes from list, frees pc */
 		json_sendpay_in_progress(pc->cmd, payment);
 	}
+
+	if (ptaken)
+		tal_free(payment);
 }
 
 void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
@@ -589,7 +589,7 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
 	}
 
 	/* Save to DB */
-	payment_store(ld, &hout->payment_hash, hout->partid);
+	payment_store(ld, payment);
 	wallet_payment_set_status(ld->wallet, &hout->payment_hash,
 				  hout->partid,
 				  PAYMENT_FAILED, NULL);
@@ -606,11 +606,6 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
 				    failmsg,
 				    fail ? fail->channel_dir : 0);
 
-	/* payment_store -> wallet_payment_store just freed `payment` from
-	 * under us (useless indirection), so reload it in order to publish
-	 * the notification. */
-	payment = wallet_payment_by_hash(tmpctx, ld->wallet,
-					 &hout->payment_hash, hout->partid);
 	tell_waiters_failed(ld, &hout->payment_hash, payment, pay_errcode,
 			    hout->failuremsg, fail, failmsg);
 }

lightningd/pay.h

@@ -18,8 +18,7 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
 		    const char *localfail);
 
 /* Inform payment system to save the payment. */
-void payment_store(struct lightningd *ld,
-		   const struct sha256 *payment_hash, u64 partid);
+void payment_store(struct lightningd *ld, struct wallet_payment *payment);
 
 /* This json will be also used in 'sendpay_success' notifictaion. */
 void json_add_payment_fields(struct json_stream *response,

lightningd/peer_htlcs.c

@@ -1241,6 +1241,7 @@ static bool update_out_htlc(struct channel *channel,
 {
 	struct lightningd *ld = channel->peer->ld;
 	struct htlc_out *hout;
+	struct wallet_payment *payment;
 
 	hout = find_htlc_out(&ld->htlcs_out, channel, id);
 	if (!hout) {
@@ -1261,9 +1262,13 @@ static bool update_out_htlc(struct channel *channel,
 		}
 
 		/* For our own HTLCs, we commit payment to db lazily */
-		if (hout->am_origin)
-			payment_store(ld,
-				      &hout->payment_hash, hout->partid);
+		if (hout->am_origin) {
+			payment = wallet_payment_by_hash(tmpctx, ld->wallet,
+							 &hout->payment_hash,
+							 hout->partid);
+			assert(payment);
+			payment_store(ld, take(payment));
+		}
 	}
 
 	if (!htlc_out_update_state(channel, hout, newstate))

wallet/test/run-wallet.c

@@ -486,7 +486,7 @@ void payment_failed(struct lightningd *ld UNNEEDED, const struct htlc_out *hout
 { fprintf(stderr, "payment_failed called!\n"); abort(); }
 /* Generated stub for payment_store */
 void payment_store(struct lightningd *ld UNNEEDED,
-		   const struct sha256 *payment_hash UNNEEDED, u64 partid UNNEEDED)
+		   struct wallet_payment *payment UNNEEDED)
 { fprintf(stderr, "payment_store called!\n"); abort(); }
 /* Generated stub for payment_succeeded */
 void payment_succeeded(struct lightningd *ld UNNEEDED, struct htlc_out *hout UNNEEDED,
@@ -1282,8 +1282,9 @@ static bool test_payment_crud(struct lightningd *ld, const tal_t *ctx)
 	t->partid = 0;
 
 	db_begin_transaction(w->db);
-	wallet_payment_setup(w, tal_dup(NULL, struct wallet_payment, t));
-	wallet_payment_store(w, &t->payment_hash, 0);
+	t2 = tal_dup(NULL, struct wallet_payment, t);
+	wallet_payment_setup(w, t2);
+	wallet_payment_store(w, take(t2));
 	t2 = wallet_payment_by_hash(ctx, w, &t->payment_hash, 0);
 	CHECK(t2 != NULL);
 	CHECK(t2->status == t->status);

wallet/wallet.c

@@ -2185,14 +2185,10 @@ void wallet_payment_setup(struct wallet *wallet, struct wallet_payment *payment)
 }
 
 void wallet_payment_store(struct wallet *wallet,
-			  const struct sha256 *payment_hash,
-			  u64 partid)
+			  struct wallet_payment *payment TAKES)
 {
 	struct db_stmt *stmt;
-	struct wallet_payment *payment;
-
-	payment = find_unstored_payment(wallet, payment_hash, partid);
-	if (!payment) {
+	if (!find_unstored_payment(wallet, &payment->payment_hash, payment->partid)) {
 		/* Already stored on-disk */
 #if DEVELOPER
 		/* Double-check that it is indeed stored to disk
@@ -2203,8 +2199,8 @@ void wallet_payment_store(struct wallet *wallet,
 		    db_prepare_v2(wallet->db, SQL("SELECT status FROM payments"
 						  " WHERE payment_hash=?"
 						  " AND partid = ?;"));
-		db_bind_sha256(stmt, 0, payment_hash);
-		db_bind_u64(stmt, 1, partid);
+		db_bind_sha256(stmt, 0, &payment->payment_hash);
+		db_bind_u64(stmt, 1, payment->partid);
 		db_query_prepared(stmt);
 		res = db_step(stmt);
 		assert(res);
@@ -2274,8 +2270,17 @@ void wallet_payment_store(struct wallet *wallet,
 	db_bind_amount_msat(stmt, 11, &payment->total_msat);
 	db_bind_u64(stmt, 12, payment->partid);
 
-	db_exec_prepared_v2(take(stmt));
-	tal_free(payment);
+	db_exec_prepared_v2(stmt);
+	payment->id = db_last_insert_id_v2(stmt);
+	assert(payment->id > 0);
+	tal_free(stmt);
+
+	if (taken(payment)) {
+		tal_free(payment);
+	}  else {
+		list_del(&payment->list);
+		tal_del_destructor(payment, destroy_unstored_payment);
+	}
 }
 
 void wallet_payment_delete(struct wallet *wallet,

wallet/wallet.h

@@ -941,8 +941,7 @@ void wallet_payment_setup(struct wallet *wallet, struct wallet_payment *payment)
  * Stores the payment in the database.
  */
 void wallet_payment_store(struct wallet *wallet,
-			  const struct sha256 *payment_hash,
-			  u64 partid);
+			  struct wallet_payment *payment TAKES);
 
 /**
  * wallet_payment_delete - Remove a payment