channeld: defer first update_fee until we have an HTLC to send.

Sending update_fee immediately after channel establishment seems to
upset LND, so work around it by deferring it.  The reason we increase
the fee after establishment is because now we might need to close the
channel in a hurry due to htlcs, but until there are htlcs that's
unnecessary.

Fixes: #3596
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

channeld/channeld.c

@@ -2749,7 +2749,12 @@ static void handle_feerates(struct peer *peer, const u8 *inmsg)
 	 */
 	if (peer->channel->funder == LOCAL) {
 		peer->desired_feerate = feerate;
-		start_commit_timer(peer);
+		/* Don't do this for the first feerate, wait until something else
+		 * happens.  LND seems to get upset in some cases otherwise:
+		 * see https://github.com/ElementsProject/lightning/issues/3596 */
+		if (peer->next_index[LOCAL] != 1
+		    || peer->next_index[REMOTE] != 1)
+			start_commit_timer(peer);
 	} else {
 		/* BOLT #2:
 		 *

tests/test_connection.py

@@ -1367,7 +1367,6 @@ def test_update_fee(node_factory, bitcoind):
 
     # Make l1 send out feechange.
     l1.set_feerates((14000, 11000, 7500, 3750))
-    l2.daemon.wait_for_log('peer updated fee to 14000')
 
     # Now make sure an HTLC works.
     # (First wait for route propagation.)
@@ -1376,6 +1375,8 @@ def test_update_fee(node_factory, bitcoind):
 
     # Make payments.
     l1.pay(l2, 200000000)
+    # First payment causes fee update.
+    l2.daemon.wait_for_log('peer updated fee to 14000')
     l2.pay(l1, 100000000)
 
     # Now shutdown cleanly.
@@ -1401,6 +1402,9 @@ def test_update_fee(node_factory, bitcoind):
 def test_fee_limits(node_factory, bitcoind):
     l1, l2 = node_factory.line_graph(2, opts={'dev-max-fee-multiplier': 5, 'may_reconnect': True}, fundchannel=True)
 
+    # Kick off fee adjustment using HTLC.
+    l1.pay(l2, 1000)
+
     # L1 asks for stupid low fee (will actually hit the floor of 253)
     l1.stop()
     l1.set_feerates((15, 15, 15, 15), False)
@@ -1449,8 +1453,8 @@ def test_fee_limits(node_factory, bitcoind):
 
 @unittest.skipIf(not DEVELOPER, "needs DEVELOPER=1")
 def test_update_fee_reconnect(node_factory, bitcoind):
-    # Disconnect after first commitsig.
-    disconnects = ['+WIRE_COMMITMENT_SIGNED']
+    # Disconnect after commitsig for fee update.
+    disconnects = ['+WIRE_COMMITMENT_SIGNED*3']
     # Feerates identical so we don't get gratuitous commit to update them
     l1 = node_factory.get_node(disconnect=disconnects, may_reconnect=True,
                                feerates=(15000, 15000, 15000, 3750))
@@ -1460,6 +1464,9 @@ def test_update_fee_reconnect(node_factory, bitcoind):
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     chan = l1.fund_channel(l2, 10**6)
 
+    # Make an HTLC just to get us to do feechanges.
+    l1.pay(l2, 1000)
+
     # Make l1 send out feechange; triggers disconnect/reconnect.
     # (Note: < 10% change, so no smoothing here!)
     l1.set_feerates((14000, 14000, 14000, 3750))
@@ -1755,13 +1762,16 @@ def test_no_fee_estimate(node_factory, bitcoind, executor):
 @unittest.skipIf(not DEVELOPER, "needs --dev-disconnect")
 def test_funder_feerate_reconnect(node_factory, bitcoind):
     # l1 updates fees, then reconnect so l2 retransmits commitment_signed.
-    disconnects = ['-WIRE_COMMITMENT_SIGNED']
+    disconnects = ['-WIRE_COMMITMENT_SIGNED*3']
     l1 = node_factory.get_node(may_reconnect=True,
                                feerates=(7500, 7500, 7500, 7500))
     l2 = node_factory.get_node(disconnect=disconnects, may_reconnect=True)
     l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
     l1.fund_channel(l2, 10**6)
 
+    # Need a payment otherwise it won't update fee.
+    l1.pay(l2, 10**9 // 2)
+
     # create fee update, causing disconnect.
     l1.set_feerates((15000, 11000, 7500, 3750))
     l2.daemon.wait_for_log(r'dev_disconnect: \-WIRE_COMMITMENT_SIGNED')

tests/test_pay.py

@@ -228,6 +228,9 @@ def test_pay_disconnect(node_factory, bitcoind):
     l1, l2 = node_factory.line_graph(2, opts={'dev-max-fee-multiplier': 5,
                                               'may_reconnect': True})
 
+    # Dummy payment to kick off update_fee messages
+    l1.pay(l2, 1000)
+
     inv = l2.rpc.invoice(123000, 'test_pay_disconnect', 'description')
     rhash = inv['payment_hash']