Plugin implementation of keysend spontaneous payments

.gitignore

@@ -44,3 +44,4 @@ contrib/pylightning/pylightning.egg-info/
 contrib/pyln-*/build/
 contrib/pyln-*/dist/
 contrib/pyln-*/pyln_*.egg-info/
+plugins/keysend

common/features.c

@@ -19,6 +19,14 @@ struct feature_style {
 	enum feature_copy_style copy_style[NUM_FEATURE_PLACE];
 };
 
+const char *feature_place_names[] = {
+	"init",
+	NULL,
+	"node",
+	"channel",
+	"invoice"
+};
+
 static const struct feature_style feature_styles[] = {
 	{ OPT_DATA_LOSS_PROTECT,
 	  .copy_style = { [INIT_FEATURE] = FEATURE_REPRESENT,

common/features.h

@@ -13,6 +13,8 @@ enum feature_place {
 };
 #define NUM_FEATURE_PLACE (BOLT11_FEATURE+1)
 
+extern const char *feature_place_names[NUM_FEATURE_PLACE];
+
 /* The complete set of features for all contexts */
 struct feature_set {
 	u8 *bits[NUM_FEATURE_PLACE];

contrib/pyln-proto/pyln/proto/onion.py

@@ -184,11 +184,27 @@ def to_bytes(self):
 
 
 class Tu32Field(TlvField):
-    pass
+    def to_bytes(self):
+        raw = struct.pack("!I", self.value)
+        while len(raw) > 1 and raw[0] == 0:
+            raw = raw[1:]
+        b = BytesIO()
+        varint_encode(self.typenum, b)
+        varint_encode(len(raw), b)
+        b.write(raw)
+        return b.getvalue()
 
 
 class Tu64Field(TlvField):
-    pass
+    def to_bytes(self):
+        raw = struct.pack("!Q", self.value)
+        while len(raw) > 1 and raw[0] == 0:
+            raw = raw[1:]
+        b = BytesIO()
+        varint_encode(self.typenum, b)
+        varint_encode(len(raw), b)
+        b.write(raw)
+        return b.getvalue()
 
 
 class ShortChannelIdField(TlvField):

contrib/pyln-proto/tests/test_onion.py

@@ -30,3 +30,31 @@ def test_tlv_payload():
     ))
 
     assert(payload.to_bytes() == tlv)
+
+
+def test_tu_fields():
+    pairs = [
+        (0, b'\x01\x01\x00'),
+        (1 << 8, b'\x01\x02\x01\x00'),
+        (1 << 16, b'\x01\x03\x01\x00\x00'),
+        (1 << 24, b'\x01\x04\x01\x00\x00\x00'),
+        ((1 << 32) - 1, b'\x01\x04\xFF\xFF\xFF\xFF'),
+    ]
+
+    # These should work for Tu32
+    for i, o in pairs:
+        f = onion.Tu32Field(1, i)
+        assert(f.to_bytes() == o)
+
+    # And these should work for Tu64
+    pairs += [
+        (1 << 32, b'\x01\x05\x01\x00\x00\x00\x00'),
+        (1 << 40, b'\x01\x06\x01\x00\x00\x00\x00\x00'),
+        (1 << 48, b'\x01\x07\x01\x00\x00\x00\x00\x00\x00'),
+        (1 << 56, b'\x01\x08\x01\x00\x00\x00\x00\x00\x00\x00'),
+        ((1 << 64) - 1, b'\x01\x08\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF'),
+    ]
+
+    for i, o in pairs:
+        f = onion.Tu64Field(1, i)
+        assert(f.to_bytes() == o)

doc/PLUGINS.md

@@ -803,6 +803,13 @@ This means that the plugin does not want to do anything special and
 if we're the recipient, or attempt to forward it otherwise. Notice that the
 usual checks such as sufficient fees and CLTV deltas are still enforced.
 
+It can also replace the `onion.payload` by specifying a `payload` in
+the response.  Note that this is always a TLV-style payload, so unlike
+`onion.payload` there is no length prefix (and it must be at least 4
+hex digits long).  This will be re-parsed; it's useful for removing
+onion fields which a plugin doesn't want lightningd to consider.
+
+
 ```json
 {
   "result": "fail",

lightningd/invoice.c

@@ -314,15 +314,22 @@ invoice_check_payment(const tal_t *ctx,
 	 *    - MUST fail the HTLC.
 	 */
 	if (feature_is_set(details->features, COMPULSORY_FEATURE(OPT_VAR_ONION))
-	    && !payment_secret)
+	    && !payment_secret) {
+		log_debug(ld->log, "Attept to pay %s without secret",
+			  type_to_string(tmpctx, struct sha256, &details->rhash));
 		return tal_free(details);
+	}
 
 	if (payment_secret) {
 		struct secret expected;
 
 		invoice_secret(&details->r, &expected);
-		if (!secret_eq_consttime(payment_secret, &expected))
+		if (!secret_eq_consttime(payment_secret, &expected)) {
+			log_debug(ld->log, "Attept to pay %s with wrong secret",
+				  type_to_string(tmpctx, struct sha256,
+						 &details->rhash));
 			return tal_free(details);
+		}
 	}
 
 	/* BOLT #4:

lightningd/lightningd.c

@@ -997,7 +997,7 @@ int main(int argc, char *argv[])
 	shutdown_subdaemons(ld);
 
 	/* Remove plugins. */
-	ld->plugins = tal_free(ld->plugins);
+	plugins_free(ld->plugins);
 
 	/* Clean up the JSON-RPC. This needs to happen in a DB transaction since
 	 * it might actually be touching the DB in some destructors, e.g.,

lightningd/peer_htlcs.c

@@ -826,17 +826,60 @@ static u8 *convert_failcode(const tal_t *ctx,
 	}
 }
 
+static void
+htlc_accepted_hook_try_resolve(struct htlc_accepted_hook_payload *request,
+			       struct preimage *payment_preimage)
+{
+	struct sha256 payment_hash;
+	struct htlc_in *hin = request->hin;
+	u8 *unknown_details;
+	/* Verify that the provided secret hashes to what we need. */
+	sha256(&payment_hash, payment_preimage, sizeof(struct preimage));
+
+	if (!sha256_eq(&payment_hash, &hin->payment_hash)) {
+		log_broken(
+		    request->channel->log,
+		    "Plugin returned a preimage (sha256(%s) = %s) that doesn't "
+		    "match the HTLC hash (%s) it tries to resolve.",
+		    type_to_string(tmpctx, struct preimage, payment_preimage),
+		    type_to_string(tmpctx, struct sha256, &payment_hash),
+		    type_to_string(tmpctx, struct sha256, &hin->payment_hash));
+
+		unknown_details = tal_arr(NULL, u8, 0);
+		towire_u16(&unknown_details, 0x400f);
+		local_fail_in_htlc(hin, take(unknown_details));
+	} else {
+		fulfill_htlc(hin, payment_preimage);
+	}
+}
+
+static u8 *prepend_length(const tal_t *ctx, const u8 *payload TAKES)
+{
+	u8 buf[BIGSIZE_MAX_LEN], *ret;
+	size_t len;
+
+	len = bigsize_put(buf, tal_bytelen(payload));
+	ret = tal_arr(ctx, u8, len + tal_bytelen(payload));
+	memcpy(ret, buf, len);
+	memcpy(ret + len, payload, tal_bytelen(payload));
+	if (taken(payload))
+		tal_free(payload);
+	return ret;
+}
+
 /**
  * Callback when a plugin answers to the htlc_accepted hook
  */
 static bool htlc_accepted_hook_deserialize(struct htlc_accepted_hook_payload *request,
 					   const char *buffer,
 					   const jsmntok_t *toks)
 {
+	struct route_step *rs = request->route_step;
 	struct htlc_in *hin = request->hin;
 	struct lightningd *ld = request->ld;
 	struct preimage payment_preimage;
-	const jsmntok_t *resulttok, *paykeytok;
+	const jsmntok_t *resulttok, *paykeytok, *payloadtok;
+	u8 *payload;
 
 	if (!toks || !buffer)
 		return true;
@@ -850,6 +893,26 @@ static bool htlc_accepted_hook_deserialize(struct htlc_accepted_hook_payload *re
 		      json_strdup(tmpctx, buffer, toks));
 	}
 
+	payloadtok = json_get_member(buffer, toks, "payload");
+	if (payloadtok) {
+		payload = json_tok_bin_from_hex(rs, buffer, payloadtok);
+		if (!payload)
+			fatal("Bad payload for htlc_accepted"
+			      " hook: %.*s",
+			      payloadtok->end - payloadtok->start,
+			      buffer + payloadtok->start);
+		tal_free(request->payload);
+		tal_free(rs->raw_payload);
+
+		rs->raw_payload = prepend_length(rs, take(payload));
+		request->payload = onion_decode(request, rs,
+						hin->blinding, &hin->blinding_ss,
+						&request->failtlvtype,
+						&request->failtlvpos);
+
+	} else
+		payload = NULL;
+
 	if (json_tok_streq(buffer, resulttok, "continue")) {
 		return true;
 	}
@@ -893,7 +956,7 @@ static bool htlc_accepted_hook_deserialize(struct htlc_accepted_hook_payload *re
 		if (!json_to_preimage(buffer, paykeytok, &payment_preimage))
 			fatal("Plugin specified an invalid 'payment_key': %s",
 			      json_tok_full(buffer, resulttok));
-		fulfill_htlc(hin, &payment_preimage);
+		htlc_accepted_hook_try_resolve(request, &payment_preimage);
 		return false;
 	} else {
 		fatal("Plugin responded with an unknown result to the "

lightningd/plugin.c

@@ -56,6 +56,19 @@ struct plugins *plugins_new(const tal_t *ctx, struct log_book *log_book,
 	return p;
 }
 
+void plugins_free(struct plugins *plugins)
+{
+	struct plugin *p;
+	/* Plugins are usually the unit of allocation, and they are internally
+	 * consistent, so let's free each plugin first. */
+	while (!list_empty(&plugins->plugins)) {
+		p = list_pop(&plugins->plugins, struct plugin, list);
+		tal_free(p);
+	}
+
+	tal_free(plugins);
+}
+
 static void destroy_plugin(struct plugin *p)
 {
 	struct plugin_rpccall *call;
@@ -883,9 +896,6 @@ static void plugin_manifest_timeout(struct plugin *plugin)
 	fatal("Can't recover from plugin failure, terminating.");
 }
 
-/* List of JSON keys matching `enum feature_place`. */
-static const char *plugin_feature_place_names[] = {"init", NULL, "node", "channel", "invoice"};
-
 bool plugin_parse_getmanifest_response(const char *buffer,
                                        const jsmntok_t *toks,
                                        const jsmntok_t *idtok,
@@ -909,16 +919,16 @@ bool plugin_parse_getmanifest_response(const char *buffer,
 		bool have_featurebits = false;
 		struct feature_set *fset = talz(tmpctx, struct feature_set);
 
-		BUILD_ASSERT(ARRAY_SIZE(plugin_feature_place_names)
+		BUILD_ASSERT(ARRAY_SIZE(feature_place_names)
 			     == ARRAY_SIZE(fset->bits));
 
 		for (int i = 0; i < ARRAY_SIZE(fset->bits); i++) {
 			/* We don't allow setting the obs global init */
-			if (!plugin_feature_place_names[i])
+			if (!feature_place_names[i])
 				continue;
 
 			tok = json_get_member(buffer, featurestok,
-					      plugin_feature_place_names[i]);
+					      feature_place_names[i]);
 
 			if (!tok)
 				continue;
@@ -1201,9 +1211,9 @@ plugin_populate_init_request(struct plugin *plugin, struct jsonrpc_request *req)
 	json_add_string(req->stream, "network", chainparams->network_name);
 	json_object_start(req->stream, "feature_set");
 	for (enum feature_place fp = 0; fp < NUM_FEATURE_PLACE; fp++) {
-		if (plugin_feature_place_names[fp]) {
+		if (feature_place_names[fp]) {
 			json_add_hex_talarr(req->stream,
-					    plugin_feature_place_names[fp],
+					    feature_place_names[fp],
 					    ld->our_features->bits[fp]);
 		}
 	}

lightningd/plugin.h

@@ -137,6 +137,26 @@ void plugins_add_default_dir(struct plugins *plugins);
  */
 void plugins_init(struct plugins *plugins, const char *dev_plugin_debug);
 
+/**
+ * Free all resources that are held by plugins in the correct order.
+ *
+ * This function ensures that the resources dangling off of the plugins struct
+ * are freed in the correct order. This is necessary since the struct manages
+ * two orthogonal sets of resources:
+ *
+ *  - Plugins
+ *  - Hook calls and notifications
+ *
+ * The tal hierarchy is organized in a plugin centric way, i.e., the plugins
+ * may exit in an arbitrary order and they'll unregister pointers in the other
+ * resources. However this will fail if `tal_free` decides to free one of the
+ * non-plugin resources (technically a sibling in the allocation tree) before
+ * the plugins we will get a use-after-free. This function fixes this by
+ * freeing in the correct order without adding additional child-relationships
+ * in the allocation structure and without adding destructors.
+ */
+void plugins_free(struct plugins *plugins);
+
 /**
  * Register a plugin for initialization and execution.
  *

lightningd/test/run-find_my_abspath.c

@@ -190,6 +190,9 @@ void per_peer_state_set_fds_arr(struct per_peer_state *pps UNNEEDED, const int *
 /* Generated stub for plugins_config */
 void plugins_config(struct plugins *plugins UNNEEDED)
 { fprintf(stderr, "plugins_config called!\n"); abort(); }
+/* Generated stub for plugins_free */
+void plugins_free(struct plugins *plugins UNNEEDED)
+{ fprintf(stderr, "plugins_free called!\n"); abort(); }
 /* Generated stub for plugins_init */
 void plugins_init(struct plugins *plugins UNNEEDED, const char *dev_plugin_debug UNNEEDED)
 { fprintf(stderr, "plugins_init called!\n"); abort(); }

plugins/Makefile

@@ -10,6 +10,9 @@ PLUGIN_FUNDCHANNEL_OBJS := $(PLUGIN_FUNDCHANNEL_SRC:.c=.o)
 PLUGIN_BCLI_SRC := plugins/bcli.c
 PLUGIN_BCLI_OBJS := $(PLUGIN_BCLI_SRC:.c=.o)
 
+PLUGIN_KEYSEND_SRC := plugins/keysend.c
+PLUGIN_KEYSEND_OBJS := $(PLUGIN_KEYSEND_SRC:.c=.o)
+
 PLUGIN_LIB_SRC := plugins/libplugin.c
 PLUGIN_LIB_HEADER := plugins/libplugin.h
 PLUGIN_LIB_OBJS := $(PLUGIN_LIB_SRC:.c=.o)
@@ -56,11 +59,13 @@ plugins/fundchannel: common/addr.o $(PLUGIN_FUNDCHANNEL_OBJS) $(PLUGIN_LIB_OBJS)
 
 plugins/bcli: bitcoin/chainparams.o $(PLUGIN_BCLI_OBJS) $(PLUGIN_LIB_OBJS) $(PLUGIN_COMMON_OBJS) $(JSMN_OBJS) $(CCAN_OBJS)
 
+plugins/keysend: bitcoin/chainparams.o wire/tlvstream.o wire/gen_onion_wire.o $(PLUGIN_KEYSEND_OBJS) $(PLUGIN_LIB_OBJS) $(PLUGIN_COMMON_OBJS) $(JSMN_OBJS) $(CCAN_OBJS)
+
 $(PLUGIN_PAY_OBJS) $(PLUGIN_AUTOCLEAN_OBJS) $(PLUGIN_FUNDCHANNEL_OBJS) $(PLUGIN_BCLI_OBJS) $(PLUGIN_LIB_OBJS): $(PLUGIN_LIB_HEADER)
 
 # Make sure these depend on everything.
-ALL_PROGRAMS += plugins/pay plugins/autoclean plugins/fundchannel plugins/bcli
-ALL_OBJS += $(PLUGIN_PAY_OBJS) $(PLUGIN_AUTOCLEAN_OBJS) $(PLUGIN_FUNDCHANNEL_OBJS) $(PLUGIN_BCLI_OBJS) $(PLUGIN_LIB_OBJS)
+ALL_PROGRAMS += plugins/pay plugins/autoclean plugins/fundchannel plugins/bcli plugins/keysend
+ALL_OBJS += $(PLUGIN_PAY_OBJS) $(PLUGIN_AUTOCLEAN_OBJS) $(PLUGIN_FUNDCHANNEL_OBJS) $(PLUGIN_BCLI_OBJS) $(PLUGIN_KEYSEND_OBJS) $(PLUGIN_LIB_OBJS)
 
 check-source: $(PLUGIN_PAY_SRC:%=check-src-include-order/%) $(PLUGIN_AUTOCLEAN_SRC:%=check-src-include-order/%) $(PLUGIN_FUNDCHANNEL_SRC:%=check-src-include-order/%) $(PLUGIN_BCLI_SRC:%=check-src-include-order/%)
 check-source-bolt: $(PLUGIN_PAY_SRC:%=bolt-check/%) $(PLUGIN_AUTOCLEAN_SRC:%=bolt-check/%) $(PLUGIN_FUNDCHANNEL_SRC:%=bolt-check/%) $(PLUGIN_BCLI_SRC:%=bolt-check/%)

plugins/autoclean.c

@@ -88,7 +88,7 @@ static const struct plugin_command commands[] = { {
 int main(int argc, char *argv[])
 {
 	setup_locale();
-	plugin_main(argv, init, PLUGIN_RESTARTABLE, commands, ARRAY_SIZE(commands),
+	plugin_main(argv, init, PLUGIN_RESTARTABLE, NULL, commands, ARRAY_SIZE(commands),
 	            NULL, 0, NULL, 0,
 		    plugin_option("autocleaninvoice-cycle",
 				  "string",

plugins/bcli.c

@@ -947,7 +947,7 @@ int main(int argc, char *argv[])
 	bitcoind->rpcport = NULL;
 	bitcoind->max_fee_multiplier = 10;
 
-	plugin_main(argv, init, PLUGIN_STATIC, commands, ARRAY_SIZE(commands),
+	plugin_main(argv, init, PLUGIN_STATIC, NULL, commands, ARRAY_SIZE(commands),
 		    NULL, 0, NULL, 0,
 		    plugin_option("bitcoin-datadir",
 				  "string",

plugins/fundchannel.c

@@ -446,6 +446,6 @@ static const struct plugin_command commands[] = { {
 int main(int argc, char *argv[])
 {
 	setup_locale();
-	plugin_main(argv, init, PLUGIN_RESTARTABLE, commands, ARRAY_SIZE(commands),
-	            NULL, 0, NULL, 0, NULL);
+	plugin_main(argv, init, PLUGIN_RESTARTABLE, NULL, commands,
+		    ARRAY_SIZE(commands), NULL, 0, NULL, 0, NULL);
 }