channeldb: convert RevocationLog to use RecordT

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.
@@ -804,10 +806,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
@@ -849,10 +851,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)

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
+type (
-	// revocation logs to the database.
+	// BigSizeAmount is a type alias for a TLV record of a btcutil.Amount.
-	//
+	BigSizeAmount = tlv.BigSizeT[btcutil.Amount]
-	// NOTE: A migration should be added whenever this list changes.
+
-	revLogOurOutputIndexType   tlv.Type = 0
+	// BigSizeMilliSatoshi is a type alias for a TLV record of a
-	revLogTheirOutputIndexType tlv.Type = 1
+	// lnwire.MilliSatoshi.
-	revLogCommitTxHashType     tlv.Type = 2
+	BigSizeMilliSatoshi = tlv.BigSizeT[lnwire.MilliSatoshi]
-	revLogOurBalanceType       tlv.Type = 3
-	revLogTheirBalanceType     tlv.Type = 4
 )
 
 var (
@@ -211,15 +211,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.
@@ -229,21 +229,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
@@ -262,15 +294,26 @@ func putRevocationLog(bucket kvdb.RwBucket, commit *ChannelCommitment,
 	}
 
 	rl := &RevocationLog{
-		OurOutputIndex:   uint16(ourOutputIndex),
+		OurOutputIndex: tlv.NewPrimitiveRecord[tlv.TlvType0](
-		TheirOutputIndex: uint16(theirOutputIndex),
+			uint16(ourOutputIndex),
-		CommitTxHash:     commit.CommitTx.TxHash(),
+		),
+		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.OurBalance = tlv.SomeRecordT(tlv.NewRecordT[tlv.TlvType3](
-		rl.TheirBalance = &commit.RemoteBalance
+			tlv.NewBigSizeT(commit.LocalBalance),
+		))
+
+		rl.TheirBalance = tlv.SomeRecordT(tlv.NewRecordT[tlv.TlvType4](
+			tlv.NewBigSizeT(commit.RemoteBalance),
+		))
 	}
 
 	for _, htlc := range commit.Htlcs {
@@ -320,31 +363,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(
+		rl.OurOutputIndex.Record(),
-			revLogOurOutputIndexType, &rl.OurOutputIndex,
+		rl.TheirOutputIndex.Record(),
-		),
+		rl.CommitTxHash.Record(),
-		tlv.MakePrimitiveRecord(
-			revLogTheirOutputIndexType, &rl.TheirOutputIndex,
-		),
-		tlv.MakePrimitiveRecord(
-			revLogCommitTxHashType, &rl.CommitTxHash,
-		),
 	}
 
 	// Now we add any optional fields that are non-nil.
-	if rl.OurBalance != nil {
+	rl.OurBalance.WhenSome(
-		lb := uint64(*rl.OurBalance)
+		func(r tlv.RecordT[tlv.TlvType3, BigSizeMilliSatoshi]) {
-		records = append(records, tlv.MakeBigSizeRecord(
+			records = append(records, r.Record())
-			revLogOurBalanceType, &lb,
+		},
-		))
+	)
-	}
 
-	if rl.TheirBalance != nil {
+	rl.TheirBalance.WhenSome(
-		rb := uint64(*rl.TheirBalance)
+		func(r tlv.RecordT[tlv.TlvType4, BigSizeMilliSatoshi]) {
-		records = append(records, tlv.MakeBigSizeRecord(
+			records = append(records, r.Record())
-			revLogTheirBalanceType, &rb,
+		},
-		))
+	)
-	}
 
 	// Create the tlv stream.
 	tlvStream, err := tlv.NewStream(records...)
@@ -382,27 +417,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 (
+	var rl RevocationLog
-		rl           RevocationLog
+
-		ourBalance   uint64
+	ourBalance := rl.OurBalance.Zero()
-		theirBalance uint64
+	theirBalance := rl.TheirBalance.Zero()
-	)
 
 	// Create the tlv stream.
 	tlvStream, err := tlv.NewStream(
-		tlv.MakePrimitiveRecord(
+		rl.OurOutputIndex.Record(),
-			revLogOurOutputIndexType, &rl.OurOutputIndex,
+		rl.TheirOutputIndex.Record(),
-		),
+		rl.CommitTxHash.Record(),
-		tlv.MakePrimitiveRecord(
+		ourBalance.Record(),
-			revLogTheirOutputIndexType, &rl.TheirOutputIndex,
+		theirBalance.Record(),
-		),
-		tlv.MakePrimitiveRecord(
-			revLogCommitTxHashType, &rl.CommitTxHash,
-		),
-		tlv.MakeBigSizeRecord(revLogOurBalanceType, &ourBalance),
-		tlv.MakeBigSizeRecord(
-			revLogTheirBalanceType, &theirBalance,
-		),
 	)
 	if err != nil {
 		return rl, err
@@ -414,14 +440,12 @@ func deserializeRevocationLog(r io.Reader) (RevocationLog, error) {
 		return rl, err
 	}
 
-	if t, ok := parsedTypes[revLogOurBalanceType]; ok && t == nil {
+	if t, ok := parsedTypes[ourBalance.TlvType()]; ok && t == nil {
-		lb := lnwire.MilliSatoshi(ourBalance)
+		rl.OurBalance = tlv.SomeRecordT(ourBalance)
-		rl.OurBalance = &lb
 	}
 
-	if t, ok := parsedTypes[revLogTheirBalanceType]; ok && t == nil {
+	if t, ok := parsedTypes[theirBalance.TlvType()]; ok && t == nil {
-		rb := lnwire.MilliSatoshi(theirBalance)
+		rl.TheirBalance = tlv.SomeRecordT(theirBalance)
-		rl.TheirBalance = &rb
 	}
 
 	// Read the HTLC entries.

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"
@@ -81,12 +82,11 @@ var (
 		}},
 	}
 
-	testRevocationLogNoAmts = RevocationLog{
+	testRevocationLogNoAmts = NewRevocationLog(
-		OurOutputIndex:   0,
+		0, 1, testChannelCommit.CommitTx.TxHash(),
-		TheirOutputIndex: 1,
+		fn.None[lnwire.MilliSatoshi](), fn.None[lnwire.MilliSatoshi](),
-		CommitTxHash:     testChannelCommit.CommitTx.TxHash(),
+		[]*HTLCEntry{&testHTLCEntry},
-		HTLCEntries:      []*HTLCEntry{&testHTLCEntry},
+	)
-	}
 	testRevocationLogNoAmtsBytes = []byte{
 		// Body length 42.
 		0x2a,
@@ -102,14 +102,11 @@ var (
 		0xc8, 0x22, 0x51, 0xb1, 0x5b, 0xa0, 0xbf, 0xd,
 	}
 
-	testRevocationLogWithAmts = RevocationLog{
+	testRevocationLogWithAmts = NewRevocationLog(
-		OurOutputIndex:   0,
+		0, 1, testChannelCommit.CommitTx.TxHash(),
-		TheirOutputIndex: 1,
+		fn.Some(localBalance), fn.Some(remoteBalance),
-		CommitTxHash:     testChannelCommit.CommitTx.TxHash(),
+		[]*HTLCEntry{&testHTLCEntry},
-		HTLCEntries:      []*HTLCEntry{&testHTLCEntry},
+	)
-		OurBalance:       &localBalance,
-		TheirBalance:     &remoteBalance,
-	}
 	testRevocationLogWithAmtsBytes = []byte{
 		// Body length 52.
 		0x34,

lnwallet/channel.go

@@ -2746,7 +2746,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 {
@@ -2759,10 +2759,10 @@ func createBreachRetribution(revokedLog *channeldb.RevocationLog,
 
 	// Construct the our outpoint.
 	ourOutpoint := wire.OutPoint{
-		Hash: commitHash,
+		Hash: commitHash.Val,
 	}
-	if revokedLog.OurOutputIndex != channeldb.OutputIndexEmpty {
+	if revokedLog.OurOutputIndex.Val != channeldb.OutputIndexEmpty {
-		ourOutpoint.Index = uint32(revokedLog.OurOutputIndex)
+		ourOutpoint.Index = uint32(revokedLog.OurOutputIndex.Val)
 
 		// If the spend transaction is provided, then we use it to get
 		// the value of our output.
@@ -2785,26 +2785,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 {
+			b, err := revokedLog.OurBalance.ValOpt().UnwrapOrErr(
-				return nil, 0, 0, ErrRevLogDataMissing
+				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 {
+	if revokedLog.TheirOutputIndex.Val != channeldb.OutputIndexEmpty {
-		theirOutpoint.Index = uint32(revokedLog.TheirOutputIndex)
+		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, "+
@@ -2822,16 +2825,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 {
+			b, err := revokedLog.TheirBalance.ValOpt().UnwrapOrErr(
-				return nil, 0, 0, ErrRevLogDataMissing
+				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,

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{
+	revokedLog := channeldb.NewRevocationLog(
-		CommitTxHash:     commitHash,
+		uint16(localIndex), uint16(remoteIndex), commitHash,
-		OurOutputIndex:   uint16(localIndex),
+		fn.Some(ourAmtMsat), fn.Some(theirAmtMsat),
-		TheirOutputIndex: uint16(remoteIndex),
+		[]*channeldb.HTLCEntry{htlc},
-		HTLCEntries:      []*channeldb.HTLCEntry{htlc},
+	)
-		TheirBalance:     &theirAmtMsat,
-		OurBalance:       &ourAmtMsat,
-	}
 
 	// 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,
 		},

tlv/record_type.go

@@ -140,6 +140,15 @@ func (o *OptionalRecordT[T, V]) UnwrapOrErrV(err error) (V, error) {
 	return inner.Val, nil
 }
 
+// ValOpt returns an Option of the underlying value. This can be used to chain
+// other option related methods to avoid needing to first go through the outter
+// record.
+func (t *OptionalRecordT[T, V]) ValOpt() fn.Option[V] {
+	return fn.MapOption(func(record RecordT[T, V]) V {
+		return record.Val
+	})(t.Option)
+}
+
 // Zero returns a zero value of the record type.
 func (t *OptionalRecordT[T, V]) Zero() RecordT[T, V] {
 	return ZeroRecordT[T, V]()