htlcswitch: key the aux traffic shaper on the evaluated channel
What changed, and why it matters
This commit fixes a routing bug in LND's Lightning payment forwarding. When a payment could take any of several parallel channels to the same next peer, the node was accidentally asking an optional 'auxiliary traffic shaper' about the channel the sender requested, rather than the channel actually being considered. That could lead to wrong bandwidth/custom-policy decisions and might leak the real channel ID if not carefully handled. The patch keys the shaper on the actual evaluated channel while keeping the sender-facing error messages on the requested (often alias) channel ID, so no real ID leaks onto the network.
Treat as a correctness/security hygiene fix and include in the next maintenance release. Users running custom AuxTrafficShaper implementations with parallel channels to peers should upgrade. No immediate emergency response is indicated, but operators should monitor for inconsistent forwarding decisions on parallel channels.
Security signals we found
Logic error: wrong channel identifier used for auxiliary policy/bandwidth check
Potential information disclosure: real SCID could leak if passed to shaper-driven wire messages; commit explicitly prevents this
Parallel-channel forwarding correctness issue
No explicit security wording in commit title/message
Regression test added to enforce correct behavior
Evidence from the diff
In htlcswitch/link.go, canSendHtlc previously passed originalScid (the sender-requested outgoing short channel ID) into l.AuxBandwidth, which forwards it to AuxTrafficShaper.ShouldHandleTraffic. During non-strict forwarding, handlePacketAdd evaluates each candidate channelLink to the next peer, so using originalScid caused the shaper to be queried about the wrong channel when parallel channels exist. The patch changes the argument to l.ShortChanID(), i.e., the candidate link’s real SCID. originalScid is still used only for createFailureWithUpdate/FailAliasUpdate, preserving alias-aware channel_update messages. A new unit test records the SCID passed to the shaper and verifies both correct keying and non-leakage of the real SCID in failure updates.
Changed components
htlcswitch/link.go: canSendHtlc / AuxBandwidth callhtlcswitch/link_test.go: new TestCheckHtlcForwardAuxShaperChannelAuxTrafficShaper interface consumersInspect captured patch +133 / −1
diff --git a/htlcswitch/link.go b/htlcswitch/link.go
index 9e3adf0..715afe7 100644
--- a/htlcswitch/link.go
+++ b/htlcswitch/link.go
@@ -2668,7 +2668,10 @@ func (l *channelLink) canSendHtlc(policy models.ForwardingPolicy,
htlcBlob = fn.Some(blob)
}
- return l.AuxBandwidth(amt, originalScid, htlcBlob, ts)
+ // Check if this link can handle the traffic.
+ return l.AuxBandwidth(
+ amt, l.ShortChanID(), htlcBlob, ts,
+ )
},
).Unpack()
if externalErr != nil {
diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go
index 2876089..72e08f3 100644
--- a/htlcswitch/link_test.go
+++ b/htlcswitch/link_test.go
@@ -40,6 +40,7 @@ import (
"github.com/lightningnetwork/lnd/lnwallet/chainfee"
"github.com/lightningnetwork/lnd/lnwire"
"github.com/lightningnetwork/lnd/ticker"
+ "github.com/lightningnetwork/lnd/tlv"
"github.com/stretchr/testify/require"
)
@@ -6373,6 +6374,134 @@ func TestCheckHtlcForward(t *testing.T) {
})
}
+// recordingAuxShaper is a minimal AuxTrafficShaper that records the channel id
+// it is asked about and declines to handle the traffic, so the normal
+// forwarding path proceeds. Only the methods reached by CheckHtlcForward are
+// implemented; the rest are inherited from the embedded (nil) interface and
+// must never be called.
+type recordingAuxShaper struct {
+ AuxTrafficShaper
+
+ gotCID lnwire.ShortChannelID
+}
+
+// ShouldHandleTraffic records the short channel ID passed to the shaper.
+func (a *recordingAuxShaper) ShouldHandleTraffic(cid lnwire.ShortChannelID,
+ _, _ fn.Option[tlv.Blob]) (bool, error) {
+
+ a.gotCID = cid
+
+ return false, nil
+}
+
+// IsCustomHTLC returns false as recordingAuxShaper handles standard HTLCs.
+func (a *recordingAuxShaper) IsCustomHTLC(_ lnwire.CustomRecords) bool {
+ return false
+}
+
+// TestCheckHtlcForwardAuxShaperChannel asserts that during non-strict
+// forwarding the aux traffic shaper is keyed on the channel actually being
+// evaluated (the link's own SCID), not the sender-requested SCID, which fixes
+// both the node-ID/blinded path (where no SCID is requested) and pre-existing
+// parallel-channel forwarding. It also asserts the real SCID handed to the
+// shaper never leaks into the sender-facing channel_update, which continues to
+// reference the requested (alias) SCID.
+func TestCheckHtlcForwardAuxShaperChannel(t *testing.T) {
+ t.Parallel()
+
+ const (
+ chanScid = 42
+ requestedScid = 99
+ )
+
+ fetchLastChannelUpdate := func(lnwire.ShortChannelID) (
+ *lnwire.ChannelUpdate1, error) {
+
+ return &lnwire.ChannelUpdate1{}, nil
+ }
+
+ // Record the SCID used to build the returned channel_update on failure.
+ var updateScid lnwire.ShortChannelID
+ failAliasUpdate := func(sid lnwire.ShortChannelID,
+ incoming bool) *lnwire.ChannelUpdate1 {
+
+ updateScid = sid
+
+ return &lnwire.ChannelUpdate1{
+ ShortChannelID: sid,
+ }
+ }
+
+ testChannel, _, err := createTestChannel(
+ t, alicePrivKey, bobPrivKey, 100000, 100000, 1000, 1000,
+ lnwire.NewShortChanIDFromInt(chanScid),
+ )
+ require.NoError(t, err)
+
+ shaper := &recordingAuxShaper{}
+ link := channelLink{
+ cfg: ChannelLinkConfig{
+ FwrdingPolicy: models.ForwardingPolicy{
+ TimeLockDelta: 20,
+ MinHTLCOut: 500,
+ MaxHTLC: 1000,
+ BaseFee: 10,
+ },
+ FetchLastChannelUpdate: fetchLastChannelUpdate,
+ MaxOutgoingCltvExpiry: DefaultMaxOutgoingCltvExpiry,
+ HtlcNotifier: &mockHTLCNotifier{},
+ },
+ log: log,
+ channel: testChannel.channel,
+ }
+ link.cfg.AuxTrafficShaper = fn.Some[AuxTrafficShaper](shaper)
+ link.attachFailAliasUpdate(failAliasUpdate)
+
+ require.Equal(
+ t, lnwire.NewShortChanIDFromInt(chanScid), link.ShortChanID(),
+ )
+
+ var hash [32]byte
+ requested := lnwire.NewShortChanIDFromInt(requestedScid)
+
+ // A satisfiable forward: the shaper must be queried about the channel
+ // being evaluated (the link's own SCID), not the requested SCID.
+ result := link.CheckHtlcForward(
+ hash, 1500, 1000, 200, 150, models.InboundFee{}, 0, requested,
+ nil,
+ )
+ require.Nil(t, result, "expected policy to be satisfied")
+ require.Equal(
+ t, link.ShortChanID(), shaper.gotCID,
+ "aux shaper must be keyed on the evaluated channel",
+ )
+ require.NotEqual(
+ t, requested, shaper.gotCID,
+ "aux shaper must not be keyed on the requested SCID",
+ )
+
+ // A failing forward: the returned channel_update must reference the
+ // requested (alias) SCID, never the real channel SCID handed to the
+ // shaper.
+ result = link.CheckHtlcForward(
+ hash, 100, 50, 200, 150, models.InboundFee{}, 0, requested, nil,
+ )
+ require.NotNil(t, result)
+ require.Equal(
+ t, requested, updateScid,
+ "channel_update must reference the requested SCID, not the "+
+ "real channel SCID",
+ )
+
+ wireErr := result.WireMessage()
+ failAmt, ok := wireErr.(*lnwire.FailAmountBelowMinimum)
+ require.True(t, ok, "expected FailAmountBelowMinimum failure")
+ require.Equal(
+ t, requested, failAmt.Update.ShortChannelID,
+ "failure update must carry the requested SCID",
+ )
+}
+
// TestChannelLinkCanceledInvoice in this test checks the interaction
// between Alice and Bob for a canceled invoice.
func TestChannelLinkCanceledInvoice(t *testing.T) {
Why this scored 44/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.