lnwallet: add new NoopAdd payDesc entry type

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.

lnwallet/channel.go

@@ -552,6 +552,20 @@
 		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 @@
 		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 @@
 		theirPkScript:      theirP2WSH,
 		theirWitnessScript: theirWitnessScript,
 		BlindingPoint:      htlc.BlindingPoint,
-		CustomRecords:      htlc.CustomRecords.Copy(),
+		CustomRecords:      customRecords,
 	}, nil
 }
 
@@ -1101,6 +1115,11 @@
 			},
 		}
 
+		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 @@
 				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 @@
 		// 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 @@
 		}
 
 		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 @@
 			)
 			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 @@
 		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 @@
 	// 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 @@
 		// 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 @@
 		// 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 @@
 			// 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 @@
 		// 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 @@
 				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 @@
 	// 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 @@
 	// 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) 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 @@
 		OnionBlob:      htlc.OnionBlob,
 		OpenCircuitKey: openKey,
 		BlindingPoint:  htlc.BlindingPoint,
-		CustomRecords:  htlc.CustomRecords.Copy(),
+		CustomRecords:  customRecords,
 	}
 }
 
@@ -6107,17 +6168,32 @@
 			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,
 		LogIndex:      lc.updateLogs.Remote.logIndex,
 		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 @@
 
 		// We don't save add updates as they are restored from the
 		// remote commitment in restoreStateLogs.
-		if pd.EntryType == Add {
+		if pd.isAdd() {
 			continue
 		}
 

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)
+}