askrene: make auto.sourcefree a real layer, too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: `getroutes` now applies `auto.sourcefree` layer in the order specified, so doesn't alter channels changed in later layers.

contrib/msggen/msggen/schema.json

@@ -14647,7 +14647,7 @@
         "",
         "Layers are generally maintained by plugins, either to contain persistent information about capacities which have been discovered, or to contain transient information for this particular payment (such as blinded paths or routehints).",
         "",
-        "There are two automatic layers: *auto.localchans* contains information on local channels from this node (including non-public ones), and their exact current spendable capacities, and *auto.sourcefree* overrides all channels leading out of the *source* to be zero fee and zero delay.  These are both useful in the case where the source is the current node."
+        "There are two automatic layers: *auto.localchans* contains information on local channels from this node (including non-public ones), and their exact current spendable capacities, and *auto.sourcefree* overrides all channels (including those from previous layers) leading out of the *source* to be zero fee and zero delay.  These are both useful in the case where the source is the current node."
       ],
       "categories": [
         "readonly"

doc/schemas/lightning-getroutes.json

@@ -14,7 +14,7 @@
     "",
     "Layers are generally maintained by plugins, either to contain persistent information about capacities which have been discovered, or to contain transient information for this particular payment (such as blinded paths or routehints).",
     "",
-    "There are two automatic layers: *auto.localchans* contains information on local channels from this node (including non-public ones), and their exact current spendable capacities, and *auto.sourcefree* overrides all channels leading out of the *source* to be zero fee and zero delay.  These are both useful in the case where the source is the current node."
+    "There are two automatic layers: *auto.localchans* contains information on local channels from this node (including non-public ones), and their exact current spendable capacities, and *auto.sourcefree* overrides all channels (including those from previous layers) leading out of the *source* to be zero fee and zero delay.  These are both useful in the case where the source is the current node."
   ],
   "categories": [
     "readonly"

plugins/askrene/askrene.c

@@ -172,21 +172,20 @@ static fp16_t *get_capacities(const tal_t *ctx,
 /* If we're the payer, we don't add delay or fee to our own outgoing
  * channels.  This wouldn't be right if we looped back through ourselves,
  * but we won't. */
-/* FIXME: We could cache this until gossmap changes... */
-static void add_free_source(struct plugin *plugin,
-			    struct gossmap *gossmap,
-			    struct gossmap_localmods *localmods,
-			    const struct node_id *source)
+/* FIXME: We could cache this until gossmap/layer changes... */
+static struct layer *source_free_layer(const tal_t *ctx,
+				       struct gossmap *gossmap,
+				       const struct node_id *source,
+				       struct gossmap_localmods *localmods)
 {
-	/* We apply existing localmods, save up mods we want, then append
-	 * them: it's not safe to modify localmods while they are applied! */
+	/* We apply existing localmods so we see *all* channels */
 	const struct gossmap_node *srcnode;
 	const struct amount_msat zero_base_fee = AMOUNT_MSAT(0);
 	const u16 zero_delay = 0;
 	const u32 zero_prop_fee = 0;
-	struct short_channel_id_dir *scidds
-		= tal_arr(tmpctx, struct short_channel_id_dir, 0);
+	struct layer *layer = new_temp_layer(ctx, "auto.sourcefree");
 
+	/* We apply this so we see any created channels */
 	gossmap_apply_localmods(gossmap, localmods);
 
 	/* If we're not in map, we complain later */
@@ -198,20 +197,14 @@ static void add_free_source(struct plugin *plugin,
 
 		c = gossmap_nth_chan(gossmap, srcnode, i, &scidd.dir);
 		scidd.scid = gossmap_chan_scid(gossmap, c);
-		tal_arr_expand(&scidds, scidd);
+		layer_add_update_channel(layer, &scidd,
+					 NULL, NULL, NULL,
+					 &zero_base_fee, &zero_prop_fee,
+					 &zero_delay);
 	}
 	gossmap_remove_localmods(gossmap, localmods);
 
-	/* Now we can update localmods: we only change fee levels and delay */
-	for (size_t i = 0; i < tal_count(scidds); i++) {
-		if (!gossmap_local_updatechan(localmods,
-					      &scidds[i],
-					      NULL, NULL, NULL,
-					      &zero_base_fee, &zero_prop_fee,
-					      &zero_delay))
-			plugin_err(plugin, "Could not zero fee/delay on %s",
-				   fmt_short_channel_id_dir(tmpctx, &scidds[i]));
-	}
+	return layer;
 }
 
 struct amount_msat get_additional_per_htlc_cost(const struct route_query *rq,
@@ -319,7 +312,8 @@ static const char *get_routes(const tal_t *ctx,
 			} else {
 				/* Handled below, after other layers */
 				assert(streq(layers[i], "auto.sourcefree"));
-				continue;
+				plugin_log(rq->plugin, LOG_DBG, "Adding auto.sourcefree");
+				l = source_free_layer(layers, askrene->gossmap, source, localmods);
 			}
 		}
 
@@ -332,10 +326,6 @@ static const char *get_routes(const tal_t *ctx,
 		layer_clear_overridden_capacities(l, askrene->gossmap, rq->capacities);
 	}
 
-	/* This also looks into localmods, to zero them */
-	if (have_layer(layers, "auto.sourcefree"))
-		add_free_source(rq->plugin, askrene->gossmap, localmods, source);
-
 	/* Clear scids with reservations, too, so we don't have to look up
 	 * all the time! */
 	reserves_clear_capacities(askrene->reserved, askrene->gossmap, rq->capacities);

tests/test_askrene.py

@@ -671,7 +671,7 @@ def test_sourcefree_on_mods(node_factory, bitcoind):
     check_route_as_expected(routes, [[{'short_channel_id_dir': '0x3x3/1',
                                        'amount_msat': 1000000, 'delay': 99}]])
 
-    # Same if we specify layers in the other order!
+    # NOT if we specify layers in the other order!
     routes = l1.rpc.getroutes(source=nodemap[0],
                               destination=l1.info['id'],
                               amount_msat=1000000,
@@ -680,7 +680,7 @@ def test_sourcefree_on_mods(node_factory, bitcoind):
                               final_cltv=99)['routes']
     # Expect no fee.
     check_route_as_expected(routes, [[{'short_channel_id_dir': '0x3x3/1',
-                                       'amount_msat': 1000000, 'delay': 99}]])
+                                       'amount_msat': 1003000, 'delay': 117}]])
 
 
 def test_live_spendable(node_factory, bitcoind):