Skip to content

Commit b3fbc48

Browse files
committed
htlcswitch: add read-only reputation hooks and panic guard
Add three nil-guarded hooks at the switch's circuit-matching layer (handlePacketAdd/handlePacketSettle/handlePacketFail) that feed forwarded HTLC add/settle/fail events to an optional ReputationManager. The switch defines the interface; it never imports the reputation package, keeping the dependency a black box that can only call observation methods. The hooks pass copies of scalar values (circuit keys, amounts, cltv, the accountable bit) — no pointers into switch state — so they cannot mutate forwarding. A guardedReputationManager wraps the manager in a recover() boundary: because the hooks run on the forwarding goroutine, a panic in the (log-only) subsystem must never take down HTLC forwarding, so on a panic the guard logs, disables the subsystem, and forwarding continues unaffected.
1 parent 316d99d commit b3fbc48

4 files changed

Lines changed: 661 additions & 0 deletions

File tree

htlcswitch/interfaces.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,25 @@ type htlcNotifier interface {
489489
info channeldb.FinalHtlcInfo)
490490
}
491491

492+
// ReputationManager is the read-only seam through which the switch feeds HTLC
493+
// forwarding lifecycle events to the (optional) local reputation subsystem.
494+
// It is a black box that only observes events to update internal reputation
495+
// state; it never affects forwarding decisions or the wire (log-only). When no
496+
// reputation manager is configured this is nil and the hooks are skipped.
497+
type ReputationManager interface {
498+
// OnForward observes a forwarded HTLC at the point the switch
499+
// commits to forwarding it to the outgoing channel.
500+
OnForward(incoming, outgoing CircuitKey, incomingAmt,
501+
outgoingAmt lnwire.MilliSatoshi, incomingCltv uint32,
502+
accountable bool)
503+
504+
// OnSettle observes the successful resolution of a forwarded HTLC.
505+
OnSettle(incoming, outgoing CircuitKey)
506+
507+
// OnFail observes the failed resolution of a forwarded HTLC.
508+
OnFail(incoming, outgoing CircuitKey)
509+
}
510+
492511
// AuxHtlcModifier is an interface that allows the sender to modify the outgoing
493512
// HTLC of a payment by changing the amount or the wire message tlv records.
494513
type AuxHtlcModifier interface {

htlcswitch/reputation_guard.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package htlcswitch
2+
3+
import (
4+
"sync/atomic"
5+
6+
"github.com/lightningnetwork/lnd/lnwire"
7+
)
8+
9+
// guardedReputationManager wraps a ReputationManager so that a panic in any of
10+
// its hooks can never propagate into the switch's forwarding goroutine. The
11+
// reputation subsystem is log-only and MUST NOT be able to degrade forwarding;
12+
// if a hook panics we log it, permanently disable the subsystem (fail open),
13+
// and continue forwarding unaffected.
14+
//
15+
// The hooks run synchronously on the switch's forwarding goroutine, so this is
16+
// the boundary that keeps a subsystem bug — a nil deref, a bad map access, an
17+
// arithmetic panic — from taking down the node's HTLC forwarding.
18+
type guardedReputationManager struct {
19+
inner ReputationManager
20+
disabled atomic.Bool
21+
}
22+
23+
// NewGuardedReputationManager wraps the given ReputationManager with a panic
24+
// boundary. It returns nil when inner is nil, so the switch's existing nil
25+
// check still short-circuits a disabled subsystem with zero overhead.
26+
func NewGuardedReputationManager(inner ReputationManager) ReputationManager {
27+
if inner == nil {
28+
return nil
29+
}
30+
31+
return &guardedReputationManager{inner: inner}
32+
}
33+
34+
// OnForward forwards the observation to the wrapped manager behind a panic
35+
// boundary.
36+
func (g *guardedReputationManager) OnForward(incoming, outgoing CircuitKey,
37+
incomingAmt, outgoingAmt lnwire.MilliSatoshi, incomingCltv uint32,
38+
accountable bool) {
39+
40+
if g.disabled.Load() {
41+
return
42+
}
43+
defer g.recoverHook("OnForward")
44+
45+
g.inner.OnForward(
46+
incoming, outgoing, incomingAmt, outgoingAmt, incomingCltv,
47+
accountable,
48+
)
49+
}
50+
51+
// OnSettle forwards the observation to the wrapped manager behind a panic
52+
// boundary.
53+
func (g *guardedReputationManager) OnSettle(incoming, outgoing CircuitKey) {
54+
if g.disabled.Load() {
55+
return
56+
}
57+
defer g.recoverHook("OnSettle")
58+
59+
g.inner.OnSettle(incoming, outgoing)
60+
}
61+
62+
// OnFail forwards the observation to the wrapped manager behind a panic
63+
// boundary.
64+
func (g *guardedReputationManager) OnFail(incoming, outgoing CircuitKey) {
65+
if g.disabled.Load() {
66+
return
67+
}
68+
defer g.recoverHook("OnFail")
69+
70+
g.inner.OnFail(incoming, outgoing)
71+
}
72+
73+
// recoverHook recovers from a panic in a reputation hook, logging it and
74+
// permanently disabling the subsystem so a deterministic bug cannot panic on
75+
// every forwarded HTLC. Forwarding is never affected.
76+
func (g *guardedReputationManager) recoverHook(method string) {
77+
if r := recover(); r != nil {
78+
log.Errorf("Reputation %s hook panicked; disabling reputation "+
79+
"subsystem (forwarding is unaffected): %v", method, r)
80+
g.disabled.Store(true)
81+
}
82+
}

0 commit comments

Comments
 (0)