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

Persist payment info to disk #90

Closed
alecchendev opened this issue Jan 5, 2023 · 2 comments · Fixed by #104
Closed

Persist payment info to disk #90

alecchendev opened this issue Jan 5, 2023 · 2 comments · Fixed by #104

Comments

@alecchendev
Copy link
Contributor

alecchendev commented Jan 5, 2023

I saw this was commented TODO in main.rs, but I wanted to open an issue to see if others think this would be valuable to implement before I start working on a PR for it.

Motivation

It seems this--keeping track of payment info between times that a node is on/offline--would be helpful to demonstrate to a user. I don't think this adds significant complexity compared to what's already here, but I'm open to what others have to say.

Implementing this

My initial thoughts for what needs to be done to implement this:

  • On startup, read inbound_payments and outbound_payments if they have been previously persisted.

  • Whenever inbound_payments or outbound_payments gets updated, persist payment info to disk.

  • Haven’t totally fleshed out what’s the best way to persist/update payment info, but considering some of the following:
    • Add custom read_payment_info and persist_payment_info functions to disk.rs.
    • Implement Writeable trait (and maybe Readable) on PaymentInfoStorage so that we can persist using FilesystemPersister similar to how other things are persisted.
    • Likely split up inbound_payments and outbound_payments into 2 different files to not complicate serializing payment info
  • Where does payment info get updated?
    • main.rs
      • inbound_payments | Event::PaymentClaimed
      • outbound_payments | Event::PaymentSent
      • outbound_payments | Event::PaymentFailed
    • cli.rs
      • inbound_payments | get_invoice
      • outbound_payments | send_payment
      • outbound_payments | keysend
@TheBlueMatt
Copy link
Contributor

Sure, if you want to. Because this is ultimately sample code, we should seek to ensure the implementation of everything is super straightforward, simple, and readable, and not care too much about performance or even feature-completeness. So, if we can't do this in a way that meets those goals, we shouldn't bother.

With that in mind, we'll probably want to abstract out the PaymentInfoStorage from a simple mutex to a struct which implements writing to some (trivial) DB. I don't think we want to bother taking on a dependency for a database, so we'll have to come up with something simple, like writing it all out to a flag file with Writeable/Readable like you suggest. One approach may be to wait for lightningdevkit/rust-lightning#1823 to land and then use that to implement Readable and Writeable so that we can persist it trivially like you suggest.

@alecchendev
Copy link
Contributor Author

One approach may be to wait for lightningdevkit/rust-lightning#1823 to land and then use that to implement Readable and Writeable so that we can persist it trivially like you suggest.

That makes sense, I'll probably wait for this.

I've mainly been looking for ways to build up my confidence contributing before jumping into rust-lightning, but I realize this sample repo probably doesn't need much work done on it, so I'll probably go pick up a good first issue there sometime soon! Anyway, thanks for the feedback!

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

Successfully merging a pull request may close this issue.

2 participants