Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should ChannelMonitor keys be converted to ChannelIds (and what to do with spliced ChannelMonitors) #3325

Open
TheBlueMatt opened this issue Sep 18, 2024 · 1 comment

Comments

@TheBlueMatt
Copy link
Collaborator

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
Copy link
Collaborator Author

cc @jkczyz maybe, and obviously cc @optout21.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant