-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Refactor Payment PR 1 #9825
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
base: master
Are you sure you want to change the base?
Refactor Payment PR 1 #9825
Conversation
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
fb6f2c6
to
3635b27
Compare
3635b27
to
cedd931
Compare
6c85629
to
da88254
Compare
In the following commits we will gradually unify the current payment db operations into an interface to later down the road support both backends (sql+kv).
We move all code from the payment_control.go into payments.go so that for later commits we can easily have all payment db related kv db code together.
da88254
to
3a02bcd
Compare
return fmt.Errorf("HTLC with ID %v not registered", | ||
attemptID) | ||
} | ||
|
||
// Make sure the shard is not already failed or settled. | ||
if htlcsBucket.Get(htlcBucketKey(htlcFailInfoKey, aid)) != nil { | ||
failInfo := htlcsBucket.Get( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: keep the comment?
// KVPaymentsDB implements persistence for payments and payment attempts. | ||
type KVPaymentsDB struct { | ||
paymentSeqMx sync.Mutex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i would have rather just renamed payment_control.go
to payments_db_kvdb.go
so that this file clearly contains only the KVDB CRUD for payments. then later, we'd add a payments_db_sql.go
where we'd implement the SQL CRUD for payments.
See an example here for the graph store. Specifically, see sql_store.go
and kv_store.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
then the payments.go
file can continue to only contain payment related items that are db-agnostic
This PR is the start of a refactor series to finally support both payment backends out of the box for LND.
This PR starts with unifying the PaymentDB code which is currently splitted across multiple files and introducing a new KVPaymentDB struct at the server level. Apart from the second commit which adds a new DB to the server level (but does not use it yet) all of the changes are only refactors so nothing changed.