diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 2ed8a59a..9b65d909 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -662,22 +662,18 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) { // machine, as a result, we'll signal the switch to // cancel the pending payment. default: + log.Warnf("Unable to handle downstream add HTLC: %v", err) + var ( - isObfuscated bool + localFailure = false reason lnwire.OpaqueReason ) - // We'll parse the sphinx packet enclosed so we - // can obtain the shared secret required to - // encrypt the error back to the source. failure := lnwire.NewTemporaryChannelFailure(nil) - onionReader := bytes.NewReader(htlc.OnionBlob[:]) - obfuscator, failCode := l.cfg.DecodeOnionObfuscator(onionReader) - switch { - // If we were unable to parse the onion blob, - // then we'll send an error back to the source. - case failCode != lnwire.CodeNone: + // Encrypt the error back to the source unless the payment was + // generated locally. + if pkt.obfuscator == nil { var b bytes.Buffer err := lnwire.EncodeFailure(&b, failure, 0) if err != nil { @@ -685,24 +681,22 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) { return } reason = lnwire.OpaqueReason(b.Bytes()) - isObfuscated = false - - // Otherwise, we'll send back a proper failure - // message. - default: - reason, err = obfuscator.EncryptFirstHop(failure) + localFailure = true + } else { + var err error + reason, err = pkt.obfuscator.EncryptFirstHop(failure) if err != nil { log.Errorf("unable to obfuscate error: %v", err) return } - isObfuscated = true } failPkt := &htlcPacket{ incomingChanID: pkt.incomingChanID, incomingHTLCID: pkt.incomingHTLCID, amount: htlc.Amount, - isObfuscated: isObfuscated, + isRouted: true, + localFailure: localFailure, htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, @@ -711,7 +705,6 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) { // TODO(roasbeef): need to identify if sent // from switch so don't need to obfuscate go l.cfg.Switch.forward(failPkt) - log.Infof("Unable to handle downstream add HTLC: %v", err) return } } @@ -1214,7 +1207,6 @@ func (l *channelLink) processLockedInHtlcs( outgoingChanID: l.ShortChanID(), outgoingHTLCID: pd.ParentIndex, amount: pd.Amount, - isObfuscated: false, htlc: &lnwire.UpdateFailHTLC{ Reason: lnwire.OpaqueReason(pd.FailReason), }, diff --git a/htlcswitch/mailbox_test.go b/htlcswitch/mailbox_test.go index 56d7cc37..a31ff5e8 100644 --- a/htlcswitch/mailbox_test.go +++ b/htlcswitch/mailbox_test.go @@ -33,7 +33,6 @@ func TestMailBoxCouriers(t *testing.T) { outgoingChanID: lnwire.NewShortChanIDFromInt(uint64(prand.Int63())), incomingChanID: lnwire.NewShortChanIDFromInt(uint64(prand.Int63())), amount: lnwire.MilliSatoshi(prand.Int63()), - isObfuscated: i%2 == 0, } sentPackets[i] = pkt diff --git a/htlcswitch/packet.go b/htlcswitch/packet.go index 0c6a8926..3038f94e 100644 --- a/htlcswitch/packet.go +++ b/htlcswitch/packet.go @@ -37,11 +37,13 @@ type htlcPacket struct { // any forwarded errors in an additional layer of encryption. obfuscator ErrorEncrypter - // isObfuscated is set to true if an error occurs as soon as the switch - // forwards a packet to the link. If so, and this is an error packet, - // then this allows the switch to avoid doubly encrypting the error. - // - // TODO(andrew.shvv) revisit after refactoring the way of returning - // errors inside the htlcswitch packet. - isObfuscated bool + // localFailure is set to true if an HTLC fails for a local payment before + // the first hop. In this case, the failure reason is simply encoded, not + // encrypted with any shared secret. + localFailure bool + + // isRouted is set to true if the incomingChanID and incomingHTLCID fields + // of a forwarded fail packet are already set and do not need to be looked + // up in the circuit map. + isRouted bool } diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index b7baaf0a..f98d98c1 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -1,6 +1,7 @@ package htlcswitch import ( + "bytes" "fmt" "sync" "sync/atomic" @@ -435,23 +436,39 @@ func (s *Switch) handleLocalDispatch(packet *htlcPacket) error { // We've just received a fail update which means we can finalize the // user payment and return fail response. case *lnwire.UpdateFailHTLC: - // We'll attempt to fully decrypt the onion encrypted error. If - // we're unable to then we'll bail early. - failure, err := payment.deobfuscator.DecryptError(htlc.Reason) - if err != nil { - userErr := fmt.Sprintf("unable to de-obfuscate "+ - "onion failure, htlc with hash(%x): %v", - payment.paymentHash[:], err) - log.Error(userErr) - payment.err <- &ForwardingError{ + var failure *ForwardingError + if packet.localFailure { + var userErr string + r := bytes.NewReader(htlc.Reason) + failureMsg, err := lnwire.DecodeFailure(r, 0) + if err != nil { + userErr = fmt.Sprintf("unable to decode onion failure, "+ + "htlc with hash(%x): %v", payment.paymentHash[:], err) + log.Error(userErr) + failureMsg = lnwire.NewTemporaryChannelFailure(nil) + } + failure = &ForwardingError{ ErrorSource: s.cfg.SelfKey, ExtraMsg: userErr, - FailureMessage: lnwire.NewTemporaryChannelFailure(nil), + FailureMessage: failureMsg, } } else { - payment.err <- failure + // We'll attempt to fully decrypt the onion encrypted error. If + // we're unable to then we'll bail early. + failure, err = payment.deobfuscator.DecryptError(htlc.Reason) + if err != nil { + userErr := fmt.Sprintf("unable to de-obfuscate onion failure, "+ + "htlc with hash(%x): %v", payment.paymentHash[:], err) + log.Error(userErr) + failure = &ForwardingError{ + ErrorSource: s.cfg.SelfKey, + ExtraMsg: userErr, + FailureMessage: lnwire.NewTemporaryChannelFailure(nil), + } + } } + payment.err <- failure payment.preimage <- zeroPreimage s.removePendingPayment(packet.incomingHTLCID) @@ -503,7 +520,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { source.HandleSwitchPacket(&htlcPacket{ incomingChanID: packet.incomingChanID, incomingHTLCID: packet.incomingHTLCID, - isObfuscated: true, + isRouted: true, htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, @@ -551,7 +568,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { source.HandleSwitchPacket(&htlcPacket{ incomingChanID: packet.incomingChanID, incomingHTLCID: packet.incomingHTLCID, - isObfuscated: true, + isRouted: true, htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, @@ -573,7 +590,7 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { // payment circuit by forwarding the settle msg to the channel from // which htlc add packet was initially received. case *lnwire.UpdateFufillHTLC, *lnwire.UpdateFailHTLC: - if packet.incomingChanID == (lnwire.ShortChannelID{}) { + if !packet.isRouted { // Use circuit map to find the link to forward settle/fail to. circuit := s.circuits.LookupByHTLC(packet.outgoingChanID, packet.outgoingHTLCID) @@ -603,20 +620,22 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { packet.incomingChanID = circuit.IncomingChanID packet.incomingHTLCID = circuit.IncomingHTLCID - // A blank IncomingChanID in a circuit indicates that it is a - // pending user-initiated payment. - if circuit.IncomingChanID == (lnwire.ShortChannelID{}) { - return s.handleLocalDispatch(packet) - } - // Obfuscate the error message for fail updates before sending back - // through the circuit. - if htlc, ok := htlc.(*lnwire.UpdateFailHTLC); ok && !packet.isObfuscated { - htlc.Reason = circuit.ErrorEncrypter.IntermediateEncrypt( - htlc.Reason) + // through the circuit unless the payment was generated locally. + if circuit.ErrorEncrypter != nil { + if htlc, ok := htlc.(*lnwire.UpdateFailHTLC); ok { + htlc.Reason = circuit.ErrorEncrypter.IntermediateEncrypt( + htlc.Reason) + } } } + // A blank IncomingChanID in a circuit indicates that it is a pending + // user-initiated payment. + if packet.incomingChanID == (lnwire.ShortChannelID{}) { + return s.handleLocalDispatch(packet) + } + source, err := s.getLinkByShortID(packet.incomingChanID) if err != nil { err := errors.Errorf("Unable to get source channel link to "+ diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 7f6eb1f4..054da4c2 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -270,7 +270,6 @@ func TestSwitchCancel(t *testing.T) { outgoingChanID: bobChannelLink.ShortChanID(), outgoingHTLCID: 0, amount: 1, - isObfuscated: true, htlc: &lnwire.UpdateFailHTLC{}, } @@ -373,7 +372,6 @@ func TestSwitchAddSamePayment(t *testing.T) { outgoingChanID: bobChannelLink.ShortChanID(), outgoingHTLCID: 0, amount: 1, - isObfuscated: true, htlc: &lnwire.UpdateFailHTLC{}, } @@ -397,7 +395,6 @@ func TestSwitchAddSamePayment(t *testing.T) { outgoingChanID: bobChannelLink.ShortChanID(), outgoingHTLCID: 1, amount: 1, - isObfuscated: true, htlc: &lnwire.UpdateFailHTLC{}, } @@ -500,7 +497,6 @@ func TestSwitchSendPayment(t *testing.T) { outgoingChanID: aliceChannelLink.ShortChanID(), outgoingHTLCID: 0, amount: 1, - isObfuscated: true, htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, @@ -522,7 +518,6 @@ func TestSwitchSendPayment(t *testing.T) { packet = &htlcPacket{ outgoingChanID: aliceChannelLink.ShortChanID(), outgoingHTLCID: 1, - isObfuscated: true, htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, diff --git a/lnd_test.go b/lnd_test.go index 72787a93..3f711baa 100644 --- a/lnd_test.go +++ b/lnd_test.go @@ -4043,7 +4043,8 @@ func testAsyncPayments(net *networkHarness, t *harnessTest) { // Open up a payment stream to Alice that we'll use to send payment to // Bob. We also create a small helper function to send payments to Bob, // consuming the payment hashes we generated above. - alicePayStream, err := net.Alice.SendPayment(ctxb) + ctxt, _ = context.WithTimeout(ctxb, time.Minute) + alicePayStream, err := net.Alice.SendPayment(ctxt) if err != nil { t.Fatalf("unable to create payment stream for alice: %v", err) }