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

Provide Utilities to Persist NetworkGraph and Scorer #1191

Closed
ConorOkus opened this issue Nov 29, 2021 · 6 comments
Closed

Provide Utilities to Persist NetworkGraph and Scorer #1191

ConorOkus opened this issue Nov 29, 2021 · 6 comments

Comments

@ConorOkus
Copy link

ConorOkus commented Nov 29, 2021

We currently don't persist the NetworkGraph or Scorer data at all, leaving it up to users to implement this. It would be nice if our existing wrappers did this for users, likely via the BackgroundProcessor.

@TheBlueMatt TheBlueMatt changed the title On shutdown persistence Provide Utilities to Persist NetworkGraph and Scorer Nov 29, 2021
@jurvis
Copy link
Contributor

jurvis commented Mar 19, 2022

hi @ConorOkus, I will like to take a look into working on this! 😄

I just watched the Creating a Sample Lightning Node with LDK video you did with @valentinewallace and it looks like it is deliberate design for developers to bring their own persistence solution for NetworkGraph and Scorer.

Is the thinking here to persist NetworkGraph and Scorer automatically like how we do for ChannelManager in BackgroundProcessor?

@TheBlueMatt
Copy link
Collaborator

Is the thinking here to persist NetworkGraph and Scorer automatically like how we do for ChannelManager in BackgroundProcessor?

Yep! Note that we already have access to the network graph via net_graph_msg_handler...network_graph() which we use to call remove_stale_channels regularly, we just have to also persist it occasionally.

@jurvis
Copy link
Contributor

jurvis commented Apr 3, 2022

@TheBlueMatt is there still a desire to work on persisting Scorer? I started playing around with seeing how we may implement it and it looks like we will need to add a parameter to BackgroundProcessor::start to have access to the scorer; unless there's something I'm missing?

@TheBlueMatt
Copy link
Collaborator

Yes, I think it makes sense to move that way. We currently do it by hand in the sample, but it'd be nice if the background processor had an Option<>al scorer and could persist that. We'll need to bound it by Writeable instead of Score, I believe, though we could do both but bindings will be sad about that (or we could do a supertrait that is WriteableScore that is auto-implemented for everything that implements Score and Writeable).

@jurvis
Copy link
Contributor

jurvis commented Apr 9, 2022

@TheBlueMatt sounds good. I have a draft implementation that uses LockableScore but I'll work on creating the WriteableScore super-trait. Hope to have something by Sunday. Thanks for the context!

@tnull
Copy link
Contributor

tnull commented Jun 28, 2022

It seems this was fully addressed by #1376 and #1416. Feel free to reopen if I'm missing something.

@tnull tnull closed this as completed Jun 28, 2022
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

4 participants