diff --git a/lnwallet/aux_signer.go b/lnwallet/aux_signer.go index 510b64b5d15..e3566ee5348 100644 --- a/lnwallet/aux_signer.go +++ b/lnwallet/aux_signer.go @@ -9,9 +9,16 @@ import ( "github.com/lightningnetwork/lnd/tlv" ) -// htlcCustomSigType is the TLV type that is used to encode the custom HTLC -// signatures within the custom data for an existing HTLC. -var htlcCustomSigType tlv.TlvType65543 +var ( + // htlcCustomSigType is the TLV type that is used to encode the custom + // HTLC signatures within the custom data for an existing HTLC. + htlcCustomSigType tlv.TlvType65543 + + // noopHtlcType is the TLV that that's used in the update_add_htlc + // message to indicate the presence of a noop HTLC. This has no encoded + // value, but is used to indicate that the HTLC is a noop. + noopHtlcType tlv.TlvType65544 +) // AuxHtlcDescriptor is a struct that contains the information needed to sign or // verify an HTLC for custom channels. diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 6486464d0fe..909cbecfb6c 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -552,6 +552,20 @@ func (lc *LightningChannel) diskHtlcToPayDesc(feeRate chainfee.SatPerKWeight, remoteOutputIndex = htlc.OutputIndex } + customRecords := htlc.CustomRecords.Copy() + + entryType := Add + + noopTLV := uint64(noopHtlcType.TypeVal()) + if _, ok := customRecords[noopTLV]; ok { + entryType = NoopAdd + } + + // The NoopAdd HTLC is an internal construct, and isn't meant to show up + // on the wire. So we'll remove the special element from the set of + // custom records. + delete(customRecords, noopTLV) + // With the scripts reconstructed (depending on if this is our commit // vs theirs or a pending commit for the remote party), we can now // re-create the original payment descriptor. @@ -560,7 +574,7 @@ func (lc *LightningChannel) diskHtlcToPayDesc(feeRate chainfee.SatPerKWeight, RHash: htlc.RHash, Timeout: htlc.RefundTimeout, Amount: htlc.Amt, - EntryType: Add, + EntryType: entryType, HtlcIndex: htlc.HtlcIndex, LogIndex: htlc.LogIndex, OnionBlob: htlc.OnionBlob, @@ -571,7 +585,7 @@ func (lc *LightningChannel) diskHtlcToPayDesc(feeRate chainfee.SatPerKWeight, theirPkScript: theirP2WSH, theirWitnessScript: theirWitnessScript, BlindingPoint: htlc.BlindingPoint, - CustomRecords: htlc.CustomRecords.Copy(), + CustomRecords: customRecords, }, nil } @@ -1101,6 +1115,11 @@ func (lc *LightningChannel) logUpdateToPayDesc(logUpdate *channeldb.LogUpdate, }, } + noopTLV := uint64(noopHtlcType.TypeVal()) + if _, ok := pd.CustomRecords[noopTLV]; ok { + pd.EntryType = NoopAdd + } + isDustRemote := HtlcIsDust( lc.channelState.ChanType, false, lntypes.Remote, feeRate, wireMsg.Amount.ToSatoshis(), remoteDustLimit, @@ -1336,6 +1355,11 @@ func (lc *LightningChannel) remoteLogUpdateToPayDesc(logUpdate *channeldb.LogUpd Local: commitHeight, }, } + noopTLV := uint64(noopHtlcType.TypeVal()) + + if _, ok := pd.CustomRecords[noopTLV]; ok { + pd.EntryType = NoopAdd + } // We don't need to generate an htlc script yet. This will be // done once we sign our remote commitment. @@ -1737,7 +1761,7 @@ func (lc *LightningChannel) restorePendingRemoteUpdates( // but this Add restoration was a no-op as every single one of // these Adds was already restored since they're all incoming // htlcs on the local commitment. - if payDesc.EntryType == Add { + if payDesc.isAdd() { continue } @@ -1882,7 +1906,7 @@ func (lc *LightningChannel) restorePendingLocalUpdates( } switch payDesc.EntryType { - case Add: + case Add, NoopAdd: // The HtlcIndex of the added HTLC _must_ be equal to // the log's htlcCounter at this point. If it is not we // panic to catch this. @@ -2974,6 +2998,20 @@ func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, ) if rmvHeight == 0 { switch { + // If this a noop add, then when we settle the + // HTLC, we actually credit the sender with the + // amount again, thus making it a noop. + case entry.EntryType == Settle && + addEntry.EntryType == NoopAdd: + + delta := int64(entry.Amount) + balanceDeltas.ModifyForParty( + party.CounterParty(), + func(acc int64) int64 { + return acc + delta + }, + ) + // If an incoming HTLC is being settled, then // this means that the preimage has been // received by the settling party Therefore, we @@ -3011,7 +3049,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, liveAdds := fn.Filter( view.Updates.GetForParty(party), func(pd *paymentDescriptor) bool { - isAdd := pd.EntryType == Add + isAdd := pd.isAdd() shouldSkip := skip.GetForParty(party). Contains(pd.HtlcIndex) @@ -3050,7 +3088,7 @@ func (lc *LightningChannel) evaluateHTLCView(view *HtlcView, // corresponding to whoseCommitmentChain. isUncommitted := func(update *paymentDescriptor) bool { switch update.EntryType { - case Add: + case Add, NoopAdd: return update.addCommitHeights.GetForParty( whoseCommitChain, ) == 0 @@ -3814,7 +3852,7 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, // Go through all updates, checking that they don't violate the // channel constraints. for _, entry := range updates { - if entry.EntryType == Add { + if entry.isAdd() { // An HTLC is being added, this will add to the // number and amount in flight. amtInFlight += entry.Amount @@ -4470,6 +4508,13 @@ func (lc *LightningChannel) ProcessChanSyncMsg(ctx context.Context, // Next, we'll need to send over any updates we sent as part of // this new proposed commitment state. for _, logUpdate := range commitDiff.LogUpdates { + if htlc, ok := logUpdate.UpdateMsg.(*lnwire.UpdateAddHTLC); ok { + delete(htlc.CustomRecords, uint64(noopHtlcType.TypeVal())) + + if len(htlc.CustomRecords) == 0 { + htlc.CustomRecords = nil + } + } commitUpdates = append( commitUpdates, logUpdate.UpdateMsg, ) @@ -4501,8 +4546,8 @@ func (lc *LightningChannel) ProcessChanSyncMsg(ctx context.Context, // updates comes after the LogUpdates+CommitSig. // // ---logupdates---> - // ---commitsig----> // ---revocation---> + // ---commitsig----> updates = append(commitUpdates, updates...) } else { // Otherwise, the revocation should come before LogUpdates @@ -5693,7 +5738,7 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) ( // don't re-forward any already processed HTLC's after a // restart. switch { - case pd.EntryType == Add && committedAdd && shouldFwdAdd: + case pd.isAdd() && committedAdd && shouldFwdAdd: // Construct a reference specifying the location that // this forwarded Add will be written in the forwarding // package constructed at this remote height. @@ -5712,7 +5757,7 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) ( addUpdatesToForward, pd.toLogUpdate(), ) - case pd.EntryType != Add && committedRmv && shouldFwdRmv: + case !pd.isAdd() && committedRmv && shouldFwdRmv: // Construct a reference specifying the location that // this forwarded Settle/Fail will be written in the // forwarding package constructed at this remote height. @@ -5951,7 +5996,7 @@ func (lc *LightningChannel) GetDustSum(whoseCommit lntypes.ChannelParty, // Grab all of our HTLCs and evaluate against the dust limit. for e := lc.updateLogs.Local.Front(); e != nil; e = e.Next() { pd := e.Value - if pd.EntryType != Add { + if !pd.isAdd() { continue } @@ -5970,7 +6015,7 @@ func (lc *LightningChannel) GetDustSum(whoseCommit lntypes.ChannelParty, // Grab all of their HTLCs and evaluate against the dust limit. for e := lc.updateLogs.Remote.Front(); e != nil; e = e.Next() { pd := e.Value - if pd.EntryType != Add { + if !pd.isAdd() { continue } @@ -6043,9 +6088,25 @@ func (lc *LightningChannel) MayAddOutgoingHtlc(amt lnwire.MilliSatoshi) error { func (lc *LightningChannel) htlcAddDescriptor(htlc *lnwire.UpdateAddHTLC, openKey *models.CircuitKey) *paymentDescriptor { + // TODO(roasbeef): can use push amt to simplify logic, not have to + // detect if remote party has enough funds for anchor + entryType := Add + + customRecords := htlc.CustomRecords.Copy() + if lc.channelState.ChanType.HasTapscriptRoot() { + entryType = NoopAdd + + // We'll add an internal custom record here to make sure that + // across restarts, we recognize this as a noop HTLC. + if customRecords == nil { + customRecords = make(lnwire.CustomRecords) + } + customRecords[uint64(noopHtlcType.TypeVal())] = nil + } + return &paymentDescriptor{ ChanID: htlc.ChanID, - EntryType: Add, + EntryType: entryType, RHash: PaymentHash(htlc.PaymentHash), Timeout: htlc.Expiry, Amount: htlc.Amount, @@ -6054,7 +6115,7 @@ func (lc *LightningChannel) htlcAddDescriptor(htlc *lnwire.UpdateAddHTLC, OnionBlob: htlc.OnionBlob, OpenCircuitKey: openKey, BlindingPoint: htlc.BlindingPoint, - CustomRecords: htlc.CustomRecords.Copy(), + CustomRecords: customRecords, } } @@ -6107,9 +6168,24 @@ func (lc *LightningChannel) ReceiveHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, lc.updateLogs.Remote.htlcCounter) } + entryType := Add + + customRecords := htlc.CustomRecords.Copy() + if lc.channelState.ChanType.HasTapscriptRoot() { + if customRecords == nil { + customRecords = make(lnwire.CustomRecords) + } + + // We'll add an internal custom record here to make sure that + // across restarts, we recognize this as a noop HTLC. + customRecords[uint64(noopHtlcType.TypeVal())] = nil + + entryType = NoopAdd + } + pd := &paymentDescriptor{ ChanID: htlc.ChanID, - EntryType: Add, + EntryType: entryType, RHash: PaymentHash(htlc.PaymentHash), Timeout: htlc.Expiry, Amount: htlc.Amount, @@ -6117,7 +6193,7 @@ func (lc *LightningChannel) ReceiveHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, HtlcIndex: lc.updateLogs.Remote.htlcCounter, OnionBlob: htlc.OnionBlob, BlindingPoint: htlc.BlindingPoint, - CustomRecords: htlc.CustomRecords.Copy(), + CustomRecords: customRecords, } localACKedIndex := lc.commitChains.Remote.tail().messageIndices.Local @@ -9765,7 +9841,7 @@ func (lc *LightningChannel) unsignedLocalUpdates(remoteMessageIndex, // We don't save add updates as they are restored from the // remote commitment in restoreStateLogs. - if pd.EntryType == Add { + if pd.isAdd() { continue } diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 9da174a3d87..50a8cc0358c 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -1383,8 +1383,8 @@ func TestForceCloseDustOutput(t *testing.T) { bobChannel.channelState.RemoteChanCfg.ChanReserve = 0 htlcAmount := lnwire.NewMSatFromSatoshis(500) - aliceAmount := aliceChannel.channelState.LocalCommitment.LocalBalance + bobAmount := bobChannel.channelState.LocalCommitment.LocalBalance // Have Bobs' to-self output be below her dust limit and check @@ -11272,3 +11272,55 @@ func TestCreateCooperativeCloseTx(t *testing.T) { }) } } + +// TestNoopAddSettle tests that adding and settling an HTLC with no-op, no +// balances are actually affected. +func TestNoopAddSettle(t *testing.T) { + t.Parallel() + + // Create a test channel which will be used for the duration of this + // unittest. The channel will be funded evenly with Alice having 5 BTC, + // and Bob having 5 BTC. + chanType := channeldb.SimpleTaprootFeatureBit | + channeldb.AnchorOutputsBit | channeldb.ZeroHtlcTxFeeBit | + channeldb.SingleFunderTweaklessBit | channeldb.TapscriptRootBit + aliceChannel, bobChannel, err := CreateTestChannels( + t, chanType, + ) + require.NoError(t, err, "unable to create test channels") + + const htlcAmt = 10_000 + htlc, preimage := createHTLC(0, htlcAmt) + + aliceBalance := aliceChannel.channelState.LocalCommitment.LocalBalance + bobBalance := bobChannel.channelState.LocalCommitment.LocalBalance + + // Have Alice add the HTLC, then lock it in with a new state transition. + aliceHtlcIndex, err := aliceChannel.AddHTLC(htlc, nil) + require.NoError(t, err, "alice unable to add htlc") + bobHtlcIndex, err := bobChannel.ReceiveHTLC(htlc) + require.NoError(t, err, "bob unable to receive htlc") + if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("Can't update the channel state: %v", err) + } + + // We'll have Bob settle the HTLC, then force another state transition. + err = bobChannel.SettleHTLC(preimage, bobHtlcIndex, nil, nil, nil) + require.NoError(t, err, "bob unable to settle inbound htlc") + err = aliceChannel.ReceiveHTLCSettle(preimage, aliceHtlcIndex) + if err != nil { + t.Fatalf("alice unable to accept settle of outbound "+ + "htlc: %v", err) + } + if err := ForceStateTransition(aliceChannel, bobChannel); err != nil { + t.Fatalf("Can't update the channel state: %v", err) + } + + aliceBalanceFinal := aliceChannel.channelState.LocalCommitment.LocalBalance //nolint:ll + bobBalanceFinal := bobChannel.channelState.LocalCommitment.LocalBalance + + // The balances of Alice and Bob should be the exact same and shouldn't + // have changed. + require.Equal(t, aliceBalance, aliceBalanceFinal) + require.Equal(t, bobBalance, bobBalanceFinal) +} diff --git a/lnwallet/payment_descriptor.go b/lnwallet/payment_descriptor.go index 49b79a139dc..f59aba02383 100644 --- a/lnwallet/payment_descriptor.go +++ b/lnwallet/payment_descriptor.go @@ -42,6 +42,12 @@ const ( // FeeUpdate is an update type sent by the channel initiator that // updates the fee rate used when signing the commitment transaction. FeeUpdate + + // NoopAdd is an update type that adds a new HTLC entry into the log. + // This differs from the normal Add type, in that when settled the + // balance will go back to the sender, rather than be credited for the + // receiver. + NoopAdd ) // String returns a human readable string that uniquely identifies the target @@ -58,6 +64,8 @@ func (u updateType) String() string { return "Settle" case FeeUpdate: return "FeeUpdate" + case NoopAdd: + return "NoopAdd" default: return "" } @@ -238,7 +246,7 @@ type paymentDescriptor struct { func (pd *paymentDescriptor) toLogUpdate() channeldb.LogUpdate { var msg lnwire.Message switch pd.EntryType { - case Add: + case Add, NoopAdd: msg = &lnwire.UpdateAddHTLC{ ChanID: pd.ChanID, ID: pd.HtlcIndex, @@ -290,7 +298,7 @@ func (pd *paymentDescriptor) setCommitHeight( whoseCommitChain lntypes.ChannelParty, nextHeight uint64) { switch pd.EntryType { - case Add: + case Add, NoopAdd: pd.addCommitHeights.SetForParty( whoseCommitChain, nextHeight, ) @@ -311,3 +319,8 @@ func (pd *paymentDescriptor) setCommitHeight( ) } } + +// isAdd returns true if the paymentDescriptor is of type Add. +func (pd *paymentDescriptor) isAdd() bool { + return pd.EntryType == Add || pd.EntryType == NoopAdd +}