invoices/sqldb: query by ChanID when updating AMP invoice preimage

[aakselrod]

Change Description

This PR fixes the following test:

make itest backend=btcd dbbackend=postgres nativesql=true icase=sendpayment_amp_invoice

Note that the sendpayment_amp_invoice_repeat test is not fixed by this. Also note that this depends on #9021 to unmask the test failure.

Steps to Test

Run the itest as above before and after applying this PR.

Pull Request Checklist

Testing

• [x] Your PR passes all CI checks.
• [x] Tests covering the positive and negative (error paths) are included.
• [x] Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

• [x] The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
• [x] Commits follow the Ideal Git Commit Structure.
• [x] Any new logging statements use an appropriate subsystem and logging level.
• [x] Any new lncli commands have appropriate tags in the comments for the rpc in the proto file.
• [x] There is a change description in the release notes, or [skip ci] in the commit message for small changes.

📝 Please see our Contribution Guidelines for further guidance.

[coderabbitai[bot]]

[!IMPORTANT]

Review skipped

Auto reviews are limited to specific labels.

Labels to auto review (1)

• llm-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[bhandras]

Please make sure that the linter is green too!

[aakselrod]

Please make sure that the linter is green too!

I have questions on this:

The lint task fails on checking submodule tidiness. However, locally, I can't reproduce this:

$ git log -n 1
commit a6e1960c61bd11b4ab9884e26c4814b59840df4a (HEAD -> add-channelid-ampsettle, alex/add-channelid-ampsettle)
Author: Alex Akselrod <[email protected]>
Date:   Wed Aug 21 17:33:37 2024 -0700

    invoices/sqldb: query by ChanID when updating AMP invoice preimage
$ git status
On branch add-channelid-ampsettle
nothing to commit, working tree clean
$ ./scripts/tidy_modules.sh 
~/go/src/github.com/lightningnetwork/lnd/queue ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd/kvdb ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd/tools ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd/ticker ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd/tlv ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd/clock ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd/healthcheck ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd/fn ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd/sqldb ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd/cert ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd/tor ~/go/src/github.com/lightningnetwork/lnd
~/go/src/github.com/lightningnetwork/lnd
$ git status
On branch add-channelid-ampsettle
nothing to commit, working tree clean
$ 

The other thing, when I make lint, it doesn't like me doing a local redirect for sqldb. Would love a suggestion for how to fix that.

$ make lint
 Checking for target Go version (v1.22.6) in Dockerfile files (*Dockerfile)
./scripts/check-go-version-dockerfile.sh 1.22.6
./docker/btcd/Dockerfile is using Go version 1.22.6.
Skipping ./tools/Dockerfile
./make/builder.Dockerfile is using Go version 1.22.6.
./lnrpc/Dockerfile is using Go version 1.22.6.
./dev.Dockerfile is using Go version 1.22.6.
./Dockerfile is using Go version 1.22.6.
All Dockerfiles pass the Go version check for Go version 1.22.6.
 Checking for target Go version (v1.22.6) in YAML files (*.yaml, *.yml)
./scripts/check-go-version-yaml.sh 1.22.6
./.golangci.yml specifies Go version 1.22.6.
./.github/workflows/release.yaml specifies Go version 1.22.6.
./.github/workflows/main.yml specifies Go version 1.22.6.
All YAML files pass the Go version check for Go version 1.22.6.
 Building tools docker image.
docker build -q -t lnd-tools tools
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            Install the buildx component to build images with BuildKit:
            https://docs.docker.com/go/buildx/

sha256:0c7cd8c3d3aac815f14073c8a6483a4e26686231dfaae460503ceb59178db57c
 Linting source.
