[1/?] Local reputation: subsystem core, read only

docs/release-notes/release-notes-0.22.0.md

@@ -59,6 +59,15 @@
 
 ## Functional Enhancements
 
+* A new experimental [local reputation
+  subsystem](https://github.com/lightningnetwork/lnd/pull/10919) tracks the
+  historical forwarding behaviour of peers, following the scoring recommended in
+  BOLT [#1280](https://github.com/lightning/bolts/pull/1280). It is enabled by
+  default but is purely observational: it watches forwarded HTLCs to compute and
+  log a per-HTLC reputation decision (whether the HTLC could stand on the
+  outgoing channel's reputation if forwarded in isolation) and does not currently
+  affect routing in any way. It can be disabled with `routing.no-reputation`.
+
 ## RPC Additions
 
 * The `routerrpc.EstimateRouteFee` RPC now supports [restricting fee estimates

htlcswitch/interfaces.go

@@ -279,6 +279,13 @@ type ChannelLink interface {
 	// policy to govern if it an incoming HTLC should be forwarded or not.
 	UpdateForwardingPolicy(models.ForwardingPolicy)
 
+	// AdvertisedFee returns the fee this link's current forwarding policy
+	// charges to forward the given outgoing amount (base fee plus the
+	// proportional fee). It is the fee the node advertised for this link,
+	// as distinct from the (possibly larger) fee actually offered by the
+	// incoming HTLC.
+	AdvertisedFee(amtToForward lnwire.MilliSatoshi) lnwire.MilliSatoshi
+
 	// CheckHtlcForward should return a nil error if the passed HTLC details
 	// satisfy the current forwarding policy fo the target link. Otherwise,
 	// a LinkError with a valid protocol failure message should be returned
@@ -515,6 +522,43 @@ type htlcNotifier interface {
 		info channeldb.FinalHtlcInfo)
 }
 
+// ReputationManager is the read-only seam through which the switch feeds HTLC
+// forwarding lifecycle events to the (optional) local reputation subsystem.
+// It is a black box that only observes events to update internal reputation
+// state; it never affects forwarding decisions or the wire (log-only). When no
+// reputation manager is configured this is nil and the hooks are skipped.
+type ReputationManager interface {
+	// OnForward observes a forwarded HTLC at the point the switch
+	// commits to forwarding it to the outgoing channel. advertisedFee is
+	// the total fee the node advertised for this forward (the outgoing
+	// link's outbound fee plus the incoming link's inbound fee, clamped
+	// at zero; not the fee offered by the incoming HTLC), height is the
+	// switch's current best block height, and accountable is the outgoing
+	// accountable bit as this node would forward it.
+	//
+	// Only the outgoing channel is identified, not the outgoing HTLC: at
+	// this point the switch has not yet handed the packet to the outgoing
+	// link, so no outgoing HTLC ID has been assigned. HTLCs are therefore
+	// tracked by their incoming circuit key, which is stable for the whole
+	// lifecycle.
+	OnForward(incoming CircuitKey, outgoing lnwire.ShortChannelID,
+		incomingAmt, outgoingAmt, advertisedFee lnwire.MilliSatoshi,
+		incomingCltv, height uint32, accountable bool)
+
+	// OnSettle observes the successful resolution of a forwarded HTLC.
+	//
+	// Resolutions identify the HTLC by its incoming circuit key alone:
+	// not every resolution path knows the outgoing channel (an add failed
+	// back through the outgoing link's mailbox, for example, never had a
+	// keystone set), so the manager matches resolutions to the forwards
+	// it recorded by circuit key.
+	OnSettle(incoming CircuitKey)
+
+	// OnFail observes the failed resolution of a forwarded HTLC. See
+	// OnSettle for how the HTLC is identified.
+	OnFail(incoming CircuitKey)
+}
+
 // AuxHtlcModifier is an interface that allows the sender to modify the outgoing
 // HTLC of a payment by changing the amount or the wire message tlv records.
 type AuxHtlcModifier interface {

htlcswitch/link.go

@@ -79,6 +79,24 @@ func ExpectedFee(f models.ForwardingPolicy,
 	return f.BaseFee + (htlcAmt*f.FeeRate)/1000000
 }
 
+// TotalForwardingFee returns the total fee this node charges to forward
+// amtToForward: the outbound fee the outgoing link charges on the outgoing
+// amount, plus the inbound fee of the incoming link, which is charged on the
+// outgoing amount plus the outbound fee.
+//
+// The two components are calculated and rounded separately on purpose. An
+// aggregate fee applied to the outgoing amount may round slightly higher than
+// the sum of the separately rounded components, which would cause failed
+// forwards for senders. The result is signed because the inbound component may
+// be a discount.
+func TotalForwardingFee(amtToForward, outFee lnwire.MilliSatoshi,
+	inboundFee models.InboundFee) int64 {
+
+	inFee := inboundFee.CalcFee(amtToForward + outFee)
+
+	return inFee + int64(outFee)
+}
+
 // ChannelLinkConfig defines the configuration for the channel link. ALL
 // elements within the configuration MUST be non-nil for channel link to carry
 // out its duties.
@@ -2482,6 +2500,20 @@ func (l *channelLink) UpdateForwardingPolicy(
 	l.cfg.FwrdingPolicy = newPolicy
 }
 
+// AdvertisedFee returns the fee this link's current forwarding policy charges
+// to forward the given outgoing amount (base fee plus the proportional fee).
+//
+// NOTE: Part of the ChannelLink interface.
+func (l *channelLink) AdvertisedFee(
+	amtToForward lnwire.MilliSatoshi) lnwire.MilliSatoshi {
+
+	l.RLock()
+	policy := l.cfg.FwrdingPolicy
+	l.RUnlock()
+
+	return ExpectedFee(policy, amtToForward)
+}
+
 // CheckHtlcForward should return a nil error if the passed HTLC details
 // satisfy the current forwarding policy fo the target link. Otherwise,
 // a LinkError with a valid protocol failure message should be returned
@@ -2501,20 +2533,9 @@ func (l *channelLink) CheckHtlcForward(payHash [32]byte, incomingHtlcAmt,
 
 	// Using the outgoing HTLC amount, we'll calculate the outgoing
 	// fee this incoming HTLC must carry in order to satisfy the constraints
-	// of the outgoing link.
+	// of the outgoing link, then add the inbound fee we charge on top.
 	outFee := ExpectedFee(policy, amtToForward)
-
-	// Then calculate the inbound fee that we charge based on the sum of
-	// outgoing HTLC amount and outgoing fee.
-	inFee := inboundFee.CalcFee(amtToForward + outFee)
-
-	// Add up both fee components. It is important to calculate both fees
-	// separately. An alternative way of calculating is to first determine
-	// an aggregate fee and apply that to the outgoing HTLC amount. However,
-	// rounding may cause the result to be slightly higher than in the case
-	// of separately rounded fee components. This potentially causes failed
-	// forwards for senders and is something to be avoided.
-	expectedFee := inFee + int64(outFee)
+	expectedFee := TotalForwardingFee(amtToForward, outFee, inboundFee)
 
 	// If the actual fee is less than our expected fee, then we'll reject
 	// this HTLC as it didn't provide a sufficient amount of fees, or the

htlcswitch/link_test.go

@@ -7764,3 +7764,43 @@ func TestLinkQuiescenceExitHopProcessingDeferred(t *testing.T) {
 
 	// TODO(proofofkeags): make sure these actions are run on resume.
 }
+
+// TestTotalForwardingFee checks that the total forwarding fee is the outbound
+// fee plus the inbound fee charged on the outgoing amount plus the outbound
+// fee, and that an inbound discount yields a signed, possibly negative, total.
+func TestTotalForwardingFee(t *testing.T) {
+	t.Parallel()
+
+	const (
+		amt    = lnwire.MilliSatoshi(100_000)
+		outFee = lnwire.MilliSatoshi(1_000)
+	)
+
+	tests := []struct {
+		name       string
+		inboundFee models.InboundFee
+		want       int64
+	}{{
+		name: "no inbound fee",
+		want: 1_000,
+	}, {
+		// 500 base + 1% of (100_000 + 1_000) = 500 + 1_010.
+		name:       "inbound fee on amount plus outbound fee",
+		inboundFee: models.InboundFee{Base: 500, Rate: 10_000},
+		want:       2_510,
+	}, {
+		// A discount larger than the outbound fee goes negative.
+		name:       "inbound discount",
+		inboundFee: models.InboundFee{Base: -1_500},
+		want:       -500,
+	}}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			got := TotalForwardingFee(amt, outFee, test.inboundFee)
+			require.Equal(t, test.want, got)
+		})
+	}
+}

htlcswitch/mock.go

@@ -738,6 +738,10 @@ type mockChannelLink struct {
 
 	checkHtlcForwardResult *LinkError
 
+	// advertisedFee is the fee returned by AdvertisedFee, letting tests
+	// control the outgoing link's advertised forwarding fee.
+	advertisedFee lnwire.MilliSatoshi
+
 	failAliasUpdate func(sid lnwire.ShortChannelID,
 		incoming bool) *lnwire.ChannelUpdate1
 
@@ -847,6 +851,12 @@ func (f *mockChannelLink) HandleChannelUpdate(lnwire.Message) {
 
 func (f *mockChannelLink) UpdateForwardingPolicy(_ models.ForwardingPolicy) {
 }
+
+func (f *mockChannelLink) AdvertisedFee(
+	_ lnwire.MilliSatoshi) lnwire.MilliSatoshi {
+
+	return f.advertisedFee
+}
 func (f *mockChannelLink) CheckHtlcForward([32]byte, lnwire.MilliSatoshi,
 	lnwire.MilliSatoshi, uint32, uint32, models.InboundFee, uint32,
 	lnwire.ShortChannelID, lnwire.CustomRecords) *LinkError {

htlcswitch/reputation_guard.go

@@ -0,0 +1,68 @@
+package htlcswitch
+
+import (
+	"github.com/lightningnetwork/lnd/lnwire"
+)
+
+// guardedReputationManager wraps a ReputationManager so that a panic in any of
+// its hooks can never propagate into the switch's forwarding goroutine. The
+// reputation subsystem is log-only and MUST NOT be able to degrade forwarding;
+// if a hook panics we log it and carry on forwarding.
+//
+// The hooks run synchronously on the switch's forwarding goroutine, so this
+// boundary keeps a subsystem bug, such as a nil deref or an arithmetic panic,
+// from taking down the node's HTLC forwarding.
+type guardedReputationManager struct {
+	inner ReputationManager
+}
+
+// NewGuardedReputationManager wraps the given ReputationManager with a panic
+// boundary. It returns nil when inner is nil, so the switch's existing nil
+// check still short-circuits a disabled subsystem with zero overhead.
+func NewGuardedReputationManager(inner ReputationManager) ReputationManager {
+	if inner == nil {
+		return nil
+	}
+
+	return &guardedReputationManager{inner: inner}
+}
+
+// OnForward forwards the observation to the wrapped manager behind a panic
+// boundary.
+func (g *guardedReputationManager) OnForward(incoming CircuitKey,
+	outgoing lnwire.ShortChannelID, incomingAmt, outgoingAmt,
+	advertisedFee lnwire.MilliSatoshi, incomingCltv, height uint32,
+	accountable bool) {
+
+	defer g.recoverHook("OnForward")
+
+	g.inner.OnForward(
+		incoming, outgoing, incomingAmt, outgoingAmt, advertisedFee,
+		incomingCltv, height, accountable,
+	)
+}
+
+// OnSettle forwards the observation to the wrapped manager behind a panic
+// boundary.
+func (g *guardedReputationManager) OnSettle(incoming CircuitKey) {
+	defer g.recoverHook("OnSettle")
+
+	g.inner.OnSettle(incoming)
+}
+
+// OnFail forwards the observation to the wrapped manager behind a panic
+// boundary.
+func (g *guardedReputationManager) OnFail(incoming CircuitKey) {
+	defer g.recoverHook("OnFail")
+
+	g.inner.OnFail(incoming)
+}
+
+// recoverHook recovers from a panic in a reputation hook and logs it, so that a
+// bug in the log-only subsystem cannot affect forwarding.
+func (g *guardedReputationManager) recoverHook(method string) {
+	if r := recover(); r != nil {
+		log.Errorf("Reputation %s hook panicked (forwarding is "+
+			"unaffected): %v", method, r)
+	}
+}