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 224d4db46..38493d240 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) + }, }, } @@ -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 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..6c743b4b3 100644 --- a/channeldb/migration30/migration.go +++ b/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 { diff --git a/channeldb/migration30/migration_test.go b/channeldb/migration30/migration_test.go index 5ec91fdff..01773292c 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,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, ) diff --git a/channeldb/migration30/revocation_log.go b/channeldb/migration30/revocation_log.go index 51d92b72e..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" @@ -16,8 +17,20 @@ 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 + 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. -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 - ) + // 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 - return tlv.NewStream( - tlv.MakePrimitiveRecord(ourOutputIndexType, &rl.OurOutputIndex), - tlv.MakePrimitiveRecord( - theirOutputIndexType, &rl.TheirOutputIndex, - ), - tlv.MakePrimitiveRecord(commitTxHashType, &rl.CommitTxHash), - ) + // 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 @@ -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 diff --git a/channeldb/migration30/test_utils.go b/channeldb/migration30/test_utils.go index aa061039a..c66b796b5 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" @@ -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), - 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{ @@ -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), - 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, @@ -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 + } +} diff --git a/channeldb/migtest/migtest.go b/channeldb/migtest/migtest.go index dacca7dac..a3884a0d6 100644 --- a/channeldb/migtest/migtest.go +++ b/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() 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 e34c7e70b..f062ac086 100644 --- a/channeldb/revocation_log.go +++ b/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 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 + 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. -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 - ) + // 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 - return tlv.NewStream( - tlv.MakePrimitiveRecord(ourOutputIndexType, &rl.OurOutputIndex), - tlv.MakePrimitiveRecord( - theirOutputIndexType, &rl.TheirOutputIndex, - ), - tlv.MakePrimitiveRecord(commitTxHashType, &rl.CommitTxHash), - ) + // 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 @@ -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 diff --git a/channeldb/revocation_log_test.go b/channeldb/revocation_log_test.go index 405955f46..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) { @@ -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. - 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() @@ -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) } 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/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md index 80ad5fa71..aebee3add 100644 --- a/docs/release-notes/release-notes-0.16.0.md +++ b/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 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/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) } 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] 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, )