From ce8e7ecfa743e8be528947313ed9e4b8622ad9ad Mon Sep 17 00:00:00 2001
From: Elle Mouton <elle.mouton@gmail.com>
Date: Thu, 2 Feb 2023 08:52:21 +0200
Subject: [PATCH 1/8] channeldb: prepare RevocationLog for optional fields

This commit is a pure refactor. It restructures the RevocationLog
serialize and deserialize methods so that optional TLV fields can be
easily added.
---
 channeldb/migration30/revocation_log.go | 80 ++++++++++++++-----------
 channeldb/revocation_log.go             | 80 ++++++++++++++-----------
 channeldb/revocation_log_test.go        |  4 +-
 3 files changed, 90 insertions(+), 74 deletions(-)

diff --git a/channeldb/migration30/revocation_log.go b/channeldb/migration30/revocation_log.go
index 51d92b72e..24382e707 100644
--- a/channeldb/migration30/revocation_log.go
+++ b/channeldb/migration30/revocation_log.go
@@ -16,8 +16,18 @@ import (
 	"github.com/lightningnetwork/lnd/tlv"
 )
 
-// OutputIndexEmpty is used when the output index doesn't exist.
-const OutputIndexEmpty = math.MaxUint16
+const (
+	// 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
+)
 
 var (
 	// revocationLogBucketDeprecated is dedicated for storing the necessary
@@ -208,29 +218,6 @@ type RevocationLog struct {
 	HTLCEntries []*HTLCEntry
 }
 
-// toTlvStream converts an RevocationLog record into a tlv representation.
-func (rl *RevocationLog) toTlvStream() (*tlv.Stream, error) {
-	const (
-		// 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
-		// changes.
-		ourOutputIndexType   tlv.Type = 0
-		theirOutputIndexType tlv.Type = 1
-		commitTxHashType     tlv.Type = 2
-	)
-
-	return tlv.NewStream(
-		tlv.MakePrimitiveRecord(ourOutputIndexType, &rl.OurOutputIndex),
-		tlv.MakePrimitiveRecord(
-			theirOutputIndexType, &rl.TheirOutputIndex,
-		),
-		tlv.MakePrimitiveRecord(commitTxHashType, &rl.CommitTxHash),
-	)
-}
-
 // putRevocationLog uses the fields `CommitTx` and `Htlcs` from a
 // ChannelCommitment to construct a revocation log entry and saves them to
 // disk. It also saves our output index and their output index, which are
@@ -304,8 +291,21 @@ 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,
+		),
+	}
+
 	// Create the tlv stream.
-	tlvStream, err := rl.toTlvStream()
+	tlvStream, err := tlv.NewStream(records...)
 	if err != nil {
 		return err
 	}
