lightningd: make local htlc failures pass a wiremsg for errors, not a failcode.

Unfortunately the invoice_payment_hook can give us a failcode, so I simply
restrict it to the two sensible ones.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-deprecated: plugins: invoice_payment_hook "failure_code" only handles simple cases now, use "failure_message".

lightningd/htlc_set.c

@@ -14,7 +14,7 @@ static void htlc_set_hin_destroyed(struct htlc_in *hin,
 			/* Don't try to re-fail this HTLC! */
 			tal_arr_remove(&set->htlcs, i);
 			/* Kind of the correct failure code. */
-			htlc_set_fail(set, WIRE_MPP_TIMEOUT);
+			htlc_set_fail(set, take(towire_mpp_timeout(NULL)));
 			return;
 		}
 	}
@@ -35,15 +35,19 @@ static void destroy_htlc_set(struct htlc_set *set,
  */
 static void timeout_htlc_set(struct htlc_set *set)
 {
-	htlc_set_fail(set, WIRE_MPP_TIMEOUT);
+	htlc_set_fail(set, take(towire_mpp_timeout(NULL)));
 }
 
-void htlc_set_fail(struct htlc_set *set, enum onion_type failcode)
+void htlc_set_fail(struct htlc_set *set, const u8 *failmsg TAKES)
 {
+	/* Don't let local_fail_in_htlc take! */
+	if (taken(failmsg))
+		tal_steal(set, failmsg);
+
 	for (size_t i = 0; i < tal_count(set->htlcs); i++) {
 		/* Don't remove from set */
 		tal_del_destructor2(set->htlcs[i], htlc_set_hin_destroyed, set);
-		fail_htlc(set->htlcs[i], failcode);
+		local_fail_in_htlc(set->htlcs[i], failmsg);
 	}
 	tal_free(set);
 }
@@ -101,7 +105,8 @@ void htlc_set_add(struct lightningd *ld,
 	details = invoice_check_payment(tmpctx, ld, &hin->payment_hash,
 					total_msat, payment_secret);
 	if (!details) {
-		fail_htlc(hin, WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS);
+		local_fail_in_htlc(hin,
+				   take(failmsg_incorrect_or_unknown(NULL, hin)));
 		return;
 	}
 
@@ -125,7 +130,7 @@ void htlc_set_add(struct lightningd *ld,
 		/* We check this now, since we want to fail with this as soon
 		 * as possible, to avoid other probing attacks. */
 		if (!payment_secret) {
-			fail_htlc(hin, WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS);
+			local_fail_in_htlc(hin, take(failmsg_incorrect_or_unknown(NULL, hin)));
 			return;
 		}
 		tal_arr_expand(&set->htlcs, hin);
@@ -147,7 +152,9 @@ void htlc_set_add(struct lightningd *ld,
 					   &set->total_msat),
 			    type_to_string(tmpctx, struct amount_msat,
 					   &total_msat));
-		htlc_set_fail(set, WIRE_FINAL_INCORRECT_HTLC_AMOUNT);
+		htlc_set_fail(set,
+			      take(towire_final_incorrect_htlc_amount(NULL,
+								      hin->msat)));
 		return;
 	}
 
@@ -164,7 +171,9 @@ void htlc_set_add(struct lightningd *ld,
 					   &set->so_far),
 			    type_to_string(tmpctx, struct amount_msat,
 					   &hin->msat));
-		htlc_set_fail(set, WIRE_FINAL_INCORRECT_HTLC_AMOUNT);
+		htlc_set_fail(set,
+			      take(towire_final_incorrect_htlc_amount(NULL,
+								      hin->msat)));
 		return;
 	}
 
@@ -183,7 +192,8 @@ void htlc_set_add(struct lightningd *ld,
 	 *   - MUST require `payment_secret` for all HTLCs in the set. */
 	/* This catches the case of the first payment in a set. */
 	if (!payment_secret) {
-		htlc_set_fail(set, WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS);
+		htlc_set_fail(set,
+			      take(failmsg_incorrect_or_unknown(NULL, hin)));
 		return;
 	}
 }

lightningd/htlc_set.h

@@ -49,7 +49,7 @@ void htlc_set_add(struct lightningd *ld,
 		  const struct secret *payment_secret);
 
 /* Fail every htlc in the set: frees set */
-void htlc_set_fail(struct htlc_set *set, enum onion_type failcode);
+void htlc_set_fail(struct htlc_set *set, const u8 *failmsg TAKES);
 
 /* Fulfill every htlc in the set: frees set */
 void htlc_set_fulfill(struct htlc_set *set, const struct preimage *preimage);

lightningd/invoice.c

@@ -162,10 +162,11 @@ static void invoice_payload_remove_set(struct htlc_set *set,
 	payload->set = NULL;
 }
 
