From c84cade2700d543ccac72c286ae6ff2b995729c8 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 16 Apr 2019 13:30:15 +0930 Subject: [PATCH] channeld: give some tolerance for empty commitments. The spec says not to send a commitment_signed without any changes, but LND does this. To understand why, you have to understand how LND works. I haven't read the code, but I'm pretty sure it works like this: 1. lnd slows down to do garbage collection, because it's in Go. 2. When an alert timer goes off, noticing it's not making process, it sends a twitter message to @roasbeef. 3. @roasbeef sshs into the user's machine and binary patches lnd to send a commitment_signed message. 4. Unfortunately he works so fast that various laws of causality are broken, meaning sometimes the commitment_signed is sent before any of thes other things happen. I'm fairly sure that this will stop as @roasbeef ages, or lnd introduces some kind of causality enforcement fix. Signed-off-by: Rusty Russell --- CHANGELOG.md | 2 +- channeld/channeld.c | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93fa6b24d..d066f62cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ changes. - `--bind-addr=` fixed for nodes using local sockets (eg. testing). - Unannounced local channels were forgotten for routing on restart until reconnection occurred. - lightning-cli: arguments containing `"` now succeed, rather than causing JSON errors. -- protocol: handle lnd sending more messages before `reestablish`; don't fail channel. +- protocol: handle lnd sending more messages before `reestablish`; don't fail channel, and handle older lnd's spurious empty commitments. ### Security diff --git a/channeld/channeld.c b/channeld/channeld.c index 35272a255..fa6e0ca40 100644 --- a/channeld/channeld.c +++ b/channeld/channeld.c @@ -157,6 +157,9 @@ struct peer { /* Additional confirmations need for local lockin. */ u32 depth_togo; + + /* Empty commitments. Spec violation, but a minor one. */ + u64 last_empty_commitment; }; static u8 *create_channel_announcement(const tal_t *ctx, struct peer *peer); @@ -1344,9 +1347,13 @@ static void handle_peer_commit_sig(struct peer *peer, const u8 *msg) * - MUST NOT send a `commitment_signed` message that does not * include any updates. */ - peer_failed(&peer->cs, - &peer->channel_id, - "commit_sig with no changes"); + status_trace("Oh hi LND! Empty commitment at #%"PRIu64, + peer->next_index[LOCAL]); + if (peer->last_empty_commitment == peer->next_index[LOCAL] - 1) + peer_failed(&peer->cs, + &peer->channel_id, + "commit_sig with no changes (again!)"); + peer->last_empty_commitment = peer->next_index[LOCAL]; } /* We were supposed to check this was affordable as we go. */ @@ -2967,6 +2974,7 @@ int main(int argc, char *argv[]) peer->last_update_timestamp = 0; /* We actually received it in the previous daemon, but near enough */ peer->last_recv = time_now(); + peer->last_empty_commitment = 0; /* We send these to HSM to get real signatures; don't have valgrind * complain. */