From 8291e8a170010c795cbceff0e2b80c7f9740b40f Mon Sep 17 00:00:00 2001 From: Tommy Volk Date: Thu, 21 Apr 2022 07:33:01 +0000 Subject: [PATCH] multi: add `keep-failed-payment-attempts` flag --- channeldb/db.go | 14 ++++++++------ channeldb/options.go | 12 ++++++++++++ config.go | 35 ++++++++++++++++++++-------------- config_builder.go | 1 + routing/control_tower_test.go | 36 +++++++++++++++++++++++------------ routing/payment_lifecycle.go | 4 ++-- sample-lnd.conf | 4 ++++ 7 files changed, 72 insertions(+), 34 deletions(-) diff --git a/channeldb/db.go b/channeldb/db.go index 8a6935941..ac0ba9642 100644 --- a/channeldb/db.go +++ b/channeldb/db.go @@ -247,10 +247,11 @@ type DB struct { // channelStateDB separates all DB operations on channel state. channelStateDB *ChannelStateDB - dbPath string - graph *ChannelGraph - clock clock.Clock - dryRun bool + dbPath string + graph *ChannelGraph + clock clock.Clock + dryRun bool + keepFailedPaymentAttempts bool } // Open opens or creates channeldb. Any necessary schemas migrations due @@ -303,8 +304,9 @@ func CreateWithBackend(backend kvdb.Backend, modifiers ...OptionModifier) (*DB, }, backend: backend, }, - clock: opts.clock, - dryRun: opts.dryRun, + clock: opts.clock, + dryRun: opts.dryRun, + keepFailedPaymentAttempts: opts.keepFailedPaymentAttempts, } // Set the parent pointer (only used in tests). diff --git a/channeldb/options.go b/channeldb/options.go index 068a7fa01..7e121d254 100644 --- a/channeldb/options.go +++ b/channeldb/options.go @@ -61,6 +61,10 @@ type Options struct { // dryRun will fail to commit a successful migration when opening the // database if set to true. dryRun bool + + // keepFailedPaymentAttempts determines whether failed htlc attempts + // are kept on disk or removed to save space. + keepFailedPaymentAttempts bool } // DefaultOptions returns an Options populated with default values. @@ -164,3 +168,11 @@ func OptionDryRunMigration(dryRun bool) OptionModifier { o.dryRun = dryRun } } + +// OptionKeepFailedPaymentAttempts controls whether failed payment attempts are +// kept on disk after a payment settles. +func OptionKeepFailedPaymentAttempts(keepFailedPaymentAttempts bool) OptionModifier { + return func(o *Options) { + o.keepFailedPaymentAttempts = keepFailedPaymentAttempts + } +} diff --git a/config.go b/config.go index d313033a6..71044e898 100644 --- a/config.go +++ b/config.go @@ -199,6 +199,10 @@ const ( // defaultCoinSelectionStrategy is the coin selection strategy that is // used by default to fund transactions. defaultCoinSelectionStrategy = "largest" + + // defaultKeepFailedPaymentAttempts is the default setting for whether + // to keep failed payments in the database. + defaultKeepFailedPaymentAttempts = false ) var ( @@ -362,6 +366,8 @@ type Config struct { ChannelCommitBatchSize uint32 `long:"channel-commit-batch-size" description:"The maximum number of channel state updates that is accumulated before signing a new commitment."` + KeepFailedPaymentAttempts bool `long:"keep-failed-payment-attempts" description:"Keeps persistent record of all failed payment attempts for successfully settled payments."` + DefaultRemoteMaxHtlcs uint16 `long:"default-remote-max-htlcs" description:"The default max_htlc applied when opening or accepting channels. This value limits the number of concurrent HTLCs that the remote party can add to the commitment. The maximum possible value is 483."` NumGraphSyncPeers int `long:"numgraphsyncpeers" description:"The number of peers that we should receive new graph updates from. This option can be tuned to save bandwidth for light clients or routing nodes."` @@ -611,20 +617,21 @@ func DefaultConfig() Config { Invoices: &lncfg.Invoices{ HoldExpiryDelta: lncfg.DefaultHoldInvoiceExpiryDelta, }, - MaxOutgoingCltvExpiry: htlcswitch.DefaultMaxOutgoingCltvExpiry, - MaxChannelFeeAllocation: htlcswitch.DefaultMaxLinkFeeAllocation, - MaxCommitFeeRateAnchors: lnwallet.DefaultAnchorsCommitMaxFeeRateSatPerVByte, - DustThreshold: uint64(htlcswitch.DefaultDustThreshold.ToSatoshis()), - LogWriter: build.NewRotatingLogWriter(), - DB: lncfg.DefaultDB(), - Cluster: lncfg.DefaultCluster(), - RPCMiddleware: lncfg.DefaultRPCMiddleware(), - registeredChains: chainreg.NewChainRegistry(), - ActiveNetParams: chainreg.BitcoinTestNetParams, - ChannelCommitInterval: defaultChannelCommitInterval, - PendingCommitInterval: defaultPendingCommitInterval, - ChannelCommitBatchSize: defaultChannelCommitBatchSize, - CoinSelectionStrategy: defaultCoinSelectionStrategy, + MaxOutgoingCltvExpiry: htlcswitch.DefaultMaxOutgoingCltvExpiry, + MaxChannelFeeAllocation: htlcswitch.DefaultMaxLinkFeeAllocation, + MaxCommitFeeRateAnchors: lnwallet.DefaultAnchorsCommitMaxFeeRateSatPerVByte, + DustThreshold: uint64(htlcswitch.DefaultDustThreshold.ToSatoshis()), + LogWriter: build.NewRotatingLogWriter(), + DB: lncfg.DefaultDB(), + Cluster: lncfg.DefaultCluster(), + RPCMiddleware: lncfg.DefaultRPCMiddleware(), + registeredChains: chainreg.NewChainRegistry(), + ActiveNetParams: chainreg.BitcoinTestNetParams, + ChannelCommitInterval: defaultChannelCommitInterval, + PendingCommitInterval: defaultPendingCommitInterval, + ChannelCommitBatchSize: defaultChannelCommitBatchSize, + CoinSelectionStrategy: defaultCoinSelectionStrategy, + KeepFailedPaymentAttempts: defaultKeepFailedPaymentAttempts, RemoteSigner: &lncfg.RemoteSigner{ Timeout: lncfg.DefaultRemoteSignerRPCTimeout, }, diff --git a/config_builder.go b/config_builder.go index 18e9a3f6f..05f8ab168 100644 --- a/config_builder.go +++ b/config_builder.go @@ -852,6 +852,7 @@ func (d *DefaultDatabaseBuilder) BuildDatabase( channeldb.OptionSetBatchCommitInterval(cfg.DB.BatchCommitInterval), channeldb.OptionDryRunMigration(cfg.DryRunMigration), channeldb.OptionSetUseGraphCache(!cfg.DB.NoGraphCache), + channeldb.OptionKeepFailedPaymentAttempts(cfg.KeepFailedPaymentAttempts), } // We want to pre-allocate the channel graph cache according to what we diff --git a/routing/control_tower_test.go b/routing/control_tower_test.go index e24870c87..267e60505 100644 --- a/routing/control_tower_test.go +++ b/routing/control_tower_test.go @@ -48,7 +48,7 @@ var ( func TestControlTowerSubscribeUnknown(t *testing.T) { t.Parallel() - db, err := initDB() + db, err := initDB(false) require.NoError(t, err, "unable to init db") pControl := NewControlTower(channeldb.NewPaymentControl(db)) @@ -65,7 +65,7 @@ func TestControlTowerSubscribeUnknown(t *testing.T) { func TestControlTowerSubscribeSuccess(t *testing.T) { t.Parallel() - db, err := initDB() + db, err := initDB(false) require.NoError(t, err, "unable to init db") pControl := NewControlTower(channeldb.NewPaymentControl(db)) @@ -165,16 +165,24 @@ func TestControlTowerSubscribeSuccess(t *testing.T) { func TestPaymentControlSubscribeFail(t *testing.T) { t.Parallel() - t.Run("register attempt", func(t *testing.T) { - testPaymentControlSubscribeFail(t, true) + t.Run("register attempt, keep failed payments", func(t *testing.T) { + testPaymentControlSubscribeFail(t, true, true) }) - t.Run("no register attempt", func(t *testing.T) { - testPaymentControlSubscribeFail(t, false) + t.Run("register attempt, delete failed payments", func(t *testing.T) { + testPaymentControlSubscribeFail(t, true, false) + }) + t.Run("no register attempt, keep failed payments", func(t *testing.T) { + testPaymentControlSubscribeFail(t, false, true) + }) + t.Run("no register attempt, delete failed payments", func(t *testing.T) { + testPaymentControlSubscribeFail(t, false, false) }) } -func testPaymentControlSubscribeFail(t *testing.T, registerAttempt bool) { - db, err := initDB() +func testPaymentControlSubscribeFail(t *testing.T, registerAttempt, + keepFailedPaymentAttempts bool) { + + db, err := initDB(keepFailedPaymentAttempts) require.NoError(t, err, "unable to init db") pControl := NewControlTower(channeldb.NewPaymentControl(db)) @@ -228,8 +236,8 @@ func testPaymentControlSubscribeFail(t *testing.T, registerAttempt bool) { subscriber2, err := pControl.SubscribePayment(info.PaymentIdentifier) require.NoError(t, err, "expected subscribe to succeed, but got") - // We expect all subscribers to now report the final outcome followed by - // no other events. + // We expect both subscribers to now report the final outcome followed + // by no other events. subscribers := []*ControlTowerSubscriber{ subscriber1, subscriber2, } @@ -286,13 +294,17 @@ func testPaymentControlSubscribeFail(t *testing.T, registerAttempt bool) { } } -func initDB() (*channeldb.DB, error) { +func initDB(keepFailedPaymentAttempts bool) (*channeldb.DB, error) { tempPath, err := ioutil.TempDir("", "routingdb") if err != nil { return nil, err } - db, err := channeldb.Open(tempPath) + db, err := channeldb.Open( + tempPath, channeldb.OptionKeepFailedPaymentAttempts( + keepFailedPaymentAttempts, + ), + ) if err != nil { return nil, err } diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index f8b3cd558..b2022cf2f 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -55,7 +55,7 @@ func (ps paymentState) terminated() bool { } // needWaitForShards returns a bool to specify whether we need to wait for the -// outcome of the shanrdHandler. +// outcome of the shardHandler. func (ps paymentState) needWaitForShards() bool { // If we have in flight shards and the payment is in final stage, we // need to wait for the outcomes from the shards. Or if we have no more @@ -279,7 +279,7 @@ lifecycle: continue lifecycle } - // If this route will consume the last remeining amount to send + // If this route will consume the last remaining amount to send // to the receiver, this will be our last shard (for now). lastShard := rt.ReceiverAmt() == currentState.remainingAmt diff --git a/sample-lnd.conf b/sample-lnd.conf index 114ad5c19..5685f8275 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -311,6 +311,10 @@ ; a new commitment. ; channel-commit-batch-size=10 +; Keeps persistent record of all failed payment attempts for successfully +; settled payments. +; keep-failed-payment-attempts=false + ; The default max_htlc applied when opening or accepting channels. This value ; limits the number of concurrent HTLCs that the remote party can add to the ; commitment. The maximum possible value is 483.