-static bool hook_gives_failcode(struct log *log,
-				const char *buffer,
-				const jsmntok_t *toks,
-				enum onion_type *failcode)
+static const u8 *hook_gives_failmsg(const tal_t *ctx,
+				    struct log *log,
+				    const struct htlc_in *hin,
+				    const char *buffer,
+				    const jsmntok_t *toks)
 {
 	const jsmntok_t *resulttok;
 	const jsmntok_t *t;
@@ -173,15 +174,14 @@ static bool hook_gives_failcode(struct log *log,
 
 	/* No plugin registered on hook at all? */
 	if (!buffer)
-		return false;
+		return NULL;
 
 	resulttok = json_get_member(buffer, toks, "result");
 	if (resulttok) {
 		if (json_tok_streq(buffer, resulttok, "continue")) {
-			return false;
+			return NULL;
 		} else if (json_tok_streq(buffer, resulttok, "reject")) {
-			*failcode = WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS;
-			return true;
+			return failmsg_incorrect_or_unknown(ctx, hin);
 		} else
 			fatal("Invalid invoice_payment hook result: %.*s",
 			      toks[0].end - toks[0].start, buffer);
@@ -213,13 +213,16 @@ static bool hook_gives_failcode(struct log *log,
 		fatal("Invalid invoice_payment_hook failure_code: %.*s",
 		      toks[0].end - toks[1].start, buffer);
 
-	/* UPDATE isn't valid for final nodes to return, and I think
-	 * we assert elsewhere that we don't do this! */
-	if (val & UPDATE)
-		fatal("Invalid invoice_payment_hook UPDATE failure_code: %.*s",
-		      toks[0].end - toks[1].start, buffer);
-	*failcode = val;
-	return true;
+	/* FIXME: Allow hook to return its own failmsg directly? */
+	if (val == WIRE_TEMPORARY_NODE_FAILURE)
+		return towire_temporary_node_failure(ctx);
+	if (val != WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS)
+		log_broken(hin->key.channel->log,
+			   "invoice_payment hook returned failcode %u,"
+			   " changing to incorrect_or_unknown_payment_details",
+			   val);
+
+	return failmsg_incorrect_or_unknown(ctx, hin);
 }
 
 static void
@@ -229,7 +232,7 @@ invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload,
 {
 	struct lightningd *ld = payload->ld;
 	struct invoice invoice;
-	enum onion_type failcode;
+	const u8 *failmsg;
 
 	/* We notify here to benefit from the payload and because the hook callback is
 	 * called even if the hook is not registered. */
@@ -249,14 +252,16 @@ invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload,
 	/* If invoice gets paid meanwhile (plugin responds out-of-order?) then
 	 * we can also fail */
 	if (!wallet_invoice_find_by_label(ld->wallet, &invoice, payload->label)) {
-		failcode = WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS;
-		htlc_set_fail(payload->set, failcode);
+		htlc_set_fail(payload->set, take(failmsg_incorrect_or_unknown(
+							 NULL, payload->set->htlcs[0])));
 		return;
 	}
 
 	/* Did we have a hook result? */
-	if (hook_gives_failcode(ld->log, buffer, toks, &failcode)) {
-		htlc_set_fail(payload->set, failcode);
+	failmsg = hook_gives_failmsg(NULL, ld->log,
+				     payload->set->htlcs[0], buffer, toks);
+	if (failmsg) {
+		htlc_set_fail(payload->set, take(failmsg));
 		return;
 	}
 

lightningd/peer_htlcs.c

@@ -295,12 +295,21 @@ static void local_fail_htlc(struct htlc_in *hin, enum onion_type failcode,
 	fail_in_htlc(hin, failcode, NULL, out_channel_update);
 }
 
-void fail_htlc(struct htlc_in *hin, enum onion_type failcode)
+void local_fail_in_htlc(struct htlc_in *hin, const u8 *failmsg TAKES)
 {
-	assert(failcode);
-	/* Final hop never sends an UPDATE. */
-	assert(!(failcode & UPDATE));
-	local_fail_htlc(hin, failcode, NULL);
+	/* FIXME: pass failmsg through! */
+	local_fail_htlc(hin, fromwire_peektype(failmsg), NULL);
+	if (taken(failmsg))
+		tal_free(failmsg);
+}
+
+/* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */
+const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx,
+				       const struct htlc_in *hin)
+{
+	return towire_incorrect_or_unknown_payment_details(
+		ctx, hin->msat,
+		get_block_height(hin->key.channel->owner->ld->topology));
 }
 
 /* localfail are for handing to the local payer if it's local. */
@@ -443,7 +452,7 @@ static void handle_localpay(struct htlc_in *hin,
 			    struct amount_msat total_msat,
 			    const struct secret *payment_secret)
 {
-	enum onion_type failcode;
+	const u8 *failmsg;
 	struct lightningd *ld = hin->key.channel->peer->ld;
 
 	/* BOLT #4:
@@ -467,7 +476,7 @@ static void handle_localpay(struct htlc_in *hin,
 		 *
 		 * The amount in the HTLC doesn't match the value in the onion.
 		 */
-		failcode = WIRE_FINAL_INCORRECT_HTLC_AMOUNT;
+		failmsg = towire_final_incorrect_htlc_amount(NULL, hin->msat);
 		goto fail;
 	}
 
@@ -480,7 +489,8 @@ static void handle_localpay(struct htlc_in *hin,
 	 * The CLTV expiry in the HTLC doesn't match the value in the onion.
 	 */
 	if (!check_cltv(hin, hin->cltv_expiry, outgoing_cltv_value, 0)) {
-		failcode = WIRE_FINAL_INCORRECT_CLTV_EXPIRY;
+		failmsg = towire_final_incorrect_cltv_expiry(NULL,
+							     hin->cltv_expiry);
 		goto fail;
 	}
 
@@ -497,7 +507,7 @@ static void handle_localpay(struct htlc_in *hin,
 			  hin->cltv_expiry,
 			  get_block_height(ld->topology),
 			  ld->config.cltv_final);
-		failcode = WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS;
+		failmsg = failmsg_incorrect_or_unknown(NULL, hin);
 		goto fail;
 	}
 
@@ -505,7 +515,7 @@ static void handle_localpay(struct htlc_in *hin,
 	return;
 
 fail:
-	fail_htlc(hin, failcode);
+	local_fail_in_htlc(hin, take(failmsg));
 }
 
 /*

lightningd/peer_htlcs.h

@@ -71,11 +71,15 @@ void htlcs_resubmit(struct lightningd *ld,
 
 /* For HTLCs which terminate here, invoice payment calls one of these. */
 void fulfill_htlc(struct htlc_in *hin, const struct preimage *preimage);
-void fail_htlc(struct htlc_in *hin, enum onion_type failcode);
+void local_fail_in_htlc(struct htlc_in *hin, const u8 *failmsg TAKES);
 
 /* This json process will be used as the serialize method for
  * forward_event_notification_gen and be used in
  * `listforwardings_add_forwardings()`. */
 void json_format_forwarding_object(struct json_stream *response, const char *fieldname,
 				   const struct forwarding *cur);
+
+/* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */
+const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx,
+				       const struct htlc_in *hin);
 #endif /* LIGHTNING_LIGHTNINGD_PEER_HTLCS_H */

lightningd/test/run-invoice-select-inchan.c

@@ -92,6 +92,10 @@ char *encode_scriptpubkey_to_addr(const tal_t *ctx UNNEEDED,
 				  const struct chainparams *chainparams UNNEEDED,
 				  const u8 *scriptPubkey UNNEEDED)
 { fprintf(stderr, "encode_scriptpubkey_to_addr called!\n"); abort(); }
+/* Generated stub for failmsg_incorrect_or_unknown */
+const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx UNNEEDED,
+				       const struct htlc_in *hin UNNEEDED)
+{ fprintf(stderr, "failmsg_incorrect_or_unknown called!\n"); abort(); }
 /* Generated stub for fatal */
 void   fatal(const char *fmt UNNEEDED, ...)
 { fprintf(stderr, "fatal called!\n"); abort(); }
@@ -147,7 +151,7 @@ bool htlc_is_trimmed(enum side htlc_owner UNNEEDED,
 		     enum side side UNNEEDED)
 { fprintf(stderr, "htlc_is_trimmed called!\n"); abort(); }
 /* Generated stub for htlc_set_fail */
-void htlc_set_fail(struct htlc_set *set UNNEEDED, enum onion_type failcode UNNEEDED)
+void htlc_set_fail(struct htlc_set *set UNNEEDED, const u8 *failmsg TAKES UNNEEDED)
 { fprintf(stderr, "htlc_set_fail called!\n"); abort(); }
 /* Generated stub for htlc_set_fulfill */
 void htlc_set_fulfill(struct htlc_set *set UNNEEDED, const struct preimage *preimage UNNEEDED)
@@ -436,6 +440,9 @@ u8 *towire_hsm_sign_invoice(const tal_t *ctx UNNEEDED, const u8 *u5bytes UNNEEDE
 /* Generated stub for towire_onchain_dev_memleak */
 u8 *towire_onchain_dev_memleak(const tal_t *ctx UNNEEDED)
 { fprintf(stderr, "towire_onchain_dev_memleak called!\n"); abort(); }
+/* Generated stub for towire_temporary_node_failure */
+u8 *towire_temporary_node_failure(const tal_t *ctx UNNEEDED)
+{ fprintf(stderr, "towire_temporary_node_failure called!\n"); abort(); }
 /* Generated stub for txfilter_add_scriptpubkey */
 void txfilter_add_scriptpubkey(struct txfilter *filter UNNEEDED, const u8 *script TAKES UNNEEDED)
 { fprintf(stderr, "txfilter_add_scriptpubkey called!\n"); abort(); }