closingd: configurable closing fee negotiation step

When negotiating the transaction fee for closing a channel [1], we used
to always pick the middle of the range between our proposal and the
peer's proposal.

Introduce a new option `fee_negotiation_step` to the close command, so
the peer who initiates the close can choose his back off step.

Partially resolves https://github.com/ElementsProject/lightning/issues/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 the closing transaction fee negotiation back off step

closingd/closing_wire.csv

@@ -21,6 +21,8 @@ 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_step,u64,
+msgdata,closing_init,fee_negotiation_step_unit,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,13 +490,14 @@ 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, u64 fee_negotiation_step,
+	     u8 fee_negotiation_step_unit)
 {
-	struct amount_sat min_plus_one, avg;
+	struct amount_sat min_plus_one, range_len, step_sat, result;
+	struct amount_msat step_msat;
 
 	/* Within 1 satoshi?  Agree. */
 	if (!amount_sat_add(&min_plus_one, feerange->min, AMOUNT_SAT(1)))
@@ -507,8 +509,15 @@ static struct amount_sat adjust_offer(struct per_peer_state *pps,
 	if (amount_sat_greater_eq(min_plus_one, feerange->max))
 		return remote_offer;
 
+	/* feerange has already been adjusted so that our new offer is ok to be
+	 * any number in [feerange->min, feerange->max] and after the following
+	 * min_fee_to_accept is in that range. Thus, pick a fee in
+	 * [min_fee_to_accept, feerange->max]. */
+	if (amount_sat_greater(feerange->min, min_fee_to_accept))
+		min_fee_to_accept = feerange->min;
+
 	/* Max is below our minimum acceptable? */
-	if (amount_sat_less(feerange->max, min_fee_to_accept))
+	if (!amount_sat_sub(&range_len, feerange->max, min_fee_to_accept))
 		peer_failed(pps, channel_id,
 			    "Feerange %s-%s"
 			    " below minimum acceptable %s",
@@ -519,18 +528,44 @@ 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. */
-	if (amount_sat_greater(feerange->min, min_fee_to_accept))
-		min_fee_to_accept = feerange->min;
+	if (fee_negotiation_step_unit ==
+	    CLOSING_FEE_NEGOTIATION_STEP_UNIT_SATOSHI) {
+		/* -1 because the range boundary has already been adjusted with
+		 * one from our previous proposal. So, if the user requested a
+		 * step of 1 satoshi at a time we should just return our end of
+		 * the range from this function. */
+		amount_msat_from_u64(&step_msat,
+				     (fee_negotiation_step - 1) * MSAT_PER_SAT);
+	} else {
+		/* fee_negotiation_step is e.g. 20 to designate 20% from
+		 * range_len (which is in satoshi), so:
+		 * range_len * fee_negotiation_step / 100 [sat]
+		 * is equivalent to:
+		 * range_len * fee_negotiation_step * 10 [msat] */
+		amount_msat_from_u64(&step_msat,
+				     range_len.satoshis /* Raw: % calc */ *
+					 fee_negotiation_step * 10);
+	}
 
-	if (!amount_sat_add(&avg, feerange->max, min_fee_to_accept))
-		peer_failed(pps, channel_id,
-			    "Fee offer %s max too large",
-			    type_to_string(tmpctx, struct amount_sat,
-					   &feerange->max));
+	step_sat = amount_msat_to_sat_round_down(step_msat);
 
-	avg.satoshis /= 2; /* Raw: average calculation */
-	return avg;
+	if (feerange->higher_side == LOCAL) {
+		if (!amount_sat_sub(&result, feerange->max, step_sat))
+			/* step_sat > feerange->max, unlikely */
+			return min_fee_to_accept;
+
+		if (amount_sat_less_eq(result, min_fee_to_accept))
+			return min_fee_to_accept;
+	} else {
+		if (!amount_sat_add(&result, min_fee_to_accept, step_sat))
+			/* overflow, unlikely */
+			return feerange->max;
+
+		if (amount_sat_greater_eq(result, feerange->max))
+			return feerange->max;
+	}
+
+	return result;
 }
 
 #if DEVELOPER
@@ -570,6 +605,9 @@ int main(int argc, char *argv[])
 	struct feerange feerange;
 	enum side funder;
 	u8 *scriptpubkey[NUM_SIDES], *funding_wscript;
+	u64 fee_negotiation_step;
+	u8 fee_negotiation_step_unit;
+	char fee_negotiation_step_str[32]; /* fee_negotiation_step + "sat" */
 	struct channel_id channel_id;
 	bool reconnected;
 	u64 next_index[NUM_SIDES], revocations_received;
@@ -597,6 +635,8 @@ int main(int argc, char *argv[])
 				   &offer[LOCAL],
 				   &scriptpubkey[LOCAL],
 				   &scriptpubkey[REMOTE],
+				   &fee_negotiation_step,
+				   &fee_negotiation_step_unit,
 				   &reconnected,
 				   &next_index[LOCAL],
 				   &next_index[REMOTE],
@@ -609,6 +649,13 @@ 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);
 
+	snprintf(fee_negotiation_step_str, sizeof(fee_negotiation_step_str),
+		 "%" PRIu64 "%s", fee_negotiation_step,
+		 fee_negotiation_step_unit ==
+			 CLOSING_FEE_NEGOTIATION_STEP_UNIT_PERCENTAGE
+		     ? "%"
+		     : "sat");
+
 	status_debug("out = %s/%s",
 		     type_to_string(tmpctx, struct amount_sat, &out[LOCAL]),
 		     type_to_string(tmpctx, struct amount_sat, &out[REMOTE]));
@@ -616,6 +663,7 @@ int main(int argc, char *argv[])
 		     type_to_string(tmpctx, struct amount_sat, &our_dust_limit));
 	status_debug("fee = %s",
 		     type_to_string(tmpctx, struct amount_sat, &offer[LOCAL]));