docker run --rm -v /home/user/.cache/go-build:/tmp/build/.cache -v /home/user/go/pkg/mod:/tmp/build/.modcache -v /tmp/go-lint-cache:/root/.cache/golangci-lint -v $(pwd):/build lnd-tools golangci-lint run -v 
level=info msg="golangci-lint has version v1.60.1 built with go1.22.6 from (unknown, modified: ?, mod sum: \"h1:DRKNqNTQRLBJZ1il5u4fvgLQCjQc7QFs0DbhksJtVJE=\") on (unknown)"
level=info msg="[config_reader] Config search paths: [./ /build / /root]"
level=info msg="[config_reader] Used config file .golangci.yml"
level=warning msg="The linter 'execinquery' is deprecated (since v1.58.0) due to: The repository of the linter has been archived by the owner. "
level=info msg="[lintersdb] Active 65 linters: [asasalint asciicheck bidichk canonicalheader containedctx decorder dupl dupword durationcheck errcheck errchkjson errorlint execinquery exportloopref fatcontext forbidigo forcetypeassert funlen gci ginkgolinter gocheckcompilerdirectives gochecksumtype goconst gocritic godot gofmt goheader goimports gomoddirectives gomodguard goprintffuncname gosec gosimple govet grouper importas ineffassign interfacebloat lll loggercheck makezero mirror misspell musttag nakedret nestif nlreturn nolintlint nonamedreturns nosprintfhostport predeclared promlinter reassign sloglint spancheck staticcheck stylecheck tagliatelle tenv testableexamples unconvert unused usestdlibvars whitespace zerologlint]"
level=info msg="[loader] Using build tags: [autopilotrpc chainrpc dev invoicesrpc neutrinorpc peersrpc signrpc walletrpc watchtowerrpc kvdb_etcd kvdb_postgres kvdb_sqlite integration]"
level=info msg="[loader] Go packages loading at mode 575 (exports_file|files|name|types_sizes|compiled_files|deps|imports) took 741.598014ms"
level=info msg="[runner/filename_unadjuster] Pre-built 0 adjustments in 257.894938ms"
level=info msg="[linters_context] importas settings found, but no aliases listed. List aliases under alias: key."
level=info msg="[linters_context/goanalysis] analyzers took 0s with no stages"
level=warning msg="[runner/nolint] Found unknown linters in //nolint directives: ifelsechain"
level=info msg="[runner] Issues before processing: 31379, after processing: 1"
level=info msg="[runner] Processors filtering stat (in/out): path_shortener: 1/1, filename_unadjuster: 31379/31379, invalid_issue: 31379/31379, path_prettifier: 31379/31379, skip_dirs: 31379/31379, identifier_marker: 9917/9917, max_from_linter: 1/1, skip_files: 31379/31379, nolint: 8073/6980, diff: 6815/1, path_prefixer: 1/1, sort_results: 1/1, uniq_by_line: 6980/6815, max_per_file_from_linter: 1/1, max_same_issues: 1/1, source_code: 1/1, cgo: 31379/31379, autogenerated_exclude: 31379/9917, exclude: 9917/9917, exclude-rules: 9917/8073, severity-rules: 1/1, fixer: 1/1"
level=info msg="[runner] processing took 870.841685ms with stages: nolint: 419.503652ms, exclude-rules: 203.227664ms, identifier_marker: 139.133924ms, diff: 47.532949ms, autogenerated_exclude: 23.356419ms, path_prettifier: 17.32278ms, invalid_issue: 11.46177ms, skip_dirs: 5.796463ms, uniq_by_line: 1.345988ms, cgo: 1.214543ms, filename_unadjuster: 883.714µs, source_code: 56.572µs, max_same_issues: 1.545µs, path_shortener: 1.019µs, max_per_file_from_linter: 670ns, max_from_linter: 586ns, fixer: 322ns, skip_files: 306ns, exclude: 264ns, sort_results: 245ns, path_prefixer: 199ns, severity-rules: 91ns"
level=info msg="[runner] linters took 1.702902941s with stages: goanalysis_metalinter: 831.933394ms"
level=info msg="File cache stats: 1 entries of total size 10.3KiB"
go.mod:208:1: local replacement are not allowed: github.com/lightningnetwork/lnd/sqldb (gomoddirectives)
replace github.com/lightningnetwork/lnd/sqldb => ./sqldb
^
level=info msg="Memory: 29 samples, avg is 91.2MB, max is 132.9MB"
level=info msg="Execution took 2.707028088s"
make: *** [Makefile:322: lint-source] Error 1
$

[guggero]

@aakselrod I get the following diff when I run make tidy-module-check:

