From 9f175deecdff6b047463e8fd9674009bef448172 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 22 Aug 2018 09:39:57 +0930 Subject: [PATCH] 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 --- channeld/channel.c | 17 ++++++++++++----- channeld/channel_wire.csv | 1 + lightningd/peer_htlcs.c | 6 ++++++ tests/test_connection.py | 1 - 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/channeld/channel.c b/channeld/channel.c index 5762cf107..779201428 100644 --- a/channeld/channel.c +++ b/channeld/channel.c @@ -1272,7 +1272,7 @@ 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), @@ -1280,7 +1280,8 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) 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 diff --git a/channeld/channel_wire.csv b/channeld/channel_wire.csv index 05d3e3ab0..245c48ea8 100644 --- a/channeld/channel_wire.csv +++ b/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 diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 8a437e989..4ea976287 100644 --- a/lightningd/peer_htlcs.c +++ b/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); diff --git a/tests/test_connection.py b/tests/test_connection.py index e06148319..301c29f51 100644 --- a/tests/test_connection.py +++ b/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.