+	status_debug("fee negotiation step = %s", fee_negotiation_step_str);
 	derive_channel_id(&channel_id, &funding_txid, funding_txout);
 
 	funding_wscript = bitcoin_redeem_2of2(ctx,
@@ -628,13 +676,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 step %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_step_str);
 
 	/* BOLT #2:
 	 *
@@ -692,7 +741,9 @@ 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_step,
+						    fee_negotiation_step_unit);
 			send_offer(pps, chainparams, &channel_id,
 				   funding_pubkey,
 				   scriptpubkey, &funding_txid, funding_txout,

common/Makefile

@@ -73,6 +73,7 @@ COMMON_SRC_NOGEN :=				\
 COMMON_SRC_GEN := common/gen_status_wire.c common/gen_peer_status_wire.c
 
 COMMON_HEADERS_NOGEN := $(COMMON_SRC_NOGEN:.c=.h)	\
+	common/closing_fee.h				\
 	common/ecdh.h					\
 	common/errcode.h				\
 	common/gossip_constants.h			\

common/closing_fee.h

@@ -0,0 +1,16 @@
+#ifndef LIGHTNING_COMMON_CLOSING_FEE_H
+#define LIGHTNING_COMMON_CLOSING_FEE_H
+
+#include "config.h"
+
+#include <ccan/short_types/short_types.h>
+
+/** During closing fee negotiation give up N% of the range between our
+ * proposal and the peer's proposal on each step. */
+static const u8 CLOSING_FEE_NEGOTIATION_STEP_UNIT_PERCENTAGE = 0;
+
+/** During closing fee negotiation give up N satoshi of the range between our
+ * proposal and the peer's proposal on each step. */
+static const u8 CLOSING_FEE_NEGOTIATION_STEP_UNIT_SATOSHI = 1;
+
+#endif /* LIGHTNING_COMMON_CLOSING_FEE_H */

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

@@ -412,11 +412,12 @@ class LightningRpc(UnixDomainSocketRpc):
                 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_step=None):
             payload = {
                 "id": peer_id,
                 "unilateraltimeout": unilateraltimeout,
-                "destination": destination
+                "destination": destination,
+                "fee_negotiation_step": fee_negotiation_step
             }
             return self.call("close", payload)
 

doc/lightning-close.7

@@ -3,7 +3,7 @@
 lightning-close - Command for closing channels with direct peers
 .SH SYNOPSIS
 
-\fBclose\fR \fIid\fR [\fIunilateraltimeout\fR] [\fIdestination\fR]
+\fBclose\fR \fIid\fR [\fIunilateraltimeout\fR] [\fIdestination\fR] [\fIfee_negotiation_step\fR]
 
 .SH DESCRIPTION
 
@@ -30,6 +30,30 @@ The \fIdestination\fR can be of any Bitcoin accepted type, including bech32\.
 If it isn't specified, the default is a c-lightning wallet address\.
 
 
