lightningd: no longer forward failures to gossipd, let caller do it.

We fix up the test by using pay, instead of sendpay (and making pay log the expected message). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Changed: sendpay no longer extracts updates from errors, the caller should do it from the `raw_message`.
2025-12-19 07:04:22 +01:00 · 2021-02-02 15:46:22 +10:30
parent 780fc25413
commit 1a85edd207
8 changed files with 7 additions and 145 deletions
--- a/gossipd/gossipd.c
+++ b/gossipd/gossipd.c
@@ -1724,92 +1724,6 @@ static struct io_plan *handle_txout_reply(struct io_conn *conn,
 	return daemon_conn_read_next(conn, daemon->master);
 }

-/* Fix up the channel_update to include the type if it doesn't currently have
- * one. See ElementsProject/lightning#1730 and lightningnetwork/lnd#1599 for the
- * in-depth discussion on why we break message parsing here... */
-static u8 *patch_channel_update(const tal_t *ctx, u8 *channel_update TAKES)
-{
-	u8 *fixed;
-	if (channel_update != NULL &&
-	    fromwire_peektype(channel_update) != WIRE_CHANNEL_UPDATE) {
-		/* This should be a channel_update, prefix with the
-		 * WIRE_CHANNEL_UPDATE type, but isn't. Let's prefix it. */
-		fixed = tal_arr(ctx, u8, 0);
-		towire_u16(&fixed, WIRE_CHANNEL_UPDATE);
-		towire(&fixed, channel_update, tal_bytelen(channel_update));
-		if (taken(channel_update))
-			tal_free(channel_update);
-		return fixed;
-	} else {
-		return tal_dup_talarr(ctx, u8, channel_update);
-	}
-}
-
-/* Return NULL if the wrapped onion error message has no channel_update field,
- * or return the embedded channel_update message otherwise. */
-static u8 *channel_update_from_onion_error(const tal_t *ctx,
-					   const u8 *onion_message)
-{
-	u8 *channel_update = NULL;
-	struct amount_msat unused_msat;
-	u32 unused32;
-
-	/* Identify failcodes that have some channel_update.
-	 *
-	 * TODO > BOLT 1.0: Add new failcodes when updating to a
-	 * new BOLT version. */
-	if (!fromwire_temporary_channel_failure(ctx,
-						onion_message,
-						&channel_update) &&
-	    !fromwire_amount_below_minimum(ctx,
-					   onion_message, &unused_msat,
-					   &channel_update) &&
-	    !fromwire_fee_insufficient(ctx,
-		    		       onion_message, &unused_msat,
-				       &channel_update) &&
-	    !fromwire_incorrect_cltv_expiry(ctx,
-		    			    onion_message, &unused32,
-					    &channel_update) &&
-	    !fromwire_expiry_too_soon(ctx,
-		    		      onion_message,
-				      &channel_update))
-		/* No channel update. */
-		return NULL;
-
-	return patch_channel_update(ctx, take(channel_update));
-}
-
-/*~ lightningd tells us when a payment has failed; we mark the channel (or
- * node) unusable here if it's a permanent failure, and unpack any
- * channel_update contained in the error. */
-static struct io_plan *handle_payment_failure(struct io_conn *conn,
-					      struct daemon *daemon,
-					      const u8 *msg)
-{
-	u8 *error;
-	u8 *channel_update;
-
-	if (!fromwire_gossipd_payment_failure(msg, msg, &error))
-		master_badmsg(WIRE_GOSSIPD_PAYMENT_FAILURE, msg);
-
-	channel_update = channel_update_from_onion_error(tmpctx, error);
-	if (channel_update) {
-		status_debug("Extracted channel_update %s from onionreply %s",
-			     tal_hex(tmpctx, channel_update),
-			     tal_hex(tmpctx, error));
-		u8 *err = handle_channel_update(daemon->rstate, channel_update,
-						NULL, NULL, true);
-		if (err) {
-			status_info("extracted bad channel_update %s from onionreply %s",
-				    sanitize_error(err, err, NULL),
-				    error);
-			tal_free(err);
-		}
-	}
-
-	return daemon_conn_read_next(conn, daemon->master);
-}
-
 /*~ lightningd tells us when about a gossip message directly, when told to by
 * the addgossip RPC call.  That's usually used when a plugin gets an update
 * returned in an payment error. */
