From: Matt Corallo <git@bluematt.me>
Date: Sat, 4 Nov 2023 23:02:18 +0000 (+0000)
Subject: Stop writing signer data as a part of channels
X-Git-Tag: v0.0.119~57^2
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=7a951b1bf7615fd7afb1e20c1c627e58a3599c84;p=rust-lightning

Stop writing signer data as a part of channels

This breaks backwards compatibility with versions of LDK prior to
0.0.113 as they expect to always read signer data.

This also substantially reduces allocations during `ChannelManager`
serialization, as we currently don't pre-allocate the `Vec` that
the signer gets written in to. We could alternatively pre-allocate
that `Vec`, but we've been set up to skip the write entirely for a
while, and 0.0.113 was released nearly a year ago. Users
downgrading to LDK 0.0.112 and before at this point should not be
expected.
---

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index e795ecb9f..350415af2 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -88,10 +88,11 @@ be covered by functional tests.
 When refactoring, structure your PR to make it easy to review and don't
 hesitate to split it into multiple small, focused PRs.
 
-The Minimum Supported Rust Version (MSRV) currently is 1.41.1 (enforced by
-our GitHub Actions). Also, the compatibility for LDK object serialization is
-currently ensured back to and including crate version 0.0.99 (see the
-[changelog](CHANGELOG.md)).
+The Minimum Supported Rust Version (MSRV) currently is 1.48.0 (enforced by
+our GitHub Actions). We support reading serialized LDK objects written by any
+version of LDK 0.0.99 and above. We support LDK versions 0.0.113 and above
+reading serialized LDK objects written by modern LDK. Any expected issues with
+upgrades or downgrades should be mentioned in the [changelog](CHANGELOG.md).
 
 Commits should cover both the issue fixed and the solution's rationale. These
 [guidelines](https://chris.beams.io/posts/git-commit/) should be kept in mind.
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 52db68c13..7d5af2774 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -39,7 +39,7 @@ use crate::chain::transaction::{OutPoint, TransactionData};
 use crate::sign::{EcdsaChannelSigner, WriteableEcdsaChannelSigner, EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient};
 use crate::events::ClosureReason;
 use crate::routing::gossip::NodeId;
-use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
+use crate::util::ser::{Readable, ReadableArgs, Writeable, Writer};
 use crate::util::logger::Logger;
 use crate::util::errors::APIError;
 use crate::util::config::{UserConfig, ChannelConfig, LegacyChannelConfig, ChannelHandshakeConfig, ChannelHandshakeLimits, MaxDustHTLCExposure};
@@ -6892,7 +6892,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
 }
 
 const SERIALIZATION_VERSION: u8 = 3;
-const MIN_SERIALIZATION_VERSION: u8 = 2;
+const MIN_SERIALIZATION_VERSION: u8 = 3;
 
 impl_writeable_tlv_based_enum!(InboundHTLCRemovalReason,;
 	(0, FailRelay),
@@ -6972,14 +6972,6 @@ impl<SP: Deref> Writeable for Channel<SP> where SP::Target: SignerProvider {
 
 		self.context.latest_monitor_update_id.write(writer)?;
 
-		let mut key_data = VecWriter(Vec::new());
-		// TODO (taproot|arik): Introduce serialization distinction for non-ECDSA signers.
-		self.context.holder_signer.as_ecdsa().expect("Only ECDSA signers may be serialized").write(&mut key_data)?;
-		assert!(key_data.0.len() < core::usize::MAX);
-		assert!(key_data.0.len() < core::u32::MAX as usize);
-		(key_data.0.len() as u32).write(writer)?;
-		writer.write_all(&key_data.0[..])?;
-
 		// Write out the old serialization for shutdown_pubkey for backwards compatibility, if
 		// deserialized from that format.
 		match self.context.shutdown_scriptpubkey.as_ref().and_then(|script| script.as_legacy_pubkey()) {
diff --git a/lightning/src/sign/type_resolver.rs b/lightning/src/sign/type_resolver.rs
index 73d2cceb3..f76650982 100644
--- a/lightning/src/sign/type_resolver.rs
+++ b/lightning/src/sign/type_resolver.rs
@@ -18,6 +18,7 @@ impl<ECS: EcdsaChannelSigner> ChannelSignerType<ECS>{
 		}
 	}
 
+	#[allow(unused)]
 	pub(crate) fn as_ecdsa(&self) -> Option<&ECS> {
 		match self {
 			ChannelSignerType::Ecdsa(ecs) => Some(ecs)
diff --git a/pending_changelog/113-channel-ser-compat.txt b/pending_changelog/113-channel-ser-compat.txt
new file mode 100644
index 000000000..9bba9fd1c
--- /dev/null
+++ b/pending_changelog/113-channel-ser-compat.txt
@@ -0,0 +1,4 @@
+ * `ChannelManager`s written with LDK 0.0.119 are no longer readable by versions
+   of LDK prior to 0.0.113. Users wishing to downgrade to LDK 0.0.112 or before
+   can read an 0.0.119-serialized `ChannelManager` with a version of LDK from
+   0.0.113 to 0.0.118, re-serialize it, and then downgrade.