lnwire+htlcswitch: minor grammatical, formatting fixes after error PR

htlcswitch/circuit.go

@@ -20,7 +20,7 @@ func (k *circuitKey) String() string {
 }
 
 // paymentCircuit is used by the htlc switch subsystem to determine the
-// fowrards/backwards path for the settle/fail HTLC messages. A payment circuit
+// forwards/backwards path for the settle/fail HTLC messages. A payment circuit
 // will be created once a channel link forwards the htlc add request and
 // removed when we receive settle/fail htlc message.
 type paymentCircuit struct {
@@ -38,7 +38,7 @@ type paymentCircuit struct {
 	// request back.
 	Dest lnwire.ShortChannelID
 
-	// Obfuscator is used to obfuscate the onion failure before sending it
+	// Obfuscator is used to re-encrypt the onion failure before sending it
 	// back to the originator of the payment.
 	Obfuscator Obfuscator
 

htlcswitch/failure.go

@@ -7,22 +7,31 @@ import (
 	"github.com/lightningnetwork/lnd/lnwire"
 )
 
-// Deobfuscator entity which is used to de-obfuscate the onion opaque reason and
-// extract failure.
+// Deobfuscator is an interface that is used to decrypt the onion encrypted
+// failure reason an extra out a well formed error.
 type Deobfuscator interface {
-	// Deobfuscate function decodes the onion error failure.
+	// Deobfuscate peels off each layer of onion encryption from the first
+	// hop, to the source of the error. A fully populated
+	// lnwire.FailureMessage is returned.
 	Deobfuscate(lnwire.OpaqueReason) (lnwire.FailureMessage, error)
 }
 
-// Obfuscator entity which is used to do the initial and backward onion
-// failure obfuscation.
+// Obfuscator is an interface that is used to encrypt HTLC related errors at
+// the source of the error, and also at each intermediate hop all the way back
+// to the source of the payment.
 type Obfuscator interface {
 	// InitialObfuscate is used to convert the failure into opaque
 	// reason.
+
+	// InitialObfuscate transforms a concrete failure message into an
+	// encrypted opaque failure reason. This method will be used at the
+	// source that the error occurs. It differs from BackwardObfuscate
+	// slightly, in that it computes a proper MAC over the error.
 	InitialObfuscate(lnwire.FailureMessage) (lnwire.OpaqueReason, error)
 
-	// BackwardObfuscate is used to make the processing over onion error
-	// when it moves backward to the htlc sender.
+	// BackwardObfuscate wraps an already encrypted opaque reason error in
+	// an additional layer of onion encryption. This process repeats until
+	// the error arrives at the source of the payment.
 	BackwardObfuscate(lnwire.OpaqueReason) lnwire.OpaqueReason
 }
 
@@ -31,35 +40,36 @@ type FailureObfuscator struct {
 	*sphinx.OnionObfuscator
 }
 
-// InitialObfuscate is used by the failure sender to decode the failure and
-// make the initial failure obfuscation with addition of the failure data hmac.
+// InitialObfuscate transforms a concrete failure message into an encrypted
+// opaque failure reason. This method will be used at the source that the error
+// occurs. It differs from BackwardObfuscate slightly, in that it computes a
+// proper MAC over the error.
 //
 // NOTE: Part of the Obfuscator interface.
-func (o *FailureObfuscator) InitialObfuscate(failure lnwire.FailureMessage) (
-	lnwire.OpaqueReason, error) {
+func (o *FailureObfuscator) InitialObfuscate(failure lnwire.FailureMessage) (lnwire.OpaqueReason, error) {
 	var b bytes.Buffer
 	if err := lnwire.EncodeFailure(&b, failure, 0); err != nil {
 		return nil, err
 	}
 
-	// Make the initial obfuscation with appending hmac.
+	// We pass a true as the first parameter to indicate that a MAC should
+	// be added.
 	return o.OnionObfuscator.Obfuscate(true, b.Bytes()), nil
 }
 
-// BackwardObfuscate is used by the forwarding nodes in order to obfuscate the
-// already obfuscated onion failure blob with the stream which have been
-// generated with our shared secret. The reason we re-encrypt the message on the
-// backwards path is to ensure that the error is computationally
-// indistinguishable from any other error seen.
+// BackwardObfuscate wraps an already encrypted opaque reason error in an
+// additional layer of onion encryption. This process repeats until the error
+// arrives at the source of the payment. We re-encrypt the message on the
+// backwards path to ensure that the error is indistinguishable from any other
+// error seen.
 //
 // NOTE: Part of the Obfuscator interface.
-func (o *FailureObfuscator) BackwardObfuscate(
-	reason lnwire.OpaqueReason) lnwire.OpaqueReason {
+func (o *FailureObfuscator) BackwardObfuscate(reason lnwire.OpaqueReason) lnwire.OpaqueReason {
 	return o.OnionObfuscator.Obfuscate(false, reason)
 }
 
-// A compile time check to ensure FailureObfuscator implements the
-// Obfuscator interface.
+// A compile time check to ensure FailureObfuscator implements the Obfuscator
+// interface.
 var _ Obfuscator = (*FailureObfuscator)(nil)
 
 // FailureDeobfuscator wraps the sphinx data obfuscator and adds awareness of
@@ -68,11 +78,13 @@ type FailureDeobfuscator struct {
 	*sphinx.OnionDeobfuscator
 }
 
-// Deobfuscate decodes the obfuscated onion failure.
+// Deobfuscate peels off each layer of onion encryption from the first hop, to
+// the source of the error. A fully populated lnwire.FailureMessage is
+// returned.
 //
 // NOTE: Part of the Obfuscator interface.
-func (o *FailureDeobfuscator) Deobfuscate(reason lnwire.OpaqueReason) (lnwire.FailureMessage,
-	error) {
+func (o *FailureDeobfuscator) Deobfuscate(reason lnwire.OpaqueReason) (lnwire.FailureMessage, error) {
+
 	_, failureData, err := o.OnionDeobfuscator.Deobfuscate(reason)
 	if err != nil {
 		return nil, err

htlcswitch/iterator.go

@@ -191,10 +191,11 @@ func (p *OnionProcessor) DecodeHopIterator(r io.Reader, rHash []byte) (HopIterat
 	}, lnwire.CodeNone
 }
 
-// DecodeOnionObfuscator takes the onion blob as input extract the shared secret
-// and return the entity which is able to obfuscate failure data.
-func (p *OnionProcessor) DecodeOnionObfuscator(r io.Reader) (Obfuscator,
-	lnwire.FailCode) {
+// DecodeOnionObfuscator takes an io.Reader which should contain the onion
+// packet as original received by a forwarding node and creates an Obfuscator
+// instance using the derived shared secret. In the case that en error occurs,
+// a lnwire failure code detailing the parsing failure will be returned.
+func (p *OnionProcessor) DecodeOnionObfuscator(r io.Reader) (Obfuscator, lnwire.FailCode) {
 	onionPkt := &sphinx.OnionPacket{}
 	if err := onionPkt.Decode(r); err != nil {
 		return nil, lnwire.CodeTemporaryChannelFailure

htlcswitch/link.go

@@ -83,17 +83,18 @@ type ChannelLinkConfig struct {
 	// contained in the forwarding blob within each HTLC.
 	Switch *Switch
 
-	// DecodeHopIterator function is responsible for decoding htlc Sphinx onion
-	// blob, and creating hop iterator which will give us next destination
-	// of htlc.
+	// DecodeHopIterator function is responsible for decoding htlc Sphinx
+	// onion blob, and creating hop iterator which will give us next
+	// destination of htlc.
 	DecodeHopIterator func(r io.Reader, rHash []byte) (HopIterator, lnwire.FailCode)
 
-	// DecodeOnionObfuscator function is responsible for decoding htlc Sphinx
-	// onion blob, and creating onion failure obfuscator.
+	// DecodeOnionObfuscator function is responsible for decoding htlc
+	// Sphinx onion blob, and creating onion failure obfuscator.
 	DecodeOnionObfuscator func(r io.Reader) (Obfuscator, lnwire.FailCode)
 
-	// GetLastChannelUpdate retrieve the topology info about the channel in
-	// order to create the channel update.
+	// GetLastChannelUpdate reterives the latest routing policy for this
+	// particualr channel. This will be used to provide payment senders our
+	// laest policy when sending encrypted error messages.
 	GetLastChannelUpdate func() (*lnwire.ChannelUpdate, error)
 
 	// Peer is a lightning network node with which we have the channel link
@@ -419,6 +420,9 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket) {
 		index, err := l.channel.AddHTLC(htlc)
 		if err != nil {
 			switch err {
+
+			// The channels spare bandwidth is fully allocated, so
+			// we'll put this HTLC into the overflow queue.
 			case lnwallet.ErrMaxHTLCNumber:
 				log.Infof("Downstream htlc add update with "+
 					"payment hash(%x) have been added to "+
@@ -428,19 +432,26 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket) {
 				l.overflowQueue.consume(pkt)
 				return
 
+			// The HTLC was unable to be added to the state
+			// machine, as a result, we'll signal the switch to
+			// cancel the pending payment.
 			default:
-				// The HTLC was unable to be added to the state
-				// machine, as a result, we'll signal the switch to
-				// cancel the pending payment.
 				var (
 					isObfuscated bool
 					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)
-				if failCode != lnwire.CodeNone {
+
+				switch {
+				// If we were unable to parse the onion blob,
+				// then we'll send an error back to the source.
+				case failCode != lnwire.CodeNone:
 					var b bytes.Buffer
 					err := lnwire.EncodeFailure(&b, failure, 0)
 					if err != nil {
@@ -449,7 +460,10 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket) {
 					}
 					reason = lnwire.OpaqueReason(b.Bytes())
 					isObfuscated = false
-				} else {
+
+				// Otherwise, we'll send back a proper failure
+				// message.
+				default:
 					reason, err = obfuscator.InitialObfuscate(failure)
 					if err != nil {
 						log.Errorf("unable to obfuscate error: %v", err)
@@ -458,13 +472,16 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket) {
 					isObfuscated = true
 				}
 
-				go l.cfg.Switch.forward(newFailPacket(
-					l.ShortChanID(),
-					&lnwire.UpdateFailHTLC{
-						Reason: reason,
-					}, htlc.PaymentHash, htlc.Amount, isObfuscated,
-				))
+				upddateFail := &lnwire.UpdateFailHTLC{
+					Reason: reason,
+				}
+				failPkt := newFailPacket(
+					l.ShortChanID(), upddateFail,
+					htlc.PaymentHash, htlc.Amount,
+					isObfuscated,
+				)
 
+				go l.cfg.Switch.forward(failPkt)
 				log.Infof("Unable to handle downstream add HTLC: %v", err)
 				return
 			}
@@ -568,16 +585,18 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
 		// TODO(roasbeef): add preimage to DB in order to swipe
 		// repeated r-values
 
-	// If remote side have been unable to parse the onion blob we have sent
-	// to it, than we should transform the malformed notification to the the
-	// usual htlc fail message.
 	case *lnwire.UpdateFailMalformedHTLC:
+		// If remote side have been unable to parse the onion blob we
+		// have sent to it, than we should transform the malformed HTLC
+		// message to the usual HTLC fail message.
 		idx := msg.ID
 		if err := l.channel.ReceiveFailHTLC(idx); err != nil {
 			l.fail("unable to handle upstream fail HTLC: %v", err)
 			return
 		}
 
+		// Convert the failure type encoded within the HTLC fail
+		// message to the proper generic lnwire error code.
 		var failure lnwire.FailureMessage
 		switch msg.FailureCode {
 		case lnwire.CodeInvalidOnionVersion:
@@ -594,11 +613,14 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
 				OnionSHA256: msg.ShaOnionBlob,
 			}
 		default:
+			// TODO(roasbeef): fail channel here?
 			log.Errorf("unable to understand code of received " +
 				"malformed error")
 			return
 		}
 
+		// With the error parsed, we'll convert the into it's opaque
+		// form.
 		var b bytes.Buffer
 		if err := lnwire.EncodeFailure(&b, failure, 0); err != nil {
 			log.Errorf("unable to encode malformed error: %v", err)
@@ -943,7 +965,7 @@ func (l *channelLink) processLockedInHtlcs(
 
 		// An incoming HTLC add has been full-locked in. As a result we
 		// can no examine the forwarding details of the HTLC, and the
-		// HTLC itself ti decide if: we should forward it, cancel it,
+		// HTLC itself to decide if: we should forward it, cancel it,
 		// or are able to settle it (and it adheres to our fee related
 		// constraints).
 		case lnwallet.Add:
@@ -952,23 +974,26 @@ func (l *channelLink) processLockedInHtlcs(
 			onionBlob := l.clearedOnionBlobs[pd.Index]
 			delete(l.clearedOnionBlobs, pd.Index)
 
-			// Retrieve onion obfuscator from onion blob in order to produce
-			// initial obfuscation of the onion failureCode.
+			// Retrieve onion obfuscator from onion blob in order
+			// to produce initial obfuscation of the onion
+			// failureCode.
 			onionReader := bytes.NewReader(onionBlob[:])
 			obfuscator, failureCode := l.cfg.DecodeOnionObfuscator(onionReader)
 			if failureCode != lnwire.CodeNone {
-				log.Error("unable to decode onion obfuscator")
-				// If we unable to process the onion blob than we should send
-				// the malformed htlc error to payment sender.
+				// If we unable to process the onion blob than
+				// we should send the malformed htlc error to
+				// payment sender.
 				l.sendMalformedHTLCError(pd.RHash, failureCode, onionBlob[:])
 				needUpdate = true
+
+				log.Error("unable to decode onion obfuscator")
 				continue
 			}
 
 			// Before adding the new htlc to the state machine,
 			// parse the onion object in order to obtain the
-			// routing information with DecodeHopIterator function which
-			// process the Sphinx packet.
+			// routing information with DecodeHopIterator function
+			// which process the Sphinx packet.
 			//
 			// We include the payment hash of the htlc as it's
 			// authenticated within the Sphinx packet itself as
@@ -979,11 +1004,13 @@ func (l *channelLink) processLockedInHtlcs(
 			onionReader = bytes.NewReader(onionBlob[:])
 			chanIterator, failureCode := l.cfg.DecodeHopIterator(onionReader, pd.RHash[:])
 			if failureCode != lnwire.CodeNone {
-				log.Error("unable to decode onion hop iterator")
-				// If we unable to process the onion blob than we should send
-				// the malformed htlc error to payment sender.
+				// If we unable to process the onion blob than
+				// we should send the malformed htlc error to
+				// payment sender.
 				l.sendMalformedHTLCError(pd.RHash, failureCode, onionBlob[:])
 				needUpdate = true
+
+				log.Error("unable to decode onion hop iterator")
 				continue
 			}
 
@@ -1009,9 +1036,7 @@ func (l *channelLink) processLockedInHtlcs(
 				// the hop-payload included in the HTLC to
 				// ensure that it was crafted correctly by the
 				// sender and matches the HTLC we were
-				// extended. Additionally, we'll ensure that
-				// our time-lock value has been computed
-				// correctly.
+				// extended.
 				if !l.cfg.DebugHTLC &&
 					fwdInfo.AmountToForward != invoice.Terms.Value {
 
@@ -1026,6 +1051,9 @@ func (l *channelLink) processLockedInHtlcs(
 					needUpdate = true
 					continue
 				}
+
+				// We'll also ensure that our time-lock value
+				// has been computed correctly.
 				if !l.cfg.DebugHTLC &&
 					fwdInfo.OutgoingCTLV != l.cfg.FwrdingPolicy.TimeLockDelta {
 
@@ -1096,6 +1124,10 @@ func (l *channelLink) processLockedInHtlcs(
 						pd.RHash[:], l.cfg.FwrdingPolicy.MinHTLC,
 						pd.Amount)
 
+					// As part of the returned error, we'll
+					// send our latest routing policy so
+					// the sending node obtains the most up
+					// to date data.
 					var failure lnwire.FailureMessage
 					update, err := l.cfg.GetLastChannelUpdate()
 					if err != nil {
@@ -1134,6 +1166,10 @@ func (l *channelLink) processLockedInHtlcs(
 						btcutil.Amount(pd.Amount-fwdInfo.AmountToForward),
 						btcutil.Amount(expectedFee))
 
+					// As part of the returned error, we'll
+					// send our latest routing policy so
+					// the sending node obtains the most up
+					// to date data.
 					var failure lnwire.FailureMessage
 					update, err := l.cfg.GetLastChannelUpdate()
 					if err != nil {
@@ -1163,6 +1199,9 @@ func (l *channelLink) processLockedInHtlcs(
 						pd.RHash[:], pd.Timeout-timeDelta,
 						fwdInfo.OutgoingCTLV)
 
+					// Grab the latest routing policy so
+					// the sending node is up to date with
+					// our current policy.
 					update, err := l.cfg.GetLastChannelUpdate()
 					if err != nil {
 						l.fail("unable to create channel update "+
@@ -1211,7 +1250,7 @@ func (l *channelLink) processLockedInHtlcs(
 
 	if needUpdate {
 		// With all the settle/cancel updates added to the local and
-		// remote htlc logs, initiate a state transition by updating
+		// remote HTLC logs, initiate a state transition by updating
 		// the remote commitment chain.
 		if err := l.updateCommitTx(); err != nil {
 			l.fail("unable to update commitment: %v", err)
@@ -1222,8 +1261,8 @@ func (l *channelLink) processLockedInHtlcs(
 	return packetsToForward
 }
 
-// sendHTLCError functions cancels htlc and send cancel message back to the
-// peer from which htlc was received.
+// sendHTLCError functions cancels HTLC and send cancel message back to the
+// peer from which HTLC was received.
 func (l *channelLink) sendHTLCError(rHash [32]byte, failure lnwire.FailureMessage,
 	obfuscator Obfuscator) {
 	reason, err := obfuscator.InitialObfuscate(failure)
@@ -1245,7 +1284,7 @@ func (l *channelLink) sendHTLCError(rHash [32]byte, failure lnwire.FailureMessag
 	})
 }
 
-// sendMalformedHTLCError helper function which sends the malformed htlc update
+// sendMalformedHTLCError helper function which sends the malformed HTLC update
 // to the payment sender.
 func (l *channelLink) sendMalformedHTLCError(rHash [32]byte, code lnwire.FailCode,
 	onionBlob []byte) {
@@ -1263,8 +1302,8 @@ func (l *channelLink) sendMalformedHTLCError(rHash [32]byte, code lnwire.FailCod
 	})
 }
 
-// fail helper function which is used to encapsulate the action neccessary
-// for proper disconnect.
+// fail helper function which is used to encapsulate the action necessary for
+// proper disconnect.
 func (l *channelLink) fail(format string, a ...interface{}) {
 	reason := errors.Errorf(format, a...)
 	log.Error(reason)

htlcswitch/packet.go

@@ -35,20 +35,19 @@ type htlcPacket struct {
 	// htlc lnwire message type of which depends on switch request type.
 	htlc lnwire.Message
 
-	// obfuscator is entity which is needed to make the obfuscation of the
-	// onion failure, it is carried inside the packet from channel
-	// link to the switch because we have to create onion error inside the
-	// switch to, but we unable to restore obfuscator from the onion, because
-	// on stage of forwarding onion inside payment belongs to the remote node.
-	// TODO(andrew.shvv) revisit after refactoring the way of returning errors
-	// inside the htlcswitch packet.
+	// obfuscator contains the necessary state to allow the switch to wrap
+	// any forwarded errors in an additional layer of encryption.
+	//
+	// TODO(andrew.shvv) revisit after refactoring the way of returning
+	// errors inside the htlcswitch packet.
 	obfuscator Obfuscator
 
-	// isObfuscated is used in case if switch sent the packet to the link,
-	// but error have occurred locally, in this case we shouldn't obfuscate
-	// it again.
-	// TODO(andrew.shvv) revisit after refactoring the way of returning errors
-	// inside the htlcswitch packet.
+	// 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
 }
 

htlcswitch/switch.go

@@ -304,23 +304,27 @@ func (s *Switch) handleLocalDispatch(payment *pendingPayment, packet *htlcPacket
 		payment.preimage <- htlc.PaymentPreimage
 		s.removePendingPayment(payment.amount, payment.paymentHash)
 
-	// We've just received a fail update which means we can finalize
-	// the user payment and return fail response.
+	// We've just received a fail update which means we can finalize the
+	// user payment and return fail response.
 	case *lnwire.UpdateFailHTLC:
-		// Retrieving the fail code from byte representation of error.
 		var userErr error
 
+		// We'll attempt to fully decrypt the onion encrypted error. If
+		// we're unable to then we'll bail early.
 		failure, err := payment.deobfuscator.Deobfuscate(htlc.Reason)
 		if err != nil {
 			userErr = errors.Errorf("unable to de-obfuscate "+
-				"onion failure, htlc with hash(%v): %v", payment.paymentHash[:],
-				err)
+				"onion failure, htlc with hash(%v): %v",
+				payment.paymentHash[:], err)
 			log.Error(userErr)
 		} else {
-			// Process payment failure by updating the lightning network
-			// topology by using router subsystem handler.
+			// Process payment failure by updating the lightning
+			// network topology by using router subsystem handler.
 			var update *lnwire.ChannelUpdate
 
+			// Only a few error message actually contain a channel
+			// update message, so we'll filter out for those that
+			// do.
 			switch failure := failure.(type) {
 			case *lnwire.FailTemporaryChannelFailure:
 				update = failure.Update
@@ -336,10 +340,21 @@ func (s *Switch) handleLocalDispatch(payment *pendingPayment, packet *htlcPacket
 				update = &failure.Update
 			}
 
+			// If we've been sent an error that includes an update,
+			// then we'll apply it to the local graph.
+			//
+			// TODO(roasbeef): instead, make all onion errors the
+			// error interface, and handle this within the router.
+			// Will allow us more flexibility w.r.t how we handle
+			// the error.
 			if update != nil {
-				log.Info("Received payment failure(%v), applying lightning "+
-					"network topology update", failure.Code())
-				s.cfg.UpdateTopology(update)
+				log.Info("Received payment failure(%v), "+
+					"applying lightning network topology update",
+					failure.Code())
+
+				if err := s.cfg.UpdateTopology(update); err != nil {
+					log.Errorf("unable to update topology: %v", err)
+				}
 			}
 
 			userErr = errors.New(failure.Code())
@@ -611,15 +626,23 @@ func (s *Switch) htlcForwarder() {
 
 	for {
 		select {
+		// A local close request has arrived, we'll forward this to the
+		// relevant link (if it exists) so the channel can be
+		// cooperatively closed (if possible).
 		case req := <-s.chanCloseRequests:
 			s.handleChanelClose(req)
 
+		// A new packet has arrived for forwarding, we'll interpret the
+		// packet concretely, then either forward it along, or
+		// interpret a return packet to a locally initialized one.
 		case cmd := <-s.htlcPlex:
 			var (
 				paymentHash lnwallet.PaymentHash
 				amount      btcutil.Amount
 			)
 
+			// Only three types of message should be forwarded:
+			// add, fails, and settles. Anything else is an error.
 			switch m := cmd.pkt.htlc.(type) {
 			case *lnwire.UpdateAddHTLC:
 				paymentHash = m.PaymentHash
@@ -632,6 +655,12 @@ func (s *Switch) htlcForwarder() {
 				return
 			}
 
+			// If we can locate this packet in our local records,
+			// then this means a local sub-system initiated it.
+			// Otherwise, this is just a packet to be forwarded, so
+			// we'll treat it as so.
+			//
+			// TODO(roasbeef): can fast path this
 			payment, err := s.findPayment(amount, paymentHash)
 			if err != nil {
 				cmd.err <- s.handlePacketForward(cmd.pkt)