@@ -1938,9 +1852,6 @@ static struct io_plan *recv_req(struct io_conn *conn,
 	case WIRE_GOSSIPD_GET_TXOUT_REPLY:
 		return handle_txout_reply(conn, daemon, msg);

-	case WIRE_GOSSIPD_PAYMENT_FAILURE:
-		return handle_payment_failure(conn, daemon, msg);
-
 	case WIRE_GOSSIPD_OUTPOINT_SPENT:
 		return handle_outpoint_spent(conn, daemon, msg);

--- a/gossipd/gossipd_wire.csv
+++ b/gossipd/gossipd_wire.csv
@@ -96,11 +96,6 @@ msgdata,gossipd_get_txout_reply,satoshis,amount_sat,
 msgdata,gossipd_get_txout_reply,len,u16,
 msgdata,gossipd_get_txout_reply,outscript,u8,len

-# master->gossipd an htlc failed with this onion error.
-msgtype,gossipd_payment_failure,3021
-msgdata,gossipd_payment_failure,len,u16,
-msgdata,gossipd_payment_failure,error,u8,len
-
 # master -> gossipd: a potential funding outpoint was spent, please forget the eventual channel
 msgtype,gossipd_outpoint_spent,3024
 msgdata,gossipd_outpoint_spent,short_channel_id,short_channel_id,
--- a/gossipd/gossipd_wiregen.c
+++ b/gossipd/gossipd_wiregen.c
@@ -36,7 +36,6 @@ const char *gossipd_wire_name(int e)
 	case WIRE_GOSSIPD_LOCAL_CHANNEL_CLOSE: return "WIRE_GOSSIPD_LOCAL_CHANNEL_CLOSE";
 	case WIRE_GOSSIPD_GET_TXOUT: return "WIRE_GOSSIPD_GET_TXOUT";
 	case WIRE_GOSSIPD_GET_TXOUT_REPLY: return "WIRE_GOSSIPD_GET_TXOUT_REPLY";
-	case WIRE_GOSSIPD_PAYMENT_FAILURE: return "WIRE_GOSSIPD_PAYMENT_FAILURE";
 	case WIRE_GOSSIPD_OUTPOINT_SPENT: return "WIRE_GOSSIPD_OUTPOINT_SPENT";
 	case WIRE_GOSSIPD_DEV_SUPPRESS: return "WIRE_GOSSIPD_DEV_SUPPRESS";
 	case WIRE_GOSSIPD_DEV_MEMLEAK: return "WIRE_GOSSIPD_DEV_MEMLEAK";
@@ -76,7 +75,6 @@ bool gossipd_wire_is_defined(u16 type)
 	case WIRE_GOSSIPD_LOCAL_CHANNEL_CLOSE:;
 	case WIRE_GOSSIPD_GET_TXOUT:;
 	case WIRE_GOSSIPD_GET_TXOUT_REPLY:;
-	case WIRE_GOSSIPD_PAYMENT_FAILURE:;
 	case WIRE_GOSSIPD_OUTPOINT_SPENT:;
 	case WIRE_GOSSIPD_DEV_SUPPRESS:;
 	case WIRE_GOSSIPD_DEV_MEMLEAK:;
@@ -619,35 +617,6 @@ bool fromwire_gossipd_get_txout_reply(const tal_t *ctx, const void *p, struct sh
 	return cursor != NULL;
 }

-/* WIRE: GOSSIPD_PAYMENT_FAILURE */
-/* master->gossipd an htlc failed with this onion error. */
-u8 *towire_gossipd_payment_failure(const tal_t *ctx, const u8 *error)
-{
-	u16 len = tal_count(error);
-	u8 *p = tal_arr(ctx, u8, 0);
-
-	towire_u16(&p, WIRE_GOSSIPD_PAYMENT_FAILURE);
-	towire_u16(&p, len);
-	towire_u8_array(&p, error, len);
-
-	return memcheck(p, tal_count(p));
-}
-bool fromwire_gossipd_payment_failure(const tal_t *ctx, const void *p, u8 **error)
-{
-	u16 len;
-
-	const u8 *cursor = p;
-	size_t plen = tal_count(p);
-
-	if (fromwire_u16(&cursor, &plen) != WIRE_GOSSIPD_PAYMENT_FAILURE)
-		return false;
- 	len = fromwire_u16(&cursor, &plen);
- 	// 2nd case error
-	*error = len ? tal_arr(ctx, u8, len) : NULL;
-	fromwire_u8_array(&cursor, &plen, *error, len);
-	return cursor != NULL;
-}
-
 /* WIRE: GOSSIPD_OUTPOINT_SPENT */
 /* master -> gossipd: a potential funding outpoint was spent */
 u8 *towire_gossipd_outpoint_spent(const tal_t *ctx, const struct short_channel_id *short_channel_id)
@@ -1088,4 +1057,4 @@ bool fromwire_gossipd_addgossip_reply(const tal_t *ctx, const void *p, wirestrin
 	*err = fromwire_wirestring(ctx, &cursor, &plen);
 	return cursor != NULL;
 }
-// SHA256STAMP:e82edc5625085e21b02b27a2293d9d757556f3090a8a20b142dcb73411307a0c
+// SHA256STAMP:5fb4bcc3bb8c5f312041142d4bf555a2187c82d82921b819d5a45410efddf6f3
--- a/gossipd/gossipd_wiregen.h
+++ b/gossipd/gossipd_wiregen.h
@@ -41,8 +41,6 @@ enum gossipd_wire {
        WIRE_GOSSIPD_GET_TXOUT = 3018,
        /*  master->gossipd here is the output */
        WIRE_GOSSIPD_GET_TXOUT_REPLY = 3118,
-        /*  master->gossipd an htlc failed with this onion error. */
-        WIRE_GOSSIPD_PAYMENT_FAILURE = 3021,
        /*  master -> gossipd: a potential funding outpoint was spent */
        WIRE_GOSSIPD_OUTPOINT_SPENT = 3024,
        /*  master -> gossipd: stop gossip timers. */
@@ -157,11 +155,6 @@ bool fromwire_gossipd_get_txout(const void *p, struct short_channel_id *short_ch
 u8 *towire_gossipd_get_txout_reply(const tal_t *ctx, const struct short_channel_id *short_channel_id, struct amount_sat satoshis, const u8 *outscript);
 bool fromwire_gossipd_get_txout_reply(const tal_t *ctx, const void *p, struct short_channel_id *short_channel_id, struct amount_sat *satoshis, u8 **outscript);

-/* WIRE: GOSSIPD_PAYMENT_FAILURE */
-/*  master->gossipd an htlc failed with this onion error. */
-u8 *towire_gossipd_payment_failure(const tal_t *ctx, const u8 *error);
-bool fromwire_gossipd_payment_failure(const tal_t *ctx, const void *p, u8 **error);
-
 /* WIRE: GOSSIPD_OUTPOINT_SPENT */
 /*  master -> gossipd: a potential funding outpoint was spent */
 u8 *towire_gossipd_outpoint_spent(const tal_t *ctx, const struct short_channel_id *short_channel_id);
@@ -232,4 +225,4 @@ bool fromwire_gossipd_addgossip_reply(const tal_t *ctx, const void *p, wirestrin


 #endif /* LIGHTNING_GOSSIPD_GOSSIPD_WIREGEN_H */
-// SHA256STAMP:e82edc5625085e21b02b27a2293d9d757556f3090a8a20b142dcb73411307a0c
+// SHA256STAMP:5fb4bcc3bb8c5f312041142d4bf555a2187c82d82921b819d5a45410efddf6f3
--- a/lightningd/gossip_control.c
+++ b/lightningd/gossip_control.c
@@ -141,7 +141,6 @@ static unsigned gossip_msg(struct subd *gossip, const u8 *msg, const int *fds)
 	case WIRE_GOSSIPD_GET_STRIPPED_CUPDATE:
 	case WIRE_GOSSIPD_GET_TXOUT_REPLY:
 	case WIRE_GOSSIPD_OUTPOINT_SPENT:
-	case WIRE_GOSSIPD_PAYMENT_FAILURE:
 	case WIRE_GOSSIPD_GET_INCOMING_CHANNELS:
 	case WIRE_GOSSIPD_DEV_SET_MAX_SCIDS_ENCODE_SIZE:
 	case WIRE_GOSSIPD_DEV_SUPPRESS:
--- a/lightningd/pay.c
+++ b/lightningd/pay.c
@@ -490,12 +490,6 @@ remote_routing_failure(const tal_t *ctx,
 			erring_node = &route_nodes[origin_index];
 	}

-	/* Tell gossipd; it will try to extract channel_update */
-	/* FIXME: sendonion caller should do this, and inform gossipd of any
-	 * permanent errors. */
-	subd_send_msg(ld->gossip,
-		      take(towire_gossipd_payment_failure(NULL, failuremsg)));
-
 	routing_failure->erring_index = (unsigned int) (origin_index + 1);
 	routing_failure->failcode = failcode;
 	routing_failure->msg = tal_dup_talarr(routing_failure, u8, failuremsg);
--- a/plugins/libplugin-pay.c
+++ b/plugins/libplugin-pay.c
@@ -1470,6 +1470,10 @@ payment_waitsendpay_finished(struct command *cmd, const char *buffer,
 	update = channel_update_from_onion_error(tmpctx, p->result->raw_message);
 	if (update) {
 		struct out_req *req;
+		paymod_log(p, LOG_DBG,
+			   "Extracted channel_update %s from onionreply %s",
+			   tal_hex(tmpctx, update),
+			   tal_hex(tmpctx, p->result->raw_message));
 		req = jsonrpc_request_start(p->plugin, NULL, "addgossip",
 					    payment_addgossip_success,
 					    payment_addgossip_failure, p);
--- a/tests/test_pay.py
+++ b/tests/test_pay.py
@@ -286,8 +286,6 @@ def test_pay_get_error_with_update(node_factory):

    inv = l3.rpc.invoice(123000, 'test_pay_get_error_with_update', 'description')

-    route = l1.rpc.getroute(l3.info['id'], 12300, 1)["route"]
-
    # Make sure l2 doesn't tell l1 directly that channel is disabled.
    l2.rpc.dev_suppress_gossip()
    l3.stop()
@@ -296,9 +294,8 @@ def test_pay_get_error_with_update(node_factory):
    wait_for(lambda: [c['active'] for c in l2.rpc.listchannels(chanid2)['channels']] == [False, False])

    assert(l1.is_channel_active(chanid2))
-    l1.rpc.sendpay(route, inv['payment_hash'])
    with pytest.raises(RpcError, match=r'WIRE_TEMPORARY_CHANNEL_FAILURE'):
-        l1.rpc.waitsendpay(inv['payment_hash'])
+        l1.rpc.pay(inv['bolt11'])

    # Make sure we get an onionreply, without the type prefix of the nested
    # channel_update, and it should patch it to include a type prefix. The