plugins/offer and plugins/fetchinvoice: send and recv errors.

This also lets us extend our testing to cover error cases.

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

plugins/fetchinvoice.c

@@ -2,8 +2,10 @@
 #include <ccan/array_size/array_size.h>
 #include <ccan/json_out/json_out.h>
 #include <ccan/mem/mem.h>
+#include <ccan/str/hex/hex.h>
 #include <ccan/tal/str/str.h>
 #include <ccan/time/time.h>
+#include <ccan/utf8/utf8.h>
 #include <common/blindedpath.h>
 #include <common/bolt11.h>
 #include <common/bolt12.h>
@@ -82,7 +84,7 @@ static struct command_result *recv_onion_message(struct command *cmd,
 						 const char *buf,
 						 const jsmntok_t *params)
 {
-	const jsmntok_t *om, *invtok, *blindingtok;
+	const jsmntok_t *om, *invtok, *errtok, *blindingtok;
 	const u8 *invbin;
 	size_t len;
 	struct tlv_invoice *inv;
@@ -98,9 +100,6 @@ static struct command_result *recv_onion_message(struct command *cmd,
 		   json_tok_full(buf, params));
 
 	om = json_get_member(buf, params, "onion_message");
-	invtok = json_get_member(buf, om, "invoice");
-	if (!invtok)
-		return command_hook_success(cmd);
 	blindingtok = json_get_member(buf, om, "blinding_in");
 	if (!blindingtok || !json_to_pubkey(buf, blindingtok, &blinding))
 		return command_hook_success(cmd);
@@ -108,14 +107,74 @@ static struct command_result *recv_onion_message(struct command *cmd,
 	sent = find_sent(&blinding);
 	if (!sent) {
 		plugin_log(cmd->plugin, LOG_DBG,
-			   "No match for received invoice %.*s",
-			   json_tok_full_len(invtok),
-			   json_tok_full(buf, invtok));
+			   "No match for onion %.*s",
+			   json_tok_full_len(om),
+			   json_tok_full(buf, om));
 		return command_hook_success(cmd);
 	}
 
 	/* From here on, we know it's genuine, so we will fail the
 	 * fetchinvoice command if the invoice is invalid */
+	errtok = json_get_member(buf, om, "invoice_error");
+	if (errtok) {
+		const u8 *data = json_tok_bin_from_hex(cmd, buf, errtok);
+		size_t dlen = tal_bytelen(data);
+		struct tlv_invoice_error *err = tlv_invoice_error_new(cmd);
+		struct json_out *details = json_out_new(cmd);
+
+		plugin_log(cmd->plugin, LOG_DBG, "errtok = %.*s",
+			   json_tok_full_len(errtok),
+			   json_tok_full(buf, errtok));
+		json_out_start(details, NULL, '{');
+		if (!fromwire_invoice_error(&data, &dlen, err)) {
+			plugin_log(cmd->plugin, LOG_DBG,
+				   "Invalid invoice_error %.*s",
+				   json_tok_full_len(errtok),
+				   json_tok_full(buf, errtok));
+			json_out_addstr(details, "invoice_error_hex",
+					tal_strndup(tmpctx,
+						    buf + errtok->start,
+						    errtok->end - errtok->start));
+		} else {
+			char *failstr;
+
+			/* FIXME: with a bit more generate-wire.py support,
+			 * we could have fieldnames and even types. */
+			if (err->erroneous_field)
+				json_out_add(details, "erroneous_field", false,
+					     "%"PRIu64, *err->erroneous_field);
+			if (err->suggested_value)
+				json_out_addstr(details, "suggested_value",
+						tal_hex(tmpctx,
+							err->suggested_value));
+			/* If they don't include this, it'll be empty */
+			failstr = tal_strndup(tmpctx,
+					      err->error,
+					      tal_bytelen(err->error));
+			json_out_addstr(details, "error", failstr);
+		}
+		json_out_end(details, '}');
+		discard_result(command_done_err(sent->cmd,
+						OFFER_BAD_INVREQ_REPLY,
+						"Remote node sent failure message",
+						details));
+		return command_hook_success(cmd);
+	}
+
+	invtok = json_get_member(buf, om, "invoice");
+	if (!invtok) {
+		plugin_log(cmd->plugin, LOG_UNUSUAL,
+			   "Neither invoice nor invoice_request_failed in reply %.*s",
+			   json_tok_full_len(om),
+			   json_tok_full(buf, om));
+		discard_result(command_fail(sent->cmd,
+					    OFFER_BAD_INVREQ_REPLY,
+					    "Neither invoice nor invoice_request_failed in reply %.*s",
+					    json_tok_full_len(om),
+					    json_tok_full(buf, om)));
+		return command_hook_success(cmd);
+	}
+
 	invbin = json_tok_bin_from_hex(cmd, buf, invtok);
 	len = tal_bytelen(invbin);
 	inv = tlv_invoice_new(cmd);

plugins/offers_invreq_hook.c

@@ -1,5 +1,6 @@
 #include <bitcoin/chainparams.h>
 #include <bitcoin/preimage.h>
+#include <ccan/cast/cast.h>
 #include <common/bech32_util.h>
 #include <common/bolt12.h>
 #include <common/bolt12_merkle.h>
@@ -25,6 +26,68 @@ struct invreq {
 	struct preimage preimage;
 };
 
+static struct command_result *finished(struct command *cmd,
+				       const char *buf,
+				       const jsmntok_t *result,
+				       void *unused)
+{
+	return command_hook_success(cmd);
+}
+
+/* If we get an error trying to reply, don't try again! */
+static struct command_result *error_noloop(struct command *cmd,
+					   const char *buf,
+					   const jsmntok_t *err,
+					   void *unused)
+{
+	plugin_log(cmd->plugin, LOG_BROKEN,
+		   "sendoniomessage gave JSON error: %.*s",
+		   json_tok_full_len(err),
+		   json_tok_full(buf, err));
+	return command_hook_success(cmd);
+}
+
+static struct command_result *WARN_UNUSED_RESULT
+send_onion_reply(struct command *cmd,
+		 const struct invreq *ir,
+		 const char *replyfield,
+		 const u8 *replydata)
+{
+	struct out_req *req;
+	size_t i;
+	const jsmntok_t *t;
+
+	plugin_log(cmd->plugin, LOG_DBG, "sending reply %s = %s",
+		   replyfield, tal_hex(tmpctx, replydata));
+
+	/* Send to requester, using return route. */
+	req = jsonrpc_request_start(cmd->plugin, cmd, "sendonionmessage",
+				    finished, error_noloop, NULL);
+
+	/* Add reply into last hop. */
+	json_array_start(req->js, "hops");
+	json_for_each_arr(i, t, ir->replytok) {
+		size_t j;
+		const jsmntok_t *t2;
+
+		plugin_log(cmd->plugin, LOG_DBG, "hops[%zu/%i]",
+			   i, ir->replytok->size);
+		json_object_start(req->js, NULL);
+		json_for_each_obj(j, t2, t)
+			json_add_tok(req->js,
+				     json_strdup(tmpctx, ir->buf, t2),
+				     t2+1, ir->buf);
+		if (i == ir->replytok->size - 1) {
+			plugin_log(cmd->plugin, LOG_DBG, "... adding %s",
+				   replyfield);
+			json_add_hex_talarr(req->js, replyfield, replydata);
+		}
+		json_object_end(req->js);
+	}
+	json_array_end(req->js);
+	return send_outreq(cmd->plugin, req);
+}
+
 static struct command_result *WARN_UNUSED_RESULT
 fail_invreq_level(struct command *cmd,
 		  const struct invreq *invreq,
@@ -32,6 +95,8 @@ fail_invreq_level(struct command *cmd,
 		  const char *fmt, va_list ap)
 {
 	char *full_fmt, *msg;
+	struct tlv_invoice_error *err;
+	u8 *errdata;
 
 	full_fmt = tal_fmt(tmpctx, "Failed invoice_request %s",
 			   invrequest_encode(tmpctx, invreq->invreq));
@@ -44,8 +109,18 @@ fail_invreq_level(struct command *cmd,
 	msg = tal_vfmt(tmpctx, full_fmt, ap);
 	plugin_log(cmd->plugin, l, "%s", msg);
 
-	/* FIXME: send reply */
-	return command_hook_success(cmd);
+	/* Don't send back internal error details. */
+	if (l == LOG_BROKEN)
+		msg = "Internal error";
+
+	err = tlv_invoice_error_new(cmd);
+	/* Remove NUL terminator */
+	err->error = tal_dup_arr(err, char, msg, strlen(msg), 0);
+	/* FIXME: Add suggested_value / erroneous_field! */
+
+	errdata = tal_arr(cmd, u8, 0);
+	towire_invoice_error(&errdata, err);
+	return send_onion_reply(cmd, invreq, "invoice_error", errdata);
 }
 
 static struct command_result *WARN_UNUSED_RESULT
@@ -130,14 +205,6 @@ static void json_add_label(struct json_stream *js,
 	json_add_string(js, "label", label);
 }
 
-static struct command_result *finished(struct command *cmd,
-				       const char *buf,
-				       const jsmntok_t *result,
-				       void *unused)
-{
-	return command_hook_success(cmd);
-}
-
 /* Note: this can actually happen if a single-use offer is already
  * used at the same time between the check and now.
  */
@@ -159,8 +226,6 @@ static struct command_result *createinvoice_done(struct command *cmd,
 {
 	char *hrp;
 	u8 *rawinv;
-	struct out_req *req;
-	size_t i;
 	const jsmntok_t *t;
 
 	/* We have a signed invoice, use it as a reply. */
@@ -173,27 +238,7 @@ static struct command_result *createinvoice_done(struct command *cmd,
 					json_tok_full(buf, t));
 	}
 
-	/* Now, send invoice to requester, using return route. */
-	req = jsonrpc_request_start(cmd->plugin, cmd, "sendonionmessage",
-				    finished, error, ir);
-
-	/* Add invoice into last hop. */
-	json_array_start(req->js, "hops");
-	json_for_each_arr(i, t, ir->replytok) {
-		size_t j;
-		const jsmntok_t *t2;
-
-		json_object_start(req->js, NULL);
-		json_for_each_obj(j, t2, t)
-			json_add_tok(req->js,
-				     json_strdup(tmpctx, ir->buf, t2),
-				     t2+1, ir->buf);
-		if (i == ir->replytok->size - 1)
-			json_add_hex_talarr(req->js, "invoice", rawinv);
-		json_object_end(req->js);
-	}
-	json_array_end(req->js);
-	return send_outreq(cmd->plugin, req);
+	return send_onion_reply(cmd, ir, "invoice", rawinv);
 }
 
 static struct command_result *create_invoicereq(struct command *cmd,

tests/test_pay.py

@@ -3875,14 +3875,13 @@ def test_fetchinvoice(node_factory, bitcoind):
 
     l1.rpc.pay(inv1['invoice'])
 
-    # FIXME: We don't report failure yet.
-#    # We can't pay the other one now.
-#    with pytest.raises(RpcError, match='???'):
-#        l1.rpc.pay(inv2['invoice'])
-#
-#    # We can't reuse the offer, either.
-#    with pytest.raises(RpcError, match='???'):
-#        l1.rpc.call('fetchinvoice', {'offer': offer})
+    # We can't pay the other one now.
+    with pytest.raises(RpcError, match="INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS.*'erring_node': '{}'".format(l3.info['id'])):
+        l1.rpc.pay(inv2['invoice'])
+
+    # We can't reuse the offer, either.
+    with pytest.raises(RpcError, match='Offer no longer available'):
+        l1.rpc.call('fetchinvoice', {'offer': offer})
 
     # Recurring offer.
     offer = l2.rpc.call('offer', {'amount': '1msat',
@@ -3913,4 +3912,24 @@ def test_fetchinvoice(node_factory, bitcoind):
     assert period2['paywindow_start'] == period2['starttime'] - 60
     assert period2['paywindow_end'] == period2['endtime']
 
+    # Can't request 2 before paying 1.
+    with pytest.raises(RpcError, match='previous invoice has not been paid'):
+        l1.rpc.call('fetchinvoice', {'offer': offer,
+                                     'recurrence_counter': 2,
+                                     'recurrence_label': 'test recurrence'})
+
     l1.rpc.pay(ret['invoice'], label='test recurrence')
+
+    # Now we can, but it's too early:
+    with pytest.raises(RpcError, match='Remote node sent failure message.*too early'):
+        l1.rpc.call('fetchinvoice', {'offer': offer,
+                                     'recurrence_counter': 2,
+                                     'recurrence_label': 'test recurrence'})
+
+    # Wait until the correct moment.
+    while time.time() < period1['starttime']:
+        time.sleep(1)
+
+    l1.rpc.call('fetchinvoice', {'offer': offer,
+                                 'recurrence_counter': 2,
+                                 'recurrence_label': 'test recurrence'})