Fixup min-capacity-msat semantics in preparation for anchor outputs.

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

@@ -705,26 +705,13 @@ 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;
 
 	/* Take minimal effective capacity from config min_capacity_sat */
 	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

@@ -27,6 +27,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

@@ -139,24 +139,21 @@ def test_bad_opening(node_factory):
     l2.daemon.wait_for_log('to_self_delay 100 larger than 99')
 
 
-@unittest.skipIf(EXPERIMENTAL_FEATURES, "FIXME: anchor_outputs changes numbers")
 @unittest.skipIf(not DEVELOPER, "gossip without DEVELOPER=1 is slow")
 @unittest.skipIf(TEST_NETWORK != 'regtest', "Fee computation and limits are network specific")
 @pytest.mark.slow_test
 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 +163,41 @@ 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
+    if EXPERIMENTAL_FEATURES:
+        # Gotta fund those anchors too!
+        overhead += 660
+
+    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):
@@ -264,12 +251,12 @@ def test_disconnect_opener(node_factory):
     for d in disconnects:
         l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
         with pytest.raises(RpcError):
-            l1.rpc.fundchannel(l2.info['id'], 20000)
+            l1.rpc.fundchannel(l2.info['id'], 25000)
         assert l1.rpc.getpeer(l2.info['id']) is None
 
     # This one will succeed.
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-    l1.rpc.fundchannel(l2.info['id'], 20000)
+    l1.rpc.fundchannel(l2.info['id'], 25000)
 
     # Should still only have one peer!
     assert len(l1.rpc.listpeers()) == 1
@@ -290,12 +277,12 @@ def test_disconnect_fundee(node_factory):
     for d in disconnects:
         l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
         with pytest.raises(RpcError):
-            l1.rpc.fundchannel(l2.info['id'], 20000)
+            l1.rpc.fundchannel(l2.info['id'], 25000)
         assert l1.rpc.getpeer(l2.info['id']) is None
 
     # This one will succeed.
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-    l1.rpc.fundchannel(l2.info['id'], 20000)
+    l1.rpc.fundchannel(l2.info['id'], 25000)
 
     # Should still only have one peer!
     assert len(l1.rpc.listpeers()) == 1
@@ -314,7 +301,7 @@ def test_disconnect_half_signed(node_factory):
 
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     with pytest.raises(RpcError):
-        l1.rpc.fundchannel(l2.info['id'], 20000)
+        l1.rpc.fundchannel(l2.info['id'], 25000)
 
     # Peer remembers, opener doesn't.
     assert l1.rpc.getpeer(l2.info['id']) is None
@@ -332,7 +319,7 @@ def test_reconnect_signed(node_factory):
     l1.fundwallet(2000000)
 
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-    l1.rpc.fundchannel(l2.info['id'], 20000)
+    l1.rpc.fundchannel(l2.info['id'], 25000)
 
     # They haven't forgotten each other.
     assert l1.rpc.getpeer(l2.info['id'])['id'] == l2.info['id']
@@ -361,7 +348,7 @@ def test_reconnect_openingd(node_factory):
 
     # l2 closes on l1, l1 forgets.
     with pytest.raises(RpcError):
-        l1.rpc.fundchannel(l2.info['id'], 20000)
+        l1.rpc.fundchannel(l2.info['id'], 25000)
     assert l1.rpc.getpeer(l2.info['id']) is None
 
     # Reconnect.
@@ -372,7 +359,7 @@ def test_reconnect_openingd(node_factory):
     l2.daemon.wait_for_log('openingd.*Handed peer, entering loop')
 
     # Should work fine.
-    l1.rpc.fundchannel(l2.info['id'], 20000)
+    l1.rpc.fundchannel(l2.info['id'], 25000)
     l1.daemon.wait_for_log('sendrawtx exit 0')
 
     l1.bitcoin.generate_block(3)

tests/test_gossip.py

@@ -462,11 +462,11 @@ def test_routing_gossip_reconnect(node_factory):
                                               {'may_reconnect': True},
                                               {}])
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
-    l1.openchannel(l2, 20000)
+    l1.openchannel(l2, 25000)
 
     # Now open new channels and everybody should sync
     l2.rpc.connect(l3.info['id'], 'localhost', l3.port)
-    l2.openchannel(l3, 20000)
+    l2.openchannel(l3, 25000)
 
     # Settle the gossip
     for n in [l1, l2, l3]:
@@ -508,7 +508,7 @@ def test_routing_gossip(node_factory, bitcoind):
     for i in range(len(nodes) - 1):
         src, dst = nodes[i], nodes[i + 1]
         src.rpc.connect(dst.info['id'], 'localhost', dst.port)
-        src.openchannel(dst, 20000)
+        src.openchannel(dst, 25000)
 
     # Allow announce messages.
     bitcoind.generate_block(5)

tests/test_misc.py

@@ -467,7 +467,7 @@ def test_bech32_funding(node_factory, chainparams):
     l1, l2 = node_factory.line_graph(2, opts={'random_hsm': True}, fundchannel=False)
 
     # fund a bech32 address and then open a channel with it
-    res = l1.openchannel(l2, 20000, 'bech32')
+    res = l1.openchannel(l2, 25000, 'bech32')
     address = res['address']
     assert address.startswith(chainparams['bip173_prefix'])
 

tests/test_pay.py

@@ -1382,7 +1382,7 @@ def test_forward_local_failed_stats(node_factory, bitcoind, executor):
     c12 = l1.fund_channel(l2, 10**6)
     c23 = l2.fund_channel(l3, 10**6)
     c24 = l2.fund_channel(l4, 10**6)
-    c25 = l2.fund_channel(l5, 10**4 * 2)
+    c25 = l2.fund_channel(l5, 10**4 * 3)
     l6.fund_channel(l1, 10**6)
 
     # Make sure routes finalized.

tests/test_plugin.py

@@ -1211,9 +1211,9 @@ def test_bcli(node_factory, bitcoind, chainparams):
 
     l1.fundwallet(10**5)
     l1.connect(l2)
-    fc = l1.rpc.fundchannel(l2.info["id"], 10**4 * 2)
+    fc = l1.rpc.fundchannel(l2.info["id"], 10**4 * 3)
     txo = l1.rpc.call("getutxout", {"txid": fc['txid'], "vout": fc['outnum']})
-    assert (Millisatoshi(txo["amount"]) == Millisatoshi(10**4 * 2 * 10**3)
+    assert (Millisatoshi(txo["amount"]) == Millisatoshi(10**4 * 3 * 10**3)
             and txo["script"].startswith("0020"))
     l1.rpc.close(l2.info["id"])
     # When output is spent, it should give us null !