@@ -351,13 +351,20 @@ func deserializeRevocationLog(r io.Reader) (RevocationLog, error) {
 	var rl RevocationLog
 
 	// Create the tlv stream.
-	tlvStream, err := rl.toTlvStream()
-	if err != nil {
-		return rl, err
-	}
+	tlvStream, err := tlv.NewStream(
+		tlv.MakePrimitiveRecord(
+			revLogOurOutputIndexType, &rl.OurOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogTheirOutputIndexType, &rl.TheirOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogCommitTxHashType, &rl.CommitTxHash,
+		),
+	)
 
 	// Read the tlv stream.
-	if err := readTlvStream(r, tlvStream); err != nil {
+	if _, err := readTlvStream(r, tlvStream); err != nil {
 		return rl, err
 	}
 
@@ -382,7 +389,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 +434,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 +443,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
diff --git a/channeldb/revocation_log.go b/channeldb/revocation_log.go
index e34c7e70b..30405ba8a 100644
--- a/channeldb/revocation_log.go
+++ b/channeldb/revocation_log.go
@@ -12,8 +12,18 @@ import (
 	"github.com/lightningnetwork/lnd/tlv"
 )
 
-// OutputIndexEmpty is used when the output index doesn't exist.
-const OutputIndexEmpty = math.MaxUint16
+const (
+	// 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
+)
 
 var (
 	// revocationLogBucketDeprecated is dedicated for storing the necessary
@@ -196,29 +206,6 @@ type RevocationLog struct {
 	HTLCEntries []*HTLCEntry
 }
 
-// toTlvStream converts an RevocationLog record into a tlv representation.
-func (rl *RevocationLog) toTlvStream() (*tlv.Stream, error) {
-	const (
-		// 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
-		// changes.
-		ourOutputIndexType   tlv.Type = 0
-		theirOutputIndexType tlv.Type = 1
-		commitTxHashType     tlv.Type = 2
-	)
-
-	return tlv.NewStream(
-		tlv.MakePrimitiveRecord(ourOutputIndexType, &rl.OurOutputIndex),
-		tlv.MakePrimitiveRecord(
-			theirOutputIndexType, &rl.TheirOutputIndex,
-		),
-		tlv.MakePrimitiveRecord(commitTxHashType, &rl.CommitTxHash),
-	)
-}
-
 // putRevocationLog uses the fields `CommitTx` and `Htlcs` from a
 // ChannelCommitment to construct a revocation log entry and saves them to
 // disk. It also saves our output index and their output index, which are
@@ -292,8 +279,21 @@ 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,
+		),
+	}
+
 	// Create the tlv stream.
-	tlvStream, err := rl.toTlvStream()
+	tlvStream, err := tlv.NewStream(records...)
 	if err != nil {
 		return err
 	}
@@ -339,13 +339,20 @@ func deserializeRevocationLog(r io.Reader) (RevocationLog, error) {
 	var rl RevocationLog
 
 	// Create the tlv stream.
-	tlvStream, err := rl.toTlvStream()
-	if err != nil {
-		return rl, err
-	}
+	tlvStream, err := tlv.NewStream(
+		tlv.MakePrimitiveRecord(
+			revLogOurOutputIndexType, &rl.OurOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogTheirOutputIndexType, &rl.TheirOutputIndex,
+		),
+		tlv.MakePrimitiveRecord(
+			revLogCommitTxHashType, &rl.CommitTxHash,
+		),
+	)
 
 	// Read the tlv stream.
-	if err := readTlvStream(r, tlvStream); err != nil {
+	if _, err := readTlvStream(r, tlvStream); err != nil {
 		return rl, err
 	}
 
@@ -370,7 +377,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 +422,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 +431,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
diff --git a/channeldb/revocation_log_test.go b/channeldb/revocation_log_test.go
index 405955f46..6ca5b33b0 100644
--- a/channeldb/revocation_log_test.go
+++ b/channeldb/revocation_log_test.go
@@ -127,7 +127,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 +150,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.

From 33b7b9221a84143ad6093dd1772cce32c489220c Mon Sep 17 00:00:00 2001
From: Elle Mouton <elle.mouton@gmail.com>
Date: Fri, 10 Feb 2023 09:01:39 +0200
Subject: [PATCH 2/8] channeldb+wtdb: fix migration function name lint

Rename the ApplyMigrationWithDb function to ApplyMigrationWithDB to make
the linter happy.
---
 channeldb/migration30/migration_test.go      | 4 ++--
 channeldb/migtest/migtest.go                 | 4 ++--
 watchtower/wtdb/migration4/client_db_test.go | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/channeldb/migration30/migration_test.go b/channeldb/migration30/migration_test.go
index 5ec91fdff..a97a4aea5 100644
--- a/channeldb/migration30/migration_test.go
+++ b/channeldb/migration30/migration_test.go
@@ -103,7 +103,7 @@ func TestMigrateRevocationLog(t *testing.T) {
 				return nil
 			}
 
-			migtest.ApplyMigrationWithDb(
+			migtest.ApplyMigrationWithDB(
 				t,
 				beforeMigration,
 				afterMigration,
@@ -560,7 +560,7 @@ func BenchmarkMigration(b *testing.B) {
 	}
 
 	// Run the migration test.
-	migtest.ApplyMigrationWithDb(
+	migtest.ApplyMigrationWithDB(
 		b,
 		beforeMigration,
 		nil,
diff --git a/channeldb/migtest/migtest.go b/channeldb/migtest/migtest.go
index dacca7dac..efe058287 100644
--- a/channeldb/migtest/migtest.go
+++ b/channeldb/migtest/migtest.go
@@ -84,12 +84,12 @@ 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,
 	migrationFunc func(db kvdb.Backend) error, shouldFail bool) {
 
 	t.Helper()
diff --git a/watchtower/wtdb/migration4/client_db_test.go b/watchtower/wtdb/migration4/client_db_test.go
index 2cdb7c4e9..cb0feec68 100644
--- a/watchtower/wtdb/migration4/client_db_test.go
+++ b/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,
 			)

From 70e3f8f5118b2cd5b597c17364fb200a1eed443a Mon Sep 17 00:00:00 2001
From: Elle Mouton <elle.mouton@gmail.com>
Date: Thu, 9 Feb 2023 14:40:15 +0200
Subject: [PATCH 3/8] multi: pass in a MigrationConfig to all optional
 migrations

Define a MigrationConfig interface that should be used to pass the
config of an optional migration function. An implementation of this
interface is added for migration30 (the only current optional
migration).
---
 channeldb/db.go                         | 28 +++++++++++++++++++++----
 channeldb/meta_test.go                  |  4 +++-
 channeldb/migration30/migration.go      |  9 +++++++-
 channeldb/migration30/migration_test.go | 10 +++++++--
 channeldb/migtest/migtest.go            |  3 ++-
 5 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/channeldb/db.go b/channeldb/db.go
index 224d4db46..37bee6ff1 100644
--- a/channeldb/db.go
+++ b/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
@@ -273,8 +285,12 @@ var (
 	// to determine its state.
 	optionalVersions = []optionalVersion{
 		{
-			name:      "prune revocation log",
-			migration: migration30.MigrateRevocationLog,
+			name: "prune revocation log",
+			migration: func(db kvdb.Backend,
+				cfg MigrationConfig) error {
+
+				return migration30.MigrateRevocationLog(db, cfg)
+			},
 		},
 	}
 
@@ -1545,8 +1561,12 @@ func (d *DB) applyOptionalVersions(cfg OptionalMiragtionConfig) error {
 	version := optionalVersions[0]
 	log.Infof("Performing database optional migration: %s", version.name)
 
+	migrationCfg := &MigrationConfigImpl{
+		migration30.MigrateRevLogConfigImpl{},
+	}
+
 	// 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
diff --git a/channeldb/meta_test.go b/channeldb/meta_test.go
index 211429e35..defa9f329 100644
--- a/channeldb/meta_test.go
+++ b/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
 	}
diff --git a/channeldb/migration30/migration.go b/channeldb/migration30/migration.go
index fc551ac2a..55663df5a 100644
--- a/channeldb/migration30/migration.go
+++ b/channeldb/migration30/migration.go
@@ -23,10 +23,17 @@ 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{}
+
+// MigrateRevLogConfigImpl implements the MigrationRevLogConfig interface.
+type MigrateRevLogConfigImpl struct{}
+
 // 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, _ MigrateRevLogConfig) error {
 	log.Infof("Migrating revocation logs, might take a while...")
 
 	var (
diff --git a/channeldb/migration30/migration_test.go b/channeldb/migration30/migration_test.go
index a97a4aea5..7eef265a6 100644
--- a/channeldb/migration30/migration_test.go
+++ b/channeldb/migration30/migration_test.go
@@ -103,11 +103,15 @@ func TestMigrateRevocationLog(t *testing.T) {
 				return nil
 			}
 
+			cfg := &MigrateRevLogConfigImpl{}
+
 			migtest.ApplyMigrationWithDB(
 				t,
 				beforeMigration,
 				afterMigration,
-				MigrateRevocationLog,
+				func(db kvdb.Backend) error {
+					return MigrateRevocationLog(db, cfg)
+				},
 				false,
 			)
 		})
@@ -559,6 +563,8 @@ func BenchmarkMigration(b *testing.B) {
 		return setupTestLogs(db, c, oldLogs, nil)
 	}
 
+	cfg := &MigrateRevLogConfigImpl{}
+
 	// Run the migration test.
 	migtest.ApplyMigrationWithDB(
 		b,
@@ -568,7 +574,7 @@ func BenchmarkMigration(b *testing.B) {
 			b.StartTimer()
 			defer b.StopTimer()
 
-			return MigrateRevocationLog(db)
+			return MigrateRevocationLog(db, cfg)
 		},
 		false,
 	)
diff --git a/channeldb/migtest/migtest.go b/channeldb/migtest/migtest.go
index efe058287..a3884a0d6 100644
--- a/channeldb/migtest/migtest.go
+++ b/channeldb/migtest/migtest.go
@@ -89,7 +89,8 @@ func ApplyMigration(t *testing.T,
 // 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()

From 9c01916bc0b5516bf61ddbfbd01863ec704b7d6a Mon Sep 17 00:00:00 2001
From: Elle Mouton <elle.mouton@gmail.com>
Date: Thu, 9 Feb 2023 15:01:32 +0200
Subject: [PATCH 4/8] channeldb: add NoAmountData field to
 MigrateRevLogConfigImpl

Add a NoAmountData field to the MigrateRevLogConfigImpl struct and set
it for tests. This field is still a no-op in the migration.
---
 channeldb/migration30/migration.go      | 35 ++++++++++++++++++-------
 channeldb/migration30/migration_test.go |  9 +++++--
 channeldb/migration30/test_utils.go     | 15 +++++++++++
 3 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/channeldb/migration30/migration.go b/channeldb/migration30/migration.go
index 55663df5a..4cf24bfac 100644
--- a/channeldb/migration30/migration.go
+++ b/channeldb/migration30/migration.go
@@ -25,15 +25,29 @@ const recordsPerTx = 20_000
 
 // MigrateRevLogConfig is an interface that defines the config that should be
 // passed to the MigrateRevocationLog function.
-type MigrateRevLogConfig interface{}
+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{}
+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, _ MigrateRevLogConfig) error {
+func MigrateRevocationLog(db kvdb.Backend, cfg MigrateRevLogConfig) error {
 	log.Infof("Migrating revocation logs, might take a while...")
 
 	var (
@@ -71,7 +85,7 @@ func MigrateRevocationLog(db kvdb.Backend, _ MigrateRevLogConfig) 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
 			}
@@ -121,7 +135,7 @@ func MigrateRevocationLog(db kvdb.Backend, _ MigrateRevLogConfig) 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.
@@ -141,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)
 	}
@@ -375,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)
@@ -422,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
 		}
@@ -526,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 {
diff --git a/channeldb/migration30/migration_test.go b/channeldb/migration30/migration_test.go
index 7eef265a6..a83a4f451 100644
--- a/channeldb/migration30/migration_test.go
+++ b/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,7 +104,9 @@ func TestMigrateRevocationLog(t *testing.T) {
 				return nil
 			}
 
-			cfg := &MigrateRevLogConfigImpl{}
+			cfg := &MigrateRevLogConfigImpl{
+				NoAmountData: !withAmtData,
+			}
 
 			migtest.ApplyMigrationWithDB(
 				t,
@@ -563,7 +566,9 @@ func BenchmarkMigration(b *testing.B) {
 		return setupTestLogs(db, c, oldLogs, nil)
 	}
 
-	cfg := &MigrateRevLogConfigImpl{}
+	cfg := &MigrateRevLogConfigImpl{
+		NoAmountData: !withAmtData,
+	}
 
 	// Run the migration test.
 	migtest.ApplyMigrationWithDB(
diff --git a/channeldb/migration30/test_utils.go b/channeldb/migration30/test_utils.go
index aa061039a..2a3093fb1 100644
--- a/channeldb/migration30/test_utils.go
+++ b/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"
@@ -260,6 +262,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
@@ -551,3 +559,10 @@ 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
+	}
+}

From 0730337cc71e36c4dcfaa72037f4b20aef096403 Mon Sep 17 00:00:00 2001
From: Elle Mouton <elle.mouton@gmail.com>
Date: Fri, 10 Feb 2023 10:13:34 +0200
Subject: [PATCH 5/8] multi: add new NoRevLogAmtData config option

In this commit, a new `--db.no-rev-log-amt-data` flag is added. The
config option is passed though to everywhere that it will be used. Note
that it is still a no-op in this commit. An upcoming commit will make
use of the flag.
---
 channeldb/channel.go                    |  4 ++--
 channeldb/db.go                         |  9 ++++++++-
 channeldb/migration30/revocation_log.go |  2 +-
 channeldb/migration30/test_utils.go     |  6 +++---
 channeldb/options.go                    | 12 ++++++++++++
 channeldb/revocation_log.go             |  2 +-
 channeldb/revocation_log_test.go        |  4 +++-
 config.go                               |  7 +++++++
 config_builder.go                       | 13 ++++++++++---
 lncfg/db.go                             |  2 ++
 sample-lnd.conf                         |  6 ++++++
 11 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/channeldb/channel.go b/channeldb/channel.go
index 8fbeeea5c..d71f17047 100644
--- a/channeldb/channel.go
+++ b/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,
-			ourOutputIndex, theirOutputIndex,
+			logBucket, &c.RemoteCommitment, ourOutputIndex,
+			theirOutputIndex, c.Db.parent.noRevLogAmtData,
 		)
 		if err != nil {
 			return err
diff --git a/channeldb/db.go b/channeldb/db.go
index 37bee6ff1..38493d240 100644
--- a/channeldb/db.go
+++ b/channeldb/db.go
@@ -324,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
@@ -382,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).
@@ -1562,7 +1567,9 @@ func (d *DB) applyOptionalVersions(cfg OptionalMiragtionConfig) error {
 	log.Infof("Performing database optional migration: %s", version.name)
 
 	migrationCfg := &MigrationConfigImpl{
-		migration30.MigrateRevLogConfigImpl{},
+		migration30.MigrateRevLogConfigImpl{
+			NoAmountData: d.noRevLogAmtData,
+		},
 	}
 
 	// Migrate the data.
diff --git a/channeldb/migration30/revocation_log.go b/channeldb/migration30/revocation_log.go
index 24382e707..99539736e 100644
--- a/channeldb/migration30/revocation_log.go
+++ b/channeldb/migration30/revocation_log.go
@@ -223,7 +223,7 @@ type RevocationLog struct {
 // 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 {
diff --git a/channeldb/migration30/test_utils.go b/channeldb/migration30/test_utils.go
index 2a3093fb1..97de53fb3 100644
--- a/channeldb/migration30/test_utils.go
+++ b/channeldb/migration30/test_utils.go
@@ -306,7 +306,7 @@ func setupTestLogs(db kvdb.Backend, c *mig26.OpenChannel,
 		}
 
 		// Create new logs.
-		return writeNewRevocationLogs(chanBucket, newLogs)
+		return writeNewRevocationLogs(chanBucket, newLogs, !withAmtData)
 	}, func() {})
 }
 
@@ -460,7 +460,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 {
@@ -480,7 +480,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
diff --git a/channeldb/options.go b/channeldb/options.go
index d3b26a739..3f5b472f6 100644
--- a/channeldb/options.go
+++ b/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) {
diff --git a/channeldb/revocation_log.go b/channeldb/revocation_log.go
index 30405ba8a..2df93fd5b 100644
--- a/channeldb/revocation_log.go
+++ b/channeldb/revocation_log.go
@@ -211,7 +211,7 @@ type RevocationLog struct {
 // 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 {
diff --git a/channeldb/revocation_log_test.go b/channeldb/revocation_log_test.go
index 6ca5b33b0..1f6d1f813 100644
--- a/channeldb/revocation_log_test.go
+++ b/channeldb/revocation_log_test.go
@@ -368,6 +368,7 @@ func TestPutRevocationLog(t *testing.T) {
 		commit      ChannelCommitment
 		ourIndex    uint32
 		theirIndex  uint32
+		noAmtData   bool
 		expectedErr error
 		expectedLog RevocationLog
 	}{
@@ -435,6 +436,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 +544,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)
 			}
diff --git a/config.go b/config.go
index dca79551f..41fe291ba 100644
--- a/config.go
+++ b/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, "+
diff --git a/config_builder.go b/config_builder.go
index 3b71a21b0..225c82b96 100644
--- a/config_builder.go
+++ b/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.OptionSetBatchCommitInterval(cfg.DB.BatchCommitInterval),
+		channeldb.OptionSetChannelCacheSize(
+			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
diff --git a/lncfg/db.go b/lncfg/db.go
index e446553ae..ed0bdf6b7 100644
--- a/lncfg/db.go
+++ b/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.
diff --git a/sample-lnd.conf b/sample-lnd.conf
index 96ac7e015..805db4202 100644
--- a/sample-lnd.conf
+++ b/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]
 

From 2c786ec66fca9a4fbb3608e9f7e4d9370a143126 Mon Sep 17 00:00:00 2001
From: Elle Mouton <elle.mouton@gmail.com>
Date: Thu, 2 Feb 2023 09:41:56 +0200
Subject: [PATCH 6/8] channeldb: add local and remote balance to revocation log

This commit re-adds the LocalBalance and RemoteBalance fields to the
RevocationLog. The channeldb/migration30 is also adjusted so that anyone
who has not yet run the optional migration will not lose these fields if
they run the migration after this commit.

The reason for re-adding these fields is that they are needed if we want
to reconstruct all the info of the lnwallet.BreachRetribution without
having access to the breach spend transaction. In most cases we would
have access to the spend tx since we would see it on-chain at which time
we would want to reconstruct the retribution info. However, for the
watchtower subsystem, we sometimes want to construct the retribution
info withouth having access to the spend transaction.

A user can use the `--no-rev-log-amt-data` flag to opt-out of storing
these amount fields.
---
 channeldb/migration30/migration.go      |   5 +
 channeldb/migration30/migration_test.go |   4 +
 channeldb/migration30/revocation_log.go |  72 +++++++++++-
 channeldb/migration30/test_utils.go     |  60 +++++++++-
 channeldb/revocation_log.go             |  72 +++++++++++-
 channeldb/revocation_log_test.go        | 146 +++++++++++++++++++-----
 6 files changed, 313 insertions(+), 46 deletions(-)

diff --git a/channeldb/migration30/migration.go b/channeldb/migration30/migration.go
index 4cf24bfac..6c743b4b3 100644
--- a/channeldb/migration30/migration.go
+++ b/channeldb/migration30/migration.go
@@ -560,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 {
diff --git a/channeldb/migration30/migration_test.go b/channeldb/migration30/migration_test.go
index a83a4f451..01773292c 100644
--- a/channeldb/migration30/migration_test.go
+++ b/channeldb/migration30/migration_test.go
@@ -512,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")
 
diff --git a/channeldb/migration30/revocation_log.go b/channeldb/migration30/revocation_log.go
index 99539736e..c4f3f1626 100644
--- a/channeldb/migration30/revocation_log.go
+++ b/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"
@@ -27,6 +28,8 @@ const (
 	revLogOurOutputIndexType   tlv.Type = 0
 	revLogTheirOutputIndexType tlv.Type = 1
 	revLogCommitTxHashType     tlv.Type = 2
+	revLogOurBalanceType       tlv.Type = 3
+	revLogTheirBalanceType     tlv.Type = 4
 )
 
 var (
@@ -197,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.
@@ -216,6 +216,26 @@ type RevocationLog struct {
 	// HTLCEntries is the set of HTLCEntry's that are pending at this
 	// particular commitment height.
 	HTLCEntries []*HTLCEntry
+
+	// OurBalance is the current available balance within the channel
+	// directly spendable by us. In other words, it is the value of the
+	// to_remote output on the remote parties' commitment transaction.
+	//
+	// NOTE: this is a pointer so that it is clear if the value is zero or
+	// 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.
+	OurBalance *lnwire.MilliSatoshi
+
+	// TheirBalance is the current available balance within the channel
+	// directly spendable by the remote node. In other words, it is the
+	// value of the to_local output on the remote parties' commitment.
+	//
+	// NOTE: this is a pointer so that it is clear if the value is zero or
+	// 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
@@ -240,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,6 +329,21 @@ func serializeRevocationLog(w io.Writer, rl *RevocationLog) error {
 		),
 	}
 
+	// 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 := tlv.NewStream(records...)
 	if err != nil {
@@ -348,7 +388,11 @@ 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 := tlv.NewStream(
@@ -361,13 +405,31 @@ func deserializeRevocationLog(r io.Reader) (RevocationLog, error) {
 		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)
 
diff --git a/channeldb/migration30/test_utils.go b/channeldb/migration30/test_utils.go
index 97de53fb3..c66b796b5 100644
--- a/channeldb/migration30/test_utils.go
+++ b/channeldb/migration30/test_utils.go
@@ -80,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,
@@ -87,18 +93,29 @@ var (
 		LocalHtlcIndex:  0,
 		RemoteLogIndex:  0,
 		RemoteHtlcIndex: 0,
-		LocalBalance:    lnwire.MilliSatoshi(990_950_000),
-		RemoteBalance:   0,
+		LocalBalance:    localBalance1,
+		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{
@@ -148,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,
@@ -155,13 +178,14 @@ var (
 		LocalHtlcIndex:  1,
 		RemoteLogIndex:  0,
 		RemoteHtlcIndex: 0,
-		LocalBalance:    lnwire.MilliSatoshi(888_800_000),
-		RemoteBalance:   0,
+		LocalBalance:    localBalance2,
+		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,
@@ -177,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,
@@ -565,4 +608,9 @@ func init() {
 	if rand.Intn(2) == 0 {
 		withAmtData = true
 	}
+
+	if withAmtData {
+		newLog1 = newLog1WithAmts
+		newLog2 = newLog2WithAmts
+	}
 }
diff --git a/channeldb/revocation_log.go b/channeldb/revocation_log.go
index 2df93fd5b..f062ac086 100644
--- a/channeldb/revocation_log.go
+++ b/channeldb/revocation_log.go
@@ -9,6 +9,7 @@ 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"
 )
 
@@ -23,6 +24,8 @@ const (
 	revLogOurOutputIndexType   tlv.Type = 0
 	revLogTheirOutputIndexType tlv.Type = 1
 	revLogCommitTxHashType     tlv.Type = 2
+	revLogOurBalanceType       tlv.Type = 3
+	revLogTheirBalanceType     tlv.Type = 4
 )
 
 var (
@@ -185,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.
@@ -204,6 +204,26 @@ type RevocationLog struct {
 	// HTLCEntries is the set of HTLCEntry's that are pending at this
 	// particular commitment height.
 	HTLCEntries []*HTLCEntry
+
+	// OurBalance is the current available balance within the channel
+	// directly spendable by us. In other words, it is the value of the
+	// to_remote output on the remote parties' commitment transaction.
+	//
+	// NOTE: this is a pointer so that it is clear if the value is zero or
+	// 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.
+	OurBalance *lnwire.MilliSatoshi
+
+	// TheirBalance is the current available balance within the channel
+	// directly spendable by the remote node. In other words, it is the
+	// value of the to_local output on the remote parties' commitment.
+	//
+	// NOTE: this is a pointer so that it is clear if the value is zero or
+	// 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
@@ -228,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,6 +317,21 @@ func serializeRevocationLog(w io.Writer, rl *RevocationLog) error {
 		),
 	}
 
+	// 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 := tlv.NewStream(records...)
 	if err != nil {
@@ -336,7 +376,11 @@ 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 := tlv.NewStream(
@@ -349,13 +393,31 @@ func deserializeRevocationLog(r io.Reader) (RevocationLog, error) {
 		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)
 
diff --git a/channeldb/revocation_log_test.go b/channeldb/revocation_log_test.go
index 1f6d1f813..fc5303a48 100644
--- a/channeldb/revocation_log_test.go
+++ b/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),
-		RemoteBalance: lnwire.MilliSatoshi(3000),
+		LocalBalance:  localBalance,
+		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) {
@@ -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.
-	rl := testRevocationLog
+	tests := []struct {
+		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()
 
@@ -374,12 +440,22 @@ func TestPutRevocationLog(t *testing.T) {
 	}{
 		{
 			// 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.
@@ -410,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,
 		},
 	}
 

From 91a2cd45d897a5e60de38ddcb5f6e847cec70d99 Mon Sep 17 00:00:00 2001
From: Elle Mouton <elle.mouton@gmail.com>
Date: Thu, 2 Feb 2023 10:13:44 +0200
Subject: [PATCH 7/8] lnwallet: make spendTx param in NewBreachRetribution
 optional

In this commit, the NewBreachRetribution function is adjusted so that a
caller can optionally set the spendTx parameter to nil. In this case,
the function will check the revocation log to see if the local and
remote amount fields are available there and use them if they are.
If the fields are not present, which they might not be given a previous
migration that removed the fields, then an error is returned.
---
 lnwallet/channel.go      | 92 ++++++++++++++++++++++++++++++----------
 lnwallet/channel_test.go | 72 ++++++++++++++++++++++++++++---
 2 files changed, 136 insertions(+), 28 deletions(-)

diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index 83a146be0..97b7ac44d 100644
--- a/lnwallet/channel.go
+++ b/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
-// the passed commitment transaction.
+// passed channel, at a particular revoked state number. If the spend
+// 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)
 
-		// 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,
-				ourOutpoint.Index, len(spendTx.TxOut),
-			)
+		// 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,
+					ourOutpoint.Index, len(spendTx.TxOut),
+				)
+			}
+			// Read the amounts from the breach transaction.
+			//
+			// NOTE: ourAmt here includes commit fee and anchor
+			// 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())
 		}
-		// Read the amounts from the breach transaction.
-		//
-		// NOTE: ourAmt here includes commit fee and anchor amount(if
-		// enabled).
-		ourAmt = spendTx.TxOut[ourOutpoint.Index].Value
 	}
 
 	// Construct the their outpoint.
@@ -2544,16 +2571,35 @@ func createBreachRetribution(revokedLog *channeldb.RevocationLog,
 	if revokedLog.TheirOutputIndex != channeldb.OutputIndexEmpty {
 		theirOutpoint.Index = uint32(revokedLog.TheirOutputIndex)
 
-		// Sanity check that TheirOutputIndex is within range.
-		if int(revokedLog.TheirOutputIndex) >= len(spendTx.TxOut) {
-			return nil, 0, 0, fmt.Errorf("%w: theirs=%v, "+
-				"len(TxOut)=%v", ErrOutputIndexOutOfRange,
-				revokedLog.TheirOutputIndex, len(spendTx.TxOut),
-			)
-		}
+		// 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) {
 
-		// Read the amounts from the breach transaction.
-		theirAmt = spendTx.TxOut[theirOutpoint.Index].Value
+				return nil, 0, 0, fmt.Errorf("%w: theirs=%v, "+
+					"len(TxOut)=%v",
+					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{
diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go
index db72e5db5..fc8a7bef7 100644
--- a/lnwallet/channel_test.go
+++ b/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)
 }

From 884cb2047ba2ef183b2c854d4fcfedca5f193d13 Mon Sep 17 00:00:00 2001
From: Elle Mouton <elle.mouton@gmail.com>
Date: Thu, 2 Feb 2023 10:23:33 +0200
Subject: [PATCH 8/8] docs: update release notes

---
 docs/release-notes/release-notes-0.16.0.md | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md
index 0cac35fc5..a4e0e6a94 100644
--- a/docs/release-notes/release-notes-0.16.0.md
+++ b/docs/release-notes/release-notes-0.16.0.md
@@ -404,6 +404,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