Always notify if a sendpay payment attempt is resolved

lightningd/notification.c

@@ -285,14 +285,14 @@ void notify_sendpay_failure(struct lightningd *ld,
 			    int pay_errcode,
 			    const u8 *onionreply,
 			    const struct routing_failure *fail,
-			    char *errmsg)
+			    const char *errmsg)
 {
 	void (*serialize)(struct json_stream *,
 			  const struct wallet_payment *,
 			  int,
 			  const u8 *,
 			  const struct routing_failure *,
-			  char *) = sendpay_failure_notification_gen.serialize;
+			  const char *) = sendpay_failure_notification_gen.serialize;
 
 	struct jsonrpc_notification *n =
 	    jsonrpc_notification_start(NULL, "sendpay_failure");

lightningd/notification.h

@@ -63,6 +63,6 @@ void notify_sendpay_failure(struct lightningd *ld,
 			    int pay_errcode,
 			    const u8 *onionreply,
 			    const struct routing_failure *fail,
-			    char *errmsg);
+			    const char *errmsg);
 
 #endif /* LIGHTNING_LIGHTNINGD_NOTIFICATION_H */

lightningd/pay.c

@@ -135,7 +135,6 @@ static struct command_result *sendpay_success(struct command *cmd,
 
 	assert(payment->status == PAYMENT_COMPLETE);
 
-	notify_sendpay_success(cmd->ld, payment);
 	response = json_stream_success(cmd);
 	json_add_payment_fields(response, payment);
 	return command_success(cmd, response);
@@ -191,33 +190,31 @@ void json_sendpay_fail_fields(struct json_stream *js,
 					fail->msg);
 }
 
+static const char *sendpay_errmsg_fmt(const tal_t *ctx, int pay_errcode,
+				      const struct routing_failure *fail,
+				      const char *details)
+{
+	char *errmsg;
+	if (pay_errcode == PAY_UNPARSEABLE_ONION)
+		errmsg = "Malformed error reply";
+	else {
+		assert(fail);
+		errmsg = tal_fmt(ctx, "failed: %s (%s)",
+				 onion_type_name(fail->failcode), details);
+	}
+	return errmsg;
+}
+
 /* onionreply used if pay_errcode == PAY_UNPARSEABLE_ONION */
 static struct command_result *
 sendpay_fail(struct command *cmd,
 	     const struct wallet_payment *payment,
 	     int pay_errcode,
 	     const u8 *onionreply,
 	     const struct routing_failure *fail,
-	     const char *details)
+	     const char *errmsg)
 {
 	struct json_stream *data;
-	char *errmsg;
-
-	if (pay_errcode == PAY_UNPARSEABLE_ONION)
-		errmsg = "Malformed error reply";
-	else {
-		assert(fail);
-		errmsg = tal_fmt(tmpctx, "failed: %s (%s)",
-				 onion_type_name(fail->failcode),
-				 details);
-	}
-
-	notify_sendpay_failure(cmd->ld,
-			       payment,
-			       pay_errcode,
-			       onionreply,
-			       fail,
-			       errmsg);
 
 	data = json_stream_fail(cmd, pay_errcode,
 				errmsg);
@@ -252,6 +249,8 @@ static void tell_waiters_failed(struct lightningd *ld,
 {
 	struct sendpay_command *pc;
 	struct sendpay_command *next;
+	const char *errmsg =
+	    sendpay_errmsg_fmt(tmpctx, pay_errcode, fail, details);
 
 	/* Careful: sendpay_fail deletes cmd */
 	list_for_each_safe(&ld->waitsendpay_commands, pc, next, list) {
@@ -260,9 +259,16 @@ static void tell_waiters_failed(struct lightningd *ld,
 		if (payment->partid != pc->partid)
 			continue;
 
-		sendpay_fail(pc->cmd, payment,
-			     pay_errcode, onionreply, fail, details);
+		sendpay_fail(pc->cmd, payment, pay_errcode, onionreply, fail,
+			     errmsg);
 	}
+
+	notify_sendpay_failure(ld,
+			       payment,
+			       pay_errcode,
+			       onionreply,
+			       fail,
+			       errmsg);
 }
 
 static void tell_waiters_success(struct lightningd *ld,
@@ -281,6 +287,7 @@ static void tell_waiters_success(struct lightningd *ld,
 
 		sendpay_success(pc->cmd, payment);
 	}
+	notify_sendpay_success(ld, payment);
 }
 
 void payment_succeeded(struct lightningd *ld, struct htlc_out *hout,
@@ -480,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,
@@ -582,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);
@@ -599,8 +606,8 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout,
 				    failmsg,
 				    fail ? fail->channel_dir : 0);
 
-	tell_waiters_failed(ld, &hout->payment_hash, payment,
-			    pay_errcode, hout->failuremsg, fail, failmsg);
+	tell_waiters_failed(ld, &hout->payment_hash, payment, pay_errcode,
+			    hout->failuremsg, fail, failmsg);
 }
 
 /* Wait for a payment. If cmd is deleted, then wait_payment()
@@ -622,6 +629,7 @@ static struct command_result *wait_payment(struct lightningd *ld,
 	char *faildetail;
 	struct routing_failure *fail;
 	int faildirection;
+	int rpcerrorcode;
 
 	payment = wallet_payment_by_hash(tmpctx, ld->wallet,
 					 payment_hash, partid);
@@ -665,11 +673,11 @@ static struct command_result *wait_payment(struct lightningd *ld,
 					    "Payment failure reason unknown");
 		} else if (failonionreply) {
 			/* failed to parse returned onion error */
-			return sendpay_fail(cmd,
-					    payment,
-					    PAY_UNPARSEABLE_ONION,
-					    failonionreply,
-					    NULL, faildetail);
+			return sendpay_fail(
+			    cmd, payment, PAY_UNPARSEABLE_ONION, failonionreply,
+			    NULL,
+			    sendpay_errmsg_fmt(tmpctx, PAY_UNPARSEABLE_ONION,
+					       NULL, faildetail));
 		} else {
 			/* Parsed onion error, get its details */
 			assert(failnode);
@@ -684,13 +692,14 @@ static struct command_result *wait_payment(struct lightningd *ld,
 			fail->channel_dir = faildirection;
 			/* FIXME: We don't store this! */
 			fail->msg = NULL;
-			return sendpay_fail(cmd,
-					    payment,
-					    faildestperm
-					    ? PAY_DESTINATION_PERM_FAIL
-					    : PAY_TRY_OTHER_ROUTE,
-					    NULL,
-					    fail, faildetail);
+
+			rpcerrorcode = faildestperm ? PAY_DESTINATION_PERM_FAIL
+						    : PAY_TRY_OTHER_ROUTE;
+
+			return sendpay_fail(
+			    cmd, payment, rpcerrorcode, NULL, fail,
+			    sendpay_errmsg_fmt(tmpctx, rpcerrorcode, fail,
+					       faildetail));
 		}
 	}
 
