From 2ecd1de7139cc837f11cf594db97a381e5e34738 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Thu, 6 May 2021 09:19:05 -0700 Subject: [PATCH] config: expose distinct accept-amp flag This mirrors the accept-keysend flag, but also permits users to eventually toggle off keysend separately from AMP. --- config.go | 2 ++ invoices/invoiceregistry.go | 49 +++++++++++++++++++------------- invoices/invoiceregistry_test.go | 24 ++++++++-------- lntest/node.go | 5 ++++ sample-lnd.conf | 4 +++ server.go | 1 + 6 files changed, 53 insertions(+), 32 deletions(-) diff --git a/config.go b/config.go index 0a23839c..f7799792 100644 --- a/config.go +++ b/config.go @@ -338,6 +338,8 @@ type Config struct { AcceptKeySend bool `long:"accept-keysend" description:"If true, spontaneous payments through keysend will be accepted. [experimental]"` + AcceptAMP bool `long:"accept-amp" description:"If true, spontaneous payments via AMP will be accepted."` + KeysendHoldTime time.Duration `long:"keysend-hold-time" description:"If non-zero, keysend payments are accepted but not immediately settled. If the payment isn't settled manually after the specified time, it is canceled automatically. [experimental]"` GcCanceledInvoicesOnStartup bool `long:"gc-canceled-invoices-on-startup" description:"If true, we'll attempt to garbage collect canceled invoices upon start."` diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index 03b07862..da4c2136 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -57,6 +57,10 @@ type RegistryConfig struct { // send payments. AcceptKeySend bool + // AcceptAMP indicates whether we want to accept spontaneous AMP + // payments. + AcceptAMP bool + // GcCanceledInvoicesOnStartup if set, we'll attempt to garbage collect // all canceled invoices upon start. GcCanceledInvoicesOnStartup bool @@ -884,28 +888,33 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash, amp: payload.AMPRecord(), } - // Process keysend if present. Do this outside of the lock, because - // AddInvoice obtains its own lock. This is no problem, because the - // operation is idempotent. - if i.cfg.AcceptKeySend { - if ctx.amp != nil { - err := i.processAMP(ctx) - if err != nil { - ctx.log(fmt.Sprintf("amp error: %v", err)) + switch { - return NewFailResolution( - circuitKey, currentHeight, ResultAmpError, - ), nil - } - } else { - err := i.processKeySend(ctx) - if err != nil { - ctx.log(fmt.Sprintf("keysend error: %v", err)) + // If we are accepting spontaneous AMP payments and this payload + // contains an AMP record, create an AMP invoice that will be settled + // below. + case i.cfg.AcceptAMP && ctx.amp != nil: + err := i.processAMP(ctx) + if err != nil { + ctx.log(fmt.Sprintf("amp error: %v", err)) - return NewFailResolution( - circuitKey, currentHeight, ResultKeySendError, - ), nil - } + return NewFailResolution( + circuitKey, currentHeight, ResultAmpError, + ), nil + } + + // If we are accepting spontaneous keysend payments, create a regular + // invoice that will be settled below. We also enforce that this is only + // done when no AMP payload is present since it will only be settle-able + // by regular HTLCs. + case i.cfg.AcceptKeySend && ctx.amp == nil: + err := i.processKeySend(ctx) + if err != nil { + ctx.log(fmt.Sprintf("keysend error: %v", err)) + + return NewFailResolution( + circuitKey, currentHeight, ResultKeySendError, + ), nil } } diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index 799d5395..c049bb53 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -1290,7 +1290,7 @@ func TestAMPWithoutMPPPayload(t *testing.T) { ctx := newTestContext(t) defer ctx.cleanup() - ctx.registry.cfg.AcceptKeySend = true + ctx.registry.cfg.AcceptAMP = true const ( shardAmt = lnwire.MilliSatoshi(10) @@ -1320,37 +1320,37 @@ func TestAMPWithoutMPPPayload(t *testing.T) { func TestSpontaneousAmpPayment(t *testing.T) { tests := []struct { name string - keySendEnabled bool + ampEnabled bool failReconstruction bool numShards int }{ { name: "enabled valid one shard", - keySendEnabled: true, + ampEnabled: true, failReconstruction: false, numShards: 1, }, { name: "enabled valid multiple shards", - keySendEnabled: true, + ampEnabled: true, failReconstruction: false, numShards: 3, }, { name: "enabled invalid one shard", - keySendEnabled: true, + ampEnabled: true, failReconstruction: true, numShards: 1, }, { name: "enabled invalid multiple shards", - keySendEnabled: true, + ampEnabled: true, failReconstruction: true, numShards: 3, }, { name: "disabled valid multiple shards", - keySendEnabled: false, + ampEnabled: false, failReconstruction: false, numShards: 3, }, @@ -1360,7 +1360,7 @@ func TestSpontaneousAmpPayment(t *testing.T) { test := test t.Run(test.name, func(t *testing.T) { testSpontaneousAmpPayment( - t, test.keySendEnabled, test.failReconstruction, + t, test.ampEnabled, test.failReconstruction, test.numShards, ) }) @@ -1369,14 +1369,14 @@ func TestSpontaneousAmpPayment(t *testing.T) { // testSpontaneousAmpPayment runs a specific spontaneous AMP test case. func testSpontaneousAmpPayment( - t *testing.T, keySendEnabled, failReconstruction bool, numShards int) { + t *testing.T, ampEnabled, failReconstruction bool, numShards int) { defer timeout()() ctx := newTestContext(t) defer ctx.cleanup() - ctx.registry.cfg.AcceptKeySend = keySendEnabled + ctx.registry.cfg.AcceptAMP = ampEnabled allSubscriptions, err := ctx.registry.SubscribeNotifications(0, 0) require.Nil(t, err) @@ -1471,7 +1471,7 @@ func testSpontaneousAmpPayment( // When keysend is disabled all HTLC should fail with invoice // not found, since one is not inserted before executing // UpdateInvoice. - if !keySendEnabled { + if !ampEnabled { require.NotNil(t, resolution) checkFailResolution(t, resolution, ResultInvoiceNotFound) continue @@ -1515,7 +1515,7 @@ func testSpontaneousAmpPayment( } // No need to check the hodl chans when keysend is not enabled. - if !keySendEnabled { + if !ampEnabled { return } diff --git a/lntest/node.go b/lntest/node.go index e81ff696..428243f0 100644 --- a/lntest/node.go +++ b/lntest/node.go @@ -210,6 +210,7 @@ type NodeConfig struct { ProfilePort int AcceptKeySend bool + AcceptAMP bool FeeURL string @@ -297,6 +298,10 @@ func (cfg NodeConfig) genArgs() []string { args = append(args, "--accept-keysend") } + if cfg.AcceptAMP { + args = append(args, "--accept-amp") + } + if cfg.Etcd { args = append(args, "--db.backend=etcd") args = append(args, "--db.etcd.embedded") diff --git a/sample-lnd.conf b/sample-lnd.conf index 2d63c619..4596171a 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -375,6 +375,10 @@ ; automatically. [experimental] ; keysend-hold-time=true +; If true, spontaneous payments through AMP will be accepted. Payments to AMP +; invoices will be accepted regardless of this setting. +; accept-amp=true + ; If set, lnd will use anchor channels by default if the remote channel party ; supports them. Note that lnd will require 1 UTXO to be reserved for this ; channel type if it is enabled. diff --git a/server.go b/server.go index eddec220..86d76511 100644 --- a/server.go +++ b/server.go @@ -427,6 +427,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr, HtlcHoldDuration: invoices.DefaultHtlcHoldDuration, Clock: clock.NewDefaultClock(), AcceptKeySend: cfg.AcceptKeySend, + AcceptAMP: cfg.AcceptAMP, GcCanceledInvoicesOnStartup: cfg.GcCanceledInvoicesOnStartup, GcCanceledInvoicesOnTheFly: cfg.GcCanceledInvoicesOnTheFly, KeysendHoldTime: cfg.KeysendHoldTime,