[v0.20.x-branch] Backport #11132: peer: answer every valid ping

docs/release-notes/release-notes-0.20.5.md

@@ -33,6 +33,11 @@
   keeping the invoice open so that other accepted sets on reusable static
   AMP invoices remain payable.
 
+* Peers now [answer every valid inbound
+  Ping](https://github.com/lightningnetwork/lnd/pull/11132) as required by
+  BOLT 1. The existing request flood limit remains the connection teardown
+  boundary instead of silently suppressing otherwise valid Pong replies.
+
 # New Features
 
 ## Functional Enhancements
@@ -85,3 +90,4 @@
 
 * elsirion
 * Gijs van Dam
+* Yong Yu

peer/brontide.go

@@ -53,6 +53,7 @@ import (
 	"github.com/lightningnetwork/lnd/ticker"
 	"github.com/lightningnetwork/lnd/tlv"
 	"github.com/lightningnetwork/lnd/watchtower/wtclient"
+	"golang.org/x/time/rate"
 )
 
 const (
@@ -542,8 +543,9 @@ type Brontide struct {
 
 	pingManager *PingManager
 
-	// pingLimits owns the two per-connection inbound Ping policies.
-	pingLimits pingLimits
+	// pingLimiter bounds inbound Ping work before message routing. Keeping
+	// the limiter on the peer gives each connection an independent budget.
+	pingLimiter *rate.Limiter
 
 	// queueLimits supplies one accounting policy to the producer and queue.
 	queueLimits queueLimits
@@ -698,7 +700,7 @@ func NewBrontide(cfg Config) *Brontide {
 		activeSignal:  make(chan struct{}),
 		sendQueue:     make(chan outgoingMsg),
 		outgoingQueue: make(chan outgoingMsg),
-		pingLimits:    defaultPingLimits(),
+		pingLimiter:   defaultPingLimiter(),
 		queueLimits:   defaultQueueLimits(),
 		addedChannels: &lnutils.SyncMap[lnwire.ChannelID, struct{}]{},
 		activeChannels: &lnutils.SyncMap[
@@ -2140,10 +2142,13 @@ out:
 		}
 
 		// Count valid Pings before routing because consuming endpoints
-		// skip the switch. Oversized requests never reach this point
-		// because this release rejects them during wire decoding.
-		if _, ok := nextMsg.(*lnwire.Ping); ok &&
-			!p.pingLimits.pingLimiter.Allow() {
+		// skip the switch. Valid requests pay proportionally for their
+		// Pong bytes. Oversized requests never reach this point because
+		// this release rejects them during wire decoding.
+		ping, ok := nextMsg.(*lnwire.Ping)
+		if ok && !p.pingLimiter.AllowN(
+			time.Now(), calcPingCost(ping),
+		) {
 
 			p.storeError(errPingFlood)
 			p.log.Warnf("%v", errPingFlood)
@@ -2188,15 +2193,9 @@ out:
 			// the relevant atomic variable.
 			p.lastPingPayload.Store(msg.PaddingBytes[:])
 
-			// BOLT 1 requires a Pong for every Ping below the size
-			// ceiling. We limit reply frequency to guard against
-			// floods; normal keepalives remain below this limit.
-			if !p.pingLimits.pongLimiter.Allow() {
-				p.log.Debugf("Pong reply rate limited")
-				continue
-			}
-			// Next, we'll send over the amount of specified pong
-			// bytes.
+			// BOLT 1 requires a Pong of the requested size for
+			// every Ping below the size ceiling. The request flood
+			// limiter above disconnects abusive peers first.
 			pong := lnwire.NewPong(p.cfg.PongBuf[0:msg.NumPongBytes])
 			p.queueMsg(pong, nil)