-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
channeldb: refactor UpdateInvoice
to make it simpler to create SQL specific implementation
#8100
Changes from all commits
eb4198b
4bf6b52
7298b2d
ef5a317
342eb4e
87044b8
08df7f4
9981569
ecbfc46
6b0931a
5e746b4
0e2b39e
7d56d53
f5abc94
bcc6a3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,11 @@ package invoices | |
|
||
import ( | ||
"context" | ||
"time" | ||
|
||
"github.com/lightningnetwork/lnd/channeldb/models" | ||
"github.com/lightningnetwork/lnd/lntypes" | ||
"github.com/lightningnetwork/lnd/lnwire" | ||
"github.com/lightningnetwork/lnd/record" | ||
) | ||
|
||
|
@@ -162,3 +164,37 @@ type InvoiceSlice struct { | |
// CircuitKey is a tuple of channel ID and HTLC ID, used to uniquely identify | ||
// HTLCs in a circuit. | ||
type CircuitKey = models.CircuitKey | ||
|
||
// InvoiceUpdater is an interface to abstract away the details of updating an | ||
// invoice in the database. The methods of this interface are called during the | ||
// in-memory update of an invoice when the database needs to be updated or the | ||
// updated state needs to be marked as needing to be written to the database. | ||
type InvoiceUpdater interface { | ||
// AddHtlc adds a new htlc to the invoice. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already mentioned this in the comments below, but I think these functions need a more detailed name because by just looking at the current naming I would not be able to explain why the kv-value implementation doesn't need an AddHtlc function, because htlcs are added as well in a general sense to the invoice ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again I'm not quite sure if we need to leak any implementation details through naming of this interface (see my other reply too). Do you maybe have a suggestion? |
||
AddHtlc(circuitKey CircuitKey, newHtlc *InvoiceHTLC) error | ||
|
||
// ResolveHtlc marks an htlc as resolved with the given state. | ||
ResolveHtlc(circuitKey CircuitKey, state HtlcState, | ||
resolveTime time.Time) error | ||
|
||
// AddAmpHtlcPreimage adds a preimage of an AMP htlc to the AMP invoice | ||
// identified by the setID. | ||
AddAmpHtlcPreimage(setID [32]byte, circuitKey CircuitKey, | ||
preimage lntypes.Preimage) error | ||
|
||
// UpdateInvoiceState updates the invoice state to the new state. | ||
UpdateInvoiceState(newState ContractState, | ||
preimage *lntypes.Preimage) error | ||
|
||
// UpdateInvoiceAmtPaid updates the invoice amount paid to the new | ||
// amount. | ||
UpdateInvoiceAmtPaid(amtPaid lnwire.MilliSatoshi) error | ||
|
||
// UpdateAmpState updates the state of the AMP invoice identified by | ||
// the setID. | ||
UpdateAmpState(setID [32]byte, newState InvoiceStateAMP, | ||
circuitKey models.CircuitKey) error | ||
|
||
// Finalize finalizes the update before it is written to the database. | ||
Finalize(updateType UpdateType) error | ||
} |
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.
+1 for having most of the logic here in the
invoices
packageThen we have another layer between this and the DB, that's responsible for mapping the invoice updates into concrete DB operations.
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.
My main goal for this was to completely decouple invoice tests from the
channeldb
package such that we're easily able to test against otherInvoiceDB
implementations such as the SQL one.