lightningd: make sure we don't send channel_updates for unasked for channels.

We can decide to send an HTLC down a preferred channel which leads to the same peer
as the one they asked for, but the spec is clear that you shouldn't send the "wrong"
channel_update in that case.

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

Author: rustyrussell

lightningd/channel.c

@@ -1302,10 +1302,32 @@ channel_scid_or_local_alias(const struct channel *chan)
 		return *chan->alias[LOCAL];
 }
 
+
+/* BOLT #4:
+ * An _intermediate hop_ MUST NOT, but the _final node_:
+ *...
+ *   - if it returns a `channel_update`:
+ *     - MUST set `short_channel_id` to the `short_channel_id` used
+ *       by the incoming onion.
+ */
+/* So, if the scid doesn't match (redirect or we chose an equivalent
+ * channel), we simply don't return an update. */
 const u8 *channel_update_for_error(const tal_t *ctx,
+				   const struct htlc_in *hin,
 				   struct channel *channel)
 {
-	/* FIXME: Call directly from callers */
+	if (!hin || !hin->payload || !hin->payload->forward_channel)
+		return NULL;
+
+	/* We shouldn't have forwarded to the channel if it wasn't
+	 * allowed to use that scid, so we can keep it simple here. */
+	if ((!channel->alias[LOCAL]
+	     || !short_channel_id_eq(*hin->payload->forward_channel, *channel->alias[LOCAL]))
+	    && (!channel->scid
+		|| !short_channel_id_eq(*hin->payload->forward_channel, *channel->scid))) {
+		return NULL;
+	}
+
 	return channel_gossip_update_for_error(ctx, channel);
 }
 

lightningd/channel.h

@@ -965,7 +965,9 @@ void channel_set_billboard(struct channel *channel, bool perm,
 struct htlc_in *channel_has_htlc_in(struct channel *channel);
 struct htlc_out *channel_has_htlc_out(struct channel *channel);
 
+/* hin can be NULL */
 const u8 *channel_update_for_error(const tal_t *ctx,
+				   const struct htlc_in *hin,
 				   struct channel *channel);
 
 struct amount_msat htlc_max_possible_send(const struct channel *channel);

lightningd/peer_htlcs.c

@@ -570,6 +570,7 @@ static void destroy_hout_subd_died(struct htlc_out *hout)
 
 	hout->failmsg = towire_temporary_channel_failure(hout,
 							 channel_update_for_error(tmpctx,
+										  hout->in,
 										  hout->key.channel));
 
 	/* Assign a temporary state (we're about to free it!) so checks
@@ -619,7 +620,7 @@ static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNU
 		 */
 		/* We still append the channel_update (if we have one!)  FIXME: provide an option? */
 		if (fromwire_peektype(failmsg) & UPDATE) {
-			const u8 *update = channel_update_for_error(tmpctx, hout->key.channel);
+			const u8 *update = channel_update_for_error(tmpctx, hout->in, hout->key.channel);
 			towire(&failmsg, update, tal_bytelen(update));
 		}
 		hout->failmsg = tal_steal(hout, failmsg);
@@ -725,7 +726,7 @@ const u8 *send_htlc_out(const tal_t *ctx,
 		log_info(out->log, "Attempt to send HTLC but unowned (%s)",
 			 channel_state_name(out));
 		return towire_temporary_channel_failure(ctx,
-							channel_update_for_error(tmpctx, out));
+							channel_update_for_error(tmpctx, in, out));
 	}
 
 	/* Note: we allow outgoing HTLCs before sync, for fast startup. */
@@ -865,7 +866,7 @@ static void forward_htlc(struct htlc_in *hin,
 					 next->old_feerate_ppm)) {
 			failmsg = towire_fee_insufficient(tmpctx, hin->msat,
 							  channel_update_for_error(tmpctx,
-										   next));
+										   hin, next));
 			goto fail;
 		}
 		log_info(hin->key.channel->log,
@@ -879,7 +880,7 @@ static void forward_htlc(struct htlc_in *hin,
 		    || amount_msat_less(amt_to_forward, next->old_htlc_minimum_msat)
 		    || amount_msat_greater(amt_to_forward, next->old_htlc_maximum_msat)) {
 			failmsg = towire_temporary_channel_failure(tmpctx,
-								   channel_update_for_error(tmpctx, next));
+								   channel_update_for_error(tmpctx, hin, next));
 			goto fail;
 		}
 		log_info(hin->key.channel->log,
@@ -889,7 +890,7 @@ static void forward_htlc(struct htlc_in *hin,
 	if (!check_cltv(hin, cltv_expiry, outgoing_cltv_value,
 			ld->config.cltv_expiry_delta)) {
 		failmsg = towire_incorrect_cltv_expiry(tmpctx, cltv_expiry,
-						       channel_update_for_error(tmpctx, next));
+						       channel_update_for_error(tmpctx, hin, next));
 		goto fail;
 	}
 
@@ -908,7 +909,7 @@ static void forward_htlc(struct htlc_in *hin,
 			  outgoing_cltv_value,
 			  get_block_height(ld->topology));
 		failmsg = towire_expiry_too_soon(tmpctx,
-						 channel_update_for_error(tmpctx, next));
+						 channel_update_for_error(tmpctx, hin, next));
 		goto fail;
 	}