lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.14k stars 357 forks source link

Should `ChannelMonitor` keys be converted to `ChannelId`s (and what to do with spliced `ChannelMonitor`s) #3325

Open TheBlueMatt opened 1 day ago

TheBlueMatt commented 1 day ago

ChannelMonitors are currently indexed by funding OutPoints, which is fine, except with splicing we're gonna have channels that change funding but don't change ChannelId. This made sense, I thought, because we'd just use a new ChannelMonitor for spliced channels, treating them as new channels and allowing us to remove the old ChannelMonitor once the splice is confirmed. @optout21 pointed out that this doesn't really work cause we'll end up doubling our IO write volume while the splice is pending (writing each update twice - once for the "old channel" and once for the 'new channel"). So if we, instead, use a single ChannelMonitor and just remove the old data after confirmation, we end up with a somewhat nonsense key - the "original funding OutPoint".

Instead, we could force our users to do a conversion to store ChannelMonitors by ChannelId. Technically this could cause overlap (if a peer had opened two channels with the same ID under different peer pks) but of course only one could confirm, so we'd force users to handle this (and presumably offer our own migration method). It would also mean a one-way migration - users wouldn't be able to downgrade after their migration, though technically they could do the migration without upgrading LDK (and we could encourage them to do so by providing an API that has both keys in it for one release).

One really nice side-effect of this is that we'd be able to drop the OutPoints we have in lots of structs tracking previous hops and can instead use ChannelIds, which would simplify things greatly.

TheBlueMatt commented 1 day ago

cc @jkczyz maybe, and obviously cc @optout21.