diff --git a/channeldb/channel_test.go b/channeldb/channel_test.go index 8e2984f87..8eca3e7dd 100644 --- a/channeldb/channel_test.go +++ b/channeldb/channel_test.go @@ -570,9 +570,11 @@ func assertCommitmentEqual(t *testing.T, a, b *ChannelCommitment) { func assertRevocationLogEntryEqual(t *testing.T, c *ChannelCommitment, r *RevocationLog) { + t.Helper() + // Check the common fields. require.EqualValues( - t, r.CommitTxHash, c.CommitTx.TxHash(), "CommitTx mismatch", + t, r.CommitTxHash.Val, c.CommitTx.TxHash(), "CommitTx mismatch", ) // Now check the common fields from the HTLCs. @@ -806,10 +808,10 @@ func TestChannelStateTransition(t *testing.T) { // Check the output indexes are saved as expected. require.EqualValues( - t, dummyLocalOutputIndex, diskPrevCommit.OurOutputIndex, + t, dummyLocalOutputIndex, diskPrevCommit.OurOutputIndex.Val, ) require.EqualValues( - t, dummyRemoteOutIndex, diskPrevCommit.TheirOutputIndex, + t, dummyRemoteOutIndex, diskPrevCommit.TheirOutputIndex.Val, ) // The two deltas (the original vs the on-disk version) should @@ -851,10 +853,10 @@ func TestChannelStateTransition(t *testing.T) { // Check the output indexes are saved as expected. require.EqualValues( - t, dummyLocalOutputIndex, diskPrevCommit.OurOutputIndex, + t, dummyLocalOutputIndex, diskPrevCommit.OurOutputIndex.Val, ) require.EqualValues( - t, dummyRemoteOutIndex, diskPrevCommit.TheirOutputIndex, + t, dummyRemoteOutIndex, diskPrevCommit.TheirOutputIndex.Val, ) assertRevocationLogEntryEqual(t, &oldRemoteCommit, prevCommit) diff --git a/channeldb/revocation_log.go b/channeldb/revocation_log.go index 26e4b9644..3fc4163c1 100644 --- a/channeldb/revocation_log.go +++ b/channeldb/revocation_log.go @@ -7,6 +7,7 @@ import ( "math" "github.com/btcsuite/btcd/btcutil" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" @@ -16,16 +17,15 @@ import ( 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 +type ( + // BigSizeAmount is a type alias for a TLV record of a btcutil.Amount. + BigSizeAmount = tlv.BigSizeT[btcutil.Amount] + + // BigSizeMilliSatoshi is a type alias for a TLV record of a + // lnwire.MilliSatoshi. + BigSizeMilliSatoshi = tlv.BigSizeT[lnwire.MilliSatoshi] ) var ( @@ -203,15 +203,15 @@ func NewHTLCEntryFromHTLC(htlc HTLC) *HTLCEntry { type RevocationLog struct { // OurOutputIndex specifies our output index in this commitment. In a // remote commitment transaction, this is the to remote output index. - OurOutputIndex uint16 + OurOutputIndex tlv.RecordT[tlv.TlvType0, uint16] // TheirOutputIndex specifies their output index in this commitment. In // a remote commitment transaction, this is the to local output index. - TheirOutputIndex uint16 + TheirOutputIndex tlv.RecordT[tlv.TlvType1, uint16] // CommitTxHash is the hash of the latest version of the commitment // state, broadcast able by us. - CommitTxHash [32]byte + CommitTxHash tlv.RecordT[tlv.TlvType2, [32]byte] // HTLCEntries is the set of HTLCEntry's that are pending at this // particular commitment height. @@ -221,21 +221,53 @@ type RevocationLog struct { // 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 + // NOTE: this is an option 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 + OurBalance tlv.OptionalRecordT[tlv.TlvType3, BigSizeMilliSatoshi] // 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 + // NOTE: this is an option 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 + TheirBalance tlv.OptionalRecordT[tlv.TlvType4, BigSizeMilliSatoshi] +} + +// NewRevocationLog creates a new RevocationLog from the given parameters. +func NewRevocationLog(ourOutputIndex uint16, theirOutputIndex uint16, + commitHash [32]byte, ourBalance, + theirBalance fn.Option[lnwire.MilliSatoshi], + htlcs []*HTLCEntry) RevocationLog { + + rl := RevocationLog{ + OurOutputIndex: tlv.NewPrimitiveRecord[tlv.TlvType0]( + ourOutputIndex, + ), + TheirOutputIndex: tlv.NewPrimitiveRecord[tlv.TlvType1]( + theirOutputIndex, + ), + CommitTxHash: tlv.NewPrimitiveRecord[tlv.TlvType2](commitHash), + HTLCEntries: htlcs, + } + + ourBalance.WhenSome(func(balance lnwire.MilliSatoshi) { + rl.OurBalance = tlv.SomeRecordT(tlv.NewRecordT[tlv.TlvType3]( + tlv.NewBigSizeT(balance), + )) + }) + + theirBalance.WhenSome(func(balance lnwire.MilliSatoshi) { + rl.TheirBalance = tlv.SomeRecordT(tlv.NewRecordT[tlv.TlvType4]( + tlv.NewBigSizeT(balance), + )) + }) + + return rl } // putRevocationLog uses the fields `CommitTx` and `Htlcs` from a @@ -254,15 +286,26 @@ func putRevocationLog(bucket kvdb.RwBucket, commit *ChannelCommitment, } rl := &RevocationLog{ - OurOutputIndex: uint16(ourOutputIndex), - TheirOutputIndex: uint16(theirOutputIndex), - CommitTxHash: commit.CommitTx.TxHash(), - HTLCEntries: make([]*HTLCEntry, 0, len(commit.Htlcs)), + OurOutputIndex: tlv.NewPrimitiveRecord[tlv.TlvType0]( + uint16(ourOutputIndex), + ), + TheirOutputIndex: tlv.NewPrimitiveRecord[tlv.TlvType1]( + uint16(theirOutputIndex), + ), + CommitTxHash: tlv.NewPrimitiveRecord[tlv.TlvType2, [32]byte]( + commit.CommitTx.TxHash(), + ), + HTLCEntries: make([]*HTLCEntry, 0, len(commit.Htlcs)), } if !noAmtData { - rl.OurBalance = &commit.LocalBalance - rl.TheirBalance = &commit.RemoteBalance + rl.OurBalance = tlv.SomeRecordT(tlv.NewRecordT[tlv.TlvType3]( + tlv.NewBigSizeT(commit.LocalBalance), + )) + + rl.TheirBalance = tlv.SomeRecordT(tlv.NewRecordT[tlv.TlvType4]( + tlv.NewBigSizeT(commit.RemoteBalance), + )) } for _, htlc := range commit.Htlcs { @@ -312,31 +355,23 @@ func fetchRevocationLog(log kvdb.RBucket, 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, - ), + rl.OurOutputIndex.Record(), + rl.TheirOutputIndex.Record(), + rl.CommitTxHash.Record(), } // 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, - )) - } + rl.OurBalance.WhenSome( + func(r tlv.RecordT[tlv.TlvType3, BigSizeMilliSatoshi]) { + records = append(records, r.Record()) + }, + ) - if rl.TheirBalance != nil { - rb := uint64(*rl.TheirBalance) - records = append(records, tlv.MakeBigSizeRecord( - revLogTheirBalanceType, &rb, - )) - } + rl.TheirBalance.WhenSome( + func(r tlv.RecordT[tlv.TlvType4, BigSizeMilliSatoshi]) { + records = append(records, r.Record()) + }, + ) // Create the tlv stream. tlvStream, err := tlv.NewStream(records...) @@ -374,27 +409,18 @@ 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 - ourBalance uint64 - theirBalance uint64 - ) + var rl RevocationLog + + ourBalance := rl.OurBalance.Zero() + theirBalance := rl.TheirBalance.Zero() // Create the tlv stream. 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, - ), + rl.OurOutputIndex.Record(), + rl.TheirOutputIndex.Record(), + rl.CommitTxHash.Record(), + ourBalance.Record(), + theirBalance.Record(), ) if err != nil { return rl, err @@ -406,14 +432,12 @@ func deserializeRevocationLog(r io.Reader) (RevocationLog, error) { return rl, err } - if t, ok := parsedTypes[revLogOurBalanceType]; ok && t == nil { - lb := lnwire.MilliSatoshi(ourBalance) - rl.OurBalance = &lb + if t, ok := parsedTypes[ourBalance.TlvType()]; ok && t == nil { + rl.OurBalance = tlv.SomeRecordT(ourBalance) } - if t, ok := parsedTypes[revLogTheirBalanceType]; ok && t == nil { - rb := lnwire.MilliSatoshi(theirBalance) - rl.TheirBalance = &rb + if t, ok := parsedTypes[theirBalance.TlvType()]; ok && t == nil { + rl.TheirBalance = tlv.SomeRecordT(theirBalance) } // Read the HTLC entries. diff --git a/channeldb/revocation_log_test.go b/channeldb/revocation_log_test.go index c9d2a16f7..813f70ac9 100644 --- a/channeldb/revocation_log_test.go +++ b/channeldb/revocation_log_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/btcsuite/btcd/btcutil" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lntest/channels" "github.com/lightningnetwork/lnd/lnwire" @@ -115,12 +116,11 @@ var ( }}, } - testRevocationLogNoAmts = RevocationLog{ - OurOutputIndex: 0, - TheirOutputIndex: 1, - CommitTxHash: testChannelCommit.CommitTx.TxHash(), - HTLCEntries: []*HTLCEntry{&testHTLCEntry}, - } + testRevocationLogNoAmts = NewRevocationLog( + 0, 1, testChannelCommit.CommitTx.TxHash(), + fn.None[lnwire.MilliSatoshi](), fn.None[lnwire.MilliSatoshi](), + []*HTLCEntry{&testHTLCEntry}, + ) testRevocationLogNoAmtsBytes = []byte{ // Body length 42. 0x2a, @@ -136,14 +136,11 @@ var ( 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, - } + testRevocationLogWithAmts = NewRevocationLog( + 0, 1, testChannelCommit.CommitTx.TxHash(), + fn.Some(localBalance), fn.Some(remoteBalance), + []*HTLCEntry{&testHTLCEntry}, + ) testRevocationLogWithAmtsBytes = []byte{ // Body length 52. 0x34, diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 61c9fc6bf..a2fe35c2b 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2747,7 +2747,7 @@ func createBreachRetribution(revokedLog *channeldb.RevocationLog, htlcRetributions := make([]HtlcRetribution, len(revokedLog.HTLCEntries)) for i, htlc := range revokedLog.HTLCEntries { hr, err := createHtlcRetribution( - chanState, keyRing, commitHash, + chanState, keyRing, commitHash.Val, commitmentSecret, leaseExpiry, htlc, ) if err != nil { @@ -2760,10 +2760,10 @@ func createBreachRetribution(revokedLog *channeldb.RevocationLog, // Construct the our outpoint. ourOutpoint := wire.OutPoint{ - Hash: commitHash, + Hash: commitHash.Val, } - if revokedLog.OurOutputIndex != channeldb.OutputIndexEmpty { - ourOutpoint.Index = uint32(revokedLog.OurOutputIndex) + if revokedLog.OurOutputIndex.Val != channeldb.OutputIndexEmpty { + ourOutpoint.Index = uint32(revokedLog.OurOutputIndex.Val) // If the spend transaction is provided, then we use it to get // the value of our output. @@ -2786,26 +2786,29 @@ func createBreachRetribution(revokedLog *channeldb.RevocationLog, // 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 + b, err := revokedLog.OurBalance.ValOpt().UnwrapOrErr( + ErrRevLogDataMissing, + ) + if err != nil { + return nil, 0, 0, err } - ourAmt = int64(revokedLog.OurBalance.ToSatoshis()) + ourAmt = int64(b.Int().ToSatoshis()) } } // Construct the their outpoint. theirOutpoint := wire.OutPoint{ - Hash: commitHash, + Hash: commitHash.Val, } - if revokedLog.TheirOutputIndex != channeldb.OutputIndexEmpty { - theirOutpoint.Index = uint32(revokedLog.TheirOutputIndex) + if revokedLog.TheirOutputIndex.Val != channeldb.OutputIndexEmpty { + theirOutpoint.Index = uint32(revokedLog.TheirOutputIndex.Val) // 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) >= + if int(revokedLog.TheirOutputIndex.Val) >= len(spendTx.TxOut) { return nil, 0, 0, fmt.Errorf("%w: theirs=%v, "+ @@ -2823,16 +2826,19 @@ func createBreachRetribution(revokedLog *channeldb.RevocationLog, // 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 + b, err := revokedLog.TheirBalance.ValOpt().UnwrapOrErr( + ErrRevLogDataMissing, + ) + if err != nil { + return nil, 0, 0, err } - theirAmt = int64(revokedLog.TheirBalance.ToSatoshis()) + theirAmt = int64(b.Int().ToSatoshis()) } } return &BreachRetribution{ - BreachTxHash: commitHash, + BreachTxHash: commitHash.Val, ChainHash: chanState.ChainHash, LocalOutpoint: ourOutpoint, RemoteOutpoint: theirOutpoint, diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 84c694c61..bd6b04848 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -21,6 +21,7 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/lightningnetwork/lnd/chainntnfs" "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/fn" "github.com/lightningnetwork/lnd/input" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwallet/chainfee" @@ -10021,22 +10022,19 @@ 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, - } + revokedLog := channeldb.NewRevocationLog( + uint16(localIndex), uint16(remoteIndex), commitHash, + fn.Some(ourAmtMsat), fn.Some(theirAmtMsat), + []*channeldb.HTLCEntry{htlc}, + ) // Create a log with an empty local output index. revokedLogNoLocal := revokedLog - revokedLogNoLocal.OurOutputIndex = channeldb.OutputIndexEmpty + revokedLogNoLocal.OurOutputIndex.Val = channeldb.OutputIndexEmpty // Create a log with an empty remote output index. revokedLogNoRemote := revokedLog - revokedLogNoRemote.TheirOutputIndex = channeldb.OutputIndexEmpty + revokedLogNoRemote.TheirOutputIndex.Val = channeldb.OutputIndexEmpty testCases := []struct { name string @@ -10066,14 +10064,20 @@ func TestCreateBreachRetribution(t *testing.T) { { name: "fail due to our index too big", revocationLog: &channeldb.RevocationLog{ - OurOutputIndex: uint16(htlcIndex + 1), + //nolint:lll + OurOutputIndex: tlv.NewPrimitiveRecord[tlv.TlvType0]( + uint16(htlcIndex + 1), + ), }, expectedErr: ErrOutputIndexOutOfRange, }, { name: "fail due to their index too big", revocationLog: &channeldb.RevocationLog{ - TheirOutputIndex: uint16(htlcIndex + 1), + //nolint:lll + TheirOutputIndex: tlv.NewPrimitiveRecord[tlv.TlvType1]( + uint16(htlcIndex + 1), + ), }, expectedErr: ErrOutputIndexOutOfRange, },