sqldb: InvoiceDB implementation

[bhandras]

This PR is based on the work of @positiveblue in #7357.
Also based on top of #8100 which is preliminary work refactoring the update invoice flow in order to be able to port InvoiceDB to SQL.

The PR is a collection of changes to the existing (but unreleased) schema and queries and implementation of a SQL based InvoiceDB utilizing the existing unit test coverage (moved from channeldb) and adding support for running the integration tests with native SQL invoice tables.

The PR does not implement migration for existing nodes!

To test locally, start a new regtest node with --db.backend=sqlite --db.use-native-sql flags.

---

This change is 

[bhandras]

Still a WIP draft, but now the SQL InvoiceStore works and so ready for some very early look.

[Roasbeef]

Amazing work! We're sooo close 🤩

The two main things that jumped out at me are:

1. The shift (in this PR) of the relationship between an AMP invoice and an AMP payment. Before, we had a single AMP invoice, and then HTLCs are inserted that ref that invoice. Now, we have a single logical invoice, then several AMP invoices that point to that, and then several HTLCs for a given payment.

   The original model is closer to my personal mental model. Namely, an AMP invoice is identified by a payment_addr and any payments gain a new set_id value used ot ref them. At any given time, there's only one AMP invoice in the database.

2. The shift away from the event sourced-esque model for the settle index. The original vision was that we wanted to break away from the settle index as is, as it's pretty brittle, and partially is what kept us from adding new indexes like a hold/add index.

   I think the event-sourced architecture is more flexible, and we can add additional fields to the events over time vs needing to modify the base invoice table. We'd have a root logical invoice_events table, then either a tiered approach where we ref a more specific event, like invoice_settles, or we'd have certain fields that are set based on the event type. One benefit of this, is that we can then replay ever event since they last check to the caller. This would include: adds, settles, cancel, etc. Today one would have to maintain an index for each event type, then ask for backlog for them all.

   With this approach, we can still give people just the settles, as the query filters on the event type and accumulates a counter along the way.

[bhandras]

Dismissed @Roasbeef from a discussion.
Reviewable status: 43 of 44 files reviewed, 96 unresolved discussions (waiting on @positiveblue, @Roasbeef, @yyforyongyu, and @ziggie1984)

---

scripts/gen_sqlc_docker.sh line 19 at r2 (raw file):

Previously, Roasbeef (Olaoluwa Osuntokun) wrote…

Latest release is now v1.25.0: https://github.com/sqlc-dev/sqlc/releases/tag/v1.25.0

I think this'll be the last time we'll need to update it in this PR as they do a release every few months-ish.

done

---

sqldb/invoices.go line 1006 at r4 (raw file):

Previously, Roasbeef (Olaoluwa Osuntokun) wrote…

Set what state?

Yeah this was a leftover TODO comment for myself to check later, but it's irrelevant now, so removed.

---

sqldb/sqlc/migrations/000003_invoice_events.up.sql line 12 at r4 (raw file):

Previously, Roasbeef (Olaoluwa Osuntokun) wrote…

In most of the queries that involved inserting new event types, we currently use the integer ID here, so (this is for inserting a canceled event, but at a glance, you'd need to go to the initial table def to understand what 1 is):

INSERT INTO invoice_events (
    added_at, event_type, invoice_id
) VALUES (
    $1, 1, $2
);

I think we should consider instead using the description (and making that field UNIQUE), as then queries like the above are clearer as we'd have:

INSERT INTO invoice_events (
    added_at, event_type, invoice_id
) VALUES (
    $1, 'invoice_canceled', $2
);

The idea here was that we store the strings only once in the invoice_event_types tables and rather if need to have human readability we join the invoice_events table against it. This way if we have say 1m invoices and 2-3m events we wouldn't litter the db with strings unnecessarily.

---

sqldb/sqlc/queries/amp_invoices.sql line 5 at r4 (raw file):

Previously, Roasbeef (Olaoluwa Osuntokun) wrote…

Non-blocking, but it's also possbile to use the named params syntax here to make the queries a bit more readable: https://docs.sqlc.dev/en/latest/howto/named_parameters.html#naming-parameters

So this could be @set_id, @state, @created_at, @invoice_id, etc.

Yes that's a very good point and I was aware, but for now at least didn't really use the named notation because whenever we need to update a query it could lead to hard to catch bugs. An example of such bug would be if we have say multiple integer columns next to each other, and the order of the named arg is mixed up. It'd compile just fine and given many args reviewer may not spot it and depending on test coverage we may not catch it. By just adding numbered args we can always make sure that SQLC ensures the right order.

[bhandras]

Oh LGTM vanished once i answered in reviewable 😅 ptal @Roasbeef

[Roasbeef]

re-LGTM 🏄🏻‍♀️

[positiveblue]

LGTM. I did yet another pass and found nothing seems to be blocking this PR @bhandras

The failing test in the test suite for windows is a flake not related with the code in this PR... I would say we are good to go 🚀