[1/?] Local reputation: subsystem core, read only by GeorgeTsagk · Pull Request #10919 · lightningnetwork/lnd

GeorgeTsagk · 2026-06-23T15:26:33Z

Description

Adds a subsystem that implements local reputation as proposed here.

You can read more about channel jamming mitigationa here.

The current goal is to only record and calculate revenue/reputation averages in a log-only mode, meaning that:

we record HTLC add/settle/fail times
we don't affect HTLC forwarding at all: an HTLC rejection due to insufficient reputation is a no-op
we log individual HTLC "mock" decision, "would this HTLC make it into protected slots?"

This PR aims to be non-invasive to existing HTLC forwarding code paths. A reviewer treating the reputation subsystem as a black-box should be confident that by recording HTLC events via the reputation subsystem we're not interrupting any other operation.

Checklist for undrafting

Break up commits into smaller self-explanatory ones
Add better itest coverage (probably a debug API to verify reputation numbers as well)
~~[ ] (?) Handle cold start (historical traffic read)~~ for 2nd part
~~[x] (?) Properly handle in-flight HTLCs when restarting~~ for 2nd part

github-actions · 2026-06-23T15:30:42Z

PR Severity: CRITICAL

Automated classification | 21 non-test files | ~3,427 lines changed (excluding tests/generated)

CRITICAL (4 files)

htlcswitch/interfaces.go - htlcswitch package; HTLC forwarding/payment routing state machine
htlcswitch/reputation_guard.go - htlcswitch package; new reputation gating for HTLC forwarding
htlcswitch/switch.go - htlcswitch package; core switch integration
server.go - Core server coordination

MEDIUM (16 files)

