diff --git a/go.mod b/go.mod index 60af5fb3de..3a559abb68 100644 --- a/go.mod +++ b/go.mod @@ -35,7 +35,7 @@ require ( github.com/lightningnetwork/lightning-onion v1.2.1-0.20240712235311-98bd56499dfb github.com/lightningnetwork/lnd/cert v1.2.2 github.com/lightningnetwork/lnd/clock v1.1.1 - github.com/lightningnetwork/lnd/fn v1.2.1 + github.com/lightningnetwork/lnd/fn v1.2.2 github.com/lightningnetwork/lnd/healthcheck v1.2.5 github.com/lightningnetwork/lnd/kvdb v1.4.10 github.com/lightningnetwork/lnd/queue v1.1.1 diff --git a/lnwallet/aux_signer.go b/lnwallet/aux_signer.go index 5d4bc79241..01abe1aae3 100644 --- a/lnwallet/aux_signer.go +++ b/lnwallet/aux_signer.go @@ -109,10 +109,10 @@ func newAuxHtlcDescriptor(p *paymentDescriptor) AuxHtlcDescriptor { ParentIndex: p.ParentIndex, EntryType: p.EntryType, CustomRecords: p.CustomRecords.Copy(), - addCommitHeightRemote: p.addCommitHeightRemote, - addCommitHeightLocal: p.addCommitHeightLocal, - removeCommitHeightRemote: p.removeCommitHeightRemote, - removeCommitHeightLocal: p.removeCommitHeightLocal, + addCommitHeightRemote: p.addCommitHeights.Remote, + addCommitHeightLocal: p.addCommitHeights.Local, + removeCommitHeightRemote: p.removeCommitHeights.Remote, + removeCommitHeightLocal: p.removeCommitHeights.Local, } } diff --git a/lnwallet/channel.go b/lnwallet/channel.go index f3e0769506..58e02d502c 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1080,17 +1080,19 @@ func (lc *LightningChannel) logUpdateToPayDesc(logUpdate *channeldb.LogUpdate, // as we've included this HTLC in our local commitment chain // for the remote party. pd = &paymentDescriptor{ - ChanID: wireMsg.ChanID, - RHash: wireMsg.PaymentHash, - Timeout: wireMsg.Expiry, - Amount: wireMsg.Amount, - EntryType: Add, - HtlcIndex: wireMsg.ID, - LogIndex: logUpdate.LogIndex, - addCommitHeightRemote: commitHeight, - OnionBlob: wireMsg.OnionBlob, - BlindingPoint: wireMsg.BlindingPoint, - CustomRecords: wireMsg.CustomRecords.Copy(), + ChanID: wireMsg.ChanID, + RHash: wireMsg.PaymentHash, + Timeout: wireMsg.Expiry, + Amount: wireMsg.Amount, + EntryType: Add, + HtlcIndex: wireMsg.ID, + LogIndex: logUpdate.LogIndex, + OnionBlob: wireMsg.OnionBlob, + BlindingPoint: wireMsg.BlindingPoint, + CustomRecords: wireMsg.CustomRecords.Copy(), + addCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, } isDustRemote := HtlcIsDust( @@ -1125,14 +1127,16 @@ func (lc *LightningChannel) logUpdateToPayDesc(logUpdate *channeldb.LogUpdate, ogHTLC := remoteUpdateLog.lookupHtlc(wireMsg.ID) pd = &paymentDescriptor{ - ChanID: wireMsg.ChanID, - Amount: ogHTLC.Amount, - RHash: ogHTLC.RHash, - RPreimage: wireMsg.PaymentPreimage, - LogIndex: logUpdate.LogIndex, - ParentIndex: ogHTLC.HtlcIndex, - EntryType: Settle, - removeCommitHeightRemote: commitHeight, + ChanID: wireMsg.ChanID, + Amount: ogHTLC.Amount, + RHash: ogHTLC.RHash, + RPreimage: wireMsg.PaymentPreimage, + LogIndex: logUpdate.LogIndex, + ParentIndex: ogHTLC.HtlcIndex, + EntryType: Settle, + removeCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, } // If we sent a failure for a prior incoming HTLC, then we'll consult @@ -1143,14 +1147,16 @@ func (lc *LightningChannel) logUpdateToPayDesc(logUpdate *channeldb.LogUpdate, ogHTLC := remoteUpdateLog.lookupHtlc(wireMsg.ID) pd = &paymentDescriptor{ - ChanID: wireMsg.ChanID, - Amount: ogHTLC.Amount, - RHash: ogHTLC.RHash, - ParentIndex: ogHTLC.HtlcIndex, - LogIndex: logUpdate.LogIndex, - EntryType: Fail, - FailReason: wireMsg.Reason[:], - removeCommitHeightRemote: commitHeight, + ChanID: wireMsg.ChanID, + Amount: ogHTLC.Amount, + RHash: ogHTLC.RHash, + ParentIndex: ogHTLC.HtlcIndex, + LogIndex: logUpdate.LogIndex, + EntryType: Fail, + FailReason: wireMsg.Reason[:], + removeCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, } // HTLC fails due to malformed onion blobs are treated the exact same @@ -1160,15 +1166,17 @@ func (lc *LightningChannel) logUpdateToPayDesc(logUpdate *channeldb.LogUpdate, // TODO(roasbeef): err if nil? pd = &paymentDescriptor{ - ChanID: wireMsg.ChanID, - Amount: ogHTLC.Amount, - RHash: ogHTLC.RHash, - ParentIndex: ogHTLC.HtlcIndex, - LogIndex: logUpdate.LogIndex, - EntryType: MalformedFail, - FailCode: wireMsg.FailureCode, - ShaOnionBlob: wireMsg.ShaOnionBlob, - removeCommitHeightRemote: commitHeight, + ChanID: wireMsg.ChanID, + Amount: ogHTLC.Amount, + RHash: ogHTLC.RHash, + ParentIndex: ogHTLC.HtlcIndex, + LogIndex: logUpdate.LogIndex, + EntryType: MalformedFail, + FailCode: wireMsg.FailureCode, + ShaOnionBlob: wireMsg.ShaOnionBlob, + removeCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, } // For fee updates we'll create a FeeUpdate type to add to the log. We @@ -1184,9 +1192,13 @@ func (lc *LightningChannel) logUpdateToPayDesc(logUpdate *channeldb.LogUpdate, Amount: lnwire.NewMSatFromSatoshis( btcutil.Amount(wireMsg.FeePerKw), ), - EntryType: FeeUpdate, - addCommitHeightRemote: commitHeight, - removeCommitHeightRemote: commitHeight, + EntryType: FeeUpdate, + addCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, + removeCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, } } @@ -1216,14 +1228,16 @@ func (lc *LightningChannel) localLogUpdateToPayDesc(logUpdate *channeldb.LogUpda ogHTLC := remoteUpdateLog.lookupHtlc(wireMsg.ID) return &paymentDescriptor{ - ChanID: wireMsg.ChanID, - Amount: ogHTLC.Amount, - RHash: ogHTLC.RHash, - RPreimage: wireMsg.PaymentPreimage, - LogIndex: logUpdate.LogIndex, - ParentIndex: ogHTLC.HtlcIndex, - EntryType: Settle, - removeCommitHeightRemote: commitHeight, + ChanID: wireMsg.ChanID, + Amount: ogHTLC.Amount, + RHash: ogHTLC.RHash, + RPreimage: wireMsg.PaymentPreimage, + LogIndex: logUpdate.LogIndex, + ParentIndex: ogHTLC.HtlcIndex, + EntryType: Settle, + removeCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, }, nil // If we sent a failure for a prior incoming HTLC, then we'll consult the @@ -1233,14 +1247,16 @@ func (lc *LightningChannel) localLogUpdateToPayDesc(logUpdate *channeldb.LogUpda ogHTLC := remoteUpdateLog.lookupHtlc(wireMsg.ID) return &paymentDescriptor{ - ChanID: wireMsg.ChanID, - Amount: ogHTLC.Amount, - RHash: ogHTLC.RHash, - ParentIndex: ogHTLC.HtlcIndex, - LogIndex: logUpdate.LogIndex, - EntryType: Fail, - FailReason: wireMsg.Reason[:], - removeCommitHeightRemote: commitHeight, + ChanID: wireMsg.ChanID, + Amount: ogHTLC.Amount, + RHash: ogHTLC.RHash, + ParentIndex: ogHTLC.HtlcIndex, + LogIndex: logUpdate.LogIndex, + EntryType: Fail, + FailReason: wireMsg.Reason[:], + removeCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, }, nil // HTLC fails due to malformed onion blocks are treated the exact same @@ -1249,15 +1265,17 @@ func (lc *LightningChannel) localLogUpdateToPayDesc(logUpdate *channeldb.LogUpda ogHTLC := remoteUpdateLog.lookupHtlc(wireMsg.ID) return &paymentDescriptor{ - ChanID: wireMsg.ChanID, - Amount: ogHTLC.Amount, - RHash: ogHTLC.RHash, - ParentIndex: ogHTLC.HtlcIndex, - LogIndex: logUpdate.LogIndex, - EntryType: MalformedFail, - FailCode: wireMsg.FailureCode, - ShaOnionBlob: wireMsg.ShaOnionBlob, - removeCommitHeightRemote: commitHeight, + ChanID: wireMsg.ChanID, + Amount: ogHTLC.Amount, + RHash: ogHTLC.RHash, + ParentIndex: ogHTLC.HtlcIndex, + LogIndex: logUpdate.LogIndex, + EntryType: MalformedFail, + FailCode: wireMsg.FailureCode, + ShaOnionBlob: wireMsg.ShaOnionBlob, + removeCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, }, nil case *lnwire.UpdateFee: @@ -1267,9 +1285,13 @@ func (lc *LightningChannel) localLogUpdateToPayDesc(logUpdate *channeldb.LogUpda Amount: lnwire.NewMSatFromSatoshis( btcutil.Amount(wireMsg.FeePerKw), ), - EntryType: FeeUpdate, - addCommitHeightRemote: commitHeight, - removeCommitHeightRemote: commitHeight, + EntryType: FeeUpdate, + addCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, + removeCommitHeights: lntypes.Dual[uint64]{ + Remote: commitHeight, + }, }, nil default: @@ -1294,17 +1316,19 @@ func (lc *LightningChannel) remoteLogUpdateToPayDesc(logUpdate *channeldb.LogUpd switch wireMsg := logUpdate.UpdateMsg.(type) { case *lnwire.UpdateAddHTLC: pd := &paymentDescriptor{ - ChanID: wireMsg.ChanID, - RHash: wireMsg.PaymentHash, - Timeout: wireMsg.Expiry, - Amount: wireMsg.Amount, - EntryType: Add, - HtlcIndex: wireMsg.ID, - LogIndex: logUpdate.LogIndex, - addCommitHeightLocal: commitHeight, - OnionBlob: wireMsg.OnionBlob, - BlindingPoint: wireMsg.BlindingPoint, - CustomRecords: wireMsg.CustomRecords.Copy(), + ChanID: wireMsg.ChanID, + RHash: wireMsg.PaymentHash, + Timeout: wireMsg.Expiry, + Amount: wireMsg.Amount, + EntryType: Add, + HtlcIndex: wireMsg.ID, + LogIndex: logUpdate.LogIndex, + OnionBlob: wireMsg.OnionBlob, + BlindingPoint: wireMsg.BlindingPoint, + CustomRecords: wireMsg.CustomRecords.Copy(), + addCommitHeights: lntypes.Dual[uint64]{ + Local: commitHeight, + }, } // We don't need to generate an htlc script yet. This will be @@ -1319,14 +1343,16 @@ func (lc *LightningChannel) remoteLogUpdateToPayDesc(logUpdate *channeldb.LogUpd ogHTLC := localUpdateLog.lookupHtlc(wireMsg.ID) return &paymentDescriptor{ - ChanID: wireMsg.ChanID, - Amount: ogHTLC.Amount, - RHash: ogHTLC.RHash, - RPreimage: wireMsg.PaymentPreimage, - LogIndex: logUpdate.LogIndex, - ParentIndex: ogHTLC.HtlcIndex, - EntryType: Settle, - removeCommitHeightLocal: commitHeight, + ChanID: wireMsg.ChanID, + Amount: ogHTLC.Amount, + RHash: ogHTLC.RHash, + RPreimage: wireMsg.PaymentPreimage, + LogIndex: logUpdate.LogIndex, + ParentIndex: ogHTLC.HtlcIndex, + EntryType: Settle, + removeCommitHeights: lntypes.Dual[uint64]{ + Local: commitHeight, + }, }, nil // If we received a failure for a prior outgoing HTLC, then we'll @@ -1336,14 +1362,16 @@ func (lc *LightningChannel) remoteLogUpdateToPayDesc(logUpdate *channeldb.LogUpd ogHTLC := localUpdateLog.lookupHtlc(wireMsg.ID) return &paymentDescriptor{ - ChanID: wireMsg.ChanID, - Amount: ogHTLC.Amount, - RHash: ogHTLC.RHash, - ParentIndex: ogHTLC.HtlcIndex, - LogIndex: logUpdate.LogIndex, - EntryType: Fail, - FailReason: wireMsg.Reason[:], - removeCommitHeightLocal: commitHeight, + ChanID: wireMsg.ChanID, + Amount: ogHTLC.Amount, + RHash: ogHTLC.RHash, + ParentIndex: ogHTLC.HtlcIndex, + LogIndex: logUpdate.LogIndex, + EntryType: Fail, + FailReason: wireMsg.Reason[:], + removeCommitHeights: lntypes.Dual[uint64]{ + Local: commitHeight, + }, }, nil // HTLC fails due to malformed onion blobs are treated the exact same @@ -1352,15 +1380,17 @@ func (lc *LightningChannel) remoteLogUpdateToPayDesc(logUpdate *channeldb.LogUpd ogHTLC := localUpdateLog.lookupHtlc(wireMsg.ID) return &paymentDescriptor{ - ChanID: wireMsg.ChanID, - Amount: ogHTLC.Amount, - RHash: ogHTLC.RHash, - ParentIndex: ogHTLC.HtlcIndex, - LogIndex: logUpdate.LogIndex, - EntryType: MalformedFail, - FailCode: wireMsg.FailureCode, - ShaOnionBlob: wireMsg.ShaOnionBlob, - removeCommitHeightLocal: commitHeight, + ChanID: wireMsg.ChanID, + Amount: ogHTLC.Amount, + RHash: ogHTLC.RHash, + ParentIndex: ogHTLC.HtlcIndex, + LogIndex: logUpdate.LogIndex, + EntryType: MalformedFail, + FailCode: wireMsg.FailureCode, + ShaOnionBlob: wireMsg.ShaOnionBlob, + removeCommitHeights: lntypes.Dual[uint64]{ + Local: commitHeight, + }, }, nil // For fee updates we'll create a FeeUpdate type to add to the log. We @@ -1376,9 +1406,13 @@ func (lc *LightningChannel) remoteLogUpdateToPayDesc(logUpdate *channeldb.LogUpd Amount: lnwire.NewMSatFromSatoshis( btcutil.Amount(wireMsg.FeePerKw), ), - EntryType: FeeUpdate, - addCommitHeightLocal: commitHeight, - removeCommitHeightLocal: commitHeight, + EntryType: FeeUpdate, + addCommitHeights: lntypes.Dual[uint64]{ + Local: commitHeight, + }, + removeCommitHeights: lntypes.Dual[uint64]{ + Local: commitHeight, + }, }, nil default: @@ -1611,8 +1645,9 @@ func (lc *LightningChannel) restoreStateLogs( // map we created earlier. Note that if this HTLC is not in // incomingRemoteAddHeights, the remote add height will be set // to zero, which indicates that it is not added yet. - htlc.addCommitHeightLocal = localCommitment.height - htlc.addCommitHeightRemote = incomingRemoteAddHeights[htlc.HtlcIndex] + htlc.addCommitHeights.Local = localCommitment.height + htlc.addCommitHeights.Remote = + incomingRemoteAddHeights[htlc.HtlcIndex] // Restore the htlc back to the remote log. lc.updateLogs.Remote.restoreHtlc(&htlc) @@ -1626,8 +1661,9 @@ func (lc *LightningChannel) restoreStateLogs( // As for the incoming HTLCs, we'll use the current remote // commit height as remote add height, and consult the map // created above for the local add height. - htlc.addCommitHeightRemote = remoteCommitment.height - htlc.addCommitHeightLocal = outgoingLocalAddHeights[htlc.HtlcIndex] + htlc.addCommitHeights.Remote = remoteCommitment.height + htlc.addCommitHeights.Local = + outgoingLocalAddHeights[htlc.HtlcIndex] // Restore the htlc back to the local log. lc.updateLogs.Local.restoreHtlc(&htlc) @@ -1722,15 +1758,15 @@ func (lc *LightningChannel) restorePendingRemoteUpdates( switch payDesc.EntryType { case FeeUpdate: if heightSet { - payDesc.addCommitHeightRemote = height - payDesc.removeCommitHeightRemote = height + payDesc.addCommitHeights.Remote = height + payDesc.removeCommitHeights.Remote = height } lc.updateLogs.Remote.restoreUpdate(payDesc) default: if heightSet { - payDesc.removeCommitHeightRemote = height + payDesc.removeCommitHeights.Remote = height } lc.updateLogs.Remote.restoreUpdate(payDesc) @@ -2639,11 +2675,8 @@ type HtlcView struct { // created using this view. NextHeight uint64 - // OurUpdates are our outgoing HTLCs. - OurUpdates []*paymentDescriptor - - // TheirUpdates are their incoming HTLCs. - TheirUpdates []*paymentDescriptor + // Updates is a Dual of the Local and Remote HTLCs. + Updates lntypes.Dual[[]*paymentDescriptor] // FeePerKw is the fee rate in sat/kw of the commitment transaction. FeePerKw chainfee.SatPerKWeight @@ -2652,13 +2685,13 @@ type HtlcView struct { // AuxOurUpdates returns the outgoing HTLCs as a read-only copy of // AuxHtlcDescriptors. func (v *HtlcView) AuxOurUpdates() []AuxHtlcDescriptor { - return fn.Map(newAuxHtlcDescriptor, v.OurUpdates) + return fn.Map(newAuxHtlcDescriptor, v.Updates.Local) } // AuxTheirUpdates returns the incoming HTLCs as a read-only copy of // AuxHtlcDescriptors. func (v *HtlcView) AuxTheirUpdates() []AuxHtlcDescriptor { - return fn.Map(newAuxHtlcDescriptor, v.TheirUpdates) + return fn.Map(newAuxHtlcDescriptor, v.Updates.Remote) } // fetchHTLCView returns all the candidate HTLC updates which should be @@ -2692,8 +2725,10 @@ func (lc *LightningChannel) fetchHTLCView(theirLogIndex, } return &HtlcView{ - OurUpdates: ourHTLCs, - TheirUpdates: theirHTLCs, + Updates: lntypes.Dual[[]*paymentDescriptor]{ + Local: ourHTLCs, + Remote: theirHTLCs, + }, } } @@ -2817,15 +2852,15 @@ func (lc *LightningChannel) fetchCommitmentView( // commitment are mutated, we'll manually copy over each HTLC to its // respective slice. c.outgoingHTLCs = make( - []paymentDescriptor, len(filteredHTLCView.OurUpdates), + []paymentDescriptor, len(filteredHTLCView.Updates.Local), ) - for i, htlc := range filteredHTLCView.OurUpdates { + for i, htlc := range filteredHTLCView.Updates.Local { c.outgoingHTLCs[i] = *htlc } c.incomingHTLCs = make( - []paymentDescriptor, len(filteredHTLCView.TheirUpdates), + []paymentDescriptor, len(filteredHTLCView.Updates.Remote), ) - for i, htlc := range filteredHTLCView.TheirUpdates { + for i, htlc := range filteredHTLCView.Updates.Remote { c.incomingHTLCs[i] = *htlc } @@ -2854,16 +2889,13 @@ func fundingTxIn(chanState *channeldb.OpenChannel) wire.TxIn { // returned reflects the current state of HTLCs within the remote or local // commitment chain, and the current commitment fee rate. // -// If mutateState is set to true, then the add height of all added HTLCs -// will be set to nextHeight, and the remove height of all removed HTLCs -// will be set to nextHeight. This should therefore only be set to true -// once for each height, and only in concert with signing a new commitment. -// TODO(halseth): return htlcs to mutate instead of mutating inside -// method. -func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, ourBalance, - theirBalance *lnwire.MilliSatoshi, nextHeight uint64, - whoseCommitChain lntypes.ChannelParty, mutateState bool) (*HtlcView, - error) { +// The return values of this function are as follows: +// 1. The new htlcView reflecting the current channel state. +// 2. A Dual of the updates which have not yet been committed in +// 'whoseCommitChain's commitment chain. +func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, + whoseCommitChain lntypes.ChannelParty, nextHeight uint64) (*HtlcView, + lntypes.Dual[[]*paymentDescriptor], lntypes.Dual[int64], error) { // We initialize the view's fee rate to the fee rate of the unfiltered // view. If any fee updates are found when evaluating the view, it will @@ -2872,128 +2904,162 @@ func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, ourBalance, FeePerKw: view.FeePerKw, NextHeight: nextHeight, } + noUncommitted := lntypes.Dual[[]*paymentDescriptor]{} + + // The fee rate of our view is always the last UpdateFee message from + // the channel's OpeningParty. + openerUpdates := view.Updates.GetForParty(lc.channelState.Initiator()) + feeUpdates := fn.Filter(func(u *paymentDescriptor) bool { + return u.EntryType == FeeUpdate + }, openerUpdates) + lastFeeUpdate := fn.Last(feeUpdates) + lastFeeUpdate.WhenSome(func(pd *paymentDescriptor) { + newView.FeePerKw = chainfee.SatPerKWeight( + pd.Amount.ToSatoshis(), + ) + }) // We use two maps, one for the local log and one for the remote log to // keep track of which entries we need to skip when creating the final // htlc view. We skip an entry whenever we find a settle or a timeout // modifying an entry. - skipUs := make(map[uint64]struct{}) - skipThem := make(map[uint64]struct{}) - - // First we run through non-add entries in both logs, populating the - // skip sets and mutating the current chain state (crediting balances, - // etc) to reflect the settle/timeout entry encountered. - for _, entry := range view.OurUpdates { - switch entry.EntryType { - // Skip adds for now. They will be processed below. - case Add: - continue + skip := lntypes.Dual[fn.Set[uint64]]{ + Local: fn.NewSet[uint64](), + Remote: fn.NewSet[uint64](), + } + + balanceDeltas := lntypes.Dual[int64]{} + + parties := [2]lntypes.ChannelParty{lntypes.Local, lntypes.Remote} + for _, party := range parties { + // First we run through non-add entries in both logs, + // populating the skip sets and mutating the current chain + // state (crediting balances, etc) to reflect the + // settle/timeout entry encountered. + resolutions := fn.Filter(func(pd *paymentDescriptor) bool { + switch pd.EntryType { + case Settle, Fail, MalformedFail: + return true + default: + return false + } + }, view.Updates.GetForParty(party)) - // Process fee updates, updating the current feePerKw. - case FeeUpdate: - processFeeUpdate( - entry, nextHeight, whoseCommitChain, - mutateState, newView, + for _, entry := range resolutions { + addEntry, err := lc.fetchParent( + entry, whoseCommitChain, party.CounterParty(), ) - continue - } + if err != nil { + noDeltas := lntypes.Dual[int64]{} + return nil, noUncommitted, noDeltas, err + } - // If we're settling an inbound HTLC, and it hasn't been - // processed yet, then increment our state tracking the total - // number of satoshis we've received within the channel. - if mutateState && entry.EntryType == Settle && - whoseCommitChain.IsLocal() && - entry.removeCommitHeightLocal == 0 { + skipSet := skip.GetForParty(party.CounterParty()) + skipSet.Add(addEntry.HtlcIndex) + + rmvHeights := &entry.removeCommitHeights + rmvHeight := rmvHeights.GetForParty(whoseCommitChain) + if rmvHeight == 0 { + switch { + // If an incoming HTLC is being settled, then + // this means that the preimage has been + // received by the settling party Therefore, we + // increase the settling party's balance by the + // HTLC amount. + case entry.EntryType == Settle: + delta := int64(entry.Amount) + balanceDeltas.ModifyForParty( + party, + func(acc int64) int64 { + return acc + delta + }, + ) - lc.channelState.TotalMSatReceived += entry.Amount + // Otherwise, this HTLC is being failed out, + // therefore the value of the HTLC should + // return to the failing party's counterparty. + case entry.EntryType != Settle: + delta := int64(entry.Amount) + balanceDeltas.ModifyForParty( + party.CounterParty(), + func(acc int64) int64 { + return acc + delta + }, + ) + } + } } + } - addEntry, err := lc.fetchParent( - entry, whoseCommitChain, lntypes.Remote, - ) - if err != nil { - return nil, err + // Next we take a second pass through all the log entries, skipping any + // settled HTLCs, and debiting the chain state balance due to any newly + // added HTLCs. + for _, party := range parties { + liveAdds := fn.Filter(func(pd *paymentDescriptor) bool { + return pd.EntryType == Add && + !skip.GetForParty(party).Contains(pd.HtlcIndex) + }, view.Updates.GetForParty(party)) + + for _, entry := range liveAdds { + // Skip the entries that have already had their add + // commit height set for this commit chain. + addHeights := &entry.addCommitHeights + addHeight := addHeights.GetForParty(whoseCommitChain) + if addHeight == 0 { + // If this is a new incoming (un-committed) + // HTLC, then we need to update their balance + // accordingly by subtracting the amount of + // the HTLC that are funds pending. + // Similarly, we need to debit our balance if + // this is an out going HTLC to reflect the + // pending balance. + balanceDeltas.ModifyForParty( + party, + func(acc int64) int64 { + return acc - int64(entry.Amount) + }, + ) + } } - skipThem[addEntry.HtlcIndex] = struct{}{} - - processRemoveEntry( - entry, ourBalance, theirBalance, nextHeight, - whoseCommitChain, true, mutateState, - ) + newView.Updates.SetForParty(party, liveAdds) } - for _, entry := range view.TheirUpdates { - switch entry.EntryType { - // Skip adds for now. They will be processed below. + + // Create a function that is capable of identifying whether or not the + // paymentDescriptor has been committed in the commitment chain + // corresponding to whoseCommitmentChain. + isUncommitted := func(update *paymentDescriptor) bool { + switch update.EntryType { case Add: - continue + return 0 == update.addCommitHeights.GetForParty( + whoseCommitChain, + ) - // Process fee updates, updating the current feePerKw. case FeeUpdate: - processFeeUpdate( - entry, nextHeight, whoseCommitChain, - mutateState, newView, + return 0 == update.addCommitHeights.GetForParty( + whoseCommitChain, ) - continue - } - // If the remote party is settling one of our outbound HTLC's, - // and it hasn't been processed, yet, the increment our state - // tracking the total number of satoshis we've sent within the - // channel. - if mutateState && entry.EntryType == Settle && - whoseCommitChain.IsLocal() && - entry.removeCommitHeightLocal == 0 { - - lc.channelState.TotalMSatSent += entry.Amount - } - - addEntry, err := lc.fetchParent( - entry, whoseCommitChain, lntypes.Local, - ) - if err != nil { - return nil, err - } - - skipUs[addEntry.HtlcIndex] = struct{}{} - - processRemoveEntry( - entry, ourBalance, theirBalance, nextHeight, - whoseCommitChain, false, mutateState, - ) - } + case Settle, Fail, MalformedFail: + return 0 == update.removeCommitHeights.GetForParty( + whoseCommitChain, + ) - // Next we take a second pass through all the log entries, skipping any - // settled HTLCs, and debiting the chain state balance due to any newly - // added HTLCs. - for _, entry := range view.OurUpdates { - isAdd := entry.EntryType == Add - if _, ok := skipUs[entry.HtlcIndex]; !isAdd || ok { - continue + default: + panic("invalid paymentDescriptor EntryType") } - - processAddEntry( - entry, ourBalance, theirBalance, nextHeight, - whoseCommitChain, false, mutateState, - ) - - newView.OurUpdates = append(newView.OurUpdates, entry) } - for _, entry := range view.TheirUpdates { - isAdd := entry.EntryType == Add - if _, ok := skipThem[entry.HtlcIndex]; !isAdd || ok { - continue - } - processAddEntry( - entry, ourBalance, theirBalance, nextHeight, - whoseCommitChain, true, mutateState, - ) - - newView.TheirUpdates = append(newView.TheirUpdates, entry) - } + // Collect all of the updates that haven't had their commit heights sent + // for the commitment chain corresponding to whoseCommitmentChain. + uncommittedUpdates := lntypes.MapDual( + view.Updates, + func(us []*paymentDescriptor) []*paymentDescriptor { + return fn.Filter(isUncommitted, us) + }, + ) - return newView, nil + return newView, uncommittedUpdates, balanceDeltas, nil } // fetchParent is a helper that looks up update log parent entries in the @@ -3031,146 +3097,15 @@ func (lc *LightningChannel) fetchParent(entry *paymentDescriptor, // The parent add height should never be zero at this point. If // that's the case we probably forgot to send a new commitment. - case whoseCommitChain.IsRemote() && - addEntry.addCommitHeightRemote == 0: - + case addEntry.addCommitHeights.GetForParty(whoseCommitChain) == 0: return nil, fmt.Errorf("parent entry %d for update %d "+ - "had zero remote add height", entry.ParentIndex, - entry.LogIndex) - - case whoseCommitChain.IsLocal() && - addEntry.addCommitHeightLocal == 0: - - return nil, fmt.Errorf("parent entry %d for update %d "+ - "had zero local add height", entry.ParentIndex, - entry.LogIndex) + "had zero %v add height", entry.ParentIndex, + entry.LogIndex, whoseCommitChain) } return addEntry, nil } -// processAddEntry evaluates the effect of an add entry within the HTLC log. -// If the HTLC hasn't yet been committed in either chain, then the height it -// was committed is updated. Keeping track of this inclusion height allows us to -// later compact the log once the change is fully committed in both chains. -func processAddEntry(htlc *paymentDescriptor, ourBalance, - theirBalance *lnwire.MilliSatoshi, nextHeight uint64, - whoseCommitChain lntypes.ChannelParty, isIncoming, mutateState bool) { - - // If we're evaluating this entry for the remote chain (to create/view - // a new commitment), then we'll may be updating the height this entry - // was added to the chain. Otherwise, we may be updating the entry's - // height w.r.t the local chain. - var addHeight *uint64 - if whoseCommitChain.IsRemote() { - addHeight = &htlc.addCommitHeightRemote - } else { - addHeight = &htlc.addCommitHeightLocal - } - - if *addHeight != 0 { - return - } - - if isIncoming { - // If this is a new incoming (un-committed) HTLC, then we need - // to update their balance accordingly by subtracting the - // amount of the HTLC that are funds pending. - *theirBalance -= htlc.Amount - } else { - // Similarly, we need to debit our balance if this is an out - // going HTLC to reflect the pending balance. - *ourBalance -= htlc.Amount - } - - if mutateState { - *addHeight = nextHeight - } -} - -// processRemoveEntry processes a log entry which settles or times out a -// previously added HTLC. If the removal entry has already been processed, it -// is skipped. -func processRemoveEntry(htlc *paymentDescriptor, ourBalance, - theirBalance *lnwire.MilliSatoshi, nextHeight uint64, - whoseCommitChain lntypes.ChannelParty, isIncoming, mutateState bool) { - - var removeHeight *uint64 - if whoseCommitChain.IsRemote() { - removeHeight = &htlc.removeCommitHeightRemote - } else { - removeHeight = &htlc.removeCommitHeightLocal - } - - // Ignore any removal entries which have already been processed. - if *removeHeight != 0 { - return - } - - switch { - // If an incoming HTLC is being settled, then this means that we've - // received the preimage either from another subsystem, or the - // upstream peer in the route. Therefore, we increase our balance by - // the HTLC amount. - case isIncoming && htlc.EntryType == Settle: - *ourBalance += htlc.Amount - - // Otherwise, this HTLC is being failed out, therefore the value of the - // HTLC should return to the remote party. - case isIncoming && (htlc.EntryType == Fail || htlc.EntryType == MalformedFail): - *theirBalance += htlc.Amount - - // If an outgoing HTLC is being settled, then this means that the - // downstream party resented the preimage or learned of it via a - // downstream peer. In either case, we credit their settled value with - // the value of the HTLC. - case !isIncoming && htlc.EntryType == Settle: - *theirBalance += htlc.Amount - - // Otherwise, one of our outgoing HTLC's has timed out, so the value of - // the HTLC should be returned to our settled balance. - case !isIncoming && (htlc.EntryType == Fail || htlc.EntryType == MalformedFail): - *ourBalance += htlc.Amount - } - - if mutateState { - *removeHeight = nextHeight - } -} - -// processFeeUpdate processes a log update that updates the current commitment -// fee. -func processFeeUpdate(feeUpdate *paymentDescriptor, nextHeight uint64, - whoseCommitChain lntypes.ChannelParty, mutateState bool, - view *HtlcView) { - - // Fee updates are applied for all commitments after they are - // sent/received, so we consider them being added and removed at the - // same height. - var addHeight *uint64 - var removeHeight *uint64 - if whoseCommitChain.IsRemote() { - addHeight = &feeUpdate.addCommitHeightRemote - removeHeight = &feeUpdate.removeCommitHeightRemote - } else { - addHeight = &feeUpdate.addCommitHeightLocal - removeHeight = &feeUpdate.removeCommitHeightLocal - } - - if *addHeight != 0 { - return - } - - // If the update wasn't already locked in, update the current fee rate - // to reflect this update. - view.FeePerKw = chainfee.SatPerKWeight(feeUpdate.Amount.ToSatoshis()) - - if mutateState { - *addHeight = nextHeight - *removeHeight = nextHeight - } -} - // generateRemoteHtlcSigJobs generates a series of HTLC signature jobs for the // sig pool, along with a channel that if closed, will cancel any jobs after // they have been submitted to the sigPool. This method is to be used when @@ -3419,8 +3354,8 @@ func (lc *LightningChannel) createCommitDiff(newCommit *commitment, // If this entry wasn't committed at the exact height of this // remote commitment, then we'll skip it as it was already // lingering in the log. - if pd.addCommitHeightRemote != newCommit.height && - pd.removeCommitHeightRemote != newCommit.height { + if pd.addCommitHeights.Remote != newCommit.height && + pd.removeCommitHeights.Remote != newCommit.height { continue } @@ -3734,10 +3669,12 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, // appropriate update log, in order to validate the sanity of the // commitment resulting from _actually adding_ this HTLC to the state. if predictOurAdd != nil { - view.OurUpdates = append(view.OurUpdates, predictOurAdd) + view.Updates.Local = append(view.Updates.Local, predictOurAdd) } if predictTheirAdd != nil { - view.TheirUpdates = append(view.TheirUpdates, predictTheirAdd) + view.Updates.Remote = append( + view.Updates.Remote, predictTheirAdd, + ) } ourBalance, theirBalance, commitWeight, filteredView, err := lc.computeView( @@ -3892,7 +3829,7 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, // First check that the remote updates won't violate it's channel // constraints. err = validateUpdates( - filteredView.TheirUpdates, &lc.channelState.RemoteChanCfg, + filteredView.Updates.Remote, &lc.channelState.RemoteChanCfg, ) if err != nil { return err @@ -3901,7 +3838,7 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, // Secondly check that our updates won't violate our channel // constraints. err = validateUpdates( - filteredView.OurUpdates, &lc.channelState.LocalChanCfg, + filteredView.Updates.Local, &lc.channelState.LocalChanCfg, ) if err != nil { return err @@ -4643,13 +4580,73 @@ func (lc *LightningChannel) computeView(view *HtlcView, // channel constraints to the final commitment state. If any fee // updates are found in the logs, the commitment fee rate should be // changed, so we'll also set the feePerKw to this new value. - filteredHTLCView, err := lc.evaluateHTLCView( - view, &ourBalance, &theirBalance, nextHeight, whoseCommitChain, - updateState, + filteredHTLCView, uncommitted, deltas, err := lc.evaluateHTLCView( + view, whoseCommitChain, nextHeight, ) if err != nil { return 0, 0, 0, nil, err } + + // Add the balance deltas to the balances we got from the commitment + // state. + if deltas.Local >= 0 { + ourBalance += lnwire.MilliSatoshi(deltas.Local) + } else { + ourBalance -= lnwire.MilliSatoshi(-1 * deltas.Local) + } + if deltas.Remote >= 0 { + theirBalance += lnwire.MilliSatoshi(deltas.Remote) + } else { + theirBalance -= lnwire.MilliSatoshi(-1 * deltas.Remote) + } + + if updateState { + state := lc.channelState + received := &state.TotalMSatReceived + sent := &state.TotalMSatSent + + setHeights := func(u *paymentDescriptor) { + switch u.EntryType { + case Add: + u.addCommitHeights.SetForParty( + whoseCommitChain, nextHeight, + ) + case Settle, Fail, MalformedFail: + u.removeCommitHeights.SetForParty( + whoseCommitChain, nextHeight, + ) + case FeeUpdate: + u.addCommitHeights.SetForParty( + whoseCommitChain, nextHeight, + ) + u.removeCommitHeights.SetForParty( + whoseCommitChain, nextHeight, + ) + } + } + + recordFlow := func(acc *lnwire.MilliSatoshi, + u *paymentDescriptor) { + + if u.EntryType == Settle { + *acc += u.Amount + } + } + + for _, u := range uncommitted.Local { + setHeights(u) + if whoseCommitChain == lntypes.Local { + recordFlow(received, u) + } + } + for _, u := range uncommitted.Remote { + setHeights(u) + if whoseCommitChain == lntypes.Local { + recordFlow(sent, u) + } + } + } + feePerKw := filteredHTLCView.FeePerKw // Here we override the view's fee-rate if a dry-run fee-rate was @@ -4674,7 +4671,7 @@ func (lc *LightningChannel) computeView(view *HtlcView, // Now go through all HTLCs at this stage, to calculate the total // weight, needed to calculate the transaction fee. var totalHtlcWeight lntypes.WeightUnit - for _, htlc := range filteredHTLCView.OurUpdates { + for _, htlc := range filteredHTLCView.Updates.Local { if HtlcIsDust( lc.channelState.ChanType, false, whoseCommitChain, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, @@ -4685,7 +4682,7 @@ func (lc *LightningChannel) computeView(view *HtlcView, totalHtlcWeight += input.HTLCWeight } - for _, htlc := range filteredHTLCView.TheirUpdates { + for _, htlc := range filteredHTLCView.Updates.Remote { if HtlcIsDust( lc.channelState.ChanType, true, whoseCommitChain, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, @@ -4837,7 +4834,9 @@ func genHtlcSigValidationJobs(chanState *channeldb.OpenChannel, ) } - hashCache := input.NewTxSigHashesV0Only(successTx) + hashCache := + input.NewTxSigHashesV0Only(successTx) + sigHash, err := txscript.CalcWitnessSigHash( htlc.ourWitnessScript, hashCache, sigHashType, successTx, 0, @@ -5655,19 +5654,20 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) ( // both of the remote and local heights are non-zero. If either // of these values is zero, it has yet to be committed in both // the local and remote chains. - committedAdd := pd.addCommitHeightRemote > 0 && - pd.addCommitHeightLocal > 0 - committedRmv := pd.removeCommitHeightRemote > 0 && - pd.removeCommitHeightLocal > 0 + committedAdd := pd.addCommitHeights.Remote > 0 && + pd.addCommitHeights.Local > 0 + committedRmv := pd.removeCommitHeights.Remote > 0 && + pd.removeCommitHeights.Local > 0 // Using the height of the remote and local commitments, // preemptively compute whether or not to forward this HTLC for // the case in which this in an Add HTLC, or if this is a // Settle, Fail, or MalformedFail. - shouldFwdAdd := remoteChainTail == pd.addCommitHeightRemote && - localChainTail >= pd.addCommitHeightLocal - shouldFwdRmv := remoteChainTail == pd.removeCommitHeightRemote && - localChainTail >= pd.removeCommitHeightLocal + shouldFwdAdd := remoteChainTail == pd.addCommitHeights.Remote && + localChainTail >= pd.addCommitHeights.Local + shouldFwdRmv := remoteChainTail == + pd.removeCommitHeights.Remote && + localChainTail >= pd.removeCommitHeights.Local // We'll only forward any new HTLC additions iff, it's "freshly // locked in". Meaning that the HTLC was only *just* considered diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 3adb21636d..c3401081ee 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -7614,13 +7614,13 @@ func TestChannelRestoreCommitHeight(t *testing.T) { t.Fatalf("htlc not found in log") } - if pd.addCommitHeightLocal != expLocal { + if pd.addCommitHeights.Local != expLocal { t.Fatalf("expected local add height to be %d, was %d", - expLocal, pd.addCommitHeightLocal) + expLocal, pd.addCommitHeights.Local) } - if pd.addCommitHeightRemote != expRemote { + if pd.addCommitHeights.Remote != expRemote { t.Fatalf("expected remote add height to be %d, was %d", - expRemote, pd.addCommitHeightRemote) + expRemote, pd.addCommitHeights.Remote) } return newChannel } @@ -8402,16 +8402,20 @@ func TestFetchParent(t *testing.T) { remoteEntries: []*paymentDescriptor{ // This entry will be added at log index =0. { - HtlcIndex: 1, - addCommitHeightLocal: 100, - addCommitHeightRemote: 100, + HtlcIndex: 1, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 100, + Remote: 100, + }, }, // This entry will be added at log index =1, it // is the parent entry we are looking for. { - HtlcIndex: 2, - addCommitHeightLocal: 100, - addCommitHeightRemote: 0, + HtlcIndex: 2, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 100, + Remote: 0, + }, }, }, whoseCommitChain: lntypes.Remote, @@ -8424,16 +8428,20 @@ func TestFetchParent(t *testing.T) { remoteEntries: []*paymentDescriptor{ // This entry will be added at log index =0. { - HtlcIndex: 1, - addCommitHeightLocal: 100, - addCommitHeightRemote: 100, + HtlcIndex: 1, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 100, + Remote: 100, + }, }, // This entry will be added at log index =1, it // is the parent entry we are looking for. { - HtlcIndex: 2, - addCommitHeightLocal: 0, - addCommitHeightRemote: 100, + HtlcIndex: 2, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 0, + Remote: 100, + }, }, }, localEntries: nil, @@ -8447,16 +8455,20 @@ func TestFetchParent(t *testing.T) { localEntries: []*paymentDescriptor{ // This entry will be added at log index =0. { - HtlcIndex: 1, - addCommitHeightLocal: 100, - addCommitHeightRemote: 100, + HtlcIndex: 1, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 100, + Remote: 100, + }, }, // This entry will be added at log index =1, it // is the parent entry we are looking for. { - HtlcIndex: 2, - addCommitHeightLocal: 0, - addCommitHeightRemote: 100, + HtlcIndex: 2, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 0, + Remote: 100, + }, }, }, remoteEntries: nil, @@ -8471,16 +8483,20 @@ func TestFetchParent(t *testing.T) { localEntries: []*paymentDescriptor{ // This entry will be added at log index =0. { - HtlcIndex: 1, - addCommitHeightLocal: 100, - addCommitHeightRemote: 100, + HtlcIndex: 1, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 100, + Remote: 100, + }, }, // This entry will be added at log index =1, it // is the parent entry we are looking for. { - HtlcIndex: 2, - addCommitHeightLocal: 100, - addCommitHeightRemote: 0, + HtlcIndex: 2, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 100, + Remote: 0, + }, }, }, remoteEntries: nil, @@ -8495,16 +8511,20 @@ func TestFetchParent(t *testing.T) { remoteEntries: []*paymentDescriptor{ // This entry will be added at log index =0. { - HtlcIndex: 1, - addCommitHeightLocal: 100, - addCommitHeightRemote: 0, + HtlcIndex: 1, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 100, + Remote: 0, + }, }, // This entry will be added at log index =1, it // is the parent entry we are looking for. { - HtlcIndex: 2, - addCommitHeightLocal: 100, - addCommitHeightRemote: 100, + HtlcIndex: 2, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 100, + Remote: 100, + }, }, }, whoseCommitChain: lntypes.Remote, @@ -8518,16 +8538,20 @@ func TestFetchParent(t *testing.T) { localEntries: []*paymentDescriptor{ // This entry will be added at log index =0. { - HtlcIndex: 1, - addCommitHeightLocal: 0, - addCommitHeightRemote: 100, + HtlcIndex: 1, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 0, + Remote: 100, + }, }, // This entry will be added at log index =1, it // is the parent entry we are looking for. { - HtlcIndex: 2, - addCommitHeightLocal: 100, - addCommitHeightRemote: 100, + HtlcIndex: 2, + addCommitHeights: lntypes.Dual[uint64]{ + Local: 100, + Remote: 100, + }, }, }, remoteEntries: nil, @@ -8626,6 +8650,7 @@ func TestEvaluateView(t *testing.T) { name string ourHtlcs []*paymentDescriptor theirHtlcs []*paymentDescriptor + channelInitiator lntypes.ChannelParty whoseCommitChain lntypes.ChannelParty mutateState bool @@ -8655,6 +8680,7 @@ func TestEvaluateView(t *testing.T) { }{ { name: "our fee update is applied", + channelInitiator: lntypes.Local, whoseCommitChain: lntypes.Local, mutateState: false, ourHtlcs: []*paymentDescriptor{ @@ -8672,6 +8698,7 @@ func TestEvaluateView(t *testing.T) { }, { name: "their fee update is applied", + channelInitiator: lntypes.Remote, whoseCommitChain: lntypes.Local, mutateState: false, ourHtlcs: []*paymentDescriptor{}, @@ -8728,10 +8755,12 @@ func TestEvaluateView(t *testing.T) { mutateState: true, ourHtlcs: []*paymentDescriptor{ { - HtlcIndex: 0, - Amount: htlcAddAmount, - EntryType: Add, - addCommitHeightLocal: addHeight, + HtlcIndex: 0, + Amount: htlcAddAmount, + EntryType: Add, + addCommitHeights: lntypes.Dual[uint64]{ + Local: addHeight, + }, }, }, theirHtlcs: []*paymentDescriptor{ @@ -8763,10 +8792,12 @@ func TestEvaluateView(t *testing.T) { mutateState: false, ourHtlcs: []*paymentDescriptor{ { - HtlcIndex: 0, - Amount: htlcAddAmount, - EntryType: Add, - addCommitHeightLocal: addHeight, + HtlcIndex: 0, + Amount: htlcAddAmount, + EntryType: Add, + addCommitHeights: lntypes.Dual[uint64]{ + Local: addHeight, + }, }, }, theirHtlcs: []*paymentDescriptor{ @@ -8813,16 +8844,20 @@ func TestEvaluateView(t *testing.T) { }, theirHtlcs: []*paymentDescriptor{ { - HtlcIndex: 0, - Amount: htlcAddAmount, - EntryType: Add, - addCommitHeightLocal: addHeight, + HtlcIndex: 0, + Amount: htlcAddAmount, + EntryType: Add, + addCommitHeights: lntypes.Dual[uint64]{ + Local: addHeight, + }, }, { - HtlcIndex: 1, - Amount: htlcAddAmount, - EntryType: Add, - addCommitHeightLocal: addHeight, + HtlcIndex: 1, + Amount: htlcAddAmount, + EntryType: Add, + addCommitHeights: lntypes.Dual[uint64]{ + Local: addHeight, + }, }, }, expectedFee: feePerKw, @@ -8857,10 +8892,12 @@ func TestEvaluateView(t *testing.T) { }, theirHtlcs: []*paymentDescriptor{ { - HtlcIndex: 0, - Amount: htlcAddAmount, - EntryType: Add, - addCommitHeightLocal: addHeight, + HtlcIndex: 0, + Amount: htlcAddAmount, + EntryType: Add, + addCommitHeights: lntypes.Dual[uint64]{ + Local: addHeight, + }, }, }, expectedFee: feePerKw, @@ -8877,8 +8914,10 @@ func TestEvaluateView(t *testing.T) { test := test t.Run(test.name, func(t *testing.T) { + isInitiator := test.channelInitiator == lntypes.Local lc := LightningChannel{ channelState: &channeldb.OpenChannel{ + IsInitiator: isInitiator, TotalMSatSent: 0, TotalMSatReceived: 0, }, @@ -8907,40 +8946,99 @@ func TestEvaluateView(t *testing.T) { } view := &HtlcView{ - OurUpdates: test.ourHtlcs, - TheirUpdates: test.theirHtlcs, - FeePerKw: feePerKw, + Updates: lntypes.Dual[[]*paymentDescriptor]{ + Local: test.ourHtlcs, + Remote: test.theirHtlcs, + }, + FeePerKw: feePerKw, } - var ( - // Create vars to store balance changes. We do - // not check these values in this test because - // balance modification happens on the htlc - // processing level. - ourBalance lnwire.MilliSatoshi - theirBalance lnwire.MilliSatoshi - ) - // Evaluate the htlc view, mutate as test expects. - result, err := lc.evaluateHTLCView( - view, &ourBalance, &theirBalance, nextHeight, - test.whoseCommitChain, test.mutateState, + // We do not check the balance deltas in this test + // because balance modification happens on the htlc + // processing level. + result, uncommitted, _, err := lc.evaluateHTLCView( + view, test.whoseCommitChain, nextHeight, ) + if err != nil { t.Fatalf("unexpected error: %v", err) } + // TODO(proofofkeags): This block is here because we + // extracted this code from a previous implementation + // of evaluateHTLCView, due to a reduced scope of + // responsibility of that function. Consider removing + // it from the test altogether. + if test.mutateState { + state := lc.channelState + received := &state.TotalMSatReceived + sent := &state.TotalMSatSent + + setHeights := func(u *paymentDescriptor) { + switch u.EntryType { + case Add: + u.addCommitHeights.SetForParty( + test.whoseCommitChain, + nextHeight, + ) + case Settle, Fail, MalformedFail: + //nolint:lll + u.removeCommitHeights.SetForParty( + test.whoseCommitChain, + nextHeight, + ) + case FeeUpdate: + u.addCommitHeights.SetForParty( + test.whoseCommitChain, + nextHeight, + ) + //nolint:lll + u.removeCommitHeights.SetForParty( + test.whoseCommitChain, + nextHeight, + ) + } + } + + recordFlow := func(acc *lnwire.MilliSatoshi, + u *paymentDescriptor) { + + if u.EntryType == Settle { + *acc += u.Amount + } + } + + for _, u := range uncommitted.Local { + setHeights(u) + if test.whoseCommitChain == + lntypes.Local { + + recordFlow(received, u) + } + } + for _, u := range uncommitted.Remote { + setHeights(u) + if test.whoseCommitChain == + lntypes.Local { + + recordFlow(sent, u) + } + } + } + if result.FeePerKw != test.expectedFee { t.Fatalf("expected fee: %v, got: %v", test.expectedFee, result.FeePerKw) } checkExpectedHtlcs( - t, result.OurUpdates, test.ourExpectedHtlcs, + t, result.Updates.Local, test.ourExpectedHtlcs, ) checkExpectedHtlcs( - t, result.TheirUpdates, test.theirExpectedHtlcs, + t, result.Updates.Remote, + test.theirExpectedHtlcs, ) if lc.channelState.TotalMSatSent != test.expectSent { @@ -8987,617 +9085,6 @@ type heights struct { remoteRemove uint64 } -// TestProcessFeeUpdate tests the applying of fee updates and mutation of -// local and remote add and remove heights on update messages. -func TestProcessFeeUpdate(t *testing.T) { - const ( - // height is a non-zero height that can be used for htlcs - // heights. - height = 200 - - // nextHeight is a constant that we use for the next height in - // all unit tests. - nextHeight = 400 - - // feePerKw is the fee we start all of our unit tests with. - feePerKw = 1 - - // ourFeeUpdateAmt is an amount that we update fees to expressed - // in msat. - ourFeeUpdateAmt = 20000 - - // ourFeeUpdatePerSat is the fee rate *in satoshis* that we - // expect if we update to ourFeeUpdateAmt. - ourFeeUpdatePerSat = chainfee.SatPerKWeight(20) - ) - - tests := []struct { - name string - startHeights heights - expectedHeights heights - whoseCommitChain lntypes.ChannelParty - mutate bool - expectedFee chainfee.SatPerKWeight - }{ - { - // Looking at local chain, local add is non-zero so - // the update has been applied already; no fee change. - name: "non-zero local height, fee unchanged", - startHeights: heights{ - localAdd: height, - localRemove: 0, - remoteAdd: 0, - remoteRemove: height, - }, - expectedHeights: heights{ - localAdd: height, - localRemove: 0, - remoteAdd: 0, - remoteRemove: height, - }, - whoseCommitChain: lntypes.Local, - mutate: false, - expectedFee: feePerKw, - }, - { - // Looking at local chain, local add is zero so the - // update has not been applied yet; we expect a fee - // update. - name: "zero local height, fee changed", - startHeights: heights{ - localAdd: 0, - localRemove: 0, - remoteAdd: height, - remoteRemove: 0, - }, - expectedHeights: heights{ - localAdd: 0, - localRemove: 0, - remoteAdd: height, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Local, - mutate: false, - expectedFee: ourFeeUpdatePerSat, - }, - { - // Looking at remote chain, the remote add height is - // zero, so the update has not been applied so we expect - // a fee change. - name: "zero remote height, fee changed", - startHeights: heights{ - localAdd: height, - localRemove: 0, - remoteAdd: 0, - remoteRemove: 0, - }, - expectedHeights: heights{ - localAdd: height, - localRemove: 0, - remoteAdd: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Remote, - mutate: false, - expectedFee: ourFeeUpdatePerSat, - }, - { - // Looking at remote chain, the remote add height is - // non-zero, so the update has been applied so we expect - // no fee change. - name: "non-zero remote height, no fee change", - startHeights: heights{ - localAdd: height, - localRemove: 0, - remoteAdd: height, - remoteRemove: 0, - }, - expectedHeights: heights{ - localAdd: height, - localRemove: 0, - remoteAdd: height, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Remote, - mutate: false, - expectedFee: feePerKw, - }, - { - // Local add height is non-zero, so the update has - // already been applied; we do not expect fee to - // change or any mutations to be applied. - name: "non-zero local height, mutation not applied", - startHeights: heights{ - localAdd: height, - localRemove: 0, - remoteAdd: 0, - remoteRemove: height, - }, - expectedHeights: heights{ - localAdd: height, - localRemove: 0, - remoteAdd: 0, - remoteRemove: height, - }, - whoseCommitChain: lntypes.Local, - mutate: true, - expectedFee: feePerKw, - }, - { - // Local add is zero and we are looking at our local - // chain, so the update has not been applied yet. We - // expect the local add and remote heights to be - // mutated. - name: "zero height, fee changed, mutation applied", - startHeights: heights{ - localAdd: 0, - localRemove: 0, - remoteAdd: 0, - remoteRemove: 0, - }, - expectedHeights: heights{ - localAdd: nextHeight, - localRemove: nextHeight, - remoteAdd: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Local, - mutate: true, - expectedFee: ourFeeUpdatePerSat, - }, - } - - for _, test := range tests { - test := test - - t.Run(test.name, func(t *testing.T) { - // Create a fee update with add and remove heights as - // set in the test. - heights := test.startHeights - update := &paymentDescriptor{ - Amount: ourFeeUpdateAmt, - addCommitHeightRemote: heights.remoteAdd, - addCommitHeightLocal: heights.localAdd, - removeCommitHeightRemote: heights.remoteRemove, - removeCommitHeightLocal: heights.localRemove, - EntryType: FeeUpdate, - } - - view := &HtlcView{ - FeePerKw: chainfee.SatPerKWeight(feePerKw), - } - processFeeUpdate( - update, nextHeight, test.whoseCommitChain, - test.mutate, view, - ) - - if view.FeePerKw != test.expectedFee { - t.Fatalf("expected fee: %v, got: %v", - test.expectedFee, feePerKw) - } - - checkHeights(t, update, test.expectedHeights) - }) - } -} - -func checkHeights(t *testing.T, update *paymentDescriptor, expected heights) { - updateHeights := heights{ - localAdd: update.addCommitHeightLocal, - localRemove: update.removeCommitHeightLocal, - remoteAdd: update.addCommitHeightRemote, - remoteRemove: update.removeCommitHeightRemote, - } - - if !reflect.DeepEqual(updateHeights, expected) { - t.Fatalf("expected: %v, got: %v", expected, updateHeights) - } -} - -// TestProcessAddRemoveEntry tests the updating of our and their balances when -// we process adds, settles and fails. It also tests the mutating of add and -// remove heights. -func TestProcessAddRemoveEntry(t *testing.T) { - const ( - // addHeight is a non-zero addHeight that is used for htlc - // add heights. - addHeight = 100 - - // removeHeight is a non-zero removeHeight that is used for - // htlc remove heights. - removeHeight = 200 - - // nextHeight is a constant that we use for the nextHeight in - // all unit tests. - nextHeight = 400 - - // updateAmount is the amount that the update is set to. - updateAmount = lnwire.MilliSatoshi(10) - - // startBalance is a balance we start both sides out with - // so that balances can be incremented. - startBalance = lnwire.MilliSatoshi(100) - ) - - tests := []struct { - name string - startHeights heights - whoseCommitChain lntypes.ChannelParty - isIncoming bool - mutateState bool - ourExpectedBalance lnwire.MilliSatoshi - theirExpectedBalance lnwire.MilliSatoshi - expectedHeights heights - updateType updateType - }{ - { - name: "add, remote chain, already processed", - startHeights: heights{ - localAdd: 0, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Remote, - isIncoming: false, - mutateState: false, - ourExpectedBalance: startBalance, - theirExpectedBalance: startBalance, - expectedHeights: heights{ - localAdd: 0, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - updateType: Add, - }, - { - name: "add, local chain, already processed", - startHeights: heights{ - localAdd: addHeight, - remoteAdd: 0, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Local, - isIncoming: false, - mutateState: false, - ourExpectedBalance: startBalance, - theirExpectedBalance: startBalance, - expectedHeights: heights{ - localAdd: addHeight, - remoteAdd: 0, - localRemove: 0, - remoteRemove: 0, - }, - updateType: Add, - }, - { - name: "incoming add, local chain, not mutated", - startHeights: heights{ - localAdd: 0, - remoteAdd: 0, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Local, - isIncoming: true, - mutateState: false, - ourExpectedBalance: startBalance, - theirExpectedBalance: startBalance - updateAmount, - expectedHeights: heights{ - localAdd: 0, - remoteAdd: 0, - localRemove: 0, - remoteRemove: 0, - }, - updateType: Add, - }, - { - name: "incoming add, local chain, mutated", - startHeights: heights{ - localAdd: 0, - remoteAdd: 0, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Local, - isIncoming: true, - mutateState: true, - ourExpectedBalance: startBalance, - theirExpectedBalance: startBalance - updateAmount, - expectedHeights: heights{ - localAdd: nextHeight, - remoteAdd: 0, - localRemove: 0, - remoteRemove: 0, - }, - updateType: Add, - }, - - { - name: "outgoing add, remote chain, not mutated", - startHeights: heights{ - localAdd: 0, - remoteAdd: 0, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Remote, - isIncoming: false, - mutateState: false, - ourExpectedBalance: startBalance - updateAmount, - theirExpectedBalance: startBalance, - expectedHeights: heights{ - localAdd: 0, - remoteAdd: 0, - localRemove: 0, - remoteRemove: 0, - }, - updateType: Add, - }, - { - name: "outgoing add, remote chain, mutated", - startHeights: heights{ - localAdd: 0, - remoteAdd: 0, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Remote, - isIncoming: false, - mutateState: true, - ourExpectedBalance: startBalance - updateAmount, - theirExpectedBalance: startBalance, - expectedHeights: heights{ - localAdd: 0, - remoteAdd: nextHeight, - localRemove: 0, - remoteRemove: 0, - }, - updateType: Add, - }, - { - name: "settle, remote chain, already processed", - startHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: removeHeight, - }, - whoseCommitChain: lntypes.Remote, - isIncoming: false, - mutateState: false, - ourExpectedBalance: startBalance, - theirExpectedBalance: startBalance, - expectedHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: removeHeight, - }, - updateType: Settle, - }, - { - name: "settle, local chain, already processed", - startHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: removeHeight, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Local, - isIncoming: false, - mutateState: false, - ourExpectedBalance: startBalance, - theirExpectedBalance: startBalance, - expectedHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: removeHeight, - remoteRemove: 0, - }, - updateType: Settle, - }, - { - // Remote chain, and not processed yet. Incoming settle, - // so we expect our balance to increase. - name: "incoming settle", - startHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Remote, - isIncoming: true, - mutateState: false, - ourExpectedBalance: startBalance + updateAmount, - theirExpectedBalance: startBalance, - expectedHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - updateType: Settle, - }, - { - // Remote chain, and not processed yet. Incoming settle, - // so we expect our balance to increase. - name: "outgoing settle", - startHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Remote, - isIncoming: false, - mutateState: false, - ourExpectedBalance: startBalance, - theirExpectedBalance: startBalance + updateAmount, - expectedHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - updateType: Settle, - }, - { - // Remote chain, and not processed yet. Incoming fail, - // so we expect their balance to increase. - name: "incoming fail", - startHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Remote, - isIncoming: true, - mutateState: false, - ourExpectedBalance: startBalance, - theirExpectedBalance: startBalance + updateAmount, - expectedHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - updateType: Fail, - }, - { - // Remote chain, and not processed yet. Outgoing fail, - // so we expect our balance to increase. - name: "outgoing fail", - startHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Remote, - isIncoming: false, - mutateState: false, - ourExpectedBalance: startBalance + updateAmount, - theirExpectedBalance: startBalance, - expectedHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - updateType: Fail, - }, - { - // Local chain, and not processed yet. Incoming settle, - // so we expect our balance to increase. Mutate is - // true, so we expect our remove removeHeight to have - // changed. - name: "fail, our remove height mutated", - startHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Local, - isIncoming: true, - mutateState: true, - ourExpectedBalance: startBalance + updateAmount, - theirExpectedBalance: startBalance, - expectedHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: nextHeight, - remoteRemove: 0, - }, - updateType: Settle, - }, - { - // Remote chain, and not processed yet. Incoming settle, - // so we expect our balance to increase. Mutate is - // true, so we expect their remove removeHeight to have - // changed. - name: "fail, their remove height mutated", - startHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: 0, - }, - whoseCommitChain: lntypes.Remote, - isIncoming: true, - mutateState: true, - ourExpectedBalance: startBalance + updateAmount, - theirExpectedBalance: startBalance, - expectedHeights: heights{ - localAdd: addHeight, - remoteAdd: addHeight, - localRemove: 0, - remoteRemove: nextHeight, - }, - updateType: Settle, - }, - } - - for _, test := range tests { - test := test - - t.Run(test.name, func(t *testing.T) { - t.Parallel() - - heights := test.startHeights - update := &paymentDescriptor{ - Amount: updateAmount, - addCommitHeightLocal: heights.localAdd, - addCommitHeightRemote: heights.remoteAdd, - removeCommitHeightLocal: heights.localRemove, - removeCommitHeightRemote: heights.remoteRemove, - EntryType: test.updateType, - } - - var ( - // Start both parties off with an initial - // balance. Copy by value here so that we do - // not mutate the startBalance constant. - ourBalance, theirBalance = startBalance, - startBalance - ) - - // Choose the processing function we need based on the - // update type. Process remove is used for settles, - // fails and malformed htlcs. - process := processRemoveEntry - if test.updateType == Add { - process = processAddEntry - } - - process( - update, &ourBalance, &theirBalance, nextHeight, - test.whoseCommitChain, test.isIncoming, - test.mutateState, - ) - - // Check that balances were updated as expected. - if ourBalance != test.ourExpectedBalance { - t.Fatalf("expected our balance: %v, got: %v", - test.ourExpectedBalance, ourBalance) - } - - if theirBalance != test.theirExpectedBalance { - t.Fatalf("expected their balance: %v, got: %v", - test.theirExpectedBalance, theirBalance) - } - - // Check that heights on the update are as expected. - checkHeights(t, update, test.expectedHeights) - }) - } -} - // TestChannelUnsignedAckedFailure tests that unsigned acked updates are // properly restored after signing for them and disconnecting. // diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index 170efece1b..6d61729a41 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -702,7 +702,7 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, } numHTLCs := int64(0) - for _, htlc := range filteredHTLCView.OurUpdates { + for _, htlc := range filteredHTLCView.Updates.Local { if HtlcIsDust( cb.chanState.ChanType, false, whoseCommit, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, @@ -713,7 +713,7 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, numHTLCs++ } - for _, htlc := range filteredHTLCView.TheirUpdates { + for _, htlc := range filteredHTLCView.Updates.Remote { if HtlcIsDust( cb.chanState.ChanType, true, whoseCommit, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, @@ -827,7 +827,7 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, // purposes of sorting. cltvs := make([]uint32, len(commitTx.TxOut)) htlcIndexes := make([]input.HtlcIndex, len(commitTx.TxOut)) - for _, htlc := range filteredHTLCView.OurUpdates { + for _, htlc := range filteredHTLCView.Updates.Local { if HtlcIsDust( cb.chanState.ChanType, false, whoseCommit, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, @@ -855,7 +855,7 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, cltvs = append(cltvs, htlc.Timeout) //nolint htlcIndexes = append(htlcIndexes, htlc.HtlcIndex) //nolint } - for _, htlc := range filteredHTLCView.TheirUpdates { + for _, htlc := range filteredHTLCView.Updates.Remote { if HtlcIsDust( cb.chanState.ChanType, true, whoseCommit, feePerKw, htlc.Amount.ToSatoshis(), dustLimit, diff --git a/lnwallet/payment_descriptor.go b/lnwallet/payment_descriptor.go index 5a51f29ce5..ffa4cc8ce1 100644 --- a/lnwallet/payment_descriptor.go +++ b/lnwallet/payment_descriptor.go @@ -6,6 +6,7 @@ import ( "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/channeldb/models" "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" ) @@ -165,16 +166,14 @@ type paymentDescriptor struct { // which included this HTLC on either the remote or local commitment // chain. This value is used to determine when an HTLC is fully // "locked-in". - addCommitHeightRemote uint64 - addCommitHeightLocal uint64 + addCommitHeights lntypes.Dual[uint64] // removeCommitHeight[Remote|Local] encodes the height of the // commitment which removed the parent pointer of this // paymentDescriptor either due to a timeout or a settle. Once both // these heights are below the tail of both chains, the log entries can // safely be removed. - removeCommitHeightRemote uint64 - removeCommitHeightLocal uint64 + removeCommitHeights lntypes.Dual[uint64] // OnionBlob is an opaque blob which is used to complete multi-hop // routing. diff --git a/lnwallet/update_log.go b/lnwallet/update_log.go index 42e5373a22..2d1f65c9fa 100644 --- a/lnwallet/update_log.go +++ b/lnwallet/update_log.go @@ -153,6 +153,7 @@ func compactLogs(ourLog, theirLog *updateLog, nextA = e.Next() htlc := e.Value + rmvHeights := htlc.removeCommitHeights // We skip Adds, as they will be removed along with the // fail/settles below. @@ -162,9 +163,7 @@ func compactLogs(ourLog, theirLog *updateLog, // If the HTLC hasn't yet been removed from either // chain, the skip it. - if htlc.removeCommitHeightRemote == 0 || - htlc.removeCommitHeightLocal == 0 { - + if rmvHeights.Remote == 0 || rmvHeights.Local == 0 { continue } @@ -172,8 +171,8 @@ func compactLogs(ourLog, theirLog *updateLog, // is at least the height in which the HTLC was // removed, then evict the settle/timeout entry along // with the original add entry. - if remoteChainTail >= htlc.removeCommitHeightRemote && - localChainTail >= htlc.removeCommitHeightLocal { + if remoteChainTail >= rmvHeights.Remote && + localChainTail >= rmvHeights.Local { // Fee updates have no parent htlcs, so we only // remove the update itself.