@@ -873,8 +882,10 @@ send_payment_core(struct lightningd *ld,
 						 &first_hop->channel_id,
 						 &channel->peer->id);
 
-		return sendpay_fail(cmd, old_payment, PAY_TRY_OTHER_ROUTE,
-				    NULL, fail, "First peer not ready");
+		return sendpay_fail(
+		    cmd, old_payment, PAY_TRY_OTHER_ROUTE, NULL, fail,
+		    sendpay_errmsg_fmt(tmpctx, PAY_TRY_OTHER_ROUTE, fail,
+				       "First peer not ready"));
 	}
 
 	/* If we're retrying, delete all trace of previous one.  We delete

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))

tests/plugins/sendpay_notifications.py

@@ -14,13 +14,13 @@ def init(configuration, options, plugin):
 
 @plugin.subscribe("sendpay_success")
 def notify_sendpay_success(plugin, sendpay_success):
-    plugin.log("receive a sendpay_success recored, id: {}, payment_hash: {}".format(sendpay_success['id'], sendpay_success['payment_hash']))
+    plugin.log("Received a sendpay_success: id={}, payment_hash={}".format(sendpay_success['id'], sendpay_success['payment_hash']))
     plugin.success_list.append(sendpay_success)
 
 
 @plugin.subscribe("sendpay_failure")
 def notify_sendpay_failure(plugin, sendpay_failure):
-    plugin.log("receive a sendpay_failure recored, id: {}, payment_hash: {}".format(sendpay_failure['data']['id'],
+    plugin.log("Received a sendpay_failure: id={}, payment_hash={}".format(sendpay_failure['data']['id'],
                sendpay_failure['data']['payment_hash']))
     plugin.failure_list.append(sendpay_failure)
 

tests/test_plugin.py

@@ -758,6 +758,31 @@ def test_sendpay_notifications(node_factory, bitcoind):
     assert results['sendpay_failure'][0] == err.value.error
 
 
+def test_sendpay_notifications_nowaiter(node_factory):
+    opts = [{'plugin': os.path.join(os.getcwd(), 'tests/plugins/sendpay_notifications.py')},
+            {},
+            {'may_reconnect': False}]
+    l1, l2, l3 = node_factory.line_graph(3, opts=opts, wait_for_announce=True)
+    chanid23 = l2.get_channel_scid(l3)
+    amount = 10**8
+
+    payment_hash1 = l3.rpc.invoice(amount, "first", "desc")['payment_hash']
+    payment_hash2 = l3.rpc.invoice(amount, "second", "desc")['payment_hash']
+    route = l1.rpc.getroute(l3.info['id'], amount, 1)['route']
+
+    l1.rpc.sendpay(route, payment_hash1)
+    l1.daemon.wait_for_log(r'Received a sendpay_success')
+
+    l2.rpc.close(chanid23, 1)
+
+    l1.rpc.sendpay(route, payment_hash2)
+    l1.daemon.wait_for_log(r'Received a sendpay_failure')
+
+    results = l1.rpc.call('listsendpays_plugin')
+    assert len(results['sendpay_success']) == 1
+    assert len(results['sendpay_failure']) == 1
+
+
 def test_rpc_command_hook(node_factory):
     """Test the `sensitive_command` hook"""
     plugin = os.path.join(os.getcwd(), "tests/plugins/rpc_command.py")

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