lightningd: return min-capacity-sat to its intended purpose.

And document exactly what it does: insist that an HTLC can pass of
this value (module assumptions of feerate).

Note that we remove the "is_opener" test from the capacity calculation
for anchor fees: it doesn't matter which side it is, someone has to pay
for anchor fees to it deducts from capacity.

This change breaks the test, which we rewrite.

Changelog-Changed: config: `min-capacity-sat` is now stricter about checking usable capacity of channels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

doc/lightningd-config.5

@@ -290,8 +290,12 @@ for existing channels, use the RPC call \fBlightning-setchannelfee\fR(7)\.
 
  \fBmin-capacity-sat\fR=\fISATOSHI\fR
 Default: 10000\. This value defines the minimal effective channel
-capacity in satoshi to accept for channel opening requests\. If a peer
-tries to open a channel smaller than this, the opening will be rejected\.
+capacity in satoshi to accept for channel opening requests\. This will
+reject any opening of a channel which can't pass an HTLC of least this
+value\.  Usually this prevents a peer opening a tiny channel, but it
+can also prevent a channel you open with a reasonable amount and the peer
+requesting such a large reserve that the capacity of the channel
+falls below this\.
 
 
  \fBignore-fee-limits\fR=\fIBOOL\fR

doc/lightningd-config.5.md

@@ -234,8 +234,12 @@ for existing channels, use the RPC call lightning-setchannelfee(7).
 
  **min-capacity-sat**=*SATOSHI*
 Default: 10000. This value defines the minimal effective channel
-capacity in satoshi to accept for channel opening requests. If a peer
-tries to open a channel smaller than this, the opening will be rejected.
+capacity in satoshi to accept for channel opening requests. This will
+reject any opening of a channel which can't pass an HTLC of least this
+value.  Usually this prevents a peer opening a tiny channel, but it
+can also prevent a channel you open with a reasonable amount and the peer
+requesting such a large reserve that the capacity of the channel
+falls below this.
 
  **ignore-fee-limits**=*BOOL*
 Allow nodes which establish channels to us to set any fee they want.

lightningd/opening_control.c

