multi: introduce new traffic shaper method.

We introduce a new specific fail resolution error when the
external HTLC interceptor denies the incoming HTLC. Moreover
we introduce a new traffic shaper method which moves the
implementation of asset HTLC to the external layers.
Moreover itests are adopted to reflect this new change.

Author: ziggie1984

htlcswitch/interfaces.go

@@ -508,4 +508,9 @@ type AuxTrafficShaper interface {
 	PaymentBandwidth(htlcBlob, commitmentBlob fn.Option[tlv.Blob],
 		linkBandwidth,
 		htlcAmt lnwire.MilliSatoshi) (lnwire.MilliSatoshi, error)
+
+	// IsCustomHTLC returns true if the HTLC carries the set of relevant
+	// custom records to put it under the purview of the traffic shaper,
+	// meaning that it's from a custom channel.
+	IsCustomHTLC(htlcRecords lnwire.CustomRecords) bool
 }

htlcswitch/link.go

@@ -4164,21 +4164,20 @@ func (l *channelLink) processExitHop(add lnwire.UpdateAddHTLC,
 		return nil
 	}
 
+	// In case the traffic shaper is active, we'll check if the HTLC has
+	// custom records and skip the amount check in the onion payload below.
+	isCustomHTLC := fn.MapOptionZ(
+		l.cfg.AuxTrafficShaper,
+		func(ts AuxTrafficShaper) bool {
+			return ts.IsCustomHTLC(add.CustomRecords)
+		},
+	)
+
 	// As we're the exit hop, we'll double check the hop-payload included in
 	// the HTLC to ensure that it was crafted correctly by the sender and
