closingd: negotiate closing fee one sat at a time

When negotiating the tx fee for closing a channel (as per [1]),
do not bisect the fee range but rather be stubborn and give up
one satoshi at a time.

Partially resolves #3270

[1] https://github.com/lightningnetwork/lightning-rfc/blob/master/02-peer-protocol.md#closing-negotiation-closing_signed

Changelog-Added: New optional parameter to the `close` command to control closing fee negotiation strategy

closingd/closing_wire.csv

@@ -21,6 +21,7 @@ msgdata,closing_init,local_scriptpubkey_len,u16,
 msgdata,closing_init,local_scriptpubkey,u8,local_scriptpubkey_len
 msgdata,closing_init,remote_scriptpubkey_len,u16,
 msgdata,closing_init,remote_scriptpubkey,u8,remote_scriptpubkey_len
+msgdata,closing_init,fee_negotiation,u8,
 msgdata,closing_init,reconnected,bool,
 msgdata,closing_init,next_index_local,u64,
 msgdata,closing_init,next_index_remote,u64,

closingd/closingd.c

@@ -2,6 +2,7 @@
 #include <ccan/fdpass/fdpass.h>
 #include <closingd/gen_closing_wire.h>
 #include <common/close_tx.h>
+#include <common/closing_fee.h>
 #include <common/crypto_sync.h>
 #include <common/derive_basepoints.h>
 #include <common/htlc.h>
@@ -489,11 +490,11 @@ static void adjust_feerange(struct feerange *feerange,
 }
 
 /* Figure out what we should offer now. */
