diff --git a/htlcswitch/link.go b/htlcswitch/link.go index bb800f5e4..b1836c7c7 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -2551,17 +2551,8 @@ func (l *channelLink) CheckHtlcForward(payHash [32]byte, policy := l.cfg.FwrdingPolicy l.RUnlock() - // First check whether the outgoing htlc satisfies the channel policy. - err := l.canSendHtlc( - policy, payHash, amtToForward, outgoingTimeout, heightNow, - originalScid, - ) - if err != nil { - return err - } - - // Next, using the amount of the incoming HTLC, we'll calculate the - // expected fee this incoming HTLC must carry in order to satisfy the + // Using the amount of the incoming HTLC, we'll calculate the expected + // fee this incoming HTLC must carry in order to satisfy the // constraints of the outgoing link. expectedFee := ExpectedFee(policy, amtToForward) @@ -2569,7 +2560,8 @@ func (l *channelLink) CheckHtlcForward(payHash [32]byte, // this HTLC as it didn't provide a sufficient amount of fees, or the // values have been tampered with, or the send used incorrect/dated // information to construct the forwarding information for this hop. In - // any case, we'll cancel this HTLC. + // any case, we'll cancel this HTLC. We're checking for this case first + // to leak as little information as possible. actualFee := incomingHtlcAmt - amtToForward if incomingHtlcAmt < amtToForward || actualFee < expectedFee { l.log.Warnf("outgoing htlc(%x) has insufficient fee: "+ @@ -2585,6 +2577,15 @@ func (l *channelLink) CheckHtlcForward(payHash [32]byte, return NewLinkError(failure) } + // Check whether the outgoing htlc satisfies the channel policy. + err := l.canSendHtlc( + policy, payHash, amtToForward, outgoingTimeout, heightNow, + originalScid, + ) + if err != nil { + return err + } + // Finally, we'll ensure that the time-lock on the outgoing HTLC meets // the following constraint: the incoming time-lock minus our time-lock // delta should equal the outgoing time lock. Otherwise, whether the diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index 55d77d13d..b58c8a63b 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -5643,6 +5643,19 @@ func TestCheckHtlcForward(t *testing.T) { } }) + // Test that insufficient fee error takes preference over insufficient + // channel balance. + t.Run("insufficient fee error preference", func(t *testing.T) { + t.Parallel() + + result := link.CheckHtlcForward( + hash, 100005, 100000, 200, + 150, 0, lnwire.ShortChannelID{}, + ) + _, ok := result.WireMessage().(*lnwire.FailFeeInsufficient) + require.True(t, ok, "expected FailFeeInsufficient failure code") + }) + t.Run("expiry too soon", func(t *testing.T) { result := link.CheckHtlcForward(hash, 1500, 1000, 200, 150, 190, lnwire.ShortChannelID{}) diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 2e0400340..b1e134954 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -5339,6 +5339,7 @@ func testSwitchHandlePacketForward(t *testing.T, zeroConf, private, ogPacket := &htlcPacket{ incomingChanID: bobLink.ShortChanID(), incomingHTLCID: 0, + incomingAmount: 1000, obfuscator: NewMockObfuscator(), htlc: &lnwire.UpdateAddHTLC{ PaymentHash: rhash,