Merge pull request #9303 from yyforyongyu/fix-circuit-closed

htlcswitch+routing: handle nil pointer dereference properly

htlcswitch/packet.go

@@ -1,6 +1,8 @@
 package htlcswitch
 
 import (
+	"fmt"
+
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/graph/db/models"
 	"github.com/lightningnetwork/lnd/htlcswitch/hop"
@@ -136,3 +138,12 @@ func (p *htlcPacket) keystone() Keystone {
 		OutKey: p.outKey(),
 	}
 }
+
+// String returns a human-readable description of the packet.
+func (p *htlcPacket) String() string {
+	return fmt.Sprintf("keystone=%v, sourceRef=%v, destRef=%v, "+
+		"incomingAmount=%v, amount=%v, localFailure=%v, hasSource=%v "+
+		"isResolution=%v", p.keystone(), p.sourceRef, p.destRef,
+		p.incomingAmount, p.amount, p.localFailure, p.hasSource,
+		p.isResolution)
+}

htlcswitch/switch.go

@@ -2994,6 +2994,15 @@ func (s *Switch) handlePacketSettle(packet *htlcPacket) error {
 	// If the source of this packet has not been set, use the circuit map
 	// to lookup the origin.
 	circuit, err := s.closeCircuit(packet)
+
+	// If the circuit is in the process of closing, we will return a nil as
+	// there's another packet handling undergoing.
+	if errors.Is(err, ErrCircuitClosing) {
+		log.Debugf("Circuit is closing for packet=%v", packet)
+		return nil
+	}
+
+	// Exit early if there's another error.
 	if err != nil {
 		return err
 	}
@@ -3005,7 +3014,7 @@ func (s *Switch) handlePacketSettle(packet *htlcPacket) error {
 	// and when `UpdateFulfillHTLC` is received. After which `RevokeAndAck`
 	// is received, which invokes `processRemoteSettleFails` in its link.
 	if circuit == nil {
-		log.Debugf("Found nil circuit: packet=%v", spew.Sdump(packet))
+		log.Debugf("Circuit already closed for packet=%v", packet)
 		return nil
 	}
 

lnwallet/channel.go

@@ -1710,7 +1710,7 @@ func (lc *LightningChannel) restorePendingRemoteUpdates(
 	localCommitmentHeight uint64,
 	pendingRemoteCommit *commitment) error {
 
-	lc.log.Debugf("Restoring %v dangling remote updates",
+	lc.log.Debugf("Restoring %v dangling remote updates pending our sig",
 		len(unsignedAckedUpdates))
 
 	for _, logUpdate := range unsignedAckedUpdates {
@@ -1829,6 +1829,9 @@ func (lc *LightningChannel) restorePendingLocalUpdates(
 	pendingCommit := pendingRemoteCommitDiff.Commitment
 	pendingHeight := pendingCommit.CommitHeight
 
+	lc.log.Debugf("Restoring pending remote commitment %v at commit "+
+		"height %v", pendingCommit.CommitTx.TxHash(), pendingHeight)
+
 	auxResult, err := fn.MapOptionZ(
 		lc.leafStore,
 		func(s AuxLeafStore) fn.Result[CommitDiffAuxResult] {

routing/payment_lifecycle.go

@@ -189,10 +189,21 @@ func (p *paymentLifecycle) resumePayment(ctx context.Context) ([32]byte,
 		p.resultCollector(&a)
 	}
 
+	// Get the payment status.
+	status := payment.GetStatus()
+
 	// exitWithErr is a helper closure that logs and returns an error.
 	exitWithErr := func(err error) ([32]byte, *route.Route, error) {
-		log.Errorf("Payment %v with status=%v failed: %v",
-			p.identifier, payment.GetStatus(), err)
+		// Log an error with the latest payment status.
+		//
+		// NOTE: this `status` variable is reassigned in the loop
+		// below. We could also call `payment.GetStatus` here, but in a
+		// rare case when the critical log is triggered when using
+		// postgres as db backend, the `payment` could be nil, causing
+		// the payment fetching to return an error.
+		log.Errorf("Payment %v with status=%v failed: %v", p.identifier,
+			status, err)
+
 		return [32]byte{}, nil, err
 	}
 
@@ -213,10 +224,10 @@ lifecycle:
 		ps := payment.GetState()
 		remainingFees := p.calcFeeBudget(ps.FeesPaid)
 
+		status = payment.GetStatus()
 		log.Debugf("Payment %v: status=%v, active_shards=%v, "+
-			"rem_value=%v, fee_limit=%v", p.identifier,
-			payment.GetStatus(), ps.NumAttemptsInFlight,
-			ps.RemainingAmt, remainingFees)
+			"rem_value=%v, fee_limit=%v", p.identifier, status,
+			ps.NumAttemptsInFlight, ps.RemainingAmt, remainingFees)
 
 		// We now proceed our lifecycle with the following tasks in
 		// order,