From 26235ced95e8405ff7aa61de1f7a25caa10fd723 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 20 Jan 2025 19:26:26 -0800 Subject: [PATCH 1/3] lnwallet: add new NoopAdd payDesc entry type In this commit, we add a new NoopAdd payDesc entry type. This type is meant to be used primarily by taproot overlay channels. When we go to settle this HTLC, rather than credit the settler for the funds, we just give the funds back to the sender. This results in an add that when settled, doesn't affect the balance in the channel. This new HTLC type is intended to be used alongside a push amt, to ensure the remote party has a non-dust balance from the start. With that in place, then this new add type can be used for special overlay HTLCs. --- lnwallet/channel.go | 50 ++++++++++++++++++++++++------- lnwallet/channel_test.go | 54 +++++++++++++++++++++++++++++++++- lnwallet/payment_descriptor.go | 17 +++++++++-- 3 files changed, 107 insertions(+), 14 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 6486464d0f..31ca443104 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1737,7 +1737,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 } @@ -2974,6 +2974,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 +3025,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 +3064,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 +3828,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 @@ -5693,7 +5707,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 +5726,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 +5965,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 +5984,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 +6057,17 @@ func (lc *LightningChannel) MayAddOutgoingHtlc(amt lnwire.MilliSatoshi) error { func (lc *LightningChannel) htlcAddDescriptor(htlc *lnwire.UpdateAddHTLC, openKey *models.CircuitKey) *paymentDescriptor { + // TODO(rosabeef): can use push amt to simplify logic, not have to + // detect if remote party has enough funds for anchor + entryType := Add + + if lc.channelState.ChanType.HasTapscriptRoot() { + entryType = NoopAdd + } + return &paymentDescriptor{ ChanID: htlc.ChanID, - EntryType: Add, + EntryType: entryType, RHash: PaymentHash(htlc.PaymentHash), Timeout: htlc.Expiry, Amount: htlc.Amount, @@ -6107,9 +6129,15 @@ func (lc *LightningChannel) ReceiveHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, lc.updateLogs.Remote.htlcCounter) } + entryType := Add + + if lc.channelState.ChanType.HasTapscriptRoot() { + entryType = NoopAdd + } + pd := &paymentDescriptor{ ChanID: htlc.ChanID, - EntryType: Add, + EntryType: entryType, RHash: PaymentHash(htlc.PaymentHash), Timeout: htlc.Expiry, Amount: htlc.Amount, @@ -9765,7 +9793,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 9da174a3d8..b21530cab8 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 + 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 49b79a139d..8ee8c9a44b 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 @@ -238,7 +244,9 @@ type paymentDescriptor struct { func (pd *paymentDescriptor) toLogUpdate() channeldb.LogUpdate { var msg lnwire.Message switch pd.EntryType { - case Add: + // TODO(roasbeef): need custom record to be able to distinguish between + // restarts + 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 +} From a808f946a4510cc4fb8dd150c68f461782704011 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 22 Jan 2025 19:19:38 -0800 Subject: [PATCH 2/3] fixup! lnwallet: add new NoopAdd payDesc entry type --- lnwallet/aux_signer.go | 13 +++++-- lnwallet/channel.go | 62 ++++++++++++++++++++++++++++++---- lnwallet/payment_descriptor.go | 4 +-- 3 files changed, 67 insertions(+), 12 deletions(-) diff --git a/lnwallet/aux_signer.go b/lnwallet/aux_signer.go index 510b64b5d1..e3566ee534 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 31ca443104..909cbecfb6 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. @@ -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. @@ -4484,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, ) @@ -4515,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 @@ -6057,12 +6088,20 @@ func (lc *LightningChannel) MayAddOutgoingHtlc(amt lnwire.MilliSatoshi) error { func (lc *LightningChannel) htlcAddDescriptor(htlc *lnwire.UpdateAddHTLC, openKey *models.CircuitKey) *paymentDescriptor { - // TODO(rosabeef): can use push amt to simplify logic, not have to + // 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{ @@ -6076,7 +6115,7 @@ func (lc *LightningChannel) htlcAddDescriptor(htlc *lnwire.UpdateAddHTLC, OnionBlob: htlc.OnionBlob, OpenCircuitKey: openKey, BlindingPoint: htlc.BlindingPoint, - CustomRecords: htlc.CustomRecords.Copy(), + CustomRecords: customRecords, } } @@ -6131,7 +6170,16 @@ func (lc *LightningChannel) ReceiveHTLC(htlc *lnwire.UpdateAddHTLC) (uint64, 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 } @@ -6145,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 diff --git a/lnwallet/payment_descriptor.go b/lnwallet/payment_descriptor.go index 8ee8c9a44b..f59aba0238 100644 --- a/lnwallet/payment_descriptor.go +++ b/lnwallet/payment_descriptor.go @@ -64,6 +64,8 @@ func (u updateType) String() string { return "Settle" case FeeUpdate: return "FeeUpdate" + case NoopAdd: + return "NoopAdd" default: return "" } @@ -244,8 +246,6 @@ type paymentDescriptor struct { func (pd *paymentDescriptor) toLogUpdate() channeldb.LogUpdate { var msg lnwire.Message switch pd.EntryType { - // TODO(roasbeef): need custom record to be able to distinguish between - // restarts case Add, NoopAdd: msg = &lnwire.UpdateAddHTLC{ ChanID: pd.ChanID, From f7299ee64fb79e134780f312ef9f146fbc1b79cc Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 22 Jan 2025 19:30:29 -0800 Subject: [PATCH 3/3] fixup! lnwallet: add new NoopAdd payDesc entry type --- lnwallet/channel_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index b21530cab8..50a8cc0358 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -11316,7 +11316,7 @@ func TestNoopAddSettle(t *testing.T) { t.Fatalf("Can't update the channel state: %v", err) } - aliceBalanceFinal := aliceChannel.channelState.LocalCommitment.LocalBalance + 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