Stop writing signer data as a part of channels 2023-11-less-graph-memory-frag
authorMatt Corallo <git@bluematt.me>
Sat, 4 Nov 2023 23:02:18 +0000 (23:02 +0000)
committerMatt Corallo <git@bluematt.me>
Thu, 9 Nov 2023 22:28:08 +0000 (22:28 +0000)
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.

CONTRIBUTING.md
lightning/src/ln/channel.rs
lightning/src/sign/type_resolver.rs
pending_changelog/113-channel-ser-compat.txt [new file with mode: 0644]

index e795ecb9fba7b3c48a5f7732ce96719b28d32c16..350415af24cc0ad86d200a0199797c53a0518572 100644 (file)
@@ -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.
 
 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.
 
 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.
index 52db68c1323c771a45807fe5d21002b887d9e8d8..7d5af2774175dd1bfb12195dead1de423434c9c7 100644 (file)
@@ -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::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};
 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 SERIALIZATION_VERSION: u8 = 3;
-const MIN_SERIALIZATION_VERSION: u8 = 2;
+const MIN_SERIALIZATION_VERSION: u8 = 3;
 
 impl_writeable_tlv_based_enum!(InboundHTLCRemovalReason,;
        (0, FailRelay),
 
 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)?;
 
 
                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()) {
                // 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()) {
index 73d2cceb3e85f9d84bf2404d7d9cbbfc204598d0..f76650982c2b4f2ae8e4126975d56728b2e4e261 100644 (file)
@@ -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)
        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 (file)
index 0000000..9bba9fd
--- /dev/null
@@ -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.