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 <[email protected]>
ElementsProject · Apr 18, 2021 · 04fc6d5 · 04fc6d5
1 parent b366453
commit 04fc6d5
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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 */