From 3b6e28d19b992d71b1ab8fd40b46e37660e9c6a1 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Wed, 15 Nov 2023 03:58:54 +0800 Subject: [PATCH] channeldb+htlcswitch: make sure circuit is not nil in `teardownCircuit` --- channeldb/forwarding_package.go | 5 +++ htlcswitch/link.go | 2 +- htlcswitch/switch.go | 65 +++++++++++++-------------------- 3 files changed, 32 insertions(+), 40 deletions(-) diff --git a/channeldb/forwarding_package.go b/channeldb/forwarding_package.go index 09f5671cc..4c447fc03 100644 --- a/channeldb/forwarding_package.go +++ b/channeldb/forwarding_package.go @@ -211,6 +211,11 @@ func (f *PkgFilter) Decode(r io.Reader) error { return err } +// String returns a human-readable string. +func (f *PkgFilter) String() string { + return fmt.Sprintf("count=%v, filter=%v", f.count, f.filter) +} + // FwdPkg records all adds, settles, and fails that were locked in as a result // of the remote peer sending us a revocation. Each package is identified by // the short chanid and remote commitment height corresponding to the revocation diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 7b7688d9c..337ce636c 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -3297,7 +3297,7 @@ func (l *channelLink) processRemoteSettleFails(fwdPkg *channeldb.FwdPkg, return } - l.log.Debugf("settle-fail-filter %v", fwdPkg.SettleFailFilter) + l.log.Debugf("settle-fail-filter: %v", fwdPkg.SettleFailFilter) var switchPackets []*htlcPacket for i, pd := range settleFails { diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 9ed4202ee..1ac8f93a5 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -967,7 +967,7 @@ func (s *Switch) handleLocalResponse(pkt *htlcPacket) { // This can only happen if the circuit is still open, which is why this // ordering is chosen. if err := s.teardownCircuit(pkt); err != nil { - log.Warnf("Unable to teardown circuit %s: %v", + log.Errorf("Unable to teardown circuit %s: %v", pkt.inKey(), err) return } @@ -1359,47 +1359,34 @@ func (s *Switch) teardownCircuit(pkt *htlcPacket) error { case *lnwire.UpdateFailHTLC: pktType = "FAIL" default: - err := fmt.Errorf("cannot tear down packet of type: %T", htlc) - log.Errorf(err.Error()) + return fmt.Errorf("cannot tear down packet of type: %T", htlc) + } + + var paymentHash lntypes.Hash + + // Perform a defensive check to make sure we don't try to access a nil + // circuit. + circuit := pkt.circuit + if circuit != nil { + copy(paymentHash[:], circuit.PaymentHash[:]) + } + + log.Debugf("Tearing down circuit with %s pkt, removing circuit=%v "+ + "with keystone=%v", pktType, pkt.inKey(), pkt.outKey()) + + err := s.circuits.DeleteCircuits(pkt.inKey()) + if err != nil { + log.Warnf("Failed to tear down circuit (%s, %d) <-> (%s, %d) "+ + "with payment_hash=%v using %s pkt", pkt.incomingChanID, + pkt.incomingHTLCID, pkt.outgoingChanID, + pkt.outgoingHTLCID, pkt.circuit.PaymentHash, pktType) + return err } - switch { - case pkt.circuit.HasKeystone(): - log.Debugf("Tearing down open circuit with %s pkt, removing circuit=%v "+ - "with keystone=%v", pktType, pkt.inKey(), pkt.outKey()) - - err := s.circuits.DeleteCircuits(pkt.inKey()) - if err != nil { - log.Warnf("Failed to tear down open circuit (%s, %d) <-> (%s, %d) "+ - "with payment_hash-%v using %s pkt", - pkt.incomingChanID, pkt.incomingHTLCID, - pkt.outgoingChanID, pkt.outgoingHTLCID, - pkt.circuit.PaymentHash, pktType) - return err - } - - log.Debugf("Closed completed %s circuit for %x: "+ - "(%s, %d) <-> (%s, %d)", pktType, pkt.circuit.PaymentHash, - pkt.incomingChanID, pkt.incomingHTLCID, - pkt.outgoingChanID, pkt.outgoingHTLCID) - - default: - log.Debugf("Tearing down incomplete circuit with %s for inkey=%v", - pktType, pkt.inKey()) - - err := s.circuits.DeleteCircuits(pkt.inKey()) - if err != nil { - log.Warnf("Failed to tear down pending %s circuit for %x: "+ - "(%s, %d)", pktType, pkt.circuit.PaymentHash, - pkt.incomingChanID, pkt.incomingHTLCID) - return err - } - - log.Debugf("Removed pending onion circuit for %x: "+ - "(%s, %d)", pkt.circuit.PaymentHash, - pkt.incomingChanID, pkt.incomingHTLCID) - } + log.Debugf("Closed %s circuit for %v: (%s, %d) <-> (%s, %d)", pktType, + paymentHash, pkt.incomingChanID, pkt.incomingHTLCID, + pkt.outgoingChanID, pkt.outgoingHTLCID) return nil }