From 7bde0ead4d705df35acc252af0d070af54448d13 Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Sat, 24 Jul 2021 13:38:39 +0930
Subject: [PATCH] connectd: allow out-of-bounds fees unless they're actually
 getting *worse*.

Pointed out by @fiatjaf, and indeed it happened to me as well; a peer with
a high feerate reconnects and sends a similar (but now ludicrous) feerate,
and we get upset:

```
$ lightning-cli listpeers 039c73f53daad1050a6a72afb5353a2152f3152ee17168cd0ab28c2cb3e0050e36
{
   "peers": [
      {
         "id": "039c73f53daad1050a6a72afb5353a2152f3152ee17168cd0ab28c2cb3e0050e36",
         "connected": false,
         "channels": [
            {
               "state": "CHANNELD_NORMAL",
               "scratch_txid": "d796aa9c44920cc7169cdb61e36437bf180cedaec44103a69591ce2baac9b1d9",
               "last_tx_fee": "14329000msat",
               "last_tx_fee_msat": "14329000msat",
               "feerate": {
                  "perkw": 19791,
                  "perkb": 79164
               },
```

Then in the logs:
```
2021-07-23T19:34:56.227Z DEBUG   039c73f53daad1050a6a72afb5353a2152f3152ee17168cd0ab28c2cb3e0050e36-channeld-chan#39381: billboard perm: update_fee 17055 outside range 253-7210
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 channeld/channeld.c      | 25 ++++++++++++++++++++++---
 tests/test_connection.py | 23 +++++++++++++++++++++++
 2 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/channeld/channeld.c b/channeld/channeld.c
index 4d0ae97a1..26048824e 100644
--- a/channeld/channeld.c
+++ b/channeld/channeld.c
@@ -789,6 +789,21 @@ static void handle_peer_add_htlc(struct peer *peer, const u8 *msg)
 				 channel_add_err_name(add_err));
 }
 
+/* We don't get upset if they're outside the range, as long as they're
+ * improving (or at least, not getting worse!). */
+static bool feerate_same_or_better(const struct channel *channel,
+				   u32 feerate, u32 feerate_min, u32 feerate_max)
+{
+	u32 current = channel_feerate(channel, LOCAL);
+
+	/* Too low?  But is it going upwards?  */
+	if (feerate < feerate_min)
+		return feerate >= current;
+	if (feerate > feerate_max)
+		return feerate <= current;
+	return true;
+}
+
 static void handle_peer_feechange(struct peer *peer, const u8 *msg)
 {
 	struct channel_id channel_id;
@@ -820,10 +835,14 @@ static void handle_peer_feechange(struct peer *peer, const u8 *msg)
 	 *     unreasonably large:
 	 *     - SHOULD fail the channel.
 	 */
-	if (feerate < peer->feerate_min || feerate > peer->feerate_max)
+	if (!feerate_same_or_better(peer->channel, feerate,
+				    peer->feerate_min, peer->feerate_max))
 		peer_failed_warn(peer->pps, &peer->channel_id,
-				 "update_fee %u outside range %u-%u",
-				 feerate, peer->feerate_min, peer->feerate_max);
+				 "update_fee %u outside range %u-%u"
+				 " (currently %u)",
+				 feerate,
+				 peer->feerate_min, peer->feerate_max,
+				 channel_feerate(peer->channel, LOCAL));
 
 	/* BOLT #2:
 	 *
diff --git a/tests/test_connection.py b/tests/test_connection.py
index ad008fd27..a91dd7717 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -3710,3 +3710,26 @@ def test_htlc_failed_noclose(node_factory):
 
     time.sleep(35)
     assert l1.rpc.getpeer(l2.info['id'])['connected']
+
+
+@pytest.mark.developer("dev-no-reconnect required")
+def test_old_feerate(node_factory):
+    """Test retransmission of old, now-unacceptable, feerate"""
+    l1, l2 = node_factory.line_graph(2, opts={'feerates': (75000, 75000, 75000, 75000),
+                                              'may_reconnect': True,
+                                              'dev-no-reconnect': None})
+
+    l1.pay(l2, 1000)
+    l1.rpc.disconnect(l2.info['id'], force=True)
+
+    # Drop acceptable feerate by l2
+    l2.set_feerates((7000, 7000, 7000, 7000))
+    l2.restart()
+
+    # Minor change to l1, so it sends update_fee
+    l1.set_feerates((74900, 74900, 74900, 74900))
+    l1.restart()
+    l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
+
+    # This will timeout if l2 didn't accept fee.
+    l1.pay(l2, 1000)