Merge pull request #7379 from ellemouton/addFieldsToRevocationLog

multi: add local and remote amount fields to revocation log

channeldb/channel.go

@@ -2657,8 +2657,8 @@ func (c *OpenChannel) AdvanceCommitChainTail(fwdPkg *FwdPkg,
 		// With the commitment pointer swapped, we can now add the
 		// revoked (prior) state to the revocation log.
 		err = putRevocationLog(
-			logBucket, &c.RemoteCommitment,
+			logBucket, &c.RemoteCommitment, ourOutputIndex,
-			ourOutputIndex, theirOutputIndex,
+			theirOutputIndex, c.Db.parent.noRevLogAmtData,
 		)
 		if err != nil {
 			return err

channeldb/db.go

@@ -63,12 +63,24 @@ type mandatoryVersion struct {
 	migration migration
 }
 
+// MigrationConfig is an interface combines the config interfaces of all
+// optional migrations.
+type MigrationConfig interface {
+	migration30.MigrateRevLogConfig
+}
+
+// MigrationConfigImpl is a super set of all the various migration configs and
+// an implementation of MigrationConfig.
+type MigrationConfigImpl struct {
+	migration30.MigrateRevLogConfigImpl
+}
+
 // optionalMigration defines an optional migration function. When a migration
 // is optional, it usually involves a large scale of changes that might touch
 // millions of keys. Due to OOM concern, the update cannot be safely done
 // within one db transaction. Thus, for optional migrations, they must take the
 // db backend and construct transactions as needed.
-type optionalMigration func(db kvdb.Backend) error
+type optionalMigration func(db kvdb.Backend, cfg MigrationConfig) error
 
 // optionalVersion defines a db version that can be optionally applied. When
 // applying migrations, we must apply all the mandatory migrations first before
@@ -274,7 +286,11 @@ var (
 	optionalVersions = []optionalVersion{
 		{
 			name: "prune revocation log",
-			migration: migration30.MigrateRevocationLog,
+			migration: func(db kvdb.Backend,
+				cfg MigrationConfig) error {
+
+				return migration30.MigrateRevocationLog(db, cfg)
+			},
 		},
 	}
 
@@ -308,6 +324,10 @@ type DB struct {
 	dryRun                    bool
 	keepFailedPaymentAttempts bool
 	storeFinalHtlcResolutions bool
+
+	// noRevLogAmtData if true, means that commitment transaction amount
+	// data should not be stored in the revocation log.
+	noRevLogAmtData bool
 }
 
 // Open opens or creates channeldb. Any necessary schemas migrations due
@@ -366,6 +386,7 @@ func CreateWithBackend(backend kvdb.Backend,
 		dryRun:                    opts.dryRun,
 		keepFailedPaymentAttempts: opts.keepFailedPaymentAttempts,
 		storeFinalHtlcResolutions: opts.storeFinalHtlcResolutions,
+		noRevLogAmtData:           opts.NoRevLogAmtData,
 	}
 
 	// Set the parent pointer (only used in tests).
@@ -1545,8 +1566,14 @@ func (d *DB) applyOptionalVersions(cfg OptionalMiragtionConfig) error {
 	version := optionalVersions[0]
 	log.Infof("Performing database optional migration: %s", version.name)
 
+	migrationCfg := &MigrationConfigImpl{
+		migration30.MigrateRevLogConfigImpl{
+			NoAmountData: d.noRevLogAmtData,
+		},
+	}
+
 	// Migrate the data.
-	if err := version.migration(d); err != nil {
+	if err := version.migration(d, migrationCfg); err != nil {
 		log.Errorf("Unable to apply optional migration: %s, error: %v",
 			version.name, err)
 		return err

channeldb/meta_test.go

@@ -530,7 +530,9 @@ func TestApplyOptionalVersions(t *testing.T) {
 	// Overwrite the migration function so we can count how many times the
 	// migration has happened.
 	migrateCount := 0
-	optionalVersions[0].migration = func(_ kvdb.Backend) error {
+	optionalVersions[0].migration = func(_ kvdb.Backend,
+		_ MigrationConfig) error {
+
 		migrateCount++
 		return nil
 	}

channeldb/migration30/migration.go

@@ -23,10 +23,31 @@ import (
 // indexes.
 const recordsPerTx = 20_000
 
+// MigrateRevLogConfig is an interface that defines the config that should be
+// passed to the MigrateRevocationLog function.
+type MigrateRevLogConfig interface {
+	// GetNoAmountData returns true if the amount data of revoked commitment
+	// transactions should not be stored in the revocation log.
+	GetNoAmountData() bool
+}
+
+// MigrateRevLogConfigImpl implements the MigrationRevLogConfig interface.
+type MigrateRevLogConfigImpl struct {
+	// NoAmountData if set to true will result in the amount data of revoked
+	// commitment transactions not being stored in the revocation log.
+	NoAmountData bool
+}
+
+// GetNoAmountData returns true if the amount data of revoked commitment
+// transactions should not be stored in the revocation log.
+func (c *MigrateRevLogConfigImpl) GetNoAmountData() bool {
+	return c.NoAmountData
+}
+
 // MigrateRevocationLog migrates the old revocation logs into the newer format
 // and deletes them once finished, with the deletion only happens once ALL the
 // old logs have been migrates.
-func MigrateRevocationLog(db kvdb.Backend) error {
+func MigrateRevocationLog(db kvdb.Backend, cfg MigrateRevLogConfig) error {
 	log.Infof("Migrating revocation logs, might take a while...")
 
 	var (
@@ -64,7 +85,7 @@ func MigrateRevocationLog(db kvdb.Backend) error {
 
 		// Process the migration.
 		err = kvdb.Update(db, func(tx kvdb.RwTx) error {
-			finished, err = processMigration(tx)
+			finished, err = processMigration(tx, cfg)
 			if err != nil {
 				return err
 			}
@@ -114,7 +135,7 @@ func MigrateRevocationLog(db kvdb.Backend) error {
 // processMigration finds the next un-migrated revocation logs, reads a max
 // number of `recordsPerTx` records, converts them into the new revocation logs
 // and save them to disk.
-func processMigration(tx kvdb.RwTx) (bool, error) {
+func processMigration(tx kvdb.RwTx, cfg MigrateRevLogConfig) (bool, error) {
 	openChanBucket := tx.ReadWriteBucket(openChannelBucket)
 
 	// If no bucket is found, we can exit early.
@@ -134,7 +155,7 @@ func processMigration(tx kvdb.RwTx) (bool, error) {
 	}
 
 	// Read a list of old revocation logs.
-	entryMap, err := readOldRevocationLogs(openChanBucket, locator)
+	entryMap, err := readOldRevocationLogs(openChanBucket, locator, cfg)
 	if err != nil {
 		return false, fmt.Errorf("read old logs err: %v", err)
 	}
@@ -368,7 +389,7 @@ type result struct {
 // readOldRevocationLogs finds a list of old revocation logs and converts them
 // into the new revocation logs.
 func readOldRevocationLogs(openChanBucket kvdb.RwBucket,
-	locator *updateLocator) (logEntries, error) {
+	locator *updateLocator, cfg MigrateRevLogConfig) (logEntries, error) {
 
 	entries := make(logEntries)
 	results := make([]*result, 0)
@@ -415,7 +436,9 @@ func readOldRevocationLogs(openChanBucket kvdb.RwBucket,
 		// Convert the old logs into the new logs. We do this early in
 		// the read tx so the old large revocation log can be set to
 		// nil here so save us some memory space.
-		newLog, err := convertRevocationLog(&c, ourIndex, theirIndex)
+		newLog, err := convertRevocationLog(
+			&c, ourIndex, theirIndex, cfg.GetNoAmountData(),
+		)
 		if err != nil {
 			r.errChan <- err
 		}
@@ -519,7 +542,8 @@ func readOldRevocationLogs(openChanBucket kvdb.RwBucket,
 // convertRevocationLog uses the fields `CommitTx` and `Htlcs` from a
 // ChannelCommitment to construct a revocation log entry.
 func convertRevocationLog(commit *mig.ChannelCommitment,
-	ourOutputIndex, theirOutputIndex uint32) (*RevocationLog, error) {
+	ourOutputIndex, theirOutputIndex uint32,
+	noAmtData bool) (*RevocationLog, error) {
 
 	// Sanity check that the output indexes can be safely converted.
 	if ourOutputIndex > math.MaxUint16 {
@@ -536,6 +560,11 @@ func convertRevocationLog(commit *mig.ChannelCommitment,
 		HTLCEntries:      make([]*HTLCEntry, 0, len(commit.Htlcs)),
 	}
 
+	if !noAmtData {
+		rl.TheirBalance = &commit.RemoteBalance
+		rl.OurBalance = &commit.LocalBalance
+	}
+
 	for _, htlc := range commit.Htlcs {
 		// Skip dust HTLCs.
 		if htlc.OutputIndex < 0 {

channeldb/migration30/migration_test.go

@@ -75,6 +75,7 @@ func TestMigrateRevocationLog(t *testing.T) {
 	}
 
 	fmt.Printf("Running %d test cases...\n", len(testCases))
+	fmt.Printf("withAmtData is set to: %v\n", withAmtData)
 
 	for i, tc := range testCases {
 		tc := tc
@@ -103,11 +104,17 @@ func TestMigrateRevocationLog(t *testing.T) {
 				return nil
 			}
 
-			migtest.ApplyMigrationWithDb(
+			cfg := &MigrateRevLogConfigImpl{
+				NoAmountData: !withAmtData,
+			}
+
+			migtest.ApplyMigrationWithDB(
 				t,
 				beforeMigration,
 				afterMigration,
-				MigrateRevocationLog,
+				func(db kvdb.Backend) error {
+					return MigrateRevocationLog(db, cfg)
+				},
 				false,
 			)
 		})
@@ -505,6 +512,10 @@ func assertRevocationLog(t testing.TB, want, got RevocationLog) {
 		"wrong TheirOutputIndex")
 	require.Equal(t, want.CommitTxHash, got.CommitTxHash,
 		"wrong CommitTxHash")
+	require.Equal(t, want.TheirBalance, got.TheirBalance,
+		"wrong TheirBalance")
+	require.Equal(t, want.OurBalance, got.OurBalance,
+		"wrong OurBalance")
 	require.Equal(t, len(want.HTLCEntries), len(got.HTLCEntries),
 		"wrong HTLCEntries length")
 
@@ -559,8 +570,12 @@ func BenchmarkMigration(b *testing.B) {
 		return setupTestLogs(db, c, oldLogs, nil)
 	}
 
+	cfg := &MigrateRevLogConfigImpl{
+		NoAmountData: !withAmtData,
+	}
+
 	// Run the migration test.
-	migtest.ApplyMigrationWithDb(
+	migtest.ApplyMigrationWithDB(
 		b,
 		beforeMigration,
 		nil,
@@ -568,7 +583,7 @@ func BenchmarkMigration(b *testing.B) {
 			b.StartTimer()
 			defer b.StopTimer()
 
-			return MigrateRevocationLog(db)
+			return MigrateRevocationLog(db, cfg)
 		},
 		false,
 	)

channeldb/migration30/revocation_log.go

@@ -7,6 +7,7 @@ import (
 	"math"
 
 	"github.com/btcsuite/btcd/btcutil"
+	lnwire "github.com/lightningnetwork/lnd/channeldb/migration/lnwire21"
 	mig24 "github.com/lightningnetwork/lnd/channeldb/migration24"
 	mig25 "github.com/lightningnetwork/lnd/channeldb/migration25"
 	mig26 "github.com/lightningnetwork/lnd/channeldb/migration26"
@@ -16,8 +17,20 @@ import (
 	"github.com/lightningnetwork/lnd/tlv"
 )
 
-// OutputIndexEmpty is used when the output index doesn't exist.
+const (
-const OutputIndexEmpty = math.MaxUint16
+	// OutputIndexEmpty is used when the output index doesn't exist.
+	OutputIndexEmpty = math.MaxUint16
+
+	// A set of tlv type definitions used to serialize the body of
+	// revocation logs to the database.
+	//
+	// NOTE: A migration should be added whenever this list changes.
+	revLogOurOutputIndexType   tlv.Type = 0
+	revLogTheirOutputIndexType tlv.Type = 1
+	revLogCommitTxHashType     tlv.Type = 2
+	revLogOurBalanceType       tlv.Type = 3
+	revLogTheirBalanceType     tlv.Type = 4
+)
 
 var (
 	// revocationLogBucketDeprecated is dedicated for storing the necessary
@@ -187,9 +200,6 @@ func (h *HTLCEntry) toTlvStream() (*tlv.Stream, error) {
 // fields can be viewed as a subset of a ChannelCommitment's. In the database,
 // all historical versions of the RevocationLog are saved using the
 // CommitHeight as the key.
-//
-// NOTE: all the fields use the primitive go types so they can be made into tlv
-// records without further conversion.
 type RevocationLog struct {
 	// OurOutputIndex specifies our output index in this commitment. In a
 	// remote commitment transaction, this is the to remote output index.
@@ -206,29 +216,26 @@ type RevocationLog struct {
 	// HTLCEntries is the set of HTLCEntry's that are pending at this
 	// particular commitment height.
 	HTLCEntries []*HTLCEntry
-}
 
-// toTlvStream converts an RevocationLog record into a tlv representation.
+	// OurBalance is the current available balance within the channel
-func (rl *RevocationLog) toTlvStream() (*tlv.Stream, error) {
+	// directly spendable by us. In other words, it is the value of the
-	const (
+	// to_remote output on the remote parties' commitment transaction.
-		// A set of tlv type definitions used to serialize the body of
-		// revocation logs to the database. We define it here instead
-		// of the head of the file to avoid naming conflicts.
 	//
-		// NOTE: A migration should be added whenever this list
+	// NOTE: this is a pointer so that it is clear if the value is zero or
-		// changes.
+	// nil. Since migration 30 of the channeldb initially did not include
-		ourOutputIndexType   tlv.Type = 0
+	// this field, it could be the case that the field is not present for
-		theirOutputIndexType tlv.Type = 1
+	// all revocation logs.
-		commitTxHashType     tlv.Type = 2
+	OurBalance *lnwire.MilliSatoshi
-	)
 
-	return tlv.NewStream(
+	// TheirBalance is the current available balance within the channel
-		tlv.MakePrimitiveRecord(ourOutputIndexType, &rl.OurOutputIndex),
+	// directly spendable by the remote node. In other words, it is the
-		tlv.MakePrimitiveRecord(
+	// value of the to_local output on the remote parties' commitment.
-			theirOutputIndexType, &rl.TheirOutputIndex,
+	//
-		),
+	// NOTE: this is a pointer so that it is clear if the value is zero or
-		tlv.MakePrimitiveRecord(commitTxHashType, &rl.CommitTxHash),
+	// nil. Since migration 30 of the channeldb initially did not include
-	)
+	// this field, it could be the case that the field is not present for
+	// all revocation logs.
+	TheirBalance *lnwire.MilliSatoshi
 }
 
 // putRevocationLog uses the fields `CommitTx` and `Htlcs` from a
@@ -236,7 +243,7 @@ func (rl *RevocationLog) toTlvStream() (*tlv.Stream, error) {
 // disk. It also saves our output index and their output index, which are
 // useful when creating breach retribution.
 func putRevocationLog(bucket kvdb.RwBucket, commit *mig.ChannelCommitment,
-	ourOutputIndex, theirOutputIndex uint32) error {
+	ourOutputIndex, theirOutputIndex uint32, noAmtData bool) error {
 
 	// Sanity check that the output indexes can be safely converted.
 	if ourOutputIndex > math.MaxUint16 {
@@ -253,6 +260,11 @@ func putRevocationLog(bucket kvdb.RwBucket, commit *mig.ChannelCommitment,
 		HTLCEntries:      make([]*HTLCEntry, 0, len(commit.Htlcs)),
 	}
 
+	if !noAmtData {
+		rl.OurBalance = &commit.LocalBalance
+		rl.TheirBalance = &commit.RemoteBalance
+	}
+
 	for _, htlc := range commit.Htlcs {
 		// Skip dust HTLCs.
 		if htlc.OutputIndex < 0 {
@@ -304,8 +316,36 @@ func fetchRevocationLog(log kvdb.RBucket,
 // serializeRevocationLog serializes a RevocationLog record based on tlv
 // format.
 func serializeRevocationLog(w io.Writer, rl *RevocationLog) error {
+	// Add the tlv records for all non-optional fields.
+	records := []tlv.Record{
+		tlv.MakePrimitiveRecord(
+			revLogOurOutputIndexType, &rl.OurOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogTheirOutputIndexType, &rl.TheirOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogCommitTxHashType, &rl.CommitTxHash,
+		),
+	}
+
+	// Now we add any optional fields that are non-nil.
+	if rl.OurBalance != nil {
+		lb := uint64(*rl.OurBalance)
+		records = append(records, tlv.MakeBigSizeRecord(
+			revLogOurBalanceType, &lb,
+		))
+	}
+
+	if rl.TheirBalance != nil {
+		rb := uint64(*rl.TheirBalance)
+		records = append(records, tlv.MakeBigSizeRecord(
+			revLogTheirBalanceType, &rb,
+		))
+	}
+
 	// Create the tlv stream.
-	tlvStream, err := rl.toTlvStream()
+	tlvStream, err := tlv.NewStream(records...)
 	if err != nil {
 		return err
 	}
@@ -348,19 +388,48 @@ func serializeHTLCEntries(w io.Writer, htlcs []*HTLCEntry) error {
 
 // deserializeRevocationLog deserializes a RevocationLog based on tlv format.
 func deserializeRevocationLog(r io.Reader) (RevocationLog, error) {
-	var rl RevocationLog
+	var (
+		rl            RevocationLog
+		localBalance  uint64
+		remoteBalance uint64
+	)
 
 	// Create the tlv stream.
-	tlvStream, err := rl.toTlvStream()
+	tlvStream, err := tlv.NewStream(
+		tlv.MakePrimitiveRecord(
+			revLogOurOutputIndexType, &rl.OurOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogTheirOutputIndexType, &rl.TheirOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogCommitTxHashType, &rl.CommitTxHash,
+		),
+		tlv.MakeBigSizeRecord(revLogOurBalanceType, &localBalance),
+		tlv.MakeBigSizeRecord(
+			revLogTheirBalanceType, &remoteBalance,
+		),
+	)
 	if err != nil {
 		return rl, err
 	}
 
 	// Read the tlv stream.
-	if err := readTlvStream(r, tlvStream); err != nil {
+	parsedTypes, err := readTlvStream(r, tlvStream)
+	if err != nil {
 		return rl, err
 	}
 
+	if t, ok := parsedTypes[revLogOurBalanceType]; ok && t == nil {
+		lb := lnwire.MilliSatoshi(localBalance)
+		rl.OurBalance = &lb
+	}
+
+	if t, ok := parsedTypes[revLogTheirBalanceType]; ok && t == nil {
+		rb := lnwire.MilliSatoshi(remoteBalance)
+		rl.TheirBalance = &rb
+	}
+
 	// Read the HTLC entries.
 	rl.HTLCEntries, err = deserializeHTLCEntries(r)
 
@@ -382,7 +451,7 @@ func deserializeHTLCEntries(r io.Reader) ([]*HTLCEntry, error) {
 		}
 
 		// Read the HTLC entry.
-		if err := readTlvStream(r, tlvStream); err != nil {
+		if _, err := readTlvStream(r, tlvStream); err != nil {
 			// We've reached the end when hitting an EOF.
 			if err == io.ErrUnexpectedEOF {
 				break
@@ -427,7 +496,7 @@ func writeTlvStream(w io.Writer, s *tlv.Stream) error {
 
 // readTlvStream is a helper function that decodes the tlv stream from the
 // reader.
-func readTlvStream(r io.Reader, s *tlv.Stream) error {
+func readTlvStream(r io.Reader, s *tlv.Stream) (tlv.TypeMap, error) {
 	var bodyLen uint64
 
 	// Read the stream's length.
@@ -436,16 +505,17 @@ func readTlvStream(r io.Reader, s *tlv.Stream) error {
 	// We'll convert any EOFs to ErrUnexpectedEOF, since this results in an
 	// invalid record.
 	case err == io.EOF:
-		return io.ErrUnexpectedEOF
+		return nil, io.ErrUnexpectedEOF
 
 	// Other unexpected errors.
 	case err != nil:
-		return err
+		return nil, err
 	}
 
 	// TODO(yy): add overflow check.
 	lr := io.LimitReader(r, int64(bodyLen))
-	return s.Decode(lr)
+
+	return s.DecodeWithParsedTypes(lr)
 }
 
 // fetchLogBucket returns a read bucket by visiting both the new and the old

channeldb/migration30/test_utils.go

@@ -3,6 +3,8 @@ package migration30
 import (
 	"bytes"
 	"fmt"
+	"math/rand"
+	"time"
 
 	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/btcsuite/btcd/btcutil"
@@ -78,6 +80,12 @@ var (
 	// logHeight1 is the CommitHeight used by oldLog1.
 	logHeight1 = uint64(0)
 
+	// localBalance1 is the LocalBalance used in oldLog1.
+	localBalance1 = lnwire.MilliSatoshi(990_950_000)
+
+	// remoteBalance1 is the RemoteBalance used in oldLog1.
+	remoteBalance1 = lnwire.MilliSatoshi(0)
+
 	// oldLog1 defines an old revocation that has no HTLCs.
 	oldLog1 = mig.ChannelCommitment{
 		CommitHeight:    logHeight1,
@@ -85,18 +93,29 @@ var (
 		LocalHtlcIndex:  0,
 		RemoteLogIndex:  0,
 		RemoteHtlcIndex: 0,
-		LocalBalance:    lnwire.MilliSatoshi(990_950_000),
+		LocalBalance:    localBalance1,
-		RemoteBalance:   0,
+		RemoteBalance:   remoteBalance1,
 		CommitTx:        commitTx1,
 	}
 
-	// newLog1 is the new version of oldLog1.
+	// newLog1 is the new version of oldLog1 in the case were we don't want
+	// to store any balance data.
 	newLog1 = RevocationLog{
 		OurOutputIndex:   0,
 		TheirOutputIndex: OutputIndexEmpty,
 		CommitTxHash:     commitTx1.TxHash(),
 	}
 
+	// newLog1WithAmts is the new version of oldLog1 in the case were we do
+	// want to store balance data.
+	newLog1WithAmts = RevocationLog{
+		OurOutputIndex:   0,
+		TheirOutputIndex: OutputIndexEmpty,
+		CommitTxHash:     commitTx1.TxHash(),
+		OurBalance:       &localBalance1,
+		TheirBalance:     &remoteBalance1,
+	}
+
 	// preimage2 defines the second revocation preimage used in the test,
 	// generated from itest.
 	preimage2 = []byte{
@@ -146,6 +165,12 @@ var (
 	// logHeight2 is the CommitHeight used by oldLog2.
 	logHeight2 = uint64(1)
 
+	// localBalance2 is the LocalBalance used in oldLog2.
+	localBalance2 = lnwire.MilliSatoshi(888_800_000)
+
+	// remoteBalance2 is the RemoteBalance used in oldLog2.
+	remoteBalance2 = lnwire.MilliSatoshi(0)
+
 	// oldLog2 defines an old revocation that has one HTLC.
 	oldLog2 = mig.ChannelCommitment{
 		CommitHeight:    logHeight2,
@@ -153,13 +178,14 @@ var (
 		LocalHtlcIndex:  1,
 		RemoteLogIndex:  0,
 		RemoteHtlcIndex: 0,
-		LocalBalance:    lnwire.MilliSatoshi(888_800_000),
+		LocalBalance:    localBalance2,
-		RemoteBalance:   0,
+		RemoteBalance:   remoteBalance2,
 		CommitTx:        commitTx2,
 		Htlcs:           []mig.HTLC{htlc},
 	}
 
-	// newLog2 is the new version of the oldLog2.
+	// newLog2 is the new version of the oldLog2 in the case were we don't
+	// want to store any balance data.
 	newLog2 = RevocationLog{
 		OurOutputIndex:   1,
 		TheirOutputIndex: OutputIndexEmpty,
@@ -175,6 +201,25 @@ var (
 		},
 	}
 
+	// newLog2WithAmts is the new version of oldLog2 in the case were we do
+	// want to store balance data.
+	newLog2WithAmts = RevocationLog{
+		OurOutputIndex:   1,
+		TheirOutputIndex: OutputIndexEmpty,
+		CommitTxHash:     commitTx2.TxHash(),
+		OurBalance:       &localBalance2,
+		TheirBalance:     &remoteBalance2,
+		HTLCEntries: []*HTLCEntry{
+			{
+				RHash:         rHash,
+				RefundTimeout: 489,
+				OutputIndex:   0,
+				Incoming:      false,
+				Amt:           btcutil.Amount(100_000),
+			},
+		},
+	}
+
 	// newLog3 defines an revocation log that's been created after v0.15.0.
 	newLog3 = mig.ChannelCommitment{
 		CommitHeight:    logHeight2 + 1,
@@ -260,6 +305,12 @@ var (
 		0xf8, 0xc3, 0xfc, 0x7, 0x2d, 0x15, 0x99, 0x55,
 		0x8, 0x69, 0xf6, 0x1, 0xa2, 0xcd, 0x6b, 0xa7,
 	})
+
+	// withAmtData if set, will result in the amount data of the revoked
+	// commitment transactions also being stored in the new revocation log.
+	// The value of this variable is set randomly in the init function of
+	// this package.
+	withAmtData bool
 )
 
 // setupTestLogs takes care of creating the related buckets and inserts testing
@@ -298,7 +349,7 @@ func setupTestLogs(db kvdb.Backend, c *mig26.OpenChannel,
 		}
 
 		// Create new logs.
-		return writeNewRevocationLogs(chanBucket, newLogs)
+		return writeNewRevocationLogs(chanBucket, newLogs, !withAmtData)
 	}, func() {})
 }
 
@@ -452,7 +503,7 @@ func writeOldRevocationLogs(chanBucket kvdb.RwBucket,
 
 // writeNewRevocationLogs saves a new revocation log to db.
 func writeNewRevocationLogs(chanBucket kvdb.RwBucket,
-	oldLogs []mig.ChannelCommitment) error {
+	oldLogs []mig.ChannelCommitment, noAmtData bool) error {
 
 	// Don't bother continue if the logs are empty.
 	if len(oldLogs) == 0 {
@@ -472,7 +523,7 @@ func writeNewRevocationLogs(chanBucket kvdb.RwBucket,
 		// old commit tx. We do this intentionally so we can
 		// distinguish a newly created log from an already saved one.
 		err := putRevocationLog(
-			logBucket, &c, testOurIndex, testTheirIndex,
+			logBucket, &c, testOurIndex, testTheirIndex, noAmtData,
 		)
 		if err != nil {
 			return err
@@ -551,3 +602,15 @@ func createFinished(cdb kvdb.Backend, c *mig26.OpenChannel,
 	}
 	return setupTestLogs(cdb, c, oldLogs, newLogs)
 }
+
+func init() {
+	rand.Seed(time.Now().Unix())
+	if rand.Intn(2) == 0 {
+		withAmtData = true
+	}
+
+	if withAmtData {
+		newLog1 = newLog1WithAmts
+		newLog2 = newLog2WithAmts
+	}
+}

channeldb/migtest/migtest.go

@@ -84,12 +84,13 @@ func ApplyMigration(t *testing.T,
 	}
 }
 
-// ApplyMigrationWithDb is a helper test function that encapsulates the general
+// ApplyMigrationWithDB is a helper test function that encapsulates the general
 // steps which are needed to properly check the result of applying migration
 // function. This function differs from ApplyMigration as it requires the
 // supplied migration functions to take a db instance and construct their own
 // database transactions.
-func ApplyMigrationWithDb(t testing.TB, beforeMigration, afterMigration,
+func ApplyMigrationWithDB(t testing.TB, beforeMigration,
+	afterMigration func(db kvdb.Backend) error,
 	migrationFunc func(db kvdb.Backend) error, shouldFail bool) {
 
 	t.Helper()

channeldb/options.go

@@ -64,6 +64,10 @@ type Options struct {
 	// applications that use the channeldb package as a library.
 	NoMigration bool
 
+	// NoRevLogAmtData when set to true, indicates that amount data should
+	// not be stored in the revocation log.
+	NoRevLogAmtData bool
+
 	// clock is the time source used by the database.
 	clock clock.Clock
 
@@ -130,6 +134,14 @@ func OptionSetUseGraphCache(use bool) OptionModifier {
 	}
 }
 
+// OptionNoRevLogAmtData sets the NoRevLogAmtData option to the given value. If
+// it is set to true then amount data will not be stored in the revocation log.
+func OptionNoRevLogAmtData(noAmtData bool) OptionModifier {
+	return func(o *Options) {
+		o.NoRevLogAmtData = noAmtData
+	}
+}
+
 // OptionSetSyncFreelist allows the database to sync its freelist.
 func OptionSetSyncFreelist(b bool) OptionModifier {
 	return func(o *Options) {

channeldb/revocation_log.go

@@ -9,11 +9,24 @@ import (
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/lightningnetwork/lnd/kvdb"
 	"github.com/lightningnetwork/lnd/lntypes"
+	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/tlv"
 )
 
-// OutputIndexEmpty is used when the output index doesn't exist.
+const (
-const OutputIndexEmpty = math.MaxUint16
+	// OutputIndexEmpty is used when the output index doesn't exist.
+	OutputIndexEmpty = math.MaxUint16
+
+	// A set of tlv type definitions used to serialize the body of
+	// revocation logs to the database.
+	//
+	// NOTE: A migration should be added whenever this list changes.
+	revLogOurOutputIndexType   tlv.Type = 0
+	revLogTheirOutputIndexType tlv.Type = 1
+	revLogCommitTxHashType     tlv.Type = 2
+	revLogOurBalanceType       tlv.Type = 3
+	revLogTheirBalanceType     tlv.Type = 4
+)
 
 var (
 	// revocationLogBucketDeprecated is dedicated for storing the necessary
@@ -175,9 +188,6 @@ func (h *HTLCEntry) toTlvStream() (*tlv.Stream, error) {
 // fields can be viewed as a subset of a ChannelCommitment's. In the database,
 // all historical versions of the RevocationLog are saved using the
 // CommitHeight as the key.
-//
-// NOTE: all the fields use the primitive go types so they can be made into tlv
-// records without further conversion.
 type RevocationLog struct {
 	// OurOutputIndex specifies our output index in this commitment. In a
 	// remote commitment transaction, this is the to remote output index.
@@ -194,29 +204,26 @@ type RevocationLog struct {
 	// HTLCEntries is the set of HTLCEntry's that are pending at this
 	// particular commitment height.
 	HTLCEntries []*HTLCEntry
-}
 
-// toTlvStream converts an RevocationLog record into a tlv representation.
+	// OurBalance is the current available balance within the channel
-func (rl *RevocationLog) toTlvStream() (*tlv.Stream, error) {
+	// directly spendable by us. In other words, it is the value of the
-	const (
+	// to_remote output on the remote parties' commitment transaction.
-		// A set of tlv type definitions used to serialize the body of
-		// revocation logs to the database. We define it here instead
-		// of the head of the file to avoid naming conflicts.
 	//
-		// NOTE: A migration should be added whenever this list
+	// NOTE: this is a pointer so that it is clear if the value is zero or
-		// changes.
+	// nil. Since migration 30 of the channeldb initially did not include
-		ourOutputIndexType   tlv.Type = 0
+	// this field, it could be the case that the field is not present for
-		theirOutputIndexType tlv.Type = 1
+	// all revocation logs.
-		commitTxHashType     tlv.Type = 2
+	OurBalance *lnwire.MilliSatoshi
-	)
 
-	return tlv.NewStream(
+	// TheirBalance is the current available balance within the channel
-		tlv.MakePrimitiveRecord(ourOutputIndexType, &rl.OurOutputIndex),
+	// directly spendable by the remote node. In other words, it is the
-		tlv.MakePrimitiveRecord(
+	// value of the to_local output on the remote parties' commitment.
-			theirOutputIndexType, &rl.TheirOutputIndex,
+	//
-		),
+	// NOTE: this is a pointer so that it is clear if the value is zero or
-		tlv.MakePrimitiveRecord(commitTxHashType, &rl.CommitTxHash),
+	// nil. Since migration 30 of the channeldb initially did not include
-	)
+	// this field, it could be the case that the field is not present for
+	// all revocation logs.
+	TheirBalance *lnwire.MilliSatoshi
 }
 
 // putRevocationLog uses the fields `CommitTx` and `Htlcs` from a
@@ -224,7 +231,7 @@ func (rl *RevocationLog) toTlvStream() (*tlv.Stream, error) {
 // disk. It also saves our output index and their output index, which are
 // useful when creating breach retribution.
 func putRevocationLog(bucket kvdb.RwBucket, commit *ChannelCommitment,
-	ourOutputIndex, theirOutputIndex uint32) error {
+	ourOutputIndex, theirOutputIndex uint32, noAmtData bool) error {
 
 	// Sanity check that the output indexes can be safely converted.
 	if ourOutputIndex > math.MaxUint16 {
@@ -241,6 +248,11 @@ func putRevocationLog(bucket kvdb.RwBucket, commit *ChannelCommitment,
 		HTLCEntries:      make([]*HTLCEntry, 0, len(commit.Htlcs)),
 	}
 
+	if !noAmtData {
+		rl.OurBalance = &commit.LocalBalance
+		rl.TheirBalance = &commit.RemoteBalance
+	}
+
 	for _, htlc := range commit.Htlcs {
 		// Skip dust HTLCs.
 		if htlc.OutputIndex < 0 {
@@ -292,8 +304,36 @@ func fetchRevocationLog(log kvdb.RBucket,
 // serializeRevocationLog serializes a RevocationLog record based on tlv
 // format.
 func serializeRevocationLog(w io.Writer, rl *RevocationLog) error {
+	// Add the tlv records for all non-optional fields.
+	records := []tlv.Record{
+		tlv.MakePrimitiveRecord(
+			revLogOurOutputIndexType, &rl.OurOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogTheirOutputIndexType, &rl.TheirOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogCommitTxHashType, &rl.CommitTxHash,
+		),
+	}
+
+	// Now we add any optional fields that are non-nil.
+	if rl.OurBalance != nil {
+		lb := uint64(*rl.OurBalance)
+		records = append(records, tlv.MakeBigSizeRecord(
+			revLogOurBalanceType, &lb,
+		))
+	}
+
+	if rl.TheirBalance != nil {
+		rb := uint64(*rl.TheirBalance)
+		records = append(records, tlv.MakeBigSizeRecord(
+			revLogTheirBalanceType, &rb,
+		))
+	}
+
 	// Create the tlv stream.
-	tlvStream, err := rl.toTlvStream()
+	tlvStream, err := tlv.NewStream(records...)
 	if err != nil {
 		return err
 	}
@@ -336,19 +376,48 @@ func serializeHTLCEntries(w io.Writer, htlcs []*HTLCEntry) error {
 
 // deserializeRevocationLog deserializes a RevocationLog based on tlv format.
 func deserializeRevocationLog(r io.Reader) (RevocationLog, error) {
-	var rl RevocationLog
+	var (
+		rl           RevocationLog
+		ourBalance   uint64
+		theirBalance uint64
+	)
 
 	// Create the tlv stream.
-	tlvStream, err := rl.toTlvStream()
+	tlvStream, err := tlv.NewStream(
+		tlv.MakePrimitiveRecord(
+			revLogOurOutputIndexType, &rl.OurOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogTheirOutputIndexType, &rl.TheirOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogCommitTxHashType, &rl.CommitTxHash,
+		),
+		tlv.MakeBigSizeRecord(revLogOurBalanceType, &ourBalance),
+		tlv.MakeBigSizeRecord(
+			revLogTheirBalanceType, &theirBalance,
+		),
+	)
 	if err != nil {
 		return rl, err
 	}
 
 	// Read the tlv stream.
-	if err := readTlvStream(r, tlvStream); err != nil {
+	parsedTypes, err := readTlvStream(r, tlvStream)
+	if err != nil {
 		return rl, err
 	}
 
+	if t, ok := parsedTypes[revLogOurBalanceType]; ok && t == nil {
+		lb := lnwire.MilliSatoshi(ourBalance)
+		rl.OurBalance = &lb
+	}
+
+	if t, ok := parsedTypes[revLogTheirBalanceType]; ok && t == nil {
+		rb := lnwire.MilliSatoshi(theirBalance)
+		rl.TheirBalance = &rb
+	}
+
 	// Read the HTLC entries.
 	rl.HTLCEntries, err = deserializeHTLCEntries(r)
 
@@ -370,7 +439,7 @@ func deserializeHTLCEntries(r io.Reader) ([]*HTLCEntry, error) {
 		}
 
 		// Read the HTLC entry.
-		if err := readTlvStream(r, tlvStream); err != nil {
+		if _, err := readTlvStream(r, tlvStream); err != nil {
 			// We've reached the end when hitting an EOF.
 			if err == io.ErrUnexpectedEOF {
 				break
@@ -415,7 +484,7 @@ func writeTlvStream(w io.Writer, s *tlv.Stream) error {
 
 // readTlvStream is a helper function that decodes the tlv stream from the
 // reader.
-func readTlvStream(r io.Reader, s *tlv.Stream) error {
+func readTlvStream(r io.Reader, s *tlv.Stream) (tlv.TypeMap, error) {
 	var bodyLen uint64
 
 	// Read the stream's length.
@@ -424,16 +493,17 @@ func readTlvStream(r io.Reader, s *tlv.Stream) error {
 	// We'll convert any EOFs to ErrUnexpectedEOF, since this results in an
 	// invalid record.
 	case err == io.EOF:
-		return io.ErrUnexpectedEOF
+		return nil, io.ErrUnexpectedEOF
 
 	// Other unexpected errors.
 	case err != nil:
-		return err
+		return nil, err
 	}
 
 	// TODO(yy): add overflow check.
 	lr := io.LimitReader(r, int64(bodyLen))
-	return s.Decode(lr)
+
+	return s.DecodeWithParsedTypes(lr)
 }
 
 // fetchOldRevocationLog finds the revocation log from the deprecated

channeldb/revocation_log_test.go

@@ -56,10 +56,13 @@ var (
 		0x4, 0x5, 0xfe, 0x0, 0xf, 0x42, 0x40,
 	}
 
+	localBalance  = lnwire.MilliSatoshi(9000)
+	remoteBalance = lnwire.MilliSatoshi(3000)
+
 	testChannelCommit = ChannelCommitment{
 		CommitHeight:  999,
-		LocalBalance:  lnwire.MilliSatoshi(9000),
+		LocalBalance:  localBalance,
-		RemoteBalance: lnwire.MilliSatoshi(3000),
+		RemoteBalance: remoteBalance,
 		CommitFee:     btcutil.Amount(rand.Int63()),
 		FeePerKw:      btcutil.Amount(5000),
 		CommitTx:      channels.TestFundingTx,
@@ -74,13 +77,13 @@ var (
 		}},
 	}
 
-	testRevocationLog = RevocationLog{
+	testRevocationLogNoAmts = RevocationLog{
 		OurOutputIndex:   0,
 		TheirOutputIndex: 1,
 		CommitTxHash:     testChannelCommit.CommitTx.TxHash(),
 		HTLCEntries:      []*HTLCEntry{&testHTLCEntry},
 	}
-	testRevocationLogBytes = []byte{
+	testRevocationLogNoAmtsBytes = []byte{
 		// Body length 42.
 		0x2a,
 		// OurOutputIndex tlv.
@@ -94,6 +97,33 @@ var (
 		0xbd, 0xf7, 0xd3, 0x9b, 0x16, 0x7d, 0x0, 0xff,
 		0xc8, 0x22, 0x51, 0xb1, 0x5b, 0xa0, 0xbf, 0xd,
 	}
+
+	testRevocationLogWithAmts = RevocationLog{
+		OurOutputIndex:   0,
+		TheirOutputIndex: 1,
+		CommitTxHash:     testChannelCommit.CommitTx.TxHash(),
+		HTLCEntries:      []*HTLCEntry{&testHTLCEntry},
+		OurBalance:       &localBalance,
+		TheirBalance:     &remoteBalance,
+	}
+	testRevocationLogWithAmtsBytes = []byte{
+		// Body length 52.
+		0x34,
+		// OurOutputIndex tlv.
+		0x0, 0x2, 0x0, 0x0,
+		// TheirOutputIndex tlv.
+		0x1, 0x2, 0x0, 0x1,
+		// CommitTxHash tlv.
+		0x2, 0x20,
+		0x28, 0x76, 0x2, 0x59, 0x1d, 0x9d, 0x64, 0x86,
+		0x6e, 0x60, 0x29, 0x23, 0x1d, 0x5e, 0xc5, 0xe6,
+		0xbd, 0xf7, 0xd3, 0x9b, 0x16, 0x7d, 0x0, 0xff,
+		0xc8, 0x22, 0x51, 0xb1, 0x5b, 0xa0, 0xbf, 0xd,
+		// OurBalance.
+		0x3, 0x3, 0xfd, 0x23, 0x28,
+		// Remote Balance.
+		0x4, 0x3, 0xfd, 0x0b, 0xb8,
+	}
 )
 
 func TestWriteTLVStream(t *testing.T) {
@@ -127,7 +157,7 @@ func TestReadTLVStream(t *testing.T) {
 
 	// Read the tlv stream.
 	buf := bytes.NewBuffer(testValueBytes)
-	err = readTlvStream(buf, ts)
+	_, err = readTlvStream(buf, ts)
 	require.NoError(t, err)
 
 	// Check the bytes are read as expected.
@@ -150,7 +180,7 @@ func TestReadTLVStreamErr(t *testing.T) {
 
 	// Read the tlv stream.
 	buf := bytes.NewBuffer(b)
-	err = readTlvStream(buf, ts)
+	_, err = readTlvStream(buf, ts)
 	require.ErrorIs(t, err, io.ErrUnexpectedEOF)
 
 	// Check the bytes are not read.
@@ -208,22 +238,75 @@ func TestSerializeHTLCEntries(t *testing.T) {
 	require.Equal(t, expectedBytes, buf.Bytes())
 }
 
-func TestSerializeRevocationLog(t *testing.T) {
+// TestSerializeAndDeserializeRevLog tests the serialization and deserialization
+// of various forms of the revocation log.
+func TestSerializeAndDeserializeRevLog(t *testing.T) {
 	t.Parallel()
 
-	// Copy the testRevocationLog and testHTLCEntry.
+	tests := []struct {
-	rl := testRevocationLog
+		name        string
+		revLog      RevocationLog
+		revLogBytes []byte
+	}{
+		{
+			name:        "with no amount fields",
+			revLog:      testRevocationLogNoAmts,
+			revLogBytes: testRevocationLogNoAmtsBytes,
+		},
+		{
+			name:        "with amount fields",
+			revLog:      testRevocationLogWithAmts,
+			revLogBytes: testRevocationLogWithAmtsBytes,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			testSerializeRevocationLog(
+				t, &test.revLog, test.revLogBytes,
+			)
+
+			testDerializeRevocationLog(
+				t, &test.revLog, test.revLogBytes,
+			)
+		})
+	}
+}
+
+func testSerializeRevocationLog(t *testing.T, rl *RevocationLog,
+	revLogBytes []byte) {
+
+	// Copy the testRevocationLogWithAmts and testHTLCEntry.
 	htlc := testHTLCEntry
 	rl.HTLCEntries = []*HTLCEntry{&htlc}
 
 	// Write the tlv stream.
 	buf := bytes.NewBuffer([]byte{})
-	err := serializeRevocationLog(buf, &rl)
+	err := serializeRevocationLog(buf, rl)
 	require.NoError(t, err)
 
 	// Check the expected bytes on the body of the revocation log.
 	bodyIndex := buf.Len() - len(testHTLCEntryBytes)
-	require.Equal(t, testRevocationLogBytes, buf.Bytes()[:bodyIndex])
+	require.Equal(t, revLogBytes, buf.Bytes()[:bodyIndex])
+}
+
+func testDerializeRevocationLog(t *testing.T, revLog *RevocationLog,
+	revLogBytes []byte) {
+
+	// Construct the full bytes.
+	revLogBytes = append(revLogBytes, testHTLCEntryBytes...)
+
+	// Read the tlv stream.
+	buf := bytes.NewBuffer(revLogBytes)
+	rl, err := deserializeRevocationLog(buf)
+	require.NoError(t, err)
+
+	// Check the bytes are read as expected.
+	require.Len(t, rl.HTLCEntries, 1)
+	require.Equal(t, *revLog, rl)
 }
 
 func TestDerializeHTLCEntriesEmptyRHash(t *testing.T) {
@@ -271,23 +354,6 @@ func TestDerializeHTLCEntries(t *testing.T) {
 	require.Equal(t, &entry, htlcs[0])
 }
 
-func TestDerializeRevocationLog(t *testing.T) {
-	t.Parallel()
-
-	// Construct the full bytes.
-	b := testRevocationLogBytes
-	b = append(b, testHTLCEntryBytes...)
-
-	// Read the tlv stream.
-	buf := bytes.NewBuffer(b)
-	rl, err := deserializeRevocationLog(buf)
-	require.NoError(t, err)
-
-	// Check the bytes are read as expected.
-	require.Len(t, rl.HTLCEntries, 1)
-	require.Equal(t, testRevocationLog, rl)
-}
-
 func TestFetchLogBucket(t *testing.T) {
 	t.Parallel()
 
@@ -368,17 +434,28 @@ func TestPutRevocationLog(t *testing.T) {
 		commit      ChannelCommitment
 		ourIndex    uint32
 		theirIndex  uint32
+		noAmtData   bool
 		expectedErr error
 		expectedLog RevocationLog
 	}{
 		{
 			// Test a normal put operation.
-			name:        "successful put",
+			name:        "successful put with amount data",
 			commit:      testChannelCommit,
 			ourIndex:    0,
 			theirIndex:  1,
 			expectedErr: nil,
-			expectedLog: testRevocationLog,
+			expectedLog: testRevocationLogWithAmts,
+		},
+		{
+			// Test a normal put operation.
+			name:        "successful put with no amount data",
+			commit:      testChannelCommit,
+			ourIndex:    0,
+			theirIndex:  1,
+			noAmtData:   true,
+			expectedErr: nil,
+			expectedLog: testRevocationLogNoAmts,
 		},
 		{
 			// Test our index too big.
@@ -409,12 +486,22 @@ func TestPutRevocationLog(t *testing.T) {
 		},
 		{
 			// Test dust htlc is not saved.
-			name:        "dust htlc not saved",
+			name:        "dust htlc not saved with amout data",
 			commit:      testCommitDust,
 			ourIndex:    0,
 			theirIndex:  1,
 			expectedErr: nil,
-			expectedLog: testRevocationLog,
+			expectedLog: testRevocationLogWithAmts,
+		},
+		{
+			// Test dust htlc is not saved.
+			name:        "dust htlc not saved with no amount data",
+			commit:      testCommitDust,
+			ourIndex:    0,
+			theirIndex:  1,
+			noAmtData:   true,
+			expectedErr: nil,
+			expectedLog: testRevocationLogNoAmts,
 		},
 	}
 
@@ -435,6 +522,7 @@ func TestPutRevocationLog(t *testing.T) {
 			// Save the log.
 			err = putRevocationLog(
 				bucket, &tc.commit, tc.ourIndex, tc.theirIndex,
+				tc.noAmtData,
 			)
 			if err != nil {
 				return RevocationLog{}, err
@@ -542,7 +630,7 @@ func TestFetchRevocationLogCompatible(t *testing.T) {
 				require.NoError(t, err)
 
 				err = putRevocationLog(
-					lb, &testChannelCommit, 0, 1,
+					lb, &testChannelCommit, 0, 1, false,
 				)
 				require.NoError(t, err)
 			}

config.go

@@ -981,6 +981,13 @@ func ValidateConfig(cfg Config, interceptor signal.Interceptor, fileParser,
 		)
 	}
 
+	// Ensure that the amount data for revoked commitment transactions is
+	// stored if the watchtower client is active.
+	if cfg.DB.NoRevLogAmtData && cfg.WtClient.Active {
+		return nil, mkErr("revocation log amount data must be stored " +
+			"if the watchtower client is active")
+	}
+
 	// Ensure a valid max channel fee allocation was set.
 	if cfg.MaxChannelFeeAllocation <= 0 || cfg.MaxChannelFeeAllocation > 1 {
 		return nil, mkErr("invalid max channel fee allocation: %v, "+

config_builder.go

@@ -867,15 +867,22 @@ func (d *DefaultDatabaseBuilder) BuildDatabase(
 
 	dbOptions := []channeldb.OptionModifier{
 		channeldb.OptionSetRejectCacheSize(cfg.Caches.RejectCacheSize),
-		channeldb.OptionSetChannelCacheSize(cfg.Caches.ChannelCacheSize),
+		channeldb.OptionSetChannelCacheSize(
-		channeldb.OptionSetBatchCommitInterval(cfg.DB.BatchCommitInterval),
+			cfg.Caches.ChannelCacheSize,
+		),
+		channeldb.OptionSetBatchCommitInterval(
+			cfg.DB.BatchCommitInterval,
+		),
 		channeldb.OptionDryRunMigration(cfg.DryRunMigration),
 		channeldb.OptionSetUseGraphCache(!cfg.DB.NoGraphCache),
-		channeldb.OptionKeepFailedPaymentAttempts(cfg.KeepFailedPaymentAttempts),
+		channeldb.OptionKeepFailedPaymentAttempts(
+			cfg.KeepFailedPaymentAttempts,
+		),
 		channeldb.OptionStoreFinalHtlcResolutions(
 			cfg.StoreFinalHtlcResolutions,
 		),
 		channeldb.OptionPruneRevocationLog(cfg.DB.PruneRevocation),
+		channeldb.OptionNoRevLogAmtData(cfg.DB.NoRevLogAmtData),
 	}
 
 	// We want to pre-allocate the channel graph cache according to what we

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

@@ -408,6 +408,14 @@ in the lnwire package](https://github.com/lightningnetwork/lnd/pull/7303)
   3.5.7](https://github.com/lightningnetwork/lnd/pull/7353) to resolve linking
   issues with outdated dependencies.
 
+* [Re-add local and remote output amounts to the revocation 
+  log](https://github.com/lightningnetwork/lnd/pull/7379) and add a new 
+  `--db.no-rev-log-amt-data` flag that can be used to explicitly opt out of  
+  storing this extra data. It should be noted that setting this flag is not 
+  recommended unless the user is sure that they will never activate their 
+  watchtower client (`--wtclient.active`) in the future. The new flag can not
+  be set at all if the `--wtclient.active` flag has been set.
+
 ## Pathfinding
 
 * [Pathfinding takes capacity of edges into account to improve success

lncfg/db.go

@@ -85,6 +85,8 @@ type DB struct {
 	NoGraphCache bool `long:"no-graph-cache" description:"Don't use the in-memory graph cache for path finding. Much slower but uses less RAM. Can only be used with a bolt database backend."`
 
 	PruneRevocation bool `long:"prune-revocation" description:"Run the optional migration that prunes the revocation logs to save disk space."`
+
+	NoRevLogAmtData bool `long:"no-rev-log-amt-data" description:"If set, the to-local and to-remote output amounts of revoked commitment transactions will not be stored in the revocation log. Note that once this data is lost, a watchtower client will not be able to back up the revoked state."`
 }
 
 // DefaultDB creates and returns a new default DB config.

lnwallet/channel.go

@@ -108,6 +108,10 @@ var (
 	// ErrOutputIndexOutOfRange is returned when an output index is greater
 	// than or equal to the length of a given transaction's outputs.
 	ErrOutputIndexOutOfRange = errors.New("output index is out of range")
+
+	// ErrRevLogDataMissing is returned when a certain wanted optional field
+	// in a revocation log entry is missing.
+	ErrRevLogDataMissing = errors.New("revocation log data missing")
 )
 
 // ErrCommitSyncLocalDataLoss is returned in the case that we receive a valid
@@ -2287,8 +2291,12 @@ type BreachRetribution struct {
 }
 
 // NewBreachRetribution creates a new fully populated BreachRetribution for the
-// passed channel, at a particular revoked state number, and one which targets
+// passed channel, at a particular revoked state number. If the spend
-// the passed commitment transaction.
+// transaction that the breach retribution should target is known, then it can
+// be provided via the spendTx parameter. Otherwise, if the spendTx parameter is
+// nil, then the revocation log will be checked to see if it contains the info
+// required to construct the BreachRetribution. If the revocation log is missing
+// the required fields then ErrRevLogDataMissing will be returned.
 func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64,
 	breachHeight uint32, spendTx *wire.MsgTx) (*BreachRetribution, error) {
 
@@ -2493,7 +2501,11 @@ func createHtlcRetribution(chanState *channeldb.OpenChannel,
 
 // createBreachRetribution creates a partially initiated BreachRetribution
 // using a RevocationLog. Returns the constructed retribution, our amount,
-// their amount, and a possible non-nil error.
+// their amount, and a possible non-nil error. If the spendTx parameter is
+// non-nil, then it will be used to glean the breach transaction's to-local and
+// to-remote output amounts. Otherwise, the RevocationLog will be checked to
+// see if these fields are present there. If they are not, then
+// ErrRevLogDataMissing is returned.
 func createBreachRetribution(revokedLog *channeldb.RevocationLog,
 	spendTx *wire.MsgTx, chanState *channeldb.OpenChannel,
 	keyRing *CommitmentKeyRing, commitmentSecret *btcec.PrivateKey,
@@ -2523,18 +2535,33 @@ func createBreachRetribution(revokedLog *channeldb.RevocationLog,
 	if revokedLog.OurOutputIndex != channeldb.OutputIndexEmpty {
 		ourOutpoint.Index = uint32(revokedLog.OurOutputIndex)
 
+		// If the spend transaction is provided, then we use it to get
+		// the value of our output.
+		if spendTx != nil {
 			// Sanity check that OurOutputIndex is within range.
 			if int(ourOutpoint.Index) >= len(spendTx.TxOut) {
 				return nil, 0, 0, fmt.Errorf("%w: ours=%v, "+
-				"len(TxOut)=%v", ErrOutputIndexOutOfRange,
+					"len(TxOut)=%v",
+					ErrOutputIndexOutOfRange,
 					ourOutpoint.Index, len(spendTx.TxOut),
 				)
 			}
 			// Read the amounts from the breach transaction.
 			//
-		// NOTE: ourAmt here includes commit fee and anchor amount(if
+			// NOTE: ourAmt here includes commit fee and anchor
-		// enabled).
+			// amount (if enabled).
 			ourAmt = spendTx.TxOut[ourOutpoint.Index].Value
+		} else {
+			// Otherwise, we check to see if the revocation log
+			// contains our output amount. Due to a previous
+			// migration, this field may be empty in which case an
+			// error will be returned.
+			if revokedLog.OurBalance == nil {
+				return nil, 0, 0, ErrRevLogDataMissing
+			}
+
+			ourAmt = int64(revokedLog.OurBalance.ToSatoshis())
+		}
 	}
 
 	// Construct the their outpoint.
@@ -2544,16 +2571,35 @@ func createBreachRetribution(revokedLog *channeldb.RevocationLog,
 	if revokedLog.TheirOutputIndex != channeldb.OutputIndexEmpty {
 		theirOutpoint.Index = uint32(revokedLog.TheirOutputIndex)
 
+		// If the spend transaction is provided, then we use it to get
+		// the value of the remote parties' output.
+		if spendTx != nil {
 			// Sanity check that TheirOutputIndex is within range.
-		if int(revokedLog.TheirOutputIndex) >= len(spendTx.TxOut) {
+			if int(revokedLog.TheirOutputIndex) >=
+				len(spendTx.TxOut) {
+
 				return nil, 0, 0, fmt.Errorf("%w: theirs=%v, "+
-				"len(TxOut)=%v", ErrOutputIndexOutOfRange,
+					"len(TxOut)=%v",
-				revokedLog.TheirOutputIndex, len(spendTx.TxOut),
+					ErrOutputIndexOutOfRange,
+					revokedLog.TheirOutputIndex,
+					len(spendTx.TxOut),
 				)
 			}
 
 			// Read the amounts from the breach transaction.
 			theirAmt = spendTx.TxOut[theirOutpoint.Index].Value
+
+		} else {
+			// Otherwise, we check to see if the revocation log
+			// contains remote parties' output amount. Due to a
+			// previous migration, this field may be empty in which
+			// case an error will be returned.
+			if revokedLog.TheirBalance == nil {
+				return nil, 0, 0, ErrRevLogDataMissing
+			}
+
+			theirAmt = int64(revokedLog.TheirBalance.ToSatoshis())
+		}
 	}
 
 	return &BreachRetribution{

lnwallet/channel_test.go

@@ -9482,7 +9482,7 @@ func TestCreateHtlcRetribution(t *testing.T) {
 }
 
 // TestCreateBreachRetribution checks that `createBreachRetribution` behaves as
-// epxected.
+// expected.
 func TestCreateBreachRetribution(t *testing.T) {
 	t.Parallel()
 
@@ -9524,11 +9524,15 @@ func TestCreateBreachRetribution(t *testing.T) {
 	}
 
 	// Create a dummy revocation log.
+	ourAmtMsat := lnwire.MilliSatoshi(ourAmt * 1000)
+	theirAmtMsat := lnwire.MilliSatoshi(theirAmt * 1000)
 	revokedLog := channeldb.RevocationLog{
 		CommitTxHash:     commitHash,
 		OurOutputIndex:   uint16(localIndex),
 		TheirOutputIndex: uint16(remoteIndex),
 		HTLCEntries:      []*channeldb.HTLCEntry{htlc},
+		TheirBalance:     &theirAmtMsat,
+		OurBalance:       &ourAmtMsat,
 	}
 
 	// Create a log with an empty local output index.
@@ -9545,14 +9549,25 @@ func TestCreateBreachRetribution(t *testing.T) {
 		expectedErr      error
 		expectedOurAmt   int64
 		expectedTheirAmt int64
+		noSpendTx        bool
 	}{
 		{
-			name:             "create retribution successfully",
+			name: "create retribution successfully " +
+				"with spend tx",
 			revocationLog:    &revokedLog,
 			expectedErr:      nil,
 			expectedOurAmt:   ourAmt,
 			expectedTheirAmt: theirAmt,
 		},
+		{
+			name: "create retribution successfully " +
+				"without spend tx",
+			revocationLog:    &revokedLog,
+			expectedErr:      nil,
+			expectedOurAmt:   ourAmt,
+			expectedTheirAmt: theirAmt,
+			noSpendTx:        true,
+		},
 		{
 			name: "fail due to our index too big",
 			revocationLog: &channeldb.RevocationLog{
@@ -9568,19 +9583,39 @@ func TestCreateBreachRetribution(t *testing.T) {
 			expectedErr: ErrOutputIndexOutOfRange,
 		},
 		{
-			name:             "empty local output index",
+			name: "empty local output index with spend " +
+				"tx",
 			revocationLog:    &revokedLogNoLocal,
 			expectedErr:      nil,
 			expectedOurAmt:   0,
 			expectedTheirAmt: theirAmt,
 		},
 		{
-			name:             "empty remote output index",
+			name: "empty local output index without spend " +
+				"tx",
+			revocationLog:    &revokedLogNoLocal,
+			expectedErr:      nil,
+			expectedOurAmt:   0,
+			expectedTheirAmt: theirAmt,
+			noSpendTx:        true,
+		},
+		{
+			name: "empty remote output index with spend " +
+				"tx",
 			revocationLog:    &revokedLogNoRemote,
 			expectedErr:      nil,
 			expectedOurAmt:   ourAmt,
 			expectedTheirAmt: 0,
 		},
+		{
+			name: "empty remote output index without spend " +
+				"tx",
+			revocationLog:    &revokedLogNoRemote,
+			expectedErr:      nil,
+			expectedOurAmt:   ourAmt,
+			expectedTheirAmt: 0,
+			noSpendTx:        true,
+		},
 	}
 
 	// assertRetribution is a helper closure that checks a given breach
@@ -9623,8 +9658,13 @@ func TestCreateBreachRetribution(t *testing.T) {
 	for _, tc := range testCases {
 		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
+			tx := spendTx
+			if tc.noSpendTx {
+				tx = nil
+			}
+
 			br, our, their, err := createBreachRetribution(
-				tc.revocationLog, spendTx,
+				tc.revocationLog, tx,
 				aliceChannel.channelState, keyRing,
 				dummyPrivate, leaseExpiry,
 			)
@@ -9748,6 +9788,13 @@ func testNewBreachRetribution(t *testing.T, chanType channeldb.ChannelType) {
 	)
 	require.ErrorIs(t, err, channeldb.ErrNoPastDeltas)
 
+	// We also check that the same error is returned if no breach tx is
+	// provided.
+	_, err = NewBreachRetribution(
+		aliceChannel.channelState, stateNum, breachHeight, nil,
+	)
+	require.ErrorIs(t, err, channeldb.ErrNoPastDeltas)
+
 	// We now force a state transition which will give us a revocation log
 	// at height 0.
 	txid := aliceChannel.channelState.RemoteCommitment.CommitTx.TxHash()
@@ -9797,10 +9844,25 @@ func testNewBreachRetribution(t *testing.T, chanType channeldb.ChannelType) {
 	t.Log(spew.Sdump(breachTx))
 	assertRetribution(br, 1, 0)
 
+	// Repeat the check but with the breach tx set to nil. This should work
+	// since the necessary info should now be found in the revocation log.
+	br, err = NewBreachRetribution(
+		aliceChannel.channelState, stateNum, breachHeight, nil,
+	)
+	require.NoError(t, err)
+	assertRetribution(br, 1, 0)
+
 	// Create the retribution using a stateNum+1 and we should expect an
 	// error.
 	_, err = NewBreachRetribution(
 		aliceChannel.channelState, stateNum+1, breachHeight, breachTx,
 	)
 	require.ErrorIs(t, err, channeldb.ErrLogEntryNotFound)
+
+	// Once again, repeat the check for the case when no breach tx is
+	// provided.
+	_, err = NewBreachRetribution(
+		aliceChannel.channelState, stateNum+1, breachHeight, nil,
+	)
+	require.ErrorIs(t, err, channeldb.ErrLogEntryNotFound)
 }

sample-lnd.conf

@@ -1241,6 +1241,12 @@ litecoin.node=ltcd
 ; channels prior to lnd@v0.15.0.
 ; db.prune-revocation=false
 
+; If set to true, then the to-local and to-remote output amount data of revoked
+; commitment transactions will not be stored in the revocation log. Note that
+; this flag can only be set if --wtclient.active is not set. It is not
+; recommended to set this flag if you plan on ever setting wtclient.active in
+; the future.
+; db.no-rev-log-amt-data=false
 
 [etcd]
 

watchtower/wtdb/migration4/client_db_test.go

@@ -237,7 +237,7 @@ func TestMigrateAckedUpdates(t *testing.T) {
 			// summary bucket and a new index bucket.
 			after := after(test.shouldFail, test.pre, test.post)
 
-			migtest.ApplyMigrationWithDb(
+			migtest.ApplyMigrationWithDB(
 				t, before, after, MigrateAckedUpdates(2),
 				test.shouldFail,
 			)