Merge pull request #7341 from bottlepay/final-settle-opt-in

channeldb: final htlc resolution storage opt-in

channeldb/channel.go

@@ -1879,12 +1879,18 @@ func (c *OpenChannel) UpdateCommitment(newCommitment *ChannelCommitment,
 			return err
 		}
 
-		// Get the bucket where settled htlcs are recorded.
-		finalHtlcsBucket, err := fetchFinalHtlcsBucketRw(
-			tx, c.ShortChannelID,
-		)
-		if err != nil {
-			return err
+		// Get the bucket where settled htlcs are recorded if the user
+		// opted in to storing this information.
+		var finalHtlcsBucket kvdb.RwBucket
+		if c.Db.parent.storeFinalHtlcResolutions {
+			bucket, err := fetchFinalHtlcsBucketRw(
+				tx, c.ShortChannelID,
+			)
+			if err != nil {
+				return err
+			}
+
+			finalHtlcsBucket = bucket
 		}
 
 		var unsignedUpdates []LogUpdate
@@ -1957,15 +1963,18 @@ func processFinalHtlc(finalHtlcsBucket walletdb.ReadWriteBucket, upd LogUpdate,
 		return nil
 	}
 
-	err := putFinalHtlc(
-		finalHtlcsBucket, id,
-		FinalHtlcInfo{
-			Settled:  settled,
-			Offchain: true,
-		},
-	)
-	if err != nil {
-		return err
+	// Store the final resolution in the database if a bucket is provided.
+	if finalHtlcsBucket != nil {
+		err := putFinalHtlc(
+			finalHtlcsBucket, id,
+			FinalHtlcInfo{
+				Settled:  settled,
+				Offchain: true,
+			},
+		)
+		if err != nil {
+			return err
+		}
 	}
 
 	finalHtlcs[id] = settled

channeldb/channel_test.go

@@ -1463,7 +1463,7 @@ func TestKeyLocatorEncoding(t *testing.T) {
 func TestFinalHtlcs(t *testing.T) {
 	t.Parallel()
 
-	fullDB, err := MakeTestDB(t)
+	fullDB, err := MakeTestDB(t, OptionStoreFinalHtlcResolutions(true))
 	require.NoError(t, err, "unable to make test database")
 
 	cdb := fullDB.ChannelStateDB()

channeldb/db.go

@@ -307,6 +307,7 @@ type DB struct {
 	clock                     clock.Clock
 	dryRun                    bool
 	keepFailedPaymentAttempts bool
+	storeFinalHtlcResolutions bool
 }
 
 // Open opens or creates channeldb. Any necessary schemas migrations due
@@ -364,6 +365,7 @@ func CreateWithBackend(backend kvdb.Backend,
 		clock:                     opts.clock,
 		dryRun:                    opts.dryRun,
 		keepFailedPaymentAttempts: opts.keepFailedPaymentAttempts,
+		storeFinalHtlcResolutions: opts.storeFinalHtlcResolutions,
 	}
 
 	// Set the parent pointer (only used in tests).
@@ -1741,6 +1743,11 @@ func (c *ChannelStateDB) LookupFinalHtlc(chanID lnwire.ShortChannelID,
 func (c *ChannelStateDB) PutOnchainFinalHtlcOutcome(
 	chanID lnwire.ShortChannelID, htlcID uint64, settled bool) error {
 
+	// Skip if the user did not opt in to storing final resolutions.
+	if !c.parent.storeFinalHtlcResolutions {
+		return nil
+	}
+
 	return kvdb.Update(c.backend, func(tx kvdb.RwTx) error {
 		finalHtlcsBucket, err := fetchFinalHtlcsBucketRw(tx, chanID)
 		if err != nil {

channeldb/options.go

@@ -74,6 +74,10 @@ type Options struct {
 	// keepFailedPaymentAttempts determines whether failed htlc attempts
 	// are kept on disk or removed to save space.
 	keepFailedPaymentAttempts bool
+
+	// storeFinalHtlcResolutions determines whether to persistently store
+	// the final resolution of incoming htlcs.
+	storeFinalHtlcResolutions bool
 }
 
 // DefaultOptions returns an Options populated with default values.
@@ -187,6 +191,16 @@ func OptionKeepFailedPaymentAttempts(keepFailedPaymentAttempts bool) OptionModif
 	}
 }
 
+// OptionStoreFinalHtlcResolutions controls whether to persistently store the
+// final resolution of incoming htlcs.
+func OptionStoreFinalHtlcResolutions(
+	storeFinalHtlcResolutions bool) OptionModifier {
+
+	return func(o *Options) {
+		o.storeFinalHtlcResolutions = storeFinalHtlcResolutions
+	}
+}
+
 // OptionPruneRevocationLog specifies whether the migration for pruning
 // revocation logs needs to be applied or not.
 func OptionPruneRevocationLog(prune bool) OptionModifier {

config.go

@@ -373,6 +373,8 @@ type Config struct {
 
 	KeepFailedPaymentAttempts bool `long:"keep-failed-payment-attempts" description:"Keeps persistent record of all failed payment attempts for successfully settled payments."`
 
+	StoreFinalHtlcResolutions bool `long:"store-final-htlc-resolutions" description:"Persistently store the final resolution of incoming htlcs."`
+
 	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."`

config_builder.go

@@ -872,6 +872,9 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 		channeldb.OptionDryRunMigration(cfg.DryRunMigration),
 		channeldb.OptionSetUseGraphCache(!cfg.DB.NoGraphCache),
 		channeldb.OptionKeepFailedPaymentAttempts(cfg.KeepFailedPaymentAttempts),
+		channeldb.OptionStoreFinalHtlcResolutions(
+			cfg.StoreFinalHtlcResolutions,
+		),
 		channeldb.OptionPruneRevocationLog(cfg.DB.PruneRevocation),
 	}
 

docs/release-notes/release-notes-0.16.0.md

@@ -73,6 +73,9 @@ current gossip sync query status.
   Final resolution data will only be available for htlcs that are resolved
   after upgrading lnd.
 
+  This feature is [opt-in](https://github.com/lightningnetwork/lnd/pull/7341)
+  via a config flag.
+
 * Zero-amount private invoices [now provide hop
   hints](https://github.com/lightningnetwork/lnd/pull/7082), up to `maxHopHints`
   (20 currently).

lnwallet/channel_test.go

@@ -59,7 +59,9 @@ func assertOutputExistsByValue(t *testing.T, commitTx *wire.MsgTx,
 
 // testAddSettleWorkflow tests a simple channel scenario where Alice and Bob
 // add, the settle an HTLC between themselves.
-func testAddSettleWorkflow(t *testing.T, tweakless bool) {
+func testAddSettleWorkflow(t *testing.T, tweakless,
+	storeFinalHtlcResolutions bool) {
+
 	// Create a test channel which will be used for the duration of this
 	// unittest. The channel will be funded evenly with Alice having 5 BTC,
 	// and Bob having 5 BTC.
@@ -68,7 +70,12 @@ func testAddSettleWorkflow(t *testing.T, tweakless bool) {
 		chanType = channeldb.SingleFunderBit
 	}
 
-	aliceChannel, bobChannel, err := CreateTestChannels(t, chanType)
+	aliceChannel, bobChannel, err := CreateTestChannels(
+		t, chanType,
+		channeldb.OptionStoreFinalHtlcResolutions(
+			storeFinalHtlcResolutions,
+		),
+	)
 	require.NoError(t, err, "unable to create test channels")
 
 	paymentPreimage := bytes.Repeat([]byte{1}, 32)
@@ -241,13 +248,25 @@ func testAddSettleWorkflow(t *testing.T, tweakless bool) {
 
 	bobRevocation2, _, finalHtlcs, err := bobChannel.
 		RevokeCurrentCommitment()
-
 	require.NoError(t, err, "bob unable to revoke commitment")
 
 	// Check finalHtlcs for the expected final resolution.
 	require.Len(t, finalHtlcs, 1, "final htlc expected")
-	for _, settled := range finalHtlcs {
+	for htlcID, settled := range finalHtlcs {
 		require.True(t, settled, "final settle expected")
+
+		// Assert that final resolution was stored in Bob's database if
+		// storage is enabled.
+		finalInfo, err := bobChannel.channelState.Db.LookupFinalHtlc(
+			bobChannel.ShortChanID(), htlcID,
+		)
+		if storeFinalHtlcResolutions {
+			require.NoError(t, err)
+			require.True(t, finalInfo.Offchain)
+			require.True(t, finalInfo.Settled)
+		} else {
+			require.ErrorIs(t, err, channeldb.ErrHtlcUnknown)
+		}
 	}
 
 	fwdPkg, _, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation2)
@@ -332,9 +351,13 @@ func TestSimpleAddSettleWorkflow(t *testing.T) {
 	for _, tweakless := range []bool{true, false} {
 		tweakless := tweakless
 		t.Run(fmt.Sprintf("tweakless=%v", tweakless), func(t *testing.T) {
-			testAddSettleWorkflow(t, tweakless)
+			testAddSettleWorkflow(t, tweakless, false)
 		})
 	}
+
+	t.Run("storeFinalHtlcResolutions=true", func(t *testing.T) {
+		testAddSettleWorkflow(t, false, true)
+	})
 }
 
 // TestChannelZeroAddLocalHeight tests that we properly set the addCommitHeightLocal

lnwallet/test_utils.go

@@ -106,8 +106,9 @@ var (
 // allocated to each side. Within the channel, Alice is the initiator. If
 // tweaklessCommits is true, then the commits within the channels will use the
 // new format, otherwise the legacy format.
-func CreateTestChannels(t *testing.T, chanType channeldb.ChannelType) (
-	*LightningChannel, *LightningChannel, error) {
+func CreateTestChannels(t *testing.T, chanType channeldb.ChannelType,
+	dbModifiers ...channeldb.OptionModifier) (*LightningChannel,
+	*LightningChannel, error) {
 
 	channelCapacity, err := btcutil.NewAmount(testChannelCapacity)
 	if err != nil {
@@ -228,7 +229,7 @@ func CreateTestChannels(t *testing.T, chanType channeldb.ChannelType) (
 		return nil, nil, err
 	}
 
-	dbAlice, err := channeldb.Open(t.TempDir())
+	dbAlice, err := channeldb.Open(t.TempDir(), dbModifiers...)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -236,7 +237,7 @@ func CreateTestChannels(t *testing.T, chanType channeldb.ChannelType) (
 		require.NoError(t, dbAlice.Close())
 	})
 
-	dbBob, err := channeldb.Open(t.TempDir())
+	dbBob, err := channeldb.Open(t.TempDir(), dbModifiers...)
 	if err != nil {
 		return nil, nil, err
 	}

sample-lnd.conf

@@ -318,6 +318,9 @@
 ; settled payments.
 ; keep-failed-payment-attempts=false
 
+; Persistently store the final resolution of incoming htlcs.
+; store-final-htlc-resolutions=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.