From 4fc9f243f7d1c957770589582261389a6a74b9ee Mon Sep 17 00:00:00 2001 From: SomberNight Date: Mon, 12 Aug 2019 17:06:05 +0200 Subject: [PATCH] lnpeer: reestablish_channel - always replay unacked local updates Even if we haven't signed them yet (did not send commitment_signed). Alternatively, if they are not yet signed, we could discard them here, like we do already for remote updates above (chan.hm.discard_unsigned_remote_updates). One of these two options must be done, and before this commit we were not doing either. --- electrum/lnpeer.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/electrum/lnpeer.py b/electrum/lnpeer.py index 38e3c5ea2..2c455a2a3 100644 --- a/electrum/lnpeer.py +++ b/electrum/lnpeer.py @@ -748,20 +748,24 @@ class Peer(Logger): raise RemoteMisbehaving(f"channel reestablish: their_next_local_ctn < 0") if their_oldest_unrevoked_remote_ctn < 0: raise RemoteMisbehaving(f"channel reestablish: their_oldest_unrevoked_remote_ctn < 0") + # Replay un-acked local updates (including commitment_signed) byte-for-byte. + # If we have sent them a commitment signature that they "lost" (due to disconnect), + # we need to make sure we replay the same local updates, as otherwise they could + # end up with two (or more) signed valid commitment transactions at the same ctn. + # Multiple valid ctxs at the same ctn is a major headache for pre-signing spending txns, + # e.g. for watchtowers, hence we must ensure these ctxs coincide. + # We replay the local updates even if they were not yet committed. + for raw_upd_msg in chan.hm.get_unacked_local_updates(): + self.transport.send_bytes(raw_upd_msg) should_close_we_are_ahead = False should_close_they_are_ahead = False # compare remote ctns if next_remote_ctn != their_next_local_ctn: if their_next_local_ctn == latest_remote_ctn and chan.hm.is_revack_pending(REMOTE): - # Replay un-acked local updates (including commitment_signed) byte-for-byte. - # If we have sent them a commitment signature that they "lost" (due to disconnect), - # we need to make sure we replay the same local updates, as otherwise they could - # end up with two (or more) signed valid commitment transactions at the same ctn. - # Multiple valid ctxs at the same ctn is a major headache for pre-signing spending txns, - # e.g. for watchtowers, hence we must ensure these ctxs coincide. - for raw_upd_msg in chan.hm.get_unacked_local_updates(): - self.transport.send_bytes(raw_upd_msg) + # We replayed the local updates (see above), which should have contained a commitment_signed + # (due to is_revack_pending being true), and this should have remedied this situation. + pass else: self.logger.warning(f"channel_reestablish: expected remote ctn {next_remote_ctn}, got {their_next_local_ctn}") if their_next_local_ctn < next_remote_ctn: