From 04fc6d5869ead4ef912cddb50e4079d7f771b7e5 Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Sun, 18 Apr 2021 14:13:30 +0930
Subject: [PATCH] channeld: don't ever send back-to-back feerate changes.

There are several reports of desynchronizaion with LND here; a simple
approach is to only have one feerate change in flight at any time.

Even if this turns out to be our fault, it's been a historic area of
confusion, so this restriction seems reasonable.

Fixes: #4152
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 channeld/channeld.c | 35 ++++++++++++++++++++++++-----------
 common/fee_states.c | 10 ++++++++++
 common/fee_states.h |  4 ++++
 3 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/channeld/channeld.c b/channeld/channeld.c
index 42cdfa61c310..a35057c4281d 100644
--- a/channeld/channeld.c
+++ b/channeld/channeld.c
@@ -944,6 +944,25 @@ static struct bitcoin_signature *unraw_sigs(const tal_t *ctx,
 	return sigs;
 }
 
+/* Do we want to update fees? */
+static bool want_fee_update(const struct peer *peer)
+{
+	u32 feerate, max;
+
+	if (peer->channel->opener != LOCAL)
+		return false;
+
+	max = approx_max_feerate(peer->channel);
+	feerate = peer->desired_feerate;
+
+	/* FIXME: We should avoid adding HTLCs until we can meet this
+	 * feerate! */
+	if (feerate > max)
+		feerate = max;
+
+	return feerate != channel_feerate(peer->channel, REMOTE);
+}
+
 static void send_commit(struct peer *peer)
 {
 	u8 *msg;
@@ -1002,17 +1021,11 @@ static void send_commit(struct peer *peer)
 	}
 
 	/* If we wanted to update fees, do it now. */
-	if (peer->channel->opener == LOCAL) {
-		u32 feerate, max = approx_max_feerate(peer->channel);
-
-		feerate = peer->desired_feerate;
-
-		/* FIXME: We should avoid adding HTLCs until we can meet this
-		 * feerate! */
-		if (feerate > max)
-			feerate = max;
-
-		if (feerate != channel_feerate(peer->channel, REMOTE)) {
+	if (want_fee_update(peer)) {
+		/* FIXME: We occasionally desynchronize with LND here, so
+		 * don't stress things by having more than one feerate change
+		 * in-flight! */
+		if (feerate_changes_done(peer->channel->fee_states)) {
 			u8 *msg;
 
 			if (!channel_update_feerate(peer->channel, feerate))
diff --git a/common/fee_states.c b/common/fee_states.c
index 59497d73861a..c12470911e1d 100644
--- a/common/fee_states.c
+++ b/common/fee_states.c
@@ -73,6 +73,16 @@ u32 get_feerate(const struct fee_states *fee_states,
 	abort();
 }
 
+/* Are feerates all agreed by both sides? */
+bool feerate_changes_done(const struct fee_states *fee_states)
+{
+	size_t num_feerates = 0;
+	for (size_t i = 0; i < ARRAY_SIZE(fee_states->feerate); i++) {
+		num_feerates += (fee_states->feerate[i] != NULL);
+	}
+	return num_feerates == 1;
+}
+
 void start_fee_update(struct fee_states *fee_states,
 		      enum side opener,
 		      u32 feerate_per_kw)
diff --git a/common/fee_states.h b/common/fee_states.h
index 5eb2029fe2b5..40cc18ddccd3 100644
--- a/common/fee_states.h
+++ b/common/fee_states.h
@@ -85,4 +85,8 @@ struct fee_states *fromwire_fee_states(const tal_t *ctx,
  * Is this fee_state struct valid for this side?
  */
 bool fee_states_valid(const struct fee_states *fee_states, enum side opener);
+
+/* Are therre no more fee changes in-flight? */
+bool feerate_changes_done(const struct fee_states *fee_states);
+
 #endif /* LIGHTNING_COMMON_FEE_STATES_H */