From b1aa950f4fd0333465fb9193a7a70e4e20cd2abe Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 19 Jul 2021 18:32:11 +0000 Subject: [PATCH] Do not serialize `Channel::last_sent_closing_fee` to disk We're supposed to write `Channel` to disk as if `remove_uncommitted_htlcs_and_mark_paused` had just run, however we were writing `last_sent_closing_fee` to disk (if it is not-None), whereas `remove_uncommitted_htlcs_and_mark_paused` clears it. Indeed, the BOLTs say fee "... negotiation restarts on reconnection." --- lightning/src/ln/channel.rs | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e41d6d28..d47b4e72 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -4973,15 +4973,11 @@ impl Writeable for Channel { self.update_time_counter.write(writer)?; self.feerate_per_kw.write(writer)?; - match self.last_sent_closing_fee { - Some((feerate, fee, sig)) => { - 1u8.write(writer)?; - feerate.write(writer)?; - fee.write(writer)?; - sig.write(writer)?; - }, - None => 0u8.write(writer)?, - } + // Versions prior to 0.0.100 expected to read the fields of `last_sent_closing_fee` here, + // however we are supposed to restart shutdown fee negotiation on reconnect (and wipe + // `last_send_closing_fee` in `remove_uncommitted_htlcs_and_mark_paused`) so we should never + // consider the stale state on reload. + 0u8.write(writer)?; self.funding_tx_confirmed_in.write(writer)?; self.funding_tx_confirmation_height.write(writer)?; @@ -5189,11 +5185,19 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel let update_time_counter = Readable::read(reader)?; let feerate_per_kw = Readable::read(reader)?; - let last_sent_closing_fee = match ::read(reader)? { - 0 => None, - 1 => Some((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?)), + // Versions prior to 0.0.100 expected to read the fields of `last_sent_closing_fee` here, + // however we are supposed to restart shutdown fee negotiation on reconnect (and wipe + // `last_send_closing_fee` in `remove_uncommitted_htlcs_and_mark_paused`) so we should never + // consider the stale state on reload. + match ::read(reader)? { + 0 => {}, + 1 => { + let _: u32 = Readable::read(reader)?; + let _: u64 = Readable::read(reader)?; + let _: Signature = Readable::read(reader)?; + }, _ => return Err(DecodeError::InvalidValue), - }; + } let funding_tx_confirmed_in = Readable::read(reader)?; let funding_tx_confirmation_height = Readable::read(reader)?; @@ -5321,7 +5325,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel #[cfg(debug_assertions)] counterparty_max_commitment_tx_output: Mutex::new((0, 0)), - last_sent_closing_fee, + last_sent_closing_fee: None, funding_tx_confirmed_in, funding_tx_confirmation_height, -- 2.30.2