diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index 592d03a1c..dab25aaeb 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -1000,6 +1000,8 @@ func (s *Switch) extractResult(deobfuscator ErrorDecrypter, n *networkResult, // We've received a fail update which means we can finalize the // user payment and return fail response. case *lnwire.UpdateFailHTLC: + // TODO(yy): construct deobfuscator here to avoid creating it + // in paymentLifecycle even for settled HTLCs. paymentErr := s.parseFailedPayment( deobfuscator, attemptID, paymentHash, n.unencrypted, n.isResolution, htlc, diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index d0f43d7b0..38489b141 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -456,15 +456,27 @@ func (p *paymentLifecycle) collectResult(attempt *channeldb.HTLCAttempt) ( // below. hash, err := p.shardTracker.GetHash(attempt.AttemptID) if err != nil { - return nil, err + return p.failAttempt(attempt.AttemptID, err) } // Regenerate the circuit for this attempt. _, circuit, err := generateSphinxPacket( &attempt.Route, hash[:], attempt.SessionKey(), ) + // TODO(yy): We generate this circuit to create the error decryptor, + // which is then used in htlcswitch as the deobfuscator to decode the + // error from `UpdateFailHTLC`. However, suppose it's an + // `UpdateFulfillHTLC` message yet for some reason the sphinx packet is + // failed to be generated, we'd miss settling it. This means we should + // give it a second chance to try the settlement path in case + // `GetAttemptResult` gives us back the preimage. And move the circuit + // creation into htlcswitch so it's only constructed when there's a + // failure message we need to decode. if err != nil { - return nil, err + log.Debugf("Unable to generate circuit for attempt %v: %v", + attempt.AttemptID, err) + + return p.failAttempt(attempt.AttemptID, err) } // Using the created circuit, initialize the error decrypter so we can @@ -476,6 +488,12 @@ func (p *paymentLifecycle) collectResult(attempt *channeldb.HTLCAttempt) ( // Now ask the switch to return the result of the payment when // available. + // + // TODO(yy): consider using htlcswitch to create the `errorDecryptor` + // since the htlc is already in db. This will also make the interface + // `PaymentAttemptDispatcher` deeper and easier to use. Moreover, we'd + // only create the decryptor when received a failure, further saving us + // a few CPU cycles. resultChan, err := p.router.cfg.Payer.GetAttemptResult( attempt.AttemptID, p.identifier, errorDecryptor, ) @@ -536,6 +554,9 @@ func (p *paymentLifecycle) collectResult(attempt *channeldb.HTLCAttempt) ( ) if err != nil { log.Errorf("Unable to settle payment attempt: %v", err) + + // We won't mark the attempt as failed since we already have + // the preimage. return nil, err }