plugins/multifundchannel.c: Add minchannels flag to multifundchannel.

[ changed from best_effort binary to minchannels counter -- RR]

contrib/pyln-client/pyln/client/lightning.py

@@ -846,7 +846,7 @@ class LightningRpc(UnixDomainSocketRpc):
         }
         return self.call("listsendpays", payload)
 
-    def multifundchannel(self, destinations, feerate=None, minconf=None, utxos=None, best_effort=False, **kwargs):
+    def multifundchannel(self, destinations, feerate=None, minconf=None, utxos=None, minchannels=None, **kwargs):
         """
         Fund channels to an array of {destinations},
         each entry of which is a dict of node {id}
@@ -862,7 +862,8 @@ class LightningRpc(UnixDomainSocketRpc):
             "destinations": destinations,
             "feerate": feerate,
             "minconf": minconf,
-            "utxos": utxos
+            "utxos": utxos,
+            "minchannels": minchannels,
         }
         payload.update({k: v for k, v in kwargs.items()})
         return self.call("multifundchannel", payload)

plugins/multifundchannel.c

@@ -21,6 +21,7 @@
 #include <common/utils.h>
 #include <plugins/libplugin.h>
 #include <stdarg.h>
+#include <stdlib.h>
 #include <string.h>
 
 /* Current state of the funding process.  */
@@ -99,6 +100,19 @@ struct multifundchannel_destination {
 	errcode_t code;
 };
 
+/* Stores a destination that was removed due to some failure.  */
+struct multifundchannel_removed {
+	/* The destination we removed.  */
+	struct node_id id;
+	/* The method that failed:
+	connect, fundchannel_start, fundchannel_complete.
+	*/
+	const char *method;
+	/* The error that caused this destination to be removed, in JSON.  */
+	const char *error;
+	errcode_t code;
+};
+
 /* The object for a single multifundchannel command.  */
 struct multifundchannel_command {
 	/* A unique numeric identifier for this particular
@@ -133,6 +147,12 @@ struct multifundchannel_command {
 	u32 minconf;
 	/* The set of utxos to be used.  */
 	const char *utxos_str;
+	/* How long should we keep going if things fail. */
+	size_t minchannels;
+	/* Array of destinations that were removed in a best-effort
+	attempt to fund as many channels as possible.
+	*/
+	struct multifundchannel_removed *removeds;
 
 	/* The PSBT of the funding transaction we are building.
 	Prior to `fundchannel_start` completing for all destinations,
@@ -558,6 +578,12 @@ param_destinations_array(struct command *cmd, const char *name,
 Command Processing
 -----------------------------------------------------------------------------*/
 
+/* Function to redo multifundchannel after a failure.
+*/
+static struct command_result *
+redo_multifundchannel(struct multifundchannel_command *mfc,
+		      const char *failing_method);
+
 static struct command_result *
 perform_multiconnect(struct multifundchannel_command *mfc);
 
@@ -743,7 +769,6 @@ after_multiconnect(struct multifundchannel_command *mfc)
 	/* Check if anyone failed.  */
 	for (i = 0; i < tal_count(mfc->destinations); ++i) {
 		struct multifundchannel_destination *dest;
-		struct json_stream *out;
 
 		dest = &mfc->destinations[i];
 
@@ -753,26 +778,8 @@ after_multiconnect(struct multifundchannel_command *mfc)
 		if (dest->state != MULTIFUNDCHANNEL_CONNECT_FAILED)
 			continue;
 
-		/* One of them failed, oh no.
+		/* One of them failed, oh no. */
-		Forward the error.
+		return redo_multifundchannel(mfc, "connect");
-		*/
-		plugin_log(mfc->cmd->plugin, LOG_DBG,
-			   "mfc %"PRIu64", dest %u: "
-			   "connect failure being forwarded.",
-			   mfc->id, dest->index);
-
-		out = jsonrpc_stream_fail_data(mfc->cmd,
-					       dest->code,
-					       "`connect` failed.");
-
-		json_add_node_id(out, "id", &dest->id);
-		json_add_string(out, "method", "connect");
-		json_add_jsonstr(out, "error", dest->error);
-
-		/* Close `data`.  */
-		json_object_end(out);
-
-		return mfc_finished(mfc, out);
 	}
 
 	return perform_fundpsbt(mfc);
@@ -1283,7 +1290,6 @@ after_fundchannel_start(struct multifundchannel_command *mfc)
 	/* Check if any fundchannel_start failed.  */
 	for (i = 0; i < tal_count(mfc->destinations); ++i) {
 		struct multifundchannel_destination *dest;
-		struct json_stream *out;
 
 		dest = &mfc->destinations[i];
 
@@ -1293,26 +1299,8 @@ after_fundchannel_start(struct multifundchannel_command *mfc)
 		if (dest->state != MULTIFUNDCHANNEL_START_FAILED)
 			continue;
 
-		/* One of them failed, oh no.
+		/* One of them failed, oh no.  */
-		Forward the error.
+		return redo_multifundchannel(mfc, "fundchannel_start");
-		*/
-		plugin_log(mfc->cmd->plugin, LOG_DBG,
-			   "mfc %"PRIu64", dest %u: "
-			   "fundchannel_start failure being forwarded.",
-			   mfc->id, dest->index);
-
-		out = jsonrpc_stream_fail_data(mfc->cmd,
-					       dest->code,
-					       "`fundchannel_start` failed.");
-
-		json_add_node_id(out, "id", &dest->id);
-		json_add_string(out, "method", "fundchannel_start");
-		json_add_jsonstr(out, "error", dest->error);
-
-		/* Close `data`.  */
-		json_object_end(out);
-
-		return mfc_finished(mfc, out);
 	}
 
 	/* Next step.  */
@@ -1618,7 +1606,6 @@ after_fundchannel_complete(struct multifundchannel_command *mfc)
 	/* Check if any fundchannel_complete failed.  */
 	for (i = 0; i < tal_count(mfc->destinations); ++i) {
 		struct multifundchannel_destination *dest;
-		struct json_stream *out;
 
 		dest = &mfc->destinations[i];
 
@@ -1628,27 +1615,8 @@ after_fundchannel_complete(struct multifundchannel_command *mfc)
 		if (dest->state != MULTIFUNDCHANNEL_COMPLETE_FAILED)
 			continue;
 
-		/* One of them failed, oh no.
+		/* One of them failed, oh no.  */
-		Forward the error.
+		return redo_multifundchannel(mfc, "fundchannel_complete");
-		*/
-		plugin_log(mfc->cmd->plugin, LOG_DBG,
-			   "mfc %"PRIu64", dest %u: "
-			   "fundchannel_complete failure being forwarded.",
-			   mfc->id, dest->index);
-
-		out = jsonrpc_stream_fail_data(mfc->cmd,
-					       dest->code,
-					       "`fundchannel_complete` "
-					       "failed");
-
-		json_add_node_id(out, "id", &dest->id);
-		json_add_string(out, "method", "fundchannel_complete");
-		json_add_jsonstr(out, "error", dest->error);
-
-		/* Close `data`.  */
-		json_object_end(out);
-
-		return mfc_finished(mfc, out);
 	}
 
 	return perform_sendpsbt(mfc);
@@ -1805,7 +1773,7 @@ after_sendpsbt(struct command *cmd,
 
 /*---------------------------------------------------------------------------*/
 /*~
-And finally we are done, after a thousand lines or so of code.
+And finally we are done.
 */
 
 static struct command_result *
@@ -1830,9 +1798,177 @@ multifundchannel_finished(struct multifundchannel_command *mfc)
 	}
 	json_array_end(out);
 
+	json_array_start(out, "failed");
+	for (i = 0; i < tal_count(mfc->removeds); ++i) {
+		json_object_start(out, NULL);
+		json_add_node_id(out, "id", &mfc->removeds[i].id);
+		json_add_string(out, "method", mfc->removeds[i].method);
+		json_add_jsonstr(out, "error", mfc->removeds[i].error);
+		json_object_end(out);
+	}
+	json_array_end(out);
+
 	return mfc_finished(mfc, out);
 }
 
+/*~ We do cleanup, then we remove failed destinations and if we still have
+ * the minimum number, re-run.
+*/
+struct multifundchannel_redo {
+	struct multifundchannel_command *mfc;
+	const char *failing_method;
+};
+
+static struct command_result *
+post_cleanup_redo_multifundchannel(struct multifundchannel_redo *redo);
+
+static struct command_result *
+redo_multifundchannel(struct multifundchannel_command *mfc,
+		      const char *failing_method)
+{
+	struct multifundchannel_redo *redo;
+
+	assert(mfc->pending == 0);
+
+	plugin_log(mfc->cmd->plugin, LOG_DBG,
+		   "mfc %"PRIu64": trying redo despite '%s' failure; "
+		   "will cleanup for now.",
+		   mfc->id, failing_method);
+
+	redo = tal(mfc, struct multifundchannel_redo);
+	redo->mfc = mfc;
+	redo->failing_method = failing_method;
+
+	return mfc_cleanup(mfc, &post_cleanup_redo_multifundchannel, redo);
+}
+
+/* Return true if this destination failed, false otherwise.  */
+static bool dest_failed(struct multifundchannel_destination *dest)
+{
+	switch (dest->state) {
+	case MULTIFUNDCHANNEL_START_NOT_YET:
+	case MULTIFUNDCHANNEL_STARTED:
+	case MULTIFUNDCHANNEL_DONE:
+		return false;
+
+	case MULTIFUNDCHANNEL_CONNECT_FAILED:
+	case MULTIFUNDCHANNEL_START_FAILED:
+	case MULTIFUNDCHANNEL_COMPLETE_FAILED:
+		return true;
+	}
+	abort();
+}
+
+/* Filter the failing destinations.  */
+static struct command_result *
+post_cleanup_redo_multifundchannel(struct multifundchannel_redo *redo)
+{
+	struct multifundchannel_command *mfc = redo->mfc;
+	const char *failing_method = redo->failing_method;
+	size_t i;
+	struct multifundchannel_destination *old_destinations;
+	struct multifundchannel_destination *new_destinations;
+
+	plugin_log(mfc->cmd->plugin, LOG_DBG,
+		   "mfc %"PRIu64": Filtering destinations.",
+		   mfc->id);
+
+	/* We got the args now.  */
+	tal_free(redo);
+
+	/* Clean up previous funding transaction if any.  */
+	mfc->psbt = tal_free(mfc->psbt);
+	mfc->txid = tal_free(mfc->txid);
+
+	/* Filter out destinations.  */
+	old_destinations = tal_steal(tmpctx, mfc->destinations);
+	mfc->destinations = NULL;
+	new_destinations = tal_arr(mfc, struct multifundchannel_destination,
+				   0);
+
+	for (i = 0; i < tal_count(old_destinations); ++i) {
+		struct multifundchannel_destination *dest;
+
+		dest = &old_destinations[i];
+
+		if (dest_failed(dest)) {
+			struct multifundchannel_removed removed;
+
+			plugin_log(mfc->cmd->plugin, LOG_DBG,
+				   "mfc %"PRIu64", dest %u: "
+				   "failed.",
+				   mfc->id, dest->index);
+
+			removed.id = dest->id;
+			removed.method = failing_method;
+			removed.error = dest->error;
+			removed.code = dest->code;
+			/* Add to removeds.  */
+			tal_arr_expand(&mfc->removeds, removed);
+		} else {
+			plugin_log(mfc->cmd->plugin, LOG_DBG,
+				   "mfc %"PRIu64", dest %u: "
+				   "succeeded.",
+				   mfc->id, dest->index);
+
+			/* Reset its state.  */
+			dest->state = MULTIFUNDCHANNEL_START_NOT_YET;
+			/* We do not mess with `dest->index` so we have
+			a stable id between reattempts.
+			*/
+			/* Re-add to new destinations.  */
+			tal_arr_expand(&new_destinations, *dest);
+		}
+	}
+	mfc->destinations = new_destinations;
+
+	if (tal_count(mfc->destinations) < mfc->minchannels) {
+		/* Too many failed. */
+		struct json_stream *out;
+
+		assert(tal_count(mfc->removeds) != 0);
+
+		plugin_log(mfc->cmd->plugin, LOG_DBG,
+			   "mfc %"PRIu64": %zu destinations failed, failing.",
+			   mfc->id, tal_count(mfc->removeds));
+
+		/* Blame it on the last.  */
+		i = tal_count(mfc->removeds) - 1;
+
+		out = jsonrpc_stream_fail_data(mfc->cmd,
+					       mfc->removeds[i].code,
+					       tal_fmt(tmpctx,
+						       "'%s' failed",
+						       failing_method));
+		json_add_node_id(out, "id", &mfc->removeds[i].id);
+		json_add_string(out, "method", failing_method);
+		json_add_jsonstr(out, "error", mfc->removeds[i].error);
+
+		/* Close 'data'.  */
+		json_object_end(out);
+
+		return mfc_finished(mfc, out);
+	}
+
+	/* Okay, we still have destinations to try --- reinvoke.  */
+	return perform_multifundchannel(mfc);
+}
+
+static struct command_result *param_positive_number(struct command *cmd,
+						    const char *name,
+						    const char *buffer,
+						    const jsmntok_t *tok,
+						    unsigned int **num)
+{
+	struct command_result *res = param_number(cmd, name, buffer, tok, num);
+	if (res)
+		return res;
+	if (**num == 0)
+		return command_fail_badparam(cmd, name, buffer, tok,
+					     "should be a positive integer");
+	return NULL;
+}
+
 /*-----------------------------------------------------------------------------
 Command Entry Point
 -----------------------------------------------------------------------------*/
@@ -1847,6 +1983,7 @@ json_multifundchannel(struct command *cmd,
 	const char *feerate_str;
 	u32 *minconf;
 	const jsmntok_t *utxos_tok;
+	u32 *minchannels;
 
 	struct multifundchannel_command *mfc;
 
@@ -1855,6 +1992,7 @@ json_multifundchannel(struct command *cmd,
 		   p_opt("feerate", param_string, &feerate_str),
 		   p_opt_def("minconf", param_number, &minconf, 1),
 		   p_opt("utxos", param_tok, &utxos_tok),
+		   p_opt("minchannels", param_positive_number, &minchannels),
 		   NULL))
 		return command_param_failed();
 
@@ -1877,6 +2015,9 @@ json_multifundchannel(struct command *cmd,
 					     json_tok_full_len(utxos_tok));
 	else
 		mfc->utxos_str = NULL;
+	/* Default is that all must succeed. */
+	mfc->minchannels = minchannels ? *minchannels : tal_count(mfc->destinations);
+	mfc->removeds = tal_arr(mfc, struct multifundchannel_removed, 0);
 	mfc->psbt = NULL;
 	mfc->change_scriptpubkey = NULL;
 	mfc->txid = NULL;

tests/test_connection.py

@@ -1408,6 +1408,78 @@ def test_multifunding_param_failures(node_factory):
         l1.rpc.multifundchannel(destinations)
 
 
+@unittest.skipIf(not DEVELOPER, "disconnect=... needs DEVELOPER=1")
+def test_multifunding_best_effort(node_factory, bitcoind):
+    '''
+    Check that best_effort flag works.
+    '''
+    disconnects = ["-WIRE_INIT",
+                   "-WIRE_ACCEPT_CHANNEL",
+                   "-WIRE_FUNDING_SIGNED"]
+    l1 = node_factory.get_node()
+    l2 = node_factory.get_node()
+    l3 = node_factory.get_node(disconnect=disconnects)
+    l4 = node_factory.get_node()
+
+    l1.fundwallet(2000000)
+
+    destinations = [{"id": '{}@localhost:{}'.format(l2.info['id'], l2.port),
+                     "amount": 50000},
+                    {"id": '{}@localhost:{}'.format(l3.info['id'], l3.port),
+                     "amount": 50000},
+                    {"id": '{}@localhost:{}'.format(l4.info['id'], l4.port),
+                     "amount": 50000}]
+
+    for i, d in enumerate(disconnects):
+        # Should succeed due to best-effort flag.
+        l1.rpc.multifundchannel(destinations, minchannels=2)
+        bitcoind.generate_block(6, wait_for_mempool=1)
+
+        # Only l3 should fail to have channels.
+        for node in [l1, l2, l4]:
+            node.daemon.wait_for_log(r'to CHANNELD_NORMAL')
+
+        # There should be working channels to l2 and l4.
+        for ldest in [l2, l4]:
+            inv = ldest.rpc.invoice(5000, 'i{}'.format(i), 'i{}'.format(i))['bolt11']
+            l1.rpc.pay(inv)
+
+        # Function to find the SCID of the channel that is
+        # currently open.
+        # Cannot use LightningNode.get_channel_scid since
+        # it assumes the *first* channel found is the one
+        # wanted, but in our case we close channels and
+        # open again, so multiple channels may remain
+        # listed.
+        def get_funded_channel_scid(n1, n2):
+            peers = n1.rpc.listpeers(n2.info['id'])['peers']
+            assert len(peers) == 1
+            peer = peers[0]
+            channels = peer['channels']
+            assert channels
+            for c in channels:
+                state = c['state']
+                if state in ('CHANNELD_AWAITING_LOCKIN', 'CHANNELD_NORMAL'):
+                    return c['short_channel_id']
+            assert False
+
+        # Now close channels to l2 and l4, for the next run.
+        l1.rpc.close(get_funded_channel_scid(l1, l2))
+        l1.rpc.close(get_funded_channel_scid(l1, l4))
+
+        for node in [l1, l2, l4]:
+            node.daemon.wait_for_log(r'to CLOSINGD_COMPLETE')
+
+    # With 2 down, it will fail to fund channel
+    l2.stop()
+    l3.stop()
+    with pytest.raises(RpcError, match=r'Connection refused'):
+        l1.rpc.multifundchannel(destinations, minchannels=2)
+
+    # This works though.
+    l1.rpc.multifundchannel(destinations, minchannels=1)
+
+
 def test_lockin_between_restart(node_factory, bitcoind):
     l1 = node_factory.get_node(may_reconnect=True)
     l2 = node_factory.get_node(options={'funding-confirms': 3},