-static struct amount_sat adjust_offer(struct per_peer_state *pps,
-				      const struct channel_id *channel_id,
-				      const struct feerange *feerange,
-				      struct amount_sat remote_offer,
-				      struct amount_sat min_fee_to_accept)
+static struct amount_sat
+adjust_offer(struct per_peer_state *pps, const struct channel_id *channel_id,
+	     const struct feerange *feerange, struct amount_sat remote_offer,
+	     struct amount_sat min_fee_to_accept,
+	     u8 fee_negotiation)
 {
 	struct amount_sat min_plus_one, avg;
 
@@ -519,10 +520,19 @@ static struct amount_sat adjust_offer(struct per_peer_state *pps,
 			    type_to_string(tmpctx, struct amount_sat,
 					   &min_fee_to_accept));
 
-	/* Bisect between our minimum and max. */
+	/* Pick a fee between our minimum and the range's max. */
 	if (amount_sat_greater(feerange->min, min_fee_to_accept))
 		min_fee_to_accept = feerange->min;
 
+	if (fee_negotiation == CLOSING_FEE_NEGOTIATION_STUBBORN) {
+		/* feerange has already been adjusted so that our new offer
+		 * is ok to be any number in [feerange->min, feerange->max].
+		 */
+		if (feerange->higher_side == LOCAL)
+			return feerange->max;
+		return min_fee_to_accept;
+	}
+
 	if (!amount_sat_add(&avg, feerange->max, min_fee_to_accept))
 		peer_failed(pps, channel_id,
 			    "Fee offer %s max too large",
@@ -570,6 +580,8 @@ int main(int argc, char *argv[])
 	struct feerange feerange;
 	enum side funder;
 	u8 *scriptpubkey[NUM_SIDES], *funding_wscript;
+	u8 fee_negotiation;
+	const char* fee_negotiation_str = "unknown";
 	struct channel_id channel_id;
 	bool reconnected;
 	u64 next_index[NUM_SIDES], revocations_received;
@@ -597,6 +609,7 @@ int main(int argc, char *argv[])
 				   &offer[LOCAL],
 				   &scriptpubkey[LOCAL],
 				   &scriptpubkey[REMOTE],
+				   &fee_negotiation,
 				   &reconnected,
 				   &next_index[LOCAL],
 				   &next_index[REMOTE],
@@ -609,6 +622,15 @@ int main(int argc, char *argv[])
 	/* stdin == requests, 3 == peer, 4 = gossip, 5 = gossip_store, 6 = hsmd */
 	per_peer_state_set_fds(pps, 3, 4, 5);
 
+	switch (fee_negotiation) {
+	case CLOSING_FEE_NEGOTIATION_BISECT:
+		fee_negotiation_str = "bisect";
+		break;
+	case CLOSING_FEE_NEGOTIATION_STUBBORN:
+		fee_negotiation_str = "stubborn";
+		break;
+	}
+
 	status_debug("out = %s/%s",
 		     type_to_string(tmpctx, struct amount_sat, &out[LOCAL]),
 		     type_to_string(tmpctx, struct amount_sat, &out[REMOTE]));
@@ -628,13 +650,14 @@ int main(int argc, char *argv[])
 			     channel_reestablish, scriptpubkey[LOCAL],
 			     &last_remote_per_commit_secret);
 
-	peer_billboard(true, "Negotiating closing fee between %s"
-		       " and %s satoshi (ideal %s)",
-		       type_to_string(tmpctx, struct amount_sat,
-				      &min_fee_to_accept),
-		       type_to_string(tmpctx, struct amount_sat,
-				      &commitment_fee),
-		       type_to_string(tmpctx, struct amount_sat, &offer[LOCAL]));
+	peer_billboard(
+	    true,
+	    "Negotiating closing fee between %s and %s satoshi (ideal %s) "
+	    "using strategy: %s",
+	    type_to_string(tmpctx, struct amount_sat, &min_fee_to_accept),
+	    type_to_string(tmpctx, struct amount_sat, &commitment_fee),
+	    type_to_string(tmpctx, struct amount_sat, &offer[LOCAL]),
+	    fee_negotiation_str);
 
 	/* BOLT #2:
 	 *
@@ -692,7 +715,8 @@ int main(int argc, char *argv[])
 			offer[LOCAL] = adjust_offer(pps,
 						    &channel_id,
 						    &feerange, offer[REMOTE],
-						    min_fee_to_accept);
+						    min_fee_to_accept,
+						    fee_negotiation);
 			send_offer(pps, chainparams, &channel_id,
 				   funding_pubkey,
 				   scriptpubkey, &funding_txid, funding_txout,

common/closing_fee.h

@@ -0,0 +1,9 @@
+#ifndef LIGHTNING_COMMON_CLOSING_FEE_H
+#define LIGHTNING_COMMON_CLOSING_FEE_H
+
+#include <ccan/short_types/short_types.h>
+
+static const u8 CLOSING_FEE_NEGOTIATION_BISECT = 0;
+static const u8 CLOSING_FEE_NEGOTIATION_STUBBORN = 1;
+
+#endif /* LIGHTNING_COMMON_CLOSING_FEE_H */

contrib/pylightning/requirements.txt

@@ -1 +1 @@
-pyln-client==0.7.3
+pyln-client>=0.7.3

contrib/pyln-client/pyln/client/__init__.py

@@ -2,7 +2,7 @@
 from .plugin import Plugin, monkey_patch
 
 
-__version__ = "0.8.0"
+__version__ = "0.9.0"
 
 
 __all__ = [

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

@@ -412,11 +412,12 @@ def close(self, peer_id, *args, **kwargs):
                 if args[0] is None and isinstance(args[1], int):
                     return self._deprecated_close(peer_id, *args, **kwargs)
 
-        def _close(peer_id, unilateraltimeout=None, destination=None):
+        def _close(peer_id, unilateraltimeout=None, destination=None, fee_negotiation=None):
             payload = {
                 "id": peer_id,
                 "unilateraltimeout": unilateraltimeout,
-                "destination": destination
+                "destination": destination,
+                "fee_negotiation": fee_negotiation
             }
             return self.call("close", payload)
 

doc/lightning-close.7.md

@@ -4,7 +4,7 @@ lightning-close -- Command for closing channels with direct peers
 SYNOPSIS
 --------
 
-**close** *id* \[*unilateraltimeout*\] \[*destination*\]
+**close** *id* \[*unilateraltimeout*\] \[*destination*\] \[*fee_negotiation_strategy*\]
 
 DESCRIPTION
 -----------
@@ -28,6 +28,13 @@ The default is 2 days (172800 seconds).
 The *destination* can be of any Bitcoin accepted type, including bech32.
 If it isn't specified, the default is a c-lightning wallet address.
 
+The *fee_negotiation_strategy* parameter controls how closing fee
+negotiation is performed assuming the peer proposes a fee that is
+different than our estimate. "bisect" picks the middle of the interval
+between our fee and their fee at each negotiation iteration. "stubborn"
+insists on our estimate, giving up 1 satoshi from it at each negotiation
+iteration.
+
 The peer needs to be live and connected in order to negotiate a mutual
 close. The default of unilaterally closing after 48 hours is usually a
 reasonable indication that you can no longer contact the peer.

lightningd/channel.c

@@ -1,6 +1,7 @@
 #include <bitcoin/script.h>
 #include <ccan/crypto/hkdf_sha256/hkdf_sha256.h>
 #include <ccan/tal/str/str.h>
+#include <common/closing_fee.h>
 #include <common/fee_states.h>
 #include <common/json_command.h>
 #include <common/jsonrpc_errors.h>
@@ -242,6 +243,7 @@ struct channel *new_channel(struct peer *peer, u64 dbid,
 	channel->shutdown_scriptpubkey[REMOTE]
 		= tal_steal(channel, remote_shutdown_scriptpubkey);
 	channel->final_key_idx = final_key_idx;
+	channel->closing_fee_negotiation = CLOSING_FEE_NEGOTIATION_BISECT;
 	if (local_shutdown_scriptpubkey)
 		channel->shutdown_scriptpubkey[LOCAL]
 			= tal_steal(channel, local_shutdown_scriptpubkey);

lightningd/channel.h

@@ -93,6 +93,7 @@ struct channel {
 	const u8 *shutdown_scriptpubkey[NUM_SIDES];
 	/* Address for any final outputs */
 	u64 final_key_idx;
+	u8 closing_fee_negotiation;
 
 	/* Reestablishment stuff: last sent commit and revocation details. */
 	bool last_was_revoke;

lightningd/closing_control.c

@@ -290,6 +290,7 @@ void peer_start_closingd(struct channel *channel,
 				      minfee, feelimit, startfee,
 				      channel->shutdown_scriptpubkey[LOCAL],
 				      channel->shutdown_scriptpubkey[REMOTE],
+				      channel->closing_fee_negotiation,
 				      reconnected,
 				      channel->next_index[LOCAL],
 				      channel->next_index[REMOTE],

lightningd/peer_control.c

@@ -15,6 +15,7 @@
 #include <ccan/tal/str/str.h>
 #include <channeld/gen_channel_wire.h>
 #include <common/addr.h>
+#include <common/closing_fee.h>
 #include <common/dev_disconnect.h>
 #include <common/features.h>
 #include <common/htlc_trim.h>
@@ -1285,6 +1286,7 @@ static struct command_result *json_close(struct command *cmd,
 	const u8 *close_to_script = NULL;
 	unsigned int *old_timeout;
 	bool *old_force, close_script_set;
+	const char *fee_negotiation;
 
 	/* For generating help, give new-style. */
 	if (!params || !deprecated_apis) {
@@ -1294,6 +1296,8 @@ static struct command_result *json_close(struct command *cmd,
 				     &timeout, 48 * 3600),
 			   p_opt("destination", param_bitcoin_address,
 				 &close_to_script),
+			   p_opt("fee_negotiation", param_string,
+				 &fee_negotiation),
 			   NULL))
 			return command_param_failed();
 		do_timeout = (*timeout != 0);
@@ -1308,6 +1312,8 @@ static struct command_result *json_close(struct command *cmd,
 				 param_tok_timeout_or_force, &firsttok),
 			   p_opt("destination_or_timeout",
 				 param_tok_dest_or_timeout, &secondtok),
+			   p_opt("fee_negotiation", param_string,
+				 &fee_negotiation),
 			   NULL))
 			return command_param_failed();
 
@@ -1394,6 +1400,8 @@ static struct command_result *json_close(struct command *cmd,
 				 &close_to_script),
 			   p_opt("force", param_bool, &old_force),
 			   p_opt("timeout", param_number, &old_timeout),
+			   p_opt("fee_negotiation", param_string,
+				 &fee_negotiation),
 			   NULL))
 			return command_param_failed();
 
@@ -1478,6 +1486,15 @@ static struct command_result *json_close(struct command *cmd,
 	} else
 		close_script_set = false;
 
+	if (fee_negotiation == NULL || streq(fee_negotiation, "bisect"))
+		channel->closing_fee_negotiation = CLOSING_FEE_NEGOTIATION_BISECT;
+	else if (streq(fee_negotiation, "stubborn"))
+		channel->closing_fee_negotiation = CLOSING_FEE_NEGOTIATION_STUBBORN;
+	else
+		return command_fail(
+		    cmd, JSONRPC2_INVALID_PARAMS,
+		    "Unknown value given for fee_negotiation: \"%s\", "
+		    "should be one of \"bisect\" or \"stubborn\"", fee_negotiation);
 
 	/* Normal case.
 	 * We allow states shutting down and sigexchange; a previous

tests/test_closing.py

@@ -411,10 +411,10 @@ def closing_fee(node_factory, bitcoind, chainparams, opts):
     assert bitcoind.rpc.getmempoolinfo()['size'] == 0
 
     if opts['close_initiated_by'] == 'funder':
-        funder.rpc.close(peer_id=fundee_id)
+        funder.rpc.close(peer_id=fundee_id, fee_negotiation=opts['fee_negotiation'])
     else:
         assert opts['close_initiated_by'] == 'fundee'
-        fundee.rpc.close(peer_id=funder_id)
+        fundee.rpc.close(peer_id=funder_id, fee_negotiation=opts['fee_negotiation'])
 
     # Get the closing transaction from the bitcoind mempool and get its fee
 
@@ -457,20 +457,33 @@ def get_fee_from_status(node, peer_id, i):
 
 
 def test_closing_fee(node_factory, bitcoind, chainparams):
-    """Test that the closing negotiation strategy works"""
+    """Test that the closing fee negotiation strategy works"""
     # feerate 27625 -> closing fee negotiation starts at 20000
     # feerate 29006 -> closing fee negotiation starts at 21000
 
     opts = {
         'funder_feerate_per_kw': 29006,
-        'fundee_feerate_per_kw': 27625,
-        'close_initiated_by': 'funder',
-        'expected_close_fee': 20333
+        'fundee_feerate_per_kw': 27625
     }
 
+    opts['fee_negotiation'] = 'bisect';
+
+    opts['close_initiated_by'] = 'funder'
+    opts['expected_close_fee'] = 20333;
+    closing_fee(node_factory, bitcoind, chainparams, opts)
+
+    opts['close_initiated_by'] = 'fundee'
+    opts['expected_close_fee'] = 20333;
+    closing_fee(node_factory, bitcoind, chainparams, opts)
+
+    opts['fee_negotiation'] = 'stubborn';
+
+    opts['close_initiated_by'] = 'funder'
+    opts['expected_close_fee'] = 20989;
     closing_fee(node_factory, bitcoind, chainparams, opts)
 
     opts['close_initiated_by'] = 'fundee'
+    opts['expected_close_fee'] = 20009;
     closing_fee(node_factory, bitcoind, chainparams, opts)
 
 

wallet/test/run-wallet.c

@@ -492,6 +492,11 @@ struct command_result *param_short_channel_id(struct command *cmd UNNEEDED,
 					      const jsmntok_t *tok UNNEEDED,
 					      struct short_channel_id **scid UNNEEDED)
 { fprintf(stderr, "param_short_channel_id called!\n"); abort(); }
+/* Generated stub for param_string */
+struct command_result *param_string(struct command *cmd UNNEEDED, const char *name UNNEEDED,
+				    const char * buffer UNNEEDED, const jsmntok_t *tok UNNEEDED,
+				    const char **str UNNEEDED)
+{ fprintf(stderr, "param_string called!\n"); abort(); }
 /* Generated stub for param_tok */
 struct command_result *param_tok(struct command *cmd UNNEEDED, const char *name UNNEEDED,
 				 const char *buffer UNNEEDED, const jsmntok_t * tok UNNEEDED,