lncfg/routing.go - lncfg/* routing config
log.go - top-level logger registration
reputation_adapter.go - adapter wiring reputation manager into server
reputation/buckets.go - new reputation package (uncategorized)
reputation/channel.go - new reputation package
reputation/channels.go - new reputation package
reputation/clock.go - new reputation package
reputation/config.go - new reputation package
reputation/decaying_average.go - new reputation package
reputation/decision.go - new reputation package
reputation/htlc.go - new reputation package
reputation/log.go - new reputation package
reputation/manager.go - new reputation package
reputation/manager_startup.go - new reputation package
reputation/revenue.go - new reputation package
reputation/store.go - new reputation package

LOW (13 files -- excluded from counts)

htlcswitch/reputation_hooks_test.go, reputation/*_test.go -- test files
itest/list_on_test.go, itest/lnd_reputation_test.go -- integration tests
lntest/harness_assertion.go -- test harness
reputation/DESIGN.md -- documentation

Analysis

This PR introduces a new channel reputation system for HTLC jamming mitigation. The critical classification is driven by direct modifications to htlcswitch -- one of lnd's most sensitive packages governing HTLC forwarding and the payment routing state machine -- and to server.go (core server coordination).

Key concerns warranting careful review:

htlcswitch/switch.go: Integration of reputation gating into the HTLC forwarding path. Any regression could cause incorrect HTLC accept/reject decisions, affecting payment reliability.
htlcswitch/reputation_guard.go (new file, 82 lines): New guard logic sitting in the critical forwarding path.
htlcswitch/interfaces.go: Interface additions that all implementors must satisfy; watch for subtle behavioral changes.
reputation/manager.go (650 lines): Large new manager with in-memory state, startup logic, and a backing store -- persistence correctness and concurrency safety should be verified.

Both severity-bump thresholds are exceeded (21 non-test files, ~3,427 non-test lines), but the base severity was already CRITICAL.

To override, add a severity-override-{critical,high,medium,low} label.

carlaKC · 2026-07-08T19:34:15Z

Chatted to @GeorgeTsagk about strategies to break up this PR up and lighten review burden on the LND team!

PR Breakdown

I was talking to claude about this, and produced this plan, but zero promises because I haven't even read it - just an artifact from this discussion!

(commits marked with * are dead code for the sake of incremental steps, could be squashed if that's not okay)

1. Implement reputation tracking*

Decaying averages
Tracking peers reputation
Ability to add/remove HTLCs to this system

2. Connect to switch

Report HTLCs to reputation manager (including on restart)
Set experimental field based on accountable and reputation signal
Log reputation decision for HTLCS
No persistence, no loading historical forwards
Minimal tracking of current HTLC set (if required to update reputation)
Log HTLC reputation data

3. Restarts and in-flight

Persist revenue and reputation for peers
Perform "best effort" load from historical forwards if DB values are missing

Once we get to this point, we get a very rudimentary "would this HTLC in isolation be able to enter the protected bucket (if needed)" sanity check. It doesn't take into account that there may be other HTLCs in flight, or whether we'll actually need to use protected resources, but this is a very valuable sanity check that we can't otherwise obtain with the data that's currently surfaced in LND (because we don't have historical failed forwards).

4. Implement bucketing logic*

General bucket slot tracking
Bucket state management
Benchmarks for performance (this is a place we've identified we need to be careful!)

5. Utilize buckets

Add decision making for bucket + reputation
Persistence of bucket data for in-flight HTLCs
Connect manager to bucket system

Other RPCs/snapshots can be added after that, but if the majority of folks aren't running LND with dev server then I think the value of surfacing this information in separate APIs is minimal. Perhaps could think about adding to more mainstream APIs (like listchannels), but that decision doesn't need to happen now IMO.

Review

@elnosh and I are happy to review here! We'll be able to provide strong reviews on the jamming work, since it's our focus. I should be able to provide reasonable review on the switch interactions, though my view of this system is of course a few years stale!

GeorgeTsagk · 2026-07-09T10:12:46Z

Thanks @carlaKC for writing the summary.

So I believe the next step here is to strip some things away from this PR and only keep 1 & 2:

Decaying averages & reputation
HTLC switch read-only hooks (feeding HTLCs to the system)
Individual HTLC mock-decision log (no buckets) i.e "if this HTLC was being forwarded in isolation, could it be protected"

This should leave us with a more minimal & lean diff, leaving out any noisy parts related to restarts/persistence and cold start.

Another comment on this strategy: if we ever deploy 1&2, then reputation systems in the wild will already start recording values from forwarding, at that point I don't think it would make sense to ship historical-read as a follow-up update to this system, we are practically doing a slow-bootstrap already.

carlaKC · 2026-07-09T12:59:02Z

So I believe the next step here is to strip some things away from this PR and only keep 1 & 2

Yeah SGTM! If we're okay with a bit of temporarily dead code, I think it makes sense to do 1 / 2 as separate PRs for the sake of small incremental steps. That's a question of project preferences, so depends on how LND prefers to do things nowadays.

then reputation systems in the wild will already start recording values from forwarding, at that point I don't think it would make sense to ship historical-read as a follow-up update to this system, we are practically doing a slow-bootstrap already.

Indeed! We do need 6 months data to get realreal values, so perhaps for (3) we could just focus on persistence, because we won't get far if we lose all our data every time we restart. Just 2x fields per channel, so not too bad!

@erickcestari also agreed to help out with review ❣️

GeorgeTsagk · 2026-07-16T12:05:24Z

Ok marking this as ready for review, it now adds:

reputation subsystem and related math (+tests)
hooks into htlcswitch -> feeding HTLC traffic data into the system
devRPC methods to help expose internal values
basic e2e itest

carlaKC

Primarily reviewed the first commit, haven't looked at the tests yet.

High level thoughts:

I think it's worth spending a bit more time thinking about how this interacts with the switch, and whether a queue is the right call here.
There are a few places where this can be better aligned with how LND does things, both major things like using existing interfaces and shorter comments
I am concerned by pointing a LLM at the LDK pr, it puts us at risk of propagating bugs and makes the process of improving the spec by having to implement it weaker
Snapshot and dev rpc are pretty low value IMO, would far rather see benchmarking

GeorgeTsagk · 2026-07-27T16:51:02Z

Thanks for the feedback @carlaKC

Following your suggestions, I totally dropped the dev RPC methods.

Added the benchmark, which shows that the reputation subsystem adds an extra 0.5μs of processing time per HTLC, which is beyond acceptable IMO.

Ready for another round.

litbot-9000 · 2026-08-03T17:52:17Z

@carlaKC: review reminder
@erickcestari: review reminder

carlaKC · 2026-08-03T17:58:49Z

Will get to this early this week - it's on my list!

erickcestari

Sorry for late review 😅

I could learn a lot about how the proposed reputation model works.

It's looking really good. It's not my final review yet, but I'll get there soon.

erickcestari · 2026-08-05T19:55:45Z

+		// Scoring reputation/revenue on the offered fee would let a
+		// sender inflate or destroy reputation by over/under-paying;
+		// the advertised fee is what the node actually charges.
+		advertisedFee := destination.AdvertisedFee(packet.amount)


Should this include the inbound fee?

CheckHtlcForward treats the fee we charge as inFee + outFee (link.go:2519-2531), but AdvertisedFee returns just outFee. So we admit the HTLC against one number and score reputation on another, and on a node with inbound fees configured the score is off by the inbound component.

The spec's fees is "the fees that are charged by the local node to forward the HTLC", which I read as the total.

Hm interesting. I think we should at least be consistent in our use of one or the other.

Given that the inbound fee is also chosen by the forwarding node, I don't see a way that this could be abused by an attacker (obviously, no negative fees - if we ever live in that world).

Will think about this a bit more. @elnosh also interested to hear your thoughts here!

We could merge them and attribute to the same outgoing link. This will mess up a bit with the accuracy of reputation distribution (N incoming, 1 outgoing, N different incoming fees on each case). An outgoing link will be rewarded all the reputation under this model.

Not accounting for it also doesn't sound right. These are fees that the sender (i.e attacker) pays, and all simulations were verified based on attack cost > denied revenue. If anything we're becoming more strict by not rewarding the inFee, which is also a concern w.r.t nodes not building sufficient reputation.

First one seems like the way to go.

carlaKC

Haven't reviewed the tests in great depth, thanks for addressing previous feeback!

carlaKC · 2026-08-07T14:55:04Z

+	if warmup < 1 {
+		warmup = 1
+	}


This isn't in the specification.

Was also pointed out in the LDK PR. I'd very strongly suggest pointing LLMs to claude (not the other impl) so that we don't perpetuate bugs.

Agreed it isn't in the spec, but removing it doesn't work: as periods approaches 0 so does the divisor, so a new channel's revenue reads inflate by roughly 5 orders of magnitude and the comparison stops meaning anything. Both this implementation and LDK added a floor independently, so the spec probably needs to define the periods -> 0 case. Happy to open a spec PR. Preference between flooring the divisor at 1 and not applying the threshold until some minimum history exists?

carlaKC · 2026-08-12T13:31:08Z

Another thing that's worth taking a look at here is how this will work with non-strict forwarding. I think we'll report one outgoing channel on add and resolve with a different one - worth confirming with a test that we can handle it gracefully!

GeorgeTsagk · 2026-08-18T13:36:53Z

Another thing that's worth taking a look at here is how this will work with non-strict forwarding. I think we'll report one outgoing channel on add and resolve with a different one - worth confirming with a test that we can handle it gracefully!

Checked, we call the reputation hook right after the strict pick takes place. So it's the real pick from the get go:

lnd/htlcswitch/switch.go

Lines 3038 to 3068 in 85bd467

    
           packet.outgoingChanID = destination.ShortChanID() 
        
           // Feed the (read-only) reputation manager this forward. This only 
        
           // observes the event to update internal reputation state; it never 
        
           // affects the forwarding decision (log-only). 
        
           if s.cfg.ReputationManager != nil { 
        
           	// Use the fee the node ADVERTISED on the outgoing link for this 
        
           	// forward (base fee + proportional over the outgoing amount), 
        
           	// not the (possibly larger) fee offered by the incoming HTLC. 
        
           	// Scoring reputation/revenue on the offered fee would let a 
        
           	// sender inflate or destroy reputation by over/under-paying; 
        
           	// the advertised fee is what the node actually charges. 
        
           	advertisedFee := destination.AdvertisedFee(packet.amount) 
        
           	// Derive the outgoing accountable bit exactly as the outgoing 
        
           	// link does: only accountable if we received it accountable AND 
        
           	// this node forwards the experimental accountability signal. A 
        
           	// node running --protocol.no-experimental-accountability drops 
        
           	// the bit, so it must not penalise a peer never told the HTLC 
        
           	// was accountable. 
        
           	outgoingAccountable := htlcAccountable(htlc) && 
        
           		s.shouldFwdExpAccountability() 
        
           	s.cfg.ReputationManager.OnForward( 
        
           		CircuitKey{ 
        
           			ChanID: packet.incomingChanID, 
        
           			HtlcID: packet.incomingHTLCID, 
        
           		}, 
        
           		packet.outgoingChanID, packet.incomingAmount, 
        
           		packet.amount, advertisedFee, packet.incomingTimeout, 
        
           		s.BestHeight(), outgoingAccountable,

Edit: will add a test, won't hurt

Add the numeric primitives underlying local reputation scoring, following the "Decaying Average" and "Revenue Threshold Aggregation" sections of BOLT lightningnetwork#1280, plus a package README describing the subsystem: - saturatedI64: int64 arithmetic that clamps rather than wraps, so the long-window fee accumulators never silently flip sign. - decayingAverage: a value decaying as e^(-elapsed/window) per the spec's decay_rate. - aggregatedWindowAverage: a decaying average over several windows with the spec's exponential warm-up factor.

Add the per-channel reputation state and the BOLT lightningnetwork#1280 scoring rules built on the decaying-average primitives: - Config: the tunable parameters (resolution period, revenue window, reputation multiplier, revenue window count) with the spec defaults. - effectiveFee/opportunityCost/inFlightRisk: an HTLC's contribution to reputation and its worst-case in-flight risk. - channelReputation: the per-channel outgoing reputation, incoming revenue threshold and pending HTLCs, plus the sufficiency inequality outgoing_reputation - risk >= revenue_threshold.

Add the Manager that ties the scoring together behind the OnForward/OnSettle/ OnFail hooks. The hooks run synchronously under a single lock: OnForward records the pending HTLC and computes (and logs) the reputation decision, both for the HTLC in isolation and against the risk already in flight on its outgoing channel, while OnSettle/OnFail resolve it and update the outgoing reputation and incoming revenue averages. The subsystem is log-only and holds no persisted state, so reputation re-accrues from live traffic after a restart. Every resolution drops its own pending HTLC, so a pending that outlives the worst case time it could be held for means a resolution was never reported to us. A periodic check warns about those and deliberately leaves them in place rather than sweeping them away, so the underlying bug stays visible. Includes unit tests and benchmarks for the per-forward hook cost.

Feed forwarded HTLCs to the reputation subsystem through a read-only seam on the switch. The switch calls OnForward/OnSettle/OnFail at the circuit layer behind a nil check, so the subsystem is skipped entirely when disabled. The manager is wrapped in a panic boundary before being handed to the switch: a bug in the (log-only) subsystem can never take down HTLC forwarding. Only the outgoing channel is reported to the subsystem, not an outgoing circuit key: at forward time the switch has not yet handed the packet to the outgoing link, so no outgoing HTLC ID exists yet. The subsystem is enabled by default and can be disabled with the new routing.no-reputation flag. Includes unit tests for the switch seam: each hook fires once with the right keys, a nil manager is a no-op, local sends are skipped, a hook panic is absorbed by the guard, and a non-strict forward reports the channel the HTLC actually went out on for both the add and its resolution.

Add an integration test asserting that a forwarding node running the log-only reputation subsystem forwards, fails and restarts exactly as it would without it, while emitting the expected reputation log lines.

GeorgeTsagk · 2026-08-18T14:05:59Z

Thanks for the feedback @erickcestari and @carlaKC. I have addressed your comments (some threads still open for discussion, will resolve async as we figure things out).

GeorgeTsagk self-assigned this Jun 23, 2026

github-actions Bot added the severity-critical Requires expert review - security/consensus critical label Jun 23, 2026

GeorgeTsagk force-pushed the local-reputation-subsystem branch 2 times, most recently from 1be208a to 5bdb907 Compare June 24, 2026 12:46

github-actions Bot added severity-critical Requires expert review - security/consensus critical and removed severity-critical Requires expert review - security/consensus critical labels Jun 24, 2026

GeorgeTsagk force-pushed the local-reputation-subsystem branch 3 times, most recently from 615d701 to 516c694 Compare June 25, 2026 13:12

carlaKC mentioned this pull request Jun 29, 2026

Project Updates: Hybrid Channel Jamming Mitigation lightning/bolts#1218

Open

23 tasks

GeorgeTsagk force-pushed the local-reputation-subsystem branch from 516c694 to 8813fe2 Compare July 2, 2026 13:55

GeorgeTsagk force-pushed the local-reputation-subsystem branch from 8813fe2 to d061d4a Compare July 14, 2026 18:35

github-actions Bot added severity-critical Requires expert review - security/consensus critical and removed severity-critical Requires expert review - security/consensus critical labels Jul 14, 2026

GeorgeTsagk force-pushed the local-reputation-subsystem branch 3 times, most recently from 304508e to 81315c4 Compare July 16, 2026 11:38

GeorgeTsagk changed the title ~~Add local reputation subsystem (read-only)~~ [1/?] Local reputation: subsystem core, read only Jul 16, 2026

GeorgeTsagk marked this pull request as ready for review July 16, 2026 12:05

GeorgeTsagk added routing channel jamming Issues related to channel jamming mitigation logging Related to the logging / debug output functionality labels Jul 16, 2026

GeorgeTsagk requested review from carlaKC and erickcestari July 16, 2026 14:56

carlaKC reviewed Jul 20, 2026

View reviewed changes

GeorgeTsagk force-pushed the local-reputation-subsystem branch from 81315c4 to c6480fe Compare July 27, 2026 16:47

GeorgeTsagk requested a review from carlaKC July 27, 2026 16:51

erickcestari requested changes Aug 5, 2026

View reviewed changes

carlaKC reviewed Aug 7, 2026

View reviewed changes

erickcestari requested changes Aug 12, 2026

View reviewed changes

Comment thread reputation_adapter.go Outdated

Comment thread htlcswitch/interfaces.go

GeorgeTsagk force-pushed the local-reputation-subsystem branch from c6480fe to 85bd467 Compare August 18, 2026 13:07

GeorgeTsagk added 6 commits August 18, 2026 13:40

itest: test the local reputation subsystem end-to-end

6523ba1

Add an integration test asserting that a forwarding node running the log-only reputation subsystem forwards, fails and restarts exactly as it would without it, while emitting the expected reputation log lines.

docs: add release note for the local reputation subsystem

15630d7

GeorgeTsagk force-pushed the local-reputation-subsystem branch from 85bd467 to 15630d7 Compare August 18, 2026 13:41

GeorgeTsagk requested review from carlaKC and erickcestari August 18, 2026 14:05

Uh oh!

Conversation

GeorgeTsagk commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist for undrafting

Uh oh!

github-actions Bot commented Jun 23, 2026

PR Severity: CRITICAL

Analysis

Uh oh!

carlaKC commented Jul 8, 2026

PR Breakdown

1. Implement reputation tracking*

2. Connect to switch

3. Restarts and in-flight

4. Implement bucketing logic*

5. Utilize buckets

Review

Uh oh!

GeorgeTsagk commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carlaKC commented Jul 9, 2026

Uh oh!

GeorgeTsagk commented Jul 16, 2026

Uh oh!

carlaKC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GeorgeTsagk commented Jul 27, 2026

Uh oh!

litbot-9000 commented Aug 3, 2026

Uh oh!

carlaKC commented Aug 3, 2026

Uh oh!

erickcestari left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

erickcestari Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

carlaKC Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

GeorgeTsagk Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carlaKC left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

carlaKC Aug 7, 2026

Choose a reason for hiding this comment

Uh oh!

GeorgeTsagk Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GeorgeTsagk commented Jun 23, 2026 •

edited

Loading

GeorgeTsagk commented Jul 9, 2026 •

edited

Loading

GeorgeTsagk Aug 19, 2026 •

edited

Loading

GeorgeTsagk Aug 18, 2026 •

edited

Loading

GeorgeTsagk commented Aug 18, 2026 •

edited

Loading

GeorgeTsagk commented Aug 18, 2026 •

edited

Loading