diff --git a/go.mod b/go.mod
index 6652e6547..cbe4d40ba 100644
--- a/go.mod
+++ b/go.mod
@@ -19,7 +19,6 @@ require (
        github.com/davecgh/go-spew v1.1.1
        github.com/decred/dcrd/dcrec/secp256k1/v4 v4.3.0
        github.com/go-errors/errors v1.0.1
-       github.com/golang/protobuf v1.5.3
        github.com/gorilla/websocket v1.5.0
        github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
        github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
@@ -95,6 +94,7 @@ require (
        github.com/gogo/protobuf v1.3.2 // indirect
        github.com/golang-jwt/jwt/v4 v4.4.2 // indirect
        github.com/golang-migrate/migrate/v4 v4.17.0 // indirect
+       github.com/golang/protobuf v1.5.3 // indirect
        github.com/golang/snappy v0.0.4 // indirect
        github.com/google/btree v1.0.1 // indirect
        github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect

Maybe a question of the Go version you're using?

go.mod:208:1: local replacement are not allowed: github.com/lightningnetwork/lnd/sqldb (gomoddirectives)
replace github.com/lightningnetwork/lnd/sqldb => ./sqldb

Hmm, normally we do this by doing a PR to the submodule first, merge that and push a tag, then bump the version in the main go.mod file. But I guess since that module is still under heavy development, I'd also agree to add this to the allowed replace modules in .golangci.yml:

  gomoddirectives:
    replace-local: true
    replace-allow-list:
...

[aakselrod]

make tidy-module-check is different from running the script directly as it involves one more check,

https://github.com/lightningnetwork/lnd/blob/cc9e2b783ec983fbe3995d47d31898aa122b11c8/Makefile#L337-L339

That did it, thanks!

[aakselrod]

@aakselrod I get the following diff when I run make tidy-module-check:

I think my problem was that I wasn't running the make target but the script directly.

Hmm, normally we do this by doing a PR to the submodule first, merge that and push a tag, then bump the version in the main go.mod file. But I guess since that module is still under heavy development, I'd also agree to add this to the allowed replace modules in .golangci.yml:

  gomoddirectives:
    replace-local: true
    replace-allow-list:
...

OK, I'll make this change. Thank you!

[aakselrod]

Lint is passing now!

[guggero]

I think we should do a full CI run again once https://github.com/lightningnetwork/lnd/pull/9021 is in and this is rebased.

[aakselrod]

I'll do that rebase today!

[aakselrod]

Rebase complete, hopefully should be good to merge after CI run.

[aakselrod]

These errors are caused by https://github.com/lightningnetwork/lnd/blob/8939a217c3f1bb0b56a3a77df734fc6c74347838/invoices/invoiceregistry.go#L888 and ignored. 3 of them are from the test fixed by this PR. The remainder are from the next test, explicitly not fixed by this PR (see OP change description), which I'm still debugging and for which I was planning to submit a follow-up PR. The newly-failing test didn't execute at all during CI runs as the test fixed by this PR failed and aborted the rest of the tranche.

If this PR doesn't get merged before I find/fix this failing test, I'll include the fix here instead.

[guggero]

explicitly not fixed by this PR (see OP change description)

Ah, right. I missed that part, sorry.

was planning to submit a follow-up PR.

Sounds good. Though I think we'll want to block v0.18.3 on this fix. Or do you think it might be an issue of the itest itself and not the actual native SQL code?

[aakselrod]

I think it's an issue with invoice numbering, the behavior is different between kvdb and sql here, so it looks like a real but separate bug to me.

[aakselrod]

The invoice_exact_end_date failure is caused by the way KVDB compares timestamps (integer seconds) vs SQL (microseconds), but not sure whether that should be fixed in the test, KVDB, or SQL, BTW. It's likely not as much of a blocker.

[Roasbeef]

Looks like the native SQL itest are still failing:

  --- FAIL: TestLightningNetworkDaemon/tranche05/107-of-168/bitcoind/sendpayment_amp_invoice_repeat (19.61s)
        lnd_amp_test.go:300: 
            	Error Trace:	/home/runner/work/lnd/lnd/itest/lnd_amp_test.go:300
            	            				/home/runner/work/lnd/lnd/lntest/harness.go:386
            	            				/home/runner/work/lnd/lnd/itest/lnd_test.go:139
            	Error:      	Not equal: 
            	            	expected: 1
            	            	actual  : 2
            	Test:       	TestLightningNetworkDaemon/tranche05/107-of-168/bitcoind/sendpayment_amp_invoice_repeat

[aakselrod]

I think it's an issue with invoice numbering, the behavior is different between kvdb and sql here, so it looks like a real but separate bug to me.

I no longer think it's about invoice numbering but didn't have much time to keep digging into it today. Will keep going tomorrow.

[aakselrod]

I've spent a few hours on this today, and have gotten farther. It looks like the SQL invoice updater isn't returning only the "projected" HTLCs for the newly-settled AMP invoice, but all of the past invoices. The newly-settled AMP invoice is also returned with the incorrect settle index and time, though it seems to be written correctly to the DB and returned correctly on LookupInvoice after the fact.

I also noticed that in the KVDB code, the updater consistently logs the error

[ERR] CHDB: unable to fetch amp htlcs for inv [0 0 0 0] and setIDs [0xc0007ea100]: %!w(*errors.errorString=&{unable to locate invoice})

Not 100% sure that's relevant, but I'm going to keep going on this tomorrow.

[aakselrod]

Also will take a closer look at https://github.com/lightningnetwork/lnd/blob/1bf7ad9b435ae6a5a1aaecdab77934b59936495d/invoices/invoices_test.go#L234 and see if this runs native sql tests.

ETA: it does run native SQL tests, so that's good.

[bhandras]

I've spent a few hours on this today, and have gotten farther. It looks like the SQL invoice updater isn't returning only the "projected" HTLCs for the newly-settled AMP invoice, but all of the past invoices. The newly-settled AMP invoice is also returned with the incorrect settle index and time, though it seems to be written correctly to the DB and returned correctly on LookupInvoice after the fact.

I also noticed that in the KVDB code, the updater consistently logs the error

[ERR] CHDB: unable to fetch amp htlcs for inv [0 0 0 0] and setIDs [0xc0007ea100]: %!w(*errors.errorString=&{unable to locate invoice})

Not 100% sure that's relevant, but I'm going to keep going on this tomorrow.

I believe the issue is that when we fetch the invoice during the update the setID passed to UpdateInvoice is not used: https://github.com/lightningnetwork/lnd/blob/5c3a8e949c7d30ab055fd4326a2082ad3b0f7b0f/invoices/sql_store.go#L1301 I think simply passing in the setID won't work as the SQL query assumes that the AMP sub-invoice already exists. In the KV version the assumption is different as if the setID is passed it'll simply fetch the HTLCs in that set: https://github.com/lightningnetwork/lnd/blob/5c3a8e949c7d30ab055fd4326a2082ad3b0f7b0f/channeldb/invoices.go#L1508 I can try to put together a diff to fix this.

[aakselrod]

I believe the issue is that when we fetch the invoice during the update the setID passed to UpdateInvoice is not used:

This looks exactly right to me.

I can try to put together a diff to fix this.

Sounds good, thank you! Do you want to make a separate PR or add it to this one?

[bhandras]

I believe the issue is that when we fetch the invoice during the update the setID passed to UpdateInvoice is not used:

This looks exactly right to me.

I can try to put together a diff to fix this.

Sounds good, thank you! Do you want to make a separate PR or add it to this one?

I think it's best to add to this one, given it fails the itest.

[aakselrod]

The newly-settled AMP invoice is also returned with the incorrect settle index and time, though it seems to be written correctly to the DB and returned correctly on LookupInvoice after the fact.

I've pushed an itest change that checks for this, and then a fix. Hopefully they both run so we can see the difference...

[guggero]

Did a quick rebase to fix a merge conflict in the release notes, so we can see the CI run.

[bhandras]

Added fixes for the remaining issues. Note that the fix changes some previous behavior IMO for the better (cc @Roasbeef)

[bhandras]

I wonder if we should instead open another PR with the rest of the fixes since these seem to be unrelated to the original fix.

[bhandras]

I ran the test locally and got the following errors in the Postgres container:

2024-08-26 08:45:15.430 UTC [88] ERROR:  duplicate key value violates unique constraint "invoices_payment_addr_key"
2024-08-26 08:45:15.430 UTC [88] DETAIL:  Key (payment_addr)=(\x68d295d5d6daa959147608463bb46f5ae877f0806d3af93ff48f681e79411700) already exists.
2024-08-26 08:45:15.430 UTC [88] STATEMENT:  -- name: InsertInvoice :one
        INSERT INTO invoices (
            hash, preimage, memo, amount_msat, cltv_delta, expiry, payment_addr, 
            payment_request, payment_request_hash, state, amount_paid_msat, is_amp,
            is_hodl, is_keysend, created_at
        ) VALUES (
            $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
        ) RETURNING id
        
2024-08-26 08:45:15.440 UTC [88] ERROR:  duplicate key value violates unique constraint "invoices_payment_addr_key"
2024-08-26 08:45:15.440 UTC [88] DETAIL:  Key (payment_addr)=(\x68d295d5d6daa959147608463bb46f5ae877f0806d3af93ff48f681e79411700) already exists.
2024-08-26 08:45:15.440 UTC [88] STATEMENT:  -- name: InsertInvoice :one
        INSERT INTO invoices (
            hash, preimage, memo, amount_msat, cltv_delta, expiry, payment_addr, 
            payment_request, payment_request_hash, state, amount_paid_msat, is_amp,
            is_hodl, is_keysend, created_at
        ) VALUES (
            $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
        ) RETURNING id
        
2024-08-26 08:45:15.686 UTC [88] ERROR:  duplicate key value violates unique constraint "invoices_payment_addr_key"
2024-08-26 08:45:15.686 UTC [88] DETAIL:  Key (payment_addr)=(\x68d295d5d6daa959147608463bb46f5ae877f0806d3af93ff48f681e79411700) already exists.
2024-08-26 08:45:15.686 UTC [88] STATEMENT:  -- name: InsertInvoice :one
        INSERT INTO invoices (
            hash, preimage, memo, amount_msat, cltv_delta, expiry, payment_addr, 
            payment_request, payment_request_hash, state, amount_paid_msat, is_amp,
            is_hodl, is_keysend, created_at
        ) VALUES (
            $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
        ) RETURNING id
        
2024-08-26 08:46:10.586 UTC [346] ERROR:  duplicate key value violates unique constraint "invoices_payment_addr_key"
2024-08-26 08:46:10.586 UTC [346] DETAIL:  Key (payment_addr)=(\xc781996e8a209bfdd5e2775876b8626da735bfb088a94423985f8307a7aae2ac) already exists.
2024-08-26 08:46:10.586 UTC [346] STATEMENT:  -- name: InsertInvoice :one
        INSERT INTO invoices (
            hash, preimage, memo, amount_msat, cltv_delta, expiry, payment_addr, 
            payment_request, payment_request_hash, state, amount_paid_msat, is_amp,
            is_hodl, is_keysend, created_at
        ) VALUES (
            $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
        ) RETURNING id
        
2024-08-26 08:46:10.806 UTC [346] ERROR:  duplicate key value violates unique constraint "invoices_payment_addr_key"
2024-08-26 08:46:10.806 UTC [346] DETAIL:  Key (payment_addr)=(\xc781996e8a209bfdd5e2775876b8626da735bfb088a94423985f8307a7aae2ac) already exists.
2024-08-26 08:46:10.806 UTC [346] STATEMENT:  -- name: InsertInvoice :one
        INSERT INTO invoices (
            hash, preimage, memo, amount_msat, cltv_delta, expiry, payment_addr, 
            payment_request, payment_request_hash, state, amount_paid_msat, is_amp,
            is_hodl, is_keysend, created_at
        ) VALUES (
            $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15
        ) RETURNING id

Note that these logs are harmless and emitted when running icase=send_payment_amp. In this scenario Bob adds an AMP invoice on-demand (spontaneous AMP) and it is expected that subsequent InsertInvoice calls will fail. See here: https://github.com/lightningnetwork/lnd/blob/e8c5e7d5ce4b79117ef9ab497745fea3a1bffb36/invoices/invoiceregistry.go#L889

It is def a bit ugly that we have these logs, but perhaps we can keep them? wdyt @guggero ?

[aakselrod]

~~Should we put my last 2 committs in anothet PR as well? They're also unrelated to the original fix.~~

[bhandras]

Should we put my last 2 committs in anothet PR as well? They're also unrelated to the original fix.

PR opened here: https://github.com/lightningnetwork/lnd/pull/9050

[guggero]

Closing in favor of https://github.com/lightningnetwork/lnd/pull/9050.