@@ -716,8 +716,6 @@ static void channel_config(struct lightningd *ld,
 			   u32 *max_to_self_delay,
 			   struct amount_msat *min_effective_htlc_capacity)
 {
-	struct amount_msat dust_limit;
-
 	/* FIXME: depend on feerate. */
 	*max_to_self_delay = ld->config.locktime_max;
 
@@ -725,17 +723,6 @@ static void channel_config(struct lightningd *ld,
 	if (!amount_sat_to_msat(min_effective_htlc_capacity,
 				amount_sat(ld->config.min_capacity_sat)))
 		fatal("amount_msat overflow for config.min_capacity_sat");
-	/* Substract 2 * dust_limit, so fundchannel with min value is possible */
-	if (!amount_sat_to_msat(&dust_limit, chainparams->dust_limit))
-		fatal("amount_msat overflow for dustlimit");
-	if (!amount_msat_sub(min_effective_htlc_capacity,
-				*min_effective_htlc_capacity,
-				dust_limit))
-		*min_effective_htlc_capacity = AMOUNT_MSAT(0);
-	if (!amount_msat_sub(min_effective_htlc_capacity,
-				*min_effective_htlc_capacity,
-				dust_limit))
-		*min_effective_htlc_capacity = AMOUNT_MSAT(0);
 
 	/* BOLT #2:
 	 *

openingd/openingd.c

@@ -26,6 +26,7 @@
 #include <common/gossip_rcvd_filter.h>
 #include <common/gossip_store.h>
 #include <common/initial_channel.h>
+#include <common/initial_commit_tx.h>
 #include <common/key_derive.h>
 #include <common/memleak.h>
 #include <common/overflows.h>
@@ -205,6 +206,7 @@ static bool check_config_bounds(struct state *state,
 {
 	struct amount_sat capacity;
 	struct amount_sat reserve;
+	struct amount_sat fee;
 
 	/* BOLT #2:
 	 *
@@ -244,15 +246,14 @@ static bool check_config_bounds(struct state *state,
 		return false;
 	}
 
-	/* BOLT-a12da24dd0102c170365124782b46d9710950ac1 #2:
+	/* BOLT #2:
 	 *  - if `option_anchor_outputs` applies to this commitment
 	 *    transaction and the sending node is the funder:
 	 *   - MUST be able to additionally pay for `to_local_anchor` and
 	 *    `to_remote_anchor` above its reserve.
 	 */
-	/* (We simply include in "reserve" here if they opened). */
+	/* We simply include in "reserve" here. */
 	if (state->option_anchor_outputs
-	    && !am_opener
 	    && !amount_sat_add(&reserve, reserve, AMOUNT_SAT(660))) {
 		negotiation_failed(state, am_opener,
 				   "cannot add anchors to reserve %s",
@@ -275,6 +276,25 @@ static bool check_config_bounds(struct state *state,
 		return false;
 	}
 
+	/* They have to pay for fees, too.  Assuming HTLC is dust, though,
+	 * we don't account for an HTLC output. */
+	fee = commit_tx_base_fee(state->feerate_per_kw, 0,
+				 state->option_anchor_outputs);
+	if (!amount_sat_sub(&capacity, capacity, fee)) {
+		negotiation_failed(state, am_opener,
+				   "channel_reserve_satoshis %s"
+				   " and %s plus fee %s too large for funding %s",
+				   type_to_string(tmpctx, struct amount_sat,
+						  &remoteconf->channel_reserve),
+				   type_to_string(tmpctx, struct amount_sat,
+						  &state->localconf.channel_reserve),
+				   type_to_string(tmpctx, struct amount_sat,
+						  &fee),
+				   type_to_string(tmpctx, struct amount_sat,
+						  &state->funding));
+		return false;
+	}
+
 	/* If they set the max HTLC value to less than that number, it caps
 	 * the channel capacity. */
 	if (amount_sat_greater(capacity,

tests/test_connection.py

@@ -146,17 +146,15 @@ def test_bad_opening(node_factory):
 def test_opening_tiny_channel(node_factory):
     # Test custom min-capacity-sat parameters
     #
-    #       o---> [l2] (1000)   - old default (too little for reserves)
-    #      /
-    #  [l1]-----> [l3] (~6000)  - technical minimal value that wont be rejected
+    #  [l1]-----> [l2] (~6000)  - technical minimal value that wont be rejected
     #      \
-    #       o---> [l4] (~10000) - the current default
+    #       o---> [l3] (10000) - the current default
     #        \
-    #         o-> [l5] (20000)  - a node with a higher minimal value
+    #         o-> [l4] (20000)  - a node with a higher minimal value
     #
     # For each:
-    #  1. Try to establish channel 1sat smaller than min_capacity_sat
-    #  2. Try to establish channel exact min_capacity_sat
+    #  1. Try to establish channel with capacity 1sat smaller than min_capacity_sat
+    #  2. Try to establish channel with capacity exact min_capacity_sat
     #
     # BOLT2
     # The receiving node MAY fail the channel if:
@@ -166,51 +164,38 @@ def test_opening_tiny_channel(node_factory):
     dustlimit = 546
     reserves = 2 * dustlimit
     min_commit_tx_fees = basic_fee(7500)
-    min_for_opener = min_commit_tx_fees + dustlimit + 1
-
-    l1_min_capacity = 1000            # 1k old default, too small but used at l1 to allow small incoming channels
-    l2_min_capacity = reserves        # just enough to get past capacity filter
-    l3_min_capacity = min_for_opener  # the absolute technical minimum
-    l4_min_capacity = 10000           # the current default
-    l5_min_capacity = 20000           # a server with more than default minimum
-
-    l1, l2, l3, l4, l5 = node_factory.get_nodes(5, opts=[{'min-capacity-sat': l1_min_capacity},
-                                                         {'min-capacity-sat': l2_min_capacity},
-                                                         {'min-capacity-sat': l3_min_capacity},
-                                                         {'min-capacity-sat': l4_min_capacity},
-                                                         {'min-capacity-sat': l5_min_capacity}])
+    overhead = reserves + min_commit_tx_fees
+
+    l2_min_capacity = 1               # just enough to get past capacity filter
+    l3_min_capacity = 10000           # the current default
+    l4_min_capacity = 20000           # a server with more than default minimum
+
+    l1, l2, l3, l4 = node_factory.get_nodes(4, opts=[{'min-capacity-sat': 0},
+                                                     {'min-capacity-sat': l2_min_capacity},
+                                                     {'min-capacity-sat': l3_min_capacity},
+                                                     {'min-capacity-sat': l4_min_capacity}])
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     l1.rpc.connect(l3.info['id'], 'localhost', l3.port)
     l1.rpc.connect(l4.info['id'], 'localhost', l4.port)
-    l1.rpc.connect(l5.info['id'], 'localhost', l5.port)
-
-    # Open channel with one less than reserves should be rejected at l2
-    with pytest.raises(RpcError, match=r'channel_reserve_satoshis .*sat and .*sat too large for funding .*sat'):
-        l1.fund_channel(l2, l2_min_capacity - 1)
-    # Open a channel with exactly the minimal amount for the fundee,
-    # This will raise an exception at l1, as the opener cannot afford fees for initial_commit_tx.
-    # Note: The old default of 1k sat is below the technical minimum when accounting for dust reserves and fees
-    # This is why this must fail, for this reason the default will be raised to 10k sat.
-    with pytest.raises(RpcError, match=r'Funder cannot afford fee on initial commitment transaction'):
-        l1.fund_channel(l2, l2_min_capacity)
-
-    # Open channel with one less than technical minimum should be rejected at l3
-    with pytest.raises(RpcError, match=r'channel capacity is .*sat, which is below .*sat'):
-        l1.fund_channel(l3, l3_min_capacity - 1)
-    # When amount technical minimum matches exactly, own initial_commit_tx fees can now be covered
-    l1.fund_channel(l3, l3_min_capacity)
-
-    # Open channel with one less than default 10k sats should be rejected at l4
-    with pytest.raises(RpcError, match=r'channel capacity is .*, which is below .*msat'):
-        l1.fund_channel(l4, l4_min_capacity - 1)
-    # This must be possible with enough capacity
-    l1.fund_channel(l4, l4_min_capacity)
-
-    # Open channel with less than minimum should be rejected at l5
-    with pytest.raises(RpcError, match=r'channel capacity is .*, which is below .*msat'):
-        l1.fund_channel(l5, l5_min_capacity - 1)
-    # bigger channels must not be affected
-    l1.fund_channel(l5, l5_min_capacity * 10)
+
+    with pytest.raises(RpcError, match=r'They sent error.*channel capacity is .*, which is below .*msat'):
+        l1.fund_channel(l2, l2_min_capacity + overhead - 1)
+    l1.fund_channel(l2, l2_min_capacity + overhead)
+
+    with pytest.raises(RpcError, match=r'They sent error.*channel capacity is .*, which is below .*msat'):
+        l1.fund_channel(l3, l3_min_capacity + overhead - 1)
+    l1.fund_channel(l3, l3_min_capacity + overhead)
+
+    with pytest.raises(RpcError, match=r'They sent error.*channel capacity is .*, which is below .*msat'):
+        l1.fund_channel(l4, l4_min_capacity + overhead - 1)
+    l1.fund_channel(l4, l4_min_capacity + overhead)
+
+    # Note that this check applies locally too, so you can't open it if
+    # you would reject it.
+    l3.rpc.connect(l2.info['id'], 'localhost', l2.port)
+    with pytest.raises(RpcError, match=r"'message': 'channel capacity.* is .*, which is below .*msat"):
+        l3.fund_channel(l2, l3_min_capacity + overhead - 1)
+    l3.fund_channel(l2, l3_min_capacity + overhead)
 
 
 def test_second_channel(node_factory):