lightningdevkit / rust-lightning

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

Make on-startup monitor replay more robust #2819

Open TheBlueMatt opened 9 months ago

TheBlueMatt commented 9 months ago

Currently we require that users wait until after they've loaded their ChannelMonitors into their chain::Watch until they use the ChannelManager at all. This ensures we dont generate monitor updates prior to the monitors being loaded and ready to be updated (and is relied on by the async monitor on-startup replay logic, which will release the replays as soon as any call into the ChannelManager from the user happens). This is somewhat brittle because (a) we don't clearly document this requirement, which we really need to, (b) this only happens in the rare case, not the common case, so our users aren't going to detect it until they're in production. (a) is easy to fix, but (b) will require some API change of some form:

We could either (1) add a new API call to the ChannelManager to inform it we're done with initial monitor loading, panicing or failing all calls prior to that point, (2) we could maybe emulate the same with a new MonitorEvent that indicates at least one monitor was loaded, and implicitly call process_pending_monitor_events if we haven't decided that we're done loading. (3) try to delay all our monitors via the replay pipeline, allowing operation prior to some "we're ready to go" function.

jkczyz commented 9 months ago

(1) add a new API call to the ChannelManager to inform it we're done with initial monitor loading, panicing or failing all calls prior to that point

Could we lean on the type system here? Something like:

TheBlueMatt commented 9 months ago

Have read return a UninitializedChannelManager wrapping ChannelManager and only exposing methods that can safely be called. Presumably just chain::Confirm and chain::Listen.

Mmm, indeed we could.

Add an initialize method to UninitializedChannelManager that unwraps the ChannelManager after calling watch_channel for each ChannelMonitor.

That's a big change, but probably a good one if we can pull it off. Currently we require the user replay blocks on the monitors between reading the ChannelManager and inserting them into the chain::Watch (in part because monitors may be at different block heights and need different replay). We don't do the insertion for them, but require them to insert when they're done. Doing it for them would be a nice simplification.