+The \fIfee_negotiation_step\fR parameter controls how closing fee
+negotiation is performed assuming the peer proposes a fee that is
+different than our estimate\. On every negotiation step we must give up
+some amount from our proposal towards the peer's proposal\. This parameter
+can be an integer in which case it is interpreted as number of satoshis
+to step at a time\. Or it can be an integer followed by "%s" to designate
+a percentage of the interval to give up\. A few examples, assuming the peer
+proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000:
+
+.RS
+.IP \[bu]
+"10": our next proposal will be 4000-10=3990\.
+.IP \[bu]
+"10%": our next proposal will be 4000-(10% of (4000-3000))=3900\.
+.IP \[bu]
+"1": our next proposal will be 3999\. This is the most extreme case when we
+insist on our fee as much as possible\.
+.IP \[bu]
+"100%": our next proposal will be 3000\. This is the most relaxed case when
+we quickly accept the peer's proposal\.
+The default is "50%"\.
+
+.RE
+
 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\.

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_step*\]
 
 DESCRIPTION
 -----------
@@ -28,6 +28,22 @@ 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_step* parameter controls how closing fee
+negotiation is performed assuming the peer proposes a fee that is
+different than our estimate. On every negotiation step we must give up
+some amount from our proposal towards the peer's proposal. This parameter
+can be an integer in which case it is interpreted as number of satoshis
+to step at a time. Or it can be an integer followed by "%s" to designate
+a percentage of the interval to give up. A few examples, assuming the peer
+proposes a closing fee of 3000 satoshi and our estimate shows it must be 4000:
+* "10": our next proposal will be 4000-10=3990.
+* "10%": our next proposal will be 4000-(10% of (4000-3000))=3900.
+* "1": our next proposal will be 3999. This is the most extreme case when we
+insist on our fee as much as possible.
+* "100%": our next proposal will be 3000. This is the most relaxed case when
+we quickly accept the peer's proposal.
+The default is "50%".
+
 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,9 @@ 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_step = 50;
+	channel->closing_fee_negotiation_step_unit
+		= CLOSING_FEE_NEGOTIATION_STEP_UNIT_PERCENTAGE;
 	if (local_shutdown_scriptpubkey)
 		channel->shutdown_scriptpubkey[LOCAL]
 			= tal_steal(channel, local_shutdown_scriptpubkey);

lightningd/channel.h

@@ -94,6 +94,12 @@ struct channel {
 	/* Address for any final outputs */
 	u64 final_key_idx;
 
+	/* Amount to give up on each step of the closing fee negotiation. */
+	u64 closing_fee_negotiation_step;
+
+	/* Whether closing_fee_negotiation_step is in satoshi or %. */
+	u8 closing_fee_negotiation_step_unit;
+
 	/* Reestablishment stuff: last sent commit and revocation details. */
 	bool last_was_revoke;
 	struct changed_htlc *last_sent_commit;

lightningd/closing_control.c

@@ -290,6 +290,8 @@ void peer_start_closingd(struct channel *channel,
 				      minfee, feelimit, startfee,
 				      channel->shutdown_scriptpubkey[LOCAL],
 				      channel->shutdown_scriptpubkey[REMOTE],
+				      channel->closing_fee_negotiation_step,
+				      channel->closing_fee_negotiation_step_unit,
 				      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>
@@ -52,6 +53,8 @@
 #include <lightningd/options.h>
 #include <lightningd/peer_htlcs.h>
 #include <lightningd/plugin_hook.h>
+#include <limits.h>
+#include <stdlib.h>
 #include <unistd.h>
 #include <wally_bip32.h>
 #include <wire/gen_common_wire.h>
@@ -1285,12 +1288,16 @@ static struct command_result *json_close(struct command *cmd,
 	bool do_timeout;
 	const u8 *close_to_script = NULL;
 	bool close_script_set;
+	const char *fee_negotiation_step_str;
+	char* end;
 
 	if (!param(cmd, buffer, params,
 		   p_req("id", param_tok, &idtok),
 		   p_opt_def("unilateraltimeout", param_number, &timeout,
 			     48 * 3600),
 		   p_opt("destination", param_bitcoin_address, &close_to_script),
+		   p_opt("fee_negotiation_step", param_string,
+			 &fee_negotiation_step_str),
 		   NULL))
 		return command_param_failed();
 
@@ -1356,6 +1363,35 @@ static struct command_result *json_close(struct command *cmd,
 	} else
 		close_script_set = false;
 
+	if (fee_negotiation_step_str == NULL) {
+		channel->closing_fee_negotiation_step = 50;
+		channel->closing_fee_negotiation_step_unit =
+		    CLOSING_FEE_NEGOTIATION_STEP_UNIT_PERCENTAGE;
+	} else {
+		channel->closing_fee_negotiation_step =
+		    strtoull(fee_negotiation_step_str, &end, 10);
+		if (*end == '%') {
+			if (channel->closing_fee_negotiation_step < 1 ||
+			    channel->closing_fee_negotiation_step > 100)
+				return command_fail(
+				    cmd, JSONRPC2_INVALID_PARAMS,
+				    "Wrong value given for "
+				    "fee_negotiation_step: \"%s\", the "
+				    "percentage should be between 1 and 100",
+				    fee_negotiation_step_str);
+			channel->closing_fee_negotiation_step_unit =
+			    CLOSING_FEE_NEGOTIATION_STEP_UNIT_PERCENTAGE;
+		} else if (*end == '\0')
+			channel->closing_fee_negotiation_step_unit =
+			    CLOSING_FEE_NEGOTIATION_STEP_UNIT_SATOSHI;
+		else
+			return command_fail(
+			    cmd, JSONRPC2_INVALID_PARAMS,
+			    "Wrong value given for fee_negotiation_step: "
+			    "\"%s\", should be an integer or an integer "
+			    "followed by %%",
+			    fee_negotiation_step_str);
+	}
 
 	/* Normal case.
 	 * We allow states shutting down and sigexchange; a previous

tests/test_closing.py

@@ -360,11 +360,11 @@ def test_closing_specified_destination(node_factory, bitcoind, chainparams):
         assert 1 == bitcoind.rpc.gettxout(closetx, output_num1)['confirmations']
 
 
-def closing_fee(node_factory, bitcoind, chainparams, opts):
-    rate = opts['funder_feerate_per_kw']
+def closing_negotiation_step(node_factory, bitcoind, chainparams, opts):
+    rate = 29006  # closing fee negotiation starts at 21000
     funder = node_factory.get_node(feerates=(rate, rate, rate, rate))
 
-    rate = opts['fundee_feerate_per_kw']
+    rate = 27625  # closing fee negotiation starts at 20000
     fundee = node_factory.get_node(feerates=(rate, rate, rate, rate))
 
     funder_id = funder.info['id']
@@ -378,10 +378,32 @@ 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_step=opts['fee_negotiation_step'])
     else:
         assert opts['close_initiated_by'] == 'fundee'
-        fundee.rpc.close(peer_id=funder_id)
+        fundee.rpc.close(peer_id=funder_id, fee_negotiation_step=opts['fee_negotiation_step'])
+
+    # Get the proclaimed closing fee from the two nodes' statuses
+
+    status_agreed_regex = re.compile("agreed on a closing fee of ([0-9]+) satoshi")
+
+    # [fee_from_funder_status, fee_from_fundee_status]
+    fees_from_status = [None, None]
+
+    def get_fee_from_status(node, peer_id, i):
+        nonlocal fees_from_status
+        status = only_one(only_one(node.rpc.listpeers(peer_id)['peers'][0]['channels'])['status'])
+        m = status_agreed_regex.search(status)
+        if not m:
+            return False
+        fees_from_status[i] = int(m.group(1))
+        return True
+
+    wait_for(lambda: get_fee_from_status(funder, fundee_id, 0))
+    wait_for(lambda: get_fee_from_status(fundee, funder_id, 1))
+
+    assert opts['expected_close_fee'] == fees_from_status[0]
+    assert opts['expected_close_fee'] == fees_from_status[1]
 
     # Get the closing transaction from the bitcoind mempool and get its fee
 
@@ -399,46 +421,82 @@ def closing_fee(node_factory, bitcoind, chainparams, opts):
     close_tx_id = mempool_tx_ids[0]
     fee_mempool = round(mempool[close_tx_id]['fee'] * 10**8)
 
-    # Get the proclaimed closing fee from the two nodes' statuses
+    assert opts['expected_close_fee'] == fee_mempool
 
-    status_agreed_regex = re.compile("agreed on a closing fee of ([0-9]+) satoshi")
 
-    # [fee_from_funder_status, fee_from_fundee_status]
-    fees_from_status = [None, None]
+def test_closing_negotiation_step_30pct(node_factory, bitcoind, chainparams):
+    """Test that the closing fee negotiation step works, 30%"""
+    opts = {}
+    opts['fee_negotiation_step'] = '30%'
 
-    def get_fee_from_status(node, peer_id, i):
-        nonlocal fees_from_status
-        status = only_one(only_one(node.rpc.listpeers(peer_id)['peers'][0]['channels'])['status'])
-        m = status_agreed_regex.search(status)
-        if not m:
-            return False
-        fees_from_status[i] = int(m.group(1))
-        return True
+    opts['close_initiated_by'] = 'funder'
+    opts['expected_close_fee'] = 20537 if not chainparams['elements'] else 33870
+    closing_negotiation_step(node_factory, bitcoind, chainparams, opts)
 
-    wait_for(lambda: get_fee_from_status(funder, fundee_id, 0))
-    wait_for(lambda: get_fee_from_status(fundee, funder_id, 1))
+    opts['close_initiated_by'] = 'fundee'
+    opts['expected_close_fee'] = 20233 if not chainparams['elements'] else 33366
+    closing_negotiation_step(node_factory, bitcoind, chainparams, opts)
 
-    assert fee_mempool == fees_from_status[0]
-    assert fee_mempool == fees_from_status[1]
-    assert fee_mempool == opts['expected_close_fee']
 
+def test_closing_negotiation_step_50pct(node_factory, bitcoind, chainparams):
+    """Test that the closing fee negotiation step works, 50%, the default"""
+    opts = {}
+    opts['fee_negotiation_step'] = '50%'
 
-def test_closing_fee(node_factory, bitcoind, chainparams):
-    """Test that the closing negotiation strategy works"""
-    # feerate 27625 -> closing fee negotiation starts at 20000
-    # feerate 29006 -> closing fee negotiation starts at 21000
+    opts['close_initiated_by'] = 'funder'
+    opts['expected_close_fee'] = 20334 if not chainparams['elements'] else 33533
+    closing_negotiation_step(node_factory, bitcoind, chainparams, opts)
 
-    opts = {
-        'funder_feerate_per_kw': 29006,
-        'fundee_feerate_per_kw': 27625,
-        'close_initiated_by': 'funder',
-        'expected_close_fee': 33533 if chainparams['elements'] else 20333
-    }
+    opts['close_initiated_by'] = 'fundee'
+    opts['expected_close_fee'] = 20334 if not chainparams['elements'] else 33533
+    closing_negotiation_step(node_factory, bitcoind, chainparams, opts)
+
+
+def test_closing_negotiation_step_100pct(node_factory, bitcoind, chainparams):
+    """Test that the closing fee negotiation step works, 100%"""
+    opts = {}
+    opts['fee_negotiation_step'] = '100%'
+
+    opts['close_initiated_by'] = 'funder'
+    opts['expected_close_fee'] = 20001 if not chainparams['elements'] else 32985
+    closing_negotiation_step(node_factory, bitcoind, chainparams, opts)
+
+    # The close fee of 20499 looks strange in this case - one would expect
+    # to have a number close to 21000. This is because
+    # * the range is initially set to [20000 (fundee), 21000 (funder)]
+    # * the funder is always first to propose, he uses 50% step, so he proposes 20500
+    # * the range is narrowed to [20001, 20499] and the fundee proposes 20499
+    opts['close_initiated_by'] = 'fundee'
+    opts['expected_close_fee'] = 20499 if not chainparams['elements'] else 33808
+    closing_negotiation_step(node_factory, bitcoind, chainparams, opts)
+
+
+def test_closing_negotiation_step_1sat(node_factory, bitcoind, chainparams):
+    """Test that the closing fee negotiation step works, 1sat"""
+    opts = {}
+    opts['fee_negotiation_step'] = '1'
+
+    opts['close_initiated_by'] = 'funder'
+    opts['expected_close_fee'] = 20989 if not chainparams['elements'] else 34621
+    closing_negotiation_step(node_factory, bitcoind, chainparams, opts)
+
+    opts['close_initiated_by'] = 'fundee'
+    opts['expected_close_fee'] = 20010 if not chainparams['elements'] else 32995
+    closing_negotiation_step(node_factory, bitcoind, chainparams, opts)
+
+
+def test_closing_negotiation_step_700sat(node_factory, bitcoind, chainparams):
+    """Test that the closing fee negotiation step works, 700sat"""
+    opts = {}
+    opts['fee_negotiation_step'] = '700'
 
-    closing_fee(node_factory, bitcoind, chainparams, opts)
+    opts['close_initiated_by'] = 'funder'
+    opts['expected_close_fee'] = 20151 if not chainparams['elements'] else 33459
+    closing_negotiation_step(node_factory, bitcoind, chainparams, opts)
 
     opts['close_initiated_by'] = 'fundee'
-    closing_fee(node_factory, bitcoind, chainparams, opts)
+    opts['expected_close_fee'] = 20499 if not chainparams['elements'] else 33746
+    closing_negotiation_step(node_factory, bitcoind, chainparams, opts)
 
 
 @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")

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,