plugins/bcli.c: sendrawtransaction now has a required `allowhighfee…

…s` argument.

Changelog-Deprecated: plugin: `bcli` replacements should note that `sendrawtransaction` now has a second required Boolean argument, `allowhighfees`, which if `true`, means ignore any fee limits and just broadcast the transaction. Use `--deprecated-apis` to use older `bcli` replacement plugins that only support a single argument.

doc/PLUGINS.md

@@ -1164,8 +1164,11 @@ The plugin must respond to `gettxout` with the following fields:
 
 ### `sendrawtransaction`
 
-This call takes one parameter, a string representing a hex-encoded Bitcoin
-transaction.
+This call takes two parameters,
+a string `tx` representing a hex-encoded Bitcoin transaction,
+and a boolean `allowhighfees`, which if set means suppress
+any high-fees check implemented in the backend, since the given
+transaction may have fees that are very high.
 
 The plugin must broadcast it and respond with the following fields:
     - `success` (boolean), which is `true` if the broadcast succeeded

lightningd/bitcoind.c

@@ -21,6 +21,7 @@
 #include <ccan/tal/grab_file/grab_file.h>
 #include <ccan/tal/path/path.h>
 #include <ccan/tal/str/str.h>
+#include <common/configdir.h>
 #include <common/json_helpers.h>
 #include <common/memleak.h>
 #include <common/timeout.h>
@@ -252,6 +253,7 @@ void bitcoind_estimate_fees_(struct bitcoind *bitcoind,
 
 struct sendrawtx_call {
 	struct bitcoind *bitcoind;
+	const char *hextx;
 	void (*cb)(struct bitcoind *bitcoind,
 		   bool success,
 		   const char *err_msg,
@@ -293,28 +295,113 @@ static void sendrawtx_callback(const char *buf, const jsmntok_t *toks,
 	tal_free(call);
 }
 
-void bitcoind_sendrawtx_(struct bitcoind *bitcoind,
-			 const char *hextx,
-			 void (*cb)(struct bitcoind *bitcoind,
-				    bool success, const char *err_msg, void *),
-			 void *cb_arg)
+/* For compatibility with `sendrawtransaction` plugins written for
+ * 0.9.0 or earlier.
+ * Once this deprecated API is removed, we can just delete this function
+ * and use sendrawtx_callback directly in bitcoind_sendrawtx_ahf_.
+ */
+static void sendrawtx_compatv090_callback(const char *buf,
+					  const jsmntok_t *toks,
+					  const jsmntok_t *idtok,
+					  struct sendrawtx_call *call)
+{
+	const jsmntok_t *errtok;
+	const jsmntok_t *codetok;
+	errcode_t code;
+	struct jsonrpc_request *req;
+
+	static bool warned = false;
+
+
+	/* If deprecated APIs not enabled, fail it outright.  */
+	if (!deprecated_apis)
+		goto fallback;
+
+	/* If we got a JSONRPC2_INVALID_PARAMS error, assume it is
+	 * because it is an old plugin which does not support the
+	 * new `allowhighfees` parameter.
+	 */
+	errtok = json_get_member(buf, toks, "error");
+	if (!errtok)
+		goto fallback;
+
+	codetok = json_get_member(buf, errtok, "code");
+	if (!codetok)
+		goto fallback;
+
+	if (!json_to_errcode(buf, codetok, &code))
+		goto fallback;
+
+	if (code != JSONRPC2_INVALID_PARAMS)
+		goto fallback;
+
+	/* Possibly it is because `allowhighfees` is not understood
+	 * by the plugin.  */
+	if (!warned) {
+		warned = true;
+		log_unusual(call->bitcoind->log,
+			    "`sendrawtransaction` failed when given two "
+			    "arguments, will retry with single-argument "
+			    "deprecated API.");
+	}
+
+	/* Retry with a single argument, hextx.  */
+	req = jsonrpc_request_start(call->bitcoind, "sendrawtransaction",
+				    call->bitcoind->log,
+				    &sendrawtx_callback, call);
+	json_add_string(req->stream, "tx", call->hextx);
+	jsonrpc_request_end(req);
+	bitcoin_plugin_send(call->bitcoind, req);
+
+	return;
+
+fallback:
+	sendrawtx_callback(buf, toks, idtok, call);
+	return;
+}
+
+void bitcoind_sendrawtx_ahf_(struct bitcoind *bitcoind,
+			     const char *hextx,
+			     bool allowhighfees,
+			     void (*cb)(struct bitcoind *bitcoind,
+					bool success, const char *msg, void *),
+			     void *cb_arg)
 {
 	struct jsonrpc_request *req;
 	struct sendrawtx_call *call = tal(bitcoind, struct sendrawtx_call);
 
 	call->bitcoind = bitcoind;
+	/* For compatibility with 0.9.0 or earlier.
+	 * Once removed, we can remove the hextx field in
+	 * struct sendrawtx_call as well.
+	 */
+	if (deprecated_apis)
+		call->hextx = tal_strdup(call, hextx);
+	else
+		call->hextx = NULL;
 	call->cb = cb;
 	call->cb_arg = cb_arg;
 	log_debug(bitcoind->log, "sendrawtransaction: %s", hextx);
 
 	req = jsonrpc_request_start(bitcoind, "sendrawtransaction",
-				    bitcoind->log, sendrawtx_callback,
+				    bitcoind->log,
+				    sendrawtx_compatv090_callback,
 				    call);
 	json_add_string(req->stream, "tx", hextx);
+	json_add_bool(req->stream, "allowhighfees", allowhighfees);
 	jsonrpc_request_end(req);
 	bitcoin_plugin_send(bitcoind, req);
 }
 
+void bitcoind_sendrawtx_(struct bitcoind *bitcoind,
+			 const char *hextx,
+			 void (*cb)(struct bitcoind *bitcoind,
+				    bool success, const char *msg, void *),
+			 void *arg)
+{
+	return bitcoind_sendrawtx_ahf_(bitcoind, hextx, false, cb, arg);
+}
+
 /* `getrawblockbyheight`
  *
  * If no block were found at that height, will set each field to `null`.

lightningd/bitcoind.h

@@ -74,6 +74,21 @@ void bitcoind_estimate_fees_(struct bitcoind *bitcoind,
 						    const u32 *),	\
 				(arg))
 
+void bitcoind_sendrawtx_ahf_(struct bitcoind *bitcoind,
+			     const char *hextx,
+			     bool allowhighfees,
+			     void (*cb)(struct bitcoind *bitcoind,
+					bool success, const char *msg, void *),
+			     void *arg);
+#define bitcoind_sendrawtx_ahf(bitcoind_, hextx, allowhighfees, cb, arg)\
+	bitcoind_sendrawtx_ahf_((bitcoind_), (hextx),			\
+				(allowhighfees),			\
+				typesafe_cb_preargs(void, void *,	\
+						    (cb), (arg),	\
+						    struct bitcoind *,	\
+						    bool, const char *),\
+				(arg))
+
 void bitcoind_sendrawtx_(struct bitcoind *bitcoind,
 			 const char *hextx,
 			 void (*cb)(struct bitcoind *bitcoind,

lightningd/chaintopology.c

@@ -218,10 +218,12 @@ static void broadcast_done(struct bitcoind *bitcoind,
 	}
 }
 
-void broadcast_tx(struct chain_topology *topo,
-		  struct channel *channel, const struct bitcoin_tx *tx,
-		  void (*failed_or_success)(struct channel *channel,
-					    bool success, const char *err))
+void broadcast_tx_ahf(struct chain_topology *topo,
+		      struct channel *channel, const struct bitcoin_tx *tx,
+		      bool allowhighfees,
+		      void (*failed)(struct channel *channel,
+				     bool success,
+				     const char *err))
 {
 	/* Channel might vanish: topo owns it to start with. */
 	struct outgoing_tx *otx = tal(topo, struct outgoing_tx);
@@ -230,16 +232,26 @@ void broadcast_tx(struct chain_topology *topo,
 	otx->channel = channel;
 	bitcoin_txid(tx, &otx->txid);
 	otx->hextx = tal_hex(otx, rawtx);
-	otx->failed_or_success = failed_or_success;
+	otx->failed_or_success = failed;
 	tal_free(rawtx);
 	tal_add_destructor2(channel, clear_otx_channel, otx);
 
 	log_debug(topo->log, "Broadcasting txid %s",
 		  type_to_string(tmpctx, struct bitcoin_txid, &otx->txid));
 
 	wallet_transaction_add(topo->ld->wallet, tx->wtx, 0, 0);
-	bitcoind_sendrawtx(topo->bitcoind, otx->hextx, broadcast_done, otx);
+	bitcoind_sendrawtx_ahf(topo->bitcoind, otx->hextx, allowhighfees,
+			       broadcast_done, otx);
 }
+void broadcast_tx(struct chain_topology *topo,
+		  struct channel *channel, const struct bitcoin_tx *tx,
+		  void (*failed)(struct channel *channel,
+				 bool success,
+				 const char *err))
+{
+	return broadcast_tx_ahf(topo, channel, tx, false, failed);
+}
+
 
 static enum watch_result closeinfo_txid_confirmed(struct lightningd *ld,
 						  struct channel *channel,

lightningd/chaintopology.h

@@ -181,6 +181,14 @@ void broadcast_tx(struct chain_topology *topo,
 		  void (*failed)(struct channel *channel,
 				 bool success,
 				 const char *err));
+/* Like the above, but with an additional `allowhighfees` parameter.
+ * If true, suppress any high-fee checks in the backend.  */
+void broadcast_tx_ahf(struct chain_topology *topo,
+		      struct channel *channel, const struct bitcoin_tx *tx,
+		      bool allowhighfees,
+		      void (*failed)(struct channel *channel,
+				     bool success,
+				     const char *err));
 
 struct chain_topology *new_topology(struct lightningd *ld, struct log *log);
 void setup_topology(struct chain_topology *topology, struct timers *timers,

lightningd/onchain_control.c

@@ -284,8 +284,12 @@ static void handle_onchain_broadcast_tx(struct channel *channel,
 	wallet_transaction_annotate(w, &txid, type, channel->dbid);
 
 	/* We don't really care if it fails, we'll respond via watch. */
-	broadcast_tx(channel->peer->ld->topology, channel, tx,
-		     is_rbf ? &handle_onchain_broadcast_rbf_tx_cb : NULL);
+	/* If the onchaind signals this as RBF-able, then we also
+	 * set allowhighfees, as the transaction may be RBFed into
+	 * high feerates as protection against the MAD-HTLC attack.  */
+	broadcast_tx_ahf(channel->peer->ld->topology, channel,
+			 tx, is_rbf,
+			 is_rbf ? &handle_onchain_broadcast_rbf_tx_cb : NULL);
 }
 
 static void handle_onchain_unwatch_tx(struct channel *channel, const u8 *msg)

plugins/bcli.c

@@ -793,13 +793,29 @@ static struct command_result *sendrawtransaction(struct command *cmd,
                                                  const jsmntok_t *toks)
 {
 	const char **params = tal_arr(cmd, const char *, 1);
+	bool *allowhighfees;
 
 	/* bitcoin-cli wants strings. */
 	if (!param(cmd, buf, toks,
 	           p_req("tx", param_string, &params[0]),
+		   p_req("allowhighfees", param_bool, &allowhighfees),
 	           NULL))
 		return command_param_failed();
 
+	if (*allowhighfees) {
+		if (bitcoind->version >= 190001)
+			/* Starting in 19.0.1, second argument is
+			 * maxfeerate, which when set to 0 means
+			 * no max feerate.
+			 */
+			tal_arr_expand(&params, "0");
+		else
+			/* in older versions, second arg is allowhighfees,
+			 * set to true to allow high fees.
+			 */
+			tal_arr_expand(&params, "true");
+	}
+
 	start_bitcoin_cli(NULL, cmd, process_sendrawtransaction, true,
 			  BITCOIND_HIGH_PRIO, "sendrawtransaction", params, NULL);
 

tests/test_plugin.py

@@ -1222,7 +1222,7 @@ def test_bcli(node_factory, bitcoind, chainparams):
         "vout": fc['outnum']
     })['amount'] is None)
 
-    resp = l1.rpc.call("sendrawtransaction", {"tx": "dummy"})
+    resp = l1.rpc.call("sendrawtransaction", {"tx": "dummy", "allowhighfees": False})
     assert not resp["success"] and "decode failed" in resp["errmsg"]