pay: Move notify_sendpay_* calls out of the waiter loop

Changelog-Changed: plugin: `notify_sendpay_success` and `notify_sendpay_failure` are now always called, even if there is no command waiting on the result.

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);
@@ -213,18 +212,9 @@ sendpay_fail(struct command *cmd,
 	     int pay_errcode,
 	     const u8 *onionreply,
 	     const struct routing_failure *fail,
-	     const char *details)
+	     const char *errmsg)
 {
 	struct json_stream *data;
-	const char *errmsg =
-	    sendpay_errmsg_fmt(tmpctx, pay_errcode, fail, details);
-
-	notify_sendpay_failure(cmd->ld,
-			       payment,
-			       pay_errcode,
-			       onionreply,
-			       fail,
-			       errmsg);
 
 	data = json_stream_fail(cmd, pay_errcode,
 				errmsg);
@@ -259,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) {
@@ -267,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,
@@ -288,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,
@@ -629,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);
@@ -672,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);
@@ -691,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));
 		}
 	}
 
@@ -880,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

tests/test_plugin.py

@@ -758,7 +758,6 @@ def test_sendpay_notifications(node_factory, bitcoind):
     assert results['sendpay_failure'][0] == err.value.error
 
 
-@pytest.mark.xfail(strict=True)
 def test_sendpay_notifications_nowaiter(node_factory):
     opts = [{'plugin': os.path.join(os.getcwd(), 'tests/plugins/sendpay_notifications.py')},
             {},