The GetInvoice query used an OR IS NULL pattern for each filter
parameter:
WHERE (i.hash = $1 OR $1 IS NULL)
AND (i.payment_addr = $2 OR $2 IS NULL)
SQLite's query planner decides on an execution plan at prepare time,
before seeing any parameter values. Because either condition can be
trivially true when its parameter is NULL, the planner conservatively
falls back to a full table scan rather than using the unique indexes on
hash and payment_addr. This caused every invoice lookup and update on
the hot path (HTLC settlement) to scan the entire invoices table.
Replace the single catch-all query with three dedicated queries, each
using a direct equality on a uniquely constrained column:
- GetInvoiceByHash: WHERE hash = $1
- GetInvoiceByAddr: WHERE payment_addr = $1 (new, covers AMP path)
- GetInvoiceBySetID: existing, unchanged
Update getInvoiceByRef to route to the appropriate query based on which
fields are present in the InvoiceRef. When both hash and payment address
are provided, we look up by hash and then verify the returned invoice's
payment address matches.
GetInvoice is now unused and removed from the SQLInvoiceQueries
interface.
Replace all four FilterInvoices call sites with the focused queries
introduced in the previous commit:
FetchPendingInvoices → FetchPendingInvoices
InvoicesSettledSince → FilterInvoicesBySettleIndex
InvoicesAddedSince → FilterInvoicesByAddIndex
QueryInvoices → FilterInvoicesForward / FilterInvoicesReverse
The first three are straight 1:1 swaps — the new params structs carry
only the fields that are actually used, and the removed fields (Reverse,
PendingOnly, unused index bounds) were always left at their zero values.
QueryInvoices is restructured more substantially. The forward/reverse
branch now selects between FilterInvoicesForward and
FilterInvoicesReverse, each of which takes a concrete id bound that is
always set:
forward: AddIndexGet = IndexOffset + 1 (≥ 1 when IndexOffset = 0)
reverse: AddIndexLet = IndexOffset - 1 (or MaxInt64 when offset = 0)
Timestamp parameters are changed from nullable (sql.NullTime with
OR-based SQL fallbacks) to always-on Go-side defaults, consistent with
the approach used by the payments query:
createdAfter → time.Unix(0, 0).UTC() (epoch, before any invoice)
createdBefore → time.Date(9999, 12, 31, …) (far future, no upper cap)
This ensures the planner always sees plain range predicates on
created_at and can use the invoices_created_at_idx index.
The SQLInvoiceQueries interface is updated to expose the five new
methods and drop FilterInvoices.
In this commit, we complete the migration to lnutils.SpewLogClosure for
the remaining application-layer components. This includes the autopilot
channel management system, the funding manager, the RPC server, and the
invoice storage subsystem.
Add a re-usable implementation of the sqldb.TxOptions interface and make
use of this in the various spots (invoices, batch and graph/db) where we
had previously defined individual implementations that were all doing
the same thing.
We now cancel all HTLCs of an AMP invoice as soon as it expires.
Otherwise because we mark the invoice as cancelled we would not
allow accepted HTLCs to be resolved via the invoiceEventLoop.
Previously we'd recalculate the paid amount by summing amounts of
settled HTLCs. This approach while correct would stop the SQL migration
process as some KV invoices may have incorrectly stored paid amounts.
The current sqlc GetInvoice query experiences incremental slowdowns during
the migration of large invoice databases, primarily due to its complex
predicate set. For this specific use case, a streamlined GetInvoiceByHash
function provides a more efficient solution, maintaining near-constant
lookup times even with extensive table sizes.
Certain invoices may not have a deterministic payment hash. For such
invoices we still store the payment hashes in our KV database, but we do
not have a sufficient index to retrieve them. This PR adds such index to
the SQL database that will be used during migration to retrieve payment
hashes.
Find and replace all nolint instances refering to the `lll` linter and
replace with `ll` which is the name of our custom version of the `lll`
linter which can be used to ignore log lines during linting.
The next commit will do the configuration of the custom linter and
disable the default one.
All the structs defined in the `channeldb/models` package are graph
related. So once we move all the graph CRUD code to the graph package,
it makes sense to have the schema structs there too. So this just moves
the `models` package over to `graph/db/models`.
Previously, the SQL implementation of the invoice query simply
converted the start and end timestamps to time and used them
in SQL queries to check for inclusivity. However, this logic
failed when the start and end timestamps were equal.
This commit addresses and corrects this issue.
Previously SQL invoice updater ignored the set ID hint when updating an
AMP invoice resulting in update subscriptions returning all of the AMP
state as well as all AMP HTLCs. This commit synchornizes behavior with
the KV implementation such that we now only return relevant AMP state
and HTLCs when updating an AMP invoice.
Previously, when using the native schema, invoice expiries were incorrectly
stored as 64-bit values (expiry in nanoseconds instead of seconds), causing
overflow issues. Since we cannot determine the original values, we will set
the expiries for existing invoices to 1 hour with this migration.
Previously paginated queries offseted the add_index_get, add_index_let,
settle_index_get and settle_index_let parameters with the paginators
current page offset, however this was incorrect as we can just use
SQL's LIMIT/OFFSET to paginate. This commit fixes this issue and adds an
optional parameter to the constructor of the invoice SQL store to set
page size. This is useful when testing as we can now cover pagination
correctly with our existing unit tests.
Previously if the `reverse` named arg was unset (value of NULL), then
SQL would order by NULL instead of ID causing undifined ordering of the
returned rows. To fix that we check for NULL and also make sure to set
the `reverse` arg in the code explicitly as it in the generated code it
is an `interface{}` instead of `bool`.
For SQL transactions, we often accumulate results in variables declared
outside the closure's scope. To eliminate the need for manually clearing
these containers, we introduce a reset function to ExecTx, mirroring the
approach already adopted in kvdb.
ziggie