lightningd: update feerate upon receiving revoke_and_ack from fundee.

1. l1     update_fee ->    l2
2. l1 commitment_signed -> l2 (using new feerate)
3. l1  <- revoke_and_ack   l2
4. l1 <- commitment_signed l2 (using new feerate)
5. l1  -> revoke_and_ack   l2

When we break the connection after #3, the reconnection causes #4 to
be retransmitted, but it turns out l1 wasn't telling the master to set
the local feerate until it received the commitment_signed, so on
reconnect it uses the old feerate, with predictable results (bad
signature).

Signed-off-by: Rusty Russell <[email protected]>

Author: rustyrussell

channeld/channel.c

@@ -1272,15 +1272,16 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg)
 		dump_htlcs(peer->channel, "receiving commit_sig");
 		peer_failed(&peer->cs,
 			    &peer->channel_id,
-			    "Bad commit_sig signature %"PRIu64" %s for tx %s wscript %s key %s",
+			    "Bad commit_sig signature %"PRIu64" %s for tx %s wscript %s key %s feerate %u",
 			    peer->next_index[LOCAL],
 			    type_to_string(msg, secp256k1_ecdsa_signature,
 					   &commit_sig),
 			    type_to_string(msg, struct bitcoin_tx, txs[0]),
 			    tal_hex(msg, wscripts[0]),
 			    type_to_string(msg, struct pubkey,
 					   &peer->channel->funding_pubkey
-					   [REMOTE]));
+					   [REMOTE]),
+			    peer->channel->view[LOCAL].feerate_per_kw);
 	}
 
 	/* BOLT #2:
@@ -1332,7 +1333,8 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg)
 static u8 *got_revoke_msg(const tal_t *ctx, u64 revoke_num,
 			  const struct secret *per_commitment_secret,
 			  const struct pubkey *next_per_commit_point,
-			  const struct htlc **changed_htlcs)
+			  const struct htlc **changed_htlcs,
+			  u32 feerate)
 {
 	u8 *msg;
 	struct changed_htlc *changed = tal_arr(tmpctx, struct changed_htlc, 0);
@@ -1350,7 +1352,7 @@ static u8 *got_revoke_msg(const tal_t *ctx, u64 revoke_num,
 	}
 
 	msg = towire_channel_got_revoke(ctx, revoke_num, per_commitment_secret,
-					next_per_commit_point, changed);
+					next_per_commit_point, feerate, changed);
 	return msg;
 }
 
@@ -1409,7 +1411,8 @@ static void handle_peer_revoke_and_ack(struct peer *peer, const u8 *msg)
 	/* Tell master about things this locks in, wait for response */
 	msg = got_revoke_msg(NULL, peer->revocations_received++,
 			     &old_commit_secret, &next_per_commit,
-			     changed_htlcs);
+			     changed_htlcs,
+			     channel_feerate(peer->channel, LOCAL));
 	master_wait_sync_reply(tmpctx, peer, take(msg),
 			       WIRE_CHANNEL_GOT_REVOKE_REPLY);
 
@@ -1749,6 +1752,10 @@ static void resend_commitment(struct peer *peer, const struct changed_htlc *last
 	struct commit_sigs *commit_sigs;
 	u8 *msg;
 
+	status_trace("Retransmitting commitment, feerate LOCAL=%u REMOTE=%u",
+		     channel_feerate(peer->channel, LOCAL),
+		     channel_feerate(peer->channel, REMOTE));
+
 	/* BOLT #2:
 	 *
 	 *   - if `next_local_commitment_number` is equal to the commitment

channeld/channel_wire.csv

@@ -137,6 +137,7 @@ channel_got_revoke,,revokenum,u64
 channel_got_revoke,,per_commitment_secret,struct secret
 channel_got_revoke,,next_per_commit_point,struct pubkey
 # RCVD_ADD_ACK_REVOCATION, RCVD_REMOVE_ACK_REVOCATION, RCVD_ADD_REVOCATION, RCVD_REMOVE_REVOCATION
+channel_got_revoke,,feerate,u32
 channel_got_revoke,,num_changed,u16
 channel_got_revoke,,changed,num_changed*struct changed_htlc
 # Wait for reply, to make sure it's on disk before we continue

lightningd/peer_htlcs.c

@@ -1283,10 +1283,12 @@ void peer_got_revoke(struct channel *channel, const u8 *msg)
 	enum onion_type *failcodes;
 	size_t i;
 	struct lightningd *ld = channel->peer->ld;
+	u32 feerate;
 
 	if (!fromwire_channel_got_revoke(msg, msg,
 					 &revokenum, &per_commitment_secret,
 					 &next_per_commitment_point,
+					 &feerate,
 					 &changed)) {
 		channel_internal_error(channel, "bad fromwire_channel_got_revoke %s",
 				    tal_hex(channel, msg));
@@ -1345,6 +1347,10 @@ void peer_got_revoke(struct channel *channel, const u8 *msg)
 		return;
 	}
 
+	/* Update feerate: if we are funder, their revoke_and_ack has set
+	 * this for local feerate. */
+	channel->channel_info.feerate_per_kw[LOCAL] = feerate;
+
 	/* FIXME: Check per_commitment_secret -> per_commit_point */
 	update_per_commit_point(channel, &next_per_commitment_point);
 

tests/test_connection.py

@@ -1110,7 +1110,6 @@ def test_fundee_forget_funding_tx_unconfirmed(node_factory, bitcoind):
     assert len(l2.rpc.listpeers(l1.info['id'])['peers']) == 0
 
 
-@pytest.mark.xfail(strict=True)
 @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.