-	// is compatible with the HTLC we were extended.
-	//
-	// For a special case, if the fwdInfo doesn't have any blinded path
-	// information, and the incoming HTLC had special extra data, then
-	// we'll skip this amount check. The invoice acceptor will make sure we
-	// reject the HTLC if it's not containing the correct amount after
-	// examining the custom data.
-	hasBlindedPath := fwdInfo.NextBlinding.IsSome()
-	customHTLC := len(add.CustomRecords) > 0 && !hasBlindedPath
-	log.Tracef("Exit hop has_blinded_path=%v custom_htlc_bypass=%v",
-		hasBlindedPath, customHTLC)
-
-	if !customHTLC && add.Amount < fwdInfo.AmountToForward {
+	// is compatible with the HTLC we were extended. If an external
+	// validator is active we might bypass the amount check.
+	if !isCustomHTLC && add.Amount < fwdInfo.AmountToForward {
 		l.log.Errorf("onion payload of incoming htlc(%x) has "+
 			"incompatible value: expected <=%v, got %v",
 			add.PaymentHash, add.Amount, fwdInfo.AmountToForward)

invoices/invoiceregistry.go

@@ -1117,13 +1117,15 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked(
 				return nil, nil
 			}
 
-			// If a cancel signal was set for the htlc set, we set
-			// the resolution as a failure with an underpayment
-			// indication. Something was wrong with this htlc, so
-			// we probably can't settle the invoice at all.
+			// The error `ExternalValidationFailed` error
+			// information will be packed in the
+			// `FailIncorrectDetails` msg when sending the msg to
+			// the peer. Error codes are defined by the BOLT 04
+			// specification. The error text can be arbitrary
+			// therefore we return a custom error msg.
 			resolution = NewFailResolution(
 				ctx.circuitKey, ctx.currentHeight,
-				ResultAmountTooLow,
+				ExternalValidationFailed,
 			)
 
 			// We cancel all HTLCs which are in the accepted state.

invoices/modification_interceptor.go

@@ -137,9 +137,6 @@ func (s *HtlcModificationInterceptor) Intercept(clientRequest HtlcModifyRequest,
 	// Wait for the client to respond or an error to occur.
 	select {
 	case response := <-responseChan:
-		log.Debugf("Received invoice HTLC interceptor response: %v",
-			response)
-
 		responseCallback(*response)
 
 		return nil

invoices/resolution_result.go

@@ -120,6 +120,10 @@ const (
 	// ResultAmpReconstruction is returned when the derived child
 	// hash/preimage pairs were invalid for at least one HTLC in the set.
 	ResultAmpReconstruction
+
+	// ExternalValidationFailed is returned when the external validation
+	// failed.
+	ExternalValidationFailed
 )
 
 // String returns a string representation of the result.
@@ -189,6 +193,9 @@ func (f FailResolutionResult) FailureString() string {
 	case ResultAmpReconstruction:
 		return "amp reconstruction failed"
 
+	case ExternalValidationFailed:
+		return "external validation failed"
+
 	default:
 		return "unknown failure resolution result"
 	}
@@ -202,7 +209,8 @@ func (f FailResolutionResult) IsSetFailure() bool {
 		ResultAmpReconstruction,
 		ResultHtlcSetTotalTooLow,
 		ResultHtlcSetTotalMismatch,
-		ResultHtlcSetOverpayment:
+		ResultHtlcSetOverpayment,
+		ExternalValidationFailed:
 
 		return true
 

itest/list_on_test.go

@@ -406,14 +406,6 @@ var allTestCases = []*lntest.TestCase{
 		Name:     "forward interceptor",
 		TestFunc: testForwardInterceptorBasic,
 	},
-	{
-		Name:     "forward interceptor modified htlc",
-		TestFunc: testForwardInterceptorModifiedHtlc,
-	},
-	{
-		Name:     "forward interceptor wire records",
-		TestFunc: testForwardInterceptorWireRecords,
-	},
 	{
 		Name:     "forward interceptor restart",
 		TestFunc: testForwardInterceptorRestart,

itest/lnd_forward_interceptor_test.go

@@ -10,7 +10,6 @@ import (
 	"github.com/btcsuite/btcd/btcutil"
 	"github.com/lightningnetwork/lnd/chainreg"
 	"github.com/lightningnetwork/lnd/lnrpc"
-	"github.com/lightningnetwork/lnd/lnrpc/invoicesrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
 	"github.com/lightningnetwork/lnd/lntest"
 	"github.com/lightningnetwork/lnd/lntest/node"
@@ -346,223 +345,6 @@ func testForwardInterceptorBasic(ht *lntest.HarnessTest) {
 	}
 }
 
-// testForwardInterceptorModifiedHtlc tests that the interceptor can modify the
-// amount and custom records of an intercepted HTLC and resume it.
-func testForwardInterceptorModifiedHtlc(ht *lntest.HarnessTest) {
-	const chanAmt = btcutil.Amount(300000)
-	p := lntest.OpenChannelParams{Amt: chanAmt}
-
-	// Initialize the test context with 3 connected nodes.
-	cfgs := [][]string{nil, nil, nil}
-
-	// Open and wait for channels.
-	_, nodes := ht.CreateSimpleNetwork(cfgs, p)
-	alice, bob, carol := nodes[0], nodes[1], nodes[2]
-
-	// Init the scenario.
-	ts := &interceptorTestScenario{
-		ht:    ht,
-		alice: alice,
-		bob:   bob,
-		carol: carol,
-	}
-
-	// Connect an interceptor to Bob's node.
-	bobInterceptor, cancelBobInterceptor := bob.RPC.HtlcInterceptor()
-
-	// We're going to modify the payment amount and want Carol to accept the
-	// payment, so we set up an invoice acceptor on Dave.
-	carolAcceptor, carolCancel := carol.RPC.InvoiceHtlcModifier()
-	defer carolCancel()
-
-	// Prepare the test cases.
-	invoiceValueAmtMsat := int64(20_000_000)
-	req := &lnrpc.Invoice{ValueMsat: invoiceValueAmtMsat}
-	addResponse := carol.RPC.AddInvoice(req)
-	invoice := carol.RPC.LookupInvoice(addResponse.RHash)
-	tc := &interceptorTestCase{
-		amountMsat: invoiceValueAmtMsat,
-		invoice:    invoice,
-		payAddr:    invoice.PaymentAddr,
-	}
-
-	// We initiate a payment from Alice.
-	done := make(chan struct{})
-	go func() {
-		// Signal that all the payments have been sent.
-		defer close(done)
-
-		ts.sendPaymentAndAssertAction(tc)
-	}()
-
-	// We start the htlc interceptor with a simple implementation that saves
-	// all intercepted packets. These packets are held to simulate a
-	// pending payment.
-	packet := ht.ReceiveHtlcInterceptor(bobInterceptor)
-
-	// Resume the intercepted HTLC with a modified amount and custom
-	// records.
-	customRecords := make(map[uint64][]byte)
-
-	// Add custom records entry.
-	crKey := uint64(65537)
-	crValue := []byte("custom-records-test-value")
-	customRecords[crKey] = crValue
-
-	// Modify the amount of the HTLC, so we send out less than the original
-	// amount.
-	const modifyAmount = 5_000_000
-	newOutAmountMsat := packet.OutgoingAmountMsat - modifyAmount
-	err := bobInterceptor.Send(&routerrpc.ForwardHtlcInterceptResponse{
-		IncomingCircuitKey:   packet.IncomingCircuitKey,
-		OutAmountMsat:        newOutAmountMsat,
-		OutWireCustomRecords: customRecords,
-		Action:               actionResumeModify,
-	})
-	require.NoError(ht, err, "failed to send request")
-
-	invoicePacket := ht.ReceiveInvoiceHtlcModification(carolAcceptor)
-	require.EqualValues(
-		ht, newOutAmountMsat, invoicePacket.ExitHtlcAmt,
-	)
-	amtPaid := newOutAmountMsat + modifyAmount
-	err = carolAcceptor.Send(&invoicesrpc.HtlcModifyResponse{
-		CircuitKey: invoicePacket.ExitHtlcCircuitKey,
-		AmtPaid:    &amtPaid,
-	})
-	require.NoError(ht, err, "carol acceptor response")
-
-	// Cancel the context, which will disconnect Bob's interceptor.
-	cancelBobInterceptor()
-
-	// Make sure all goroutines are finished.
-	select {
-	case <-done:
-	case <-time.After(defaultTimeout):
-		require.Fail(ht, "timeout waiting for sending payment")
-	}
-
-	// Assert that the payment was successful.
-	var preimage lntypes.Preimage
-	copy(preimage[:], invoice.RPreimage)
-	ht.AssertPaymentStatus(alice, preimage, lnrpc.Payment_SUCCEEDED)
-}
-
-// testForwardInterceptorWireRecords tests that the interceptor can read any
-// wire custom records provided by the sender of a payment as part of the
-// update_add_htlc message.
-func testForwardInterceptorWireRecords(ht *lntest.HarnessTest) {
-	const chanAmt = btcutil.Amount(300000)
-	p := lntest.OpenChannelParams{Amt: chanAmt}
-
-	// Initialize the test context with 4 connected nodes.
-	cfgs := [][]string{nil, nil, nil, nil}
-
-	// Open and wait for channels.
-	_, nodes := ht.CreateSimpleNetwork(cfgs, p)
-	alice, bob, carol, dave := nodes[0], nodes[1], nodes[2], nodes[3]
-
-	// Connect an interceptor to Bob's node.
-	bobInterceptor, cancelBobInterceptor := bob.RPC.HtlcInterceptor()
-	defer cancelBobInterceptor()
-
-	// Also connect an interceptor on Carol's node to check whether we're
-	// relaying the TLVs send in update_add_htlc over Alice -> Bob on the
-	// Bob -> Carol link.
-	carolInterceptor, cancelCarolInterceptor := carol.RPC.HtlcInterceptor()
-	defer cancelCarolInterceptor()
-
-	// We're going to modify the payment amount and want Dave to accept the
-	// payment, so we set up an invoice acceptor on Dave.
-	daveAcceptor, daveCancel := dave.RPC.InvoiceHtlcModifier()
-	defer daveCancel()
-
-	req := &lnrpc.Invoice{ValueMsat: 20_000_000}
-	addResponse := dave.RPC.AddInvoice(req)
-	invoice := dave.RPC.LookupInvoice(addResponse.RHash)
-
-	customRecords := map[uint64][]byte{
-		65537: []byte("test"),
-	}
-	sendReq := &routerrpc.SendPaymentRequest{
-		PaymentRequest:        invoice.PaymentRequest,
-		TimeoutSeconds:        int32(wait.PaymentTimeout.Seconds()),
-		FeeLimitMsat:          noFeeLimitMsat,
-		FirstHopCustomRecords: customRecords,
-	}
-	ht.SendPaymentAssertInflight(alice, sendReq)
-
-	// We start the htlc interceptor with a simple implementation that saves
-	// all intercepted packets. These packets are held to simulate a
-	// pending payment.
-	packet := ht.ReceiveHtlcInterceptor(bobInterceptor)
-	require.Equal(ht, lntest.CustomRecordsWithUnendorsed(
-		customRecords,
-	), packet.InWireCustomRecords)
-
-	// Just resume the payment on Bob.
-	err := bobInterceptor.Send(&routerrpc.ForwardHtlcInterceptResponse{
-		IncomingCircuitKey: packet.IncomingCircuitKey,
-		Action:             actionResume,
-	})
-	require.NoError(ht, err, "failed to send request")
-
-	// Assert that the Alice -> Bob custom records in update_add_htlc are
-	// not propagated on the Bob -> Carol link, just an endorsement signal.
-	packet = ht.ReceiveHtlcInterceptor(carolInterceptor)
-	require.Equal(ht, lntest.CustomRecordsWithUnendorsed(nil),
-		packet.InWireCustomRecords)
-
-	// We're going to tell Carol to forward 5k sats less to Dave. We need to
-	// set custom records on the HTLC as well, to make sure the HTLC isn't
-	// rejected outright and actually gets to the invoice acceptor.
-	const modifyAmount = 5_000_000
-	newOutAmountMsat := packet.OutgoingAmountMsat - modifyAmount
-	err = carolInterceptor.Send(&routerrpc.ForwardHtlcInterceptResponse{
-		IncomingCircuitKey:   packet.IncomingCircuitKey,
-		OutAmountMsat:        newOutAmountMsat,
-		OutWireCustomRecords: customRecords,
-		Action:               actionResumeModify,
-	})
-	require.NoError(ht, err, "carol interceptor response")
-
-	// The payment should get to Dave, and we should be able to intercept
-	// and modify it, telling Dave to accept it.
-	invoicePacket := ht.ReceiveInvoiceHtlcModification(daveAcceptor)
-	require.EqualValues(
-		ht, newOutAmountMsat, invoicePacket.ExitHtlcAmt,
-	)
-	amtPaid := newOutAmountMsat + modifyAmount
-	err = daveAcceptor.Send(&invoicesrpc.HtlcModifyResponse{
-		CircuitKey: invoicePacket.ExitHtlcCircuitKey,
-		AmtPaid:    &amtPaid,
-	})
-	require.NoError(ht, err, "dave acceptor response")
-
-	// Assert that the payment was successful.
-	var preimage lntypes.Preimage
-	copy(preimage[:], invoice.RPreimage)
-	ht.AssertPaymentStatus(
-		alice, preimage, lnrpc.Payment_SUCCEEDED,
-		func(p *lnrpc.Payment) error {
-			recordsEqual := reflect.DeepEqual(
-				p.FirstHopCustomRecords,
-				lntest.CustomRecordsWithUnendorsed(
-					customRecords,
-				),
-			)
-			if !recordsEqual {
-				return fmt.Errorf("expected custom records to "+
-					"be equal, got %v expected %v",
-					p.FirstHopCustomRecords,
-					sendReq.FirstHopCustomRecords)
-			}
-
-			return nil
-		},
-	)
-}
-
 // testForwardInterceptorRestart tests that the interceptor can read any wire
 // custom records provided by the sender of a payment as part of the
 // update_add_htlc message and that those records are persisted correctly and

routing/bandwidth_test.go

@@ -164,3 +164,7 @@ func (*mockTrafficShaper) ProduceHtlcExtraData(totalAmount lnwire.MilliSatoshi,
 
 	return totalAmount, nil, nil
 }
+
+func (*mockTrafficShaper) IsCustomHTLC(_ lnwire.CustomRecords) bool {
+	return false
+}