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

Make on-startup monitor replay more robust #2819

Open
TheBlueMatt opened this issue Jan 9, 2024 · 2 comments
Open

Make on-startup monitor replay more robust #2819

TheBlueMatt opened this issue Jan 9, 2024 · 2 comments
Milestone

Comments

@TheBlueMatt
Copy link
Collaborator

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.

@TheBlueMatt TheBlueMatt added this to the 0.0.121 milestone Jan 9, 2024
@jkczyz
Copy link
Contributor

jkczyz commented Jan 9, 2024

(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:

  • Have read return a UninitializedChannelManager wrapping ChannelManager and only exposing methods that can safely be called. Presumably just chain::Confirm and chain::Listen.
  • Add an initialize method to UninitializedChannelManager that unwraps the ChannelManager after calling watch_channel for each ChannelMonitor.

@TheBlueMatt
Copy link
Collaborator Author

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.

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

2 participants