lightningd: figure out optimal channel *before* forward_htlc hook.

Otherwise what the hook sees is actually a lie, and if it sets it
we might override it.

The side effect is that we add an explicit "forward_to" field, and
allow hooks to override it.  This lets a *hook* control channel
choice explicitly.

Changelod-Added: Plugins: `htlc_accepted_hook` return can specify what channel to forward htlc to.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

doc/PLUGINS.md

@@ -1432,7 +1432,8 @@ The payload of the hook call has the following format:
     "cltv_expiry": 500028,
     "cltv_expiry_relative": 10,
     "payment_hash": "0000000000000000000000000000000000000000000000000000000000000000"
-  }
+  },
+  "forward_to": "0000000000000000000000000000000000000000000000000000000000000000"
 }
 ```
 
@@ -1469,6 +1470,7 @@ For detailed information about each field please refer to [BOLT 04 of the specif
      blockheight.
    - `payment_hash` is the hash whose `payment_preimage` will unlock the funds
      and allow us to claim the HTLC.
+ - `forward_to`: if set, the channel_id we intend to forward this to (will not be present if the short_channel_id was invalid or we were the final destination).
 
 The hook response must have one of the following formats:
 
@@ -1489,6 +1491,7 @@ the response.  Note that this is always a TLV-style payload, so unlike
 hex digits long).  This will be re-parsed; it's useful for removing
 onion fields which a plugin doesn't want lightningd to consider.
 
+It can also specify `forward_to` in the response, replacing the destination.  This usually only makes sense if it wants to choose an alternate channel to the same next peer, but is useful if the `payload` is also replaced.
 
 ```json
 {

lightningd/channel.h

@@ -9,7 +9,6 @@
 #include <lightningd/channel_state.h>
 #include <wallet/wallet.h>
 
-struct channel_id;
 struct uncommitted_channel;
 struct wally_psbt;
 

lightningd/peer_htlcs.c

@@ -648,11 +648,51 @@ const u8 *send_htlc_out(const tal_t *ctx,
 	return NULL;
 }
 
+/* What's the best channel to this peer?
+ * If @hint is set, channel must match that one. */
+static struct channel *best_channel(struct lightningd *ld,
+				    const struct peer *next_peer,
+				    struct amount_msat amt_to_forward,
+				    struct channel *hint)
+{
+	struct amount_msat best_spendable = AMOUNT_MSAT(0);
+	struct channel *channel, *best = hint;
+
+	/* Seek channel with largest spendable! */
+	list_for_each(&next_peer->channels, channel, list) {
+		struct amount_msat spendable;
+		if (!channel_can_add_htlc(channel))
+			continue;
+		spendable = channel_amount_spendable(channel);
+		if (!amount_msat_greater(spendable, best_spendable))
+			continue;
+
+		/* Don't override if fees differ... */
+		if (hint) {
+			if (hint->feerate_base != channel->feerate_base
+			    || hint->feerate_ppm != channel->feerate_ppm)
+				continue;
+		}
+
+		/* Or if this would be below min for channel! */
+		if (amount_msat_less(amt_to_forward,
+				     channel->channel_info.their_config.htlc_minimum))
+			continue;
+
+		best = channel;
+		best_spendable = spendable;
+	}
+	return best;
+}
+
+/* forward_to is where we're actually sending it (or NULL), and
+ * forward_scid is where they asked to send it (or NULL). */
 static void forward_htlc(struct htlc_in *hin,
 			 u32 cltv_expiry,
 			 struct amount_msat amt_to_forward,
 			 u32 outgoing_cltv_value,
-			 const struct short_channel_id *scid,
+			 const struct short_channel_id *forward_scid,
+			 const struct channel_id *forward_to,
 			 const u8 next_onion[TOTAL_PACKET_SIZE(ROUTING_INFO_SIZE)],
 			 const struct pubkey *next_blinding)
 {
@@ -660,52 +700,21 @@ static void forward_htlc(struct htlc_in *hin,
 	struct lightningd *ld = hin->key.channel->peer->ld;
 	struct channel *next;
 	struct htlc_out *hout = NULL;
-	struct short_channel_id *altscid;
 
-	/* This is a shortcut for specifying next peer; doesn't mean
-	 * the actual channel! */
-	next = any_channel_by_scid(ld, scid, false);
-	if (next) {
-		struct peer *peer = next->peer;
-		struct channel *channel;
-		struct amount_msat best_spendable = channel_amount_spendable(next);
-
-		/* Seek channel with largest spendable! */
-		list_for_each(&peer->channels, channel, list) {
-			struct amount_msat spendable;
-			if (!channel_can_add_htlc(channel))
-				continue;
-			spendable = channel_amount_spendable(channel);
-			if (!amount_msat_greater(spendable, best_spendable))
-				continue;
-
-			/* Don't override if fees differ... */
-			if (channel->feerate_base != next->feerate_base
-			    || channel->feerate_ppm != next->feerate_ppm)
-				continue;
-			/* Or if this would be below min for channel! */
-			if (amount_msat_less(amt_to_forward,
-					     channel->channel_info.their_config.htlc_minimum))
-				continue;
-
-			altscid = channel->scid != NULL ? channel->scid
-							: channel->alias[LOCAL];
-
-			/* OK, it's better! */
-			log_debug(next->log, "Chose a better channel: %s",
-				  type_to_string(tmpctx,
-						 struct short_channel_id,
-						 altscid));
-			next = channel;
-		}
-	}
+	if (forward_to) {
+		next = channel_by_cid(ld, forward_to);
+		/* Update this to where we're actually trying to send. */
+		if (next)
+			forward_scid = channel_scid_or_local_alias(next);
+	}else
+		next = NULL;
 
 	/* Unknown peer, or peer not ready. */
 	if (!next || !channel_active(next)) {
 		local_fail_in_htlc(hin, take(towire_unknown_next_peer(NULL)));
 		wallet_forwarded_payment_add(hin->key.channel->peer->ld->wallet,
 					 hin, get_onion_style(hin),
-					 scid, NULL,
+					 forward_scid, NULL,
 					 FORWARD_LOCAL_FAILED,
 					 WIRE_UNKNOWN_NEXT_PEER);
 		return;
@@ -803,7 +812,7 @@ static void forward_htlc(struct htlc_in *hin,
 fail:
 	local_fail_in_htlc(hin, failmsg);
 	wallet_forwarded_payment_add(ld->wallet,
-				 hin, get_onion_style(hin), scid, hout,
+				 hin, get_onion_style(hin), forward_scid, hout,
 				 FORWARD_LOCAL_FAILED,
 				 fromwire_peektype(failmsg));
 }
@@ -819,6 +828,8 @@ struct htlc_accepted_hook_payload {
 	struct channel *channel;
 	struct lightningd *ld;
 	struct pubkey *next_blinding;
+	/* NULL if we couldn't find it */
+	struct channel_id *fwd_channel_id;
 	u8 *next_onion;
 	u64 failtlvtype;
 	size_t failtlvpos;
@@ -907,7 +918,7 @@ static bool htlc_accepted_hook_deserialize(struct htlc_accepted_hook_payload *re
 	struct htlc_in *hin = request->hin;
 	struct lightningd *ld = request->ld;
 	struct preimage payment_preimage;
-	const jsmntok_t *resulttok, *paykeytok, *payloadtok;
+	const jsmntok_t *resulttok, *paykeytok, *payloadtok, *fwdtok;
 	u8 *payload, *failonion;
 
 	if (!toks || !buffer)
@@ -939,10 +950,22 @@ static bool htlc_accepted_hook_deserialize(struct htlc_accepted_hook_payload *re
 						ld->accept_extra_tlv_types,
 						&request->failtlvtype,
 						&request->failtlvpos);
-
 	} else
 		payload = NULL;
 
+	fwdtok = json_get_member(buffer, toks, "forward_to");
+	if (fwdtok) {
+		tal_free(request->fwd_channel_id);
+		request->fwd_channel_id = tal(request, struct channel_id);
+		if (!json_to_channel_id(buffer, fwdtok,
+					request->fwd_channel_id)) {
+			fatal("Bad forward_to for htlc_accepted"
+			      " hook: %.*s",
+			      fwdtok->end - fwdtok->start,
+			      buffer + fwdtok->start);
+		}
+	}
+
 	if (json_tok_streq(buffer, resulttok, "continue")) {
 		return true;
 	}
@@ -1074,6 +1097,9 @@ static void htlc_accepted_hook_serialize(struct htlc_accepted_hook_payload *p,
 	json_add_secret(s, "shared_secret", hin->shared_secret);
 	json_object_end(s);
 
+	if (p->fwd_channel_id)
+		json_add_channel_id(s, "forward_to", p->fwd_channel_id);
+
 	json_object_start(s, "htlc");
 	json_add_short_channel_id(
 	    s, "short_channel_id",
@@ -1117,6 +1143,7 @@ htlc_accepted_hook_final(struct htlc_accepted_hook_payload *request STEALS)
 			     request->payload->amt_to_forward,
 			     request->payload->outgoing_cltv,
 			     request->payload->forward_channel,
+			     request->fwd_channel_id,
 			     serialize_onionpacket(tmpctx, rs->next),
 			     request->next_blinding);
 	} else
@@ -1166,6 +1193,37 @@ REGISTER_PLUGIN_HOOK(htlc_accepted,
 		     struct htlc_accepted_hook_payload *);
 
 
+/* Figures out how to fwd, allocating return off hp */
+static struct channel_id *calc_forwarding_channel(struct lightningd *ld,
+						  struct htlc_accepted_hook_payload *hp,
+						  const struct route_step *rs)
+{
+	const struct onion_payload *p = hp->payload;
+	struct channel *c, *best;
+
+	if (rs->nextcase != ONION_FORWARD)
+		return NULL;
+
+	if (!p || !p->forward_channel)
+		return NULL;
+
+	c = any_channel_by_scid(ld, p->forward_channel, false);
+	if (!c)
+		return NULL;
+
+	best = best_channel(ld, c->peer, p->amt_to_forward, c);
+	if (best != c) {
+		log_debug(hp->channel->log,
+			  "Chose a better channel than %s: %s",
+			  type_to_string(tmpctx, struct short_channel_id,
+					 p->forward_channel),
+			  type_to_string(tmpctx, struct short_channel_id,
+					 channel_scid_or_local_alias(best)));
+	}
+
+	return tal_dup(hp, struct channel_id, &best->cid);
+}
+
 /**
  * Everyone is committed to this htlc of theirs
  *
@@ -1300,6 +1358,16 @@ static bool peer_accepted_htlc(const tal_t *ctx,
 #endif
 		hook_payload->next_blinding = NULL;
 
+	/* The scid is merely used to indicate the next peer, it is not
+	 * a requirement (nor, ideally, observable anyway).  We can change
+	 * to a more-preferred one now, that way the hook sees the value
+	 * we're actually going to (try to) use */
+
+	/* We don't store actual channel as it could vanish while
+	 * we're in hook */
+	hook_payload->fwd_channel_id
+		= calc_forwarding_channel(ld, hook_payload, rs);
+
 	plugin_hook_call_htlc_accepted(ld, NULL, hook_payload);
 
 	/* Falling through here is ok, after all the HTLC locked */

tests/plugins/htlc_accepted-fwdto.py

@@ -0,0 +1,31 @@
+#!/usr/bin/env python3
+"""A plugin that tells us to forward HTLCs to a specific channel.
+
+"""
+from pyln.client import Plugin
+
+
+plugin = Plugin()
+
+
+@plugin.hook("htlc_accepted")
+def on_htlc_accepted(htlc, onion, plugin, **kwargs):
+    if plugin.fwdto is None:
+        return {"result": "continue"}
+
+    return {"result": "continue", "forward_to": plugin.fwdto}
+
+
+@plugin.method("setfwdto")
+def setfailonion(plugin, fwdto):
+    """Sets the channel_id to forward to when receiving an incoming HTLC.
+    """
+    plugin.fwdto = fwdto
+
+
+@plugin.init()
+def on_init(**kwargs):
+    plugin.fwdto = None
+
+
+plugin.run()

tests/test_opening.py

@@ -1722,14 +1722,22 @@ def test_zeroconf_multichan_forward(node_factory):
     # Now create a channel that is twice as large as the real channel,
     # and don't announce it.
     l2.fundwallet(10**7)
-    l2.rpc.fundchannel(l3.info['id'], 2 * 10**6, mindepth=0)
+    zeroconf_cid = l2.rpc.fundchannel(l3.info['id'], 2 * 10**6, mindepth=0)['channel_id']
 
     l2.daemon.wait_for_log(r'peer_in WIRE_CHANNEL_READY')
     l3.daemon.wait_for_log(r'peer_in WIRE_CHANNEL_READY')
 
     inv = l3.rpc.invoice(amount_msat=10000, label='lbl1', description='desc')['bolt11']
     l1.rpc.pay(inv)
-    assert l2.daemon.is_in_log(r'Chose a better channel: .*')
+
+    for c in only_one(l2.rpc.listpeers(l3.info['id'])['peers'])['channels']:
+        if c['channel_id'] == zeroconf_cid:
+            zeroconf_scid = c['alias']['local']
+        else:
+            normal_scid = c['short_channel_id']
+
+    assert l2.daemon.is_in_log(r'Chose a better channel than {}: {}'
+                               .format(normal_scid, zeroconf_scid))
 
 
 def test_zeroreserve(node_factory, bitcoind):

tests/test_plugin.py

@@ -2381,6 +2381,24 @@ def test_htlc_accepted_hook_failonion(node_factory):
         l1.rpc.pay(inv)
 
 
+@pytest.mark.developer("Gossip without developer is slow.")
+def test_htlc_accepted_hook_fwdto(node_factory):
+    plugin = os.path.join(os.path.dirname(__file__), 'plugins/htlc_accepted-fwdto.py')
+    l1, l2, l3 = node_factory.line_graph(3, opts=[{}, {'plugin': plugin}, {}], wait_for_announce=True)
+
+    # Add some balance
+    l1.rpc.pay(l2.rpc.invoice(10**9 // 2, 'balance', '')['bolt11'])
+    wait_for(lambda: only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])['htlcs'] == [])
+
+    # make it forward back down same channel.
+    l2.rpc.setfwdto(only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])['channel_id'])
+    inv = l3.rpc.invoice(42, 'fwdto', '')['bolt11']
+    with pytest.raises(RpcError, match="WIRE_INVALID_ONION_HMAC"):
+        l1.rpc.pay(inv)
+
+    assert l2.rpc.listforwards()['forwards'][0]['out_channel'] == only_one(only_one(l1.rpc.listpeers()['peers'])['channels'])['short_channel_id']
+
+
 def test_dynamic_args(node_factory):
     plugin_path = os.path.join(os.getcwd(), 'contrib/plugins/helloworld.py')
 

wallet/test/run-wallet.c

@@ -426,6 +426,10 @@ char *json_strdup(const tal_t *ctx UNNEEDED, const char *buffer UNNEEDED, const
 /* Generated stub for json_stream_success */
 struct json_stream *json_stream_success(struct command *cmd UNNEEDED)
 { fprintf(stderr, "json_stream_success called!\n"); abort(); }
+/* Generated stub for json_to_channel_id */
+bool json_to_channel_id(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED,
+			struct channel_id *cid UNNEEDED)
+{ fprintf(stderr, "json_to_channel_id called!\n"); abort(); }
 /* Generated stub for json_to_node_id */
 bool json_to_node_id(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED,
 			       struct node_id *id UNNEEDED)