invoices: migrate legacy AMP invoice HTLCs

[ziggie1984]

Change Description

Legacy AMP invoices created before reusable AMP invoices store their HTLCs
inline and don't contain AMPState metadata.

The KV-to-SQL migration inserted the generic HTLC rows for these invoices, but
without AMPState it did not create AMP sub-invoices or associate the HTLCs
with them. SQL read-back therefore returned an empty HTLC map, and strict
migration verification aborted startup.

Reconstruct the modern AMP sub-invoice state from each inline HTLC's set ID
before insertion. This preserves the HTLC associations, AMP metadata, custom
records, amounts, and settlement metadata while keeping the compatibility
handling isolated to the migration.

The migration fails closed if the legacy data is structurally inconsistent.
Release notes are included for v0.21.3 and v0.20.4.

Steps to Test

make lint
go vet ./invoices
go test ./invoices -run '^TestMigrateLegacyAMPInvoice$' -count=20

The regression test migrates a settled legacy AMP invoice with a canceled
competing set, AMP metadata, a large child index, and custom records, then
verifies complete SQL read-back equality.

[github-actions]

🟠 PR Severity: HIGH

gh pr view | 4 files | 253 lines changed

🟠 High (1 file)

• invoices/sql_migration.go - invoice SQL migration logic (invoices/*)

🟢 Low (3 files)

• invoices/sql_migration_test.go - test-only change
• docs/release-notes/release-notes-0.20.4.md - release notes
• docs/release-notes/release-notes-0.21.3.md - release notes

Analysis

The substantive change is in invoices/sql_migration.go, which falls under invoices/* (invoice management/settlement), classified as HIGH severity. It is not a channeldb/migration*, sqldb/*, or wtdb/* path, so the automatic CRITICAL database-migration rule does not apply. No bump conditions are met: only one non-test file changed with ~110 non-test lines modified, well under the 20-file/500-line thresholds, and only a single package is touched. The remaining files are a test file and release notes, both LOW.

---

_{To override, add a severity-override-{critical,high,medium,low} label.}

[ziggie1984]

Legacy AMP invoice migration: before and after

Legacy AMP invoices store their HTLCs inline. Each HTLC contains its AMP set
ID and metadata, but the invoice predates the AMPState structure used by
reusable AMP invoices.

flowchart TB
    legacy["Legacy KV AMP invoice<br/><br/>IsAMP = true<br/>AMPState = empty"]
    htlcs["Inline HTLCs<br/><br/>Set ID<br/>AMP metadata<br/>Custom records"]

    legacy --> htlcs
    htlcs --> oldMigration
    htlcs --> normalize

    subgraph before["Before the fix"]
        direction TB

        oldMigration["Original SQL migration"]
        genericRows["Insert invoice and generic HTLC rows<br/>inside the migration transaction"]
        emptyState["Iterate AMPState<br/>0 entries"]
        missingSubInvoice["No AMP sub-invoice"]
        missingAssociation["No AMP-to-HTLC association"]
        oldReader["SQL AMP reader follows<br/>AMP associations"]
        emptyResult["Read-back result<br/><br/>Htlcs = empty<br/>AMPState = empty"]
        mismatch["Strict comparison fails"]
        rollback["Entire migration transaction rolls back"]

        oldMigration --> genericRows
        oldMigration --> emptyState
        emptyState --> missingSubInvoice
        missingSubInvoice --> missingAssociation
        genericRows --> oldReader
        missingAssociation --> oldReader
        oldReader --> emptyResult
        emptyResult --> mismatch
        mismatch --> rollback
    end

    subgraph after["With the fix"]
        direction TB

        normalize["Normalize the legacy representation<br/>in memory"]
        group["Group existing HTLCs by<br/>their embedded AMP set ID"]
        ampState["Reconstruct AMPState<br/><br/>InvoiceKeys<br/>State and AmtPaid<br/>Settle index and date"]
        existingWriter["Existing SQL migration writer"]
        invoiceHtlcs["invoice_htlcs<br/><br/>Original HTLC data<br/>and custom records"]
        subInvoices["amp_sub_invoices<br/><br/>One row per AMP set"]
        associations["amp_sub_invoice_htlcs<br/><br/>AMP-to-HTLC associations<br/>and AMP metadata"]
        newReader["SQL AMP reader"]
        restored["Read-back result<br/><br/>Htlcs restored<br/>AMPState reconstructed"]
        match["Strict comparison passes"]
        commit["Migration transaction commits"]

        normalize --> group
        group --> ampState
        ampState --> existingWriter
        existingWriter --> invoiceHtlcs
        existingWriter --> subInvoices
        existingWriter --> associations
        invoiceHtlcs --> newReader
        subInvoices --> newReader
        associations --> newReader
        newReader --> restored
        restored --> match
        match --> commit
    end

Loading

The HTLC data itself was already available in the legacy record. The missing
piece was the SQL relationship connecting each HTLC to its AMP sub-invoice.

The fix reconstructs only that derivable AMPState metadata in memory and then
uses the existing SQL writer. It does not modify the source KV database or
reinterpret the payment. Structurally inconsistent legacy records still fail
the migration, and the transaction is rolled back safely.

[ziggie1984]

Legacy AMP settlement representation in SQL

The migrated legacy invoice intentionally differs from a reusable AMP invoice:

Representation	Parent invoices row	amp_sub_invoices row
Legacy AMP	is_amp=true, state=ContractSettled, with the original settle index/date	Reconstructed set state, amount, and HTLC associations; no separate settle index/date
Reusable AMP	Remains ContractOpen, without parent-level settlement metadata	Every settled set has its own settle index/date

Before reusable AMP invoices existed, settling one AMP set made the parent
invoice terminal. The KV settle index therefore recorded a single event against
the parent invoice, not against a set ID. The parent combination
is_amp=true + ContractSettled + parent settle metadata is consequently the
indicator that this is the migrated legacy representation.

Migration reconstructs the missing AMP sub-invoice rows because SQL needs them
to associate each HTLC with its set ID. It marks the successful set as settled
and preserves canceled/competing sets, but it does not manufacture a second
settlement event on the successful sub-invoice. This keeps the invoice
non-reusable and ensures InvoicesSettledSince emits the historical payment
exactly once.

[bhandras]

LGTM 🎉

[ellemouton]

🙏

[github-actions]

Created backport PR for v0.20.x-branch:

• [v0.20.x-branch] Backport #11106: invoices: migrate legacy AMP invoice HTLCs #11107 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-11106-to-v0.20.x-branch
git worktree add --checkout .worktree/backport-11106-to-v0.20.x-branch backport-11106-to-v0.20.x-branch
cd .worktree/backport-11106-to-v0.20.x-branch
git reset --hard HEAD^
git cherry-pick -x cb9c86195251422dc217c35b915e54a55d8f8580 b7a2f153962c34b3920533da0ae70f05e01d7a4f
git push --force-with-lease

[github-actions]

Created backport PR for v0.21.x-branch:

• [v0.21.x-branch] Backport #11106: invoices: migrate legacy AMP invoice HTLCs #11108 with remaining conflicts!

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-11106-to-v0.21.x-branch
git worktree add --checkout .worktree/backport-11106-to-v0.21.x-branch backport-11106-to-v0.21.x-branch
cd .worktree/backport-11106-to-v0.21.x-branch
git reset --hard HEAD^
git cherry-pick -x b7a2f153962c34b3920533da0ae70f05e01d7a4f
git push --force-with-lease