lnwire: morph CancelHTLC into UpdateFailHTLC, remove HTLCAddReject

This commit morphs the prior CancelHTLC into the new UpdateFailHTLC
message and also gets rid of the obsolete HLTCAddReject message while
we’re at it.

The primary change from the CancelHTLC message to the UpdateFailHTLC
message is that the CancelReason is now simply called Reason and that
it’s now an opaque encrypted set of bytes. With this update the failure
messages are now more flexible (they can even carry new
ChannelUpdate’s) and also don’t reveal the exact cause of failure to
intermediate nodes.

lnwire/htlc_addreject.go

@@ -1,92 +0,0 @@
-package lnwire
-
-import (
-	"io"
-
-	"github.com/roasbeef/btcd/wire"
-)
-
-// HTLCAddReject is sent by Bob when he wishes to reject a particular HTLC that
-// Alice attempted to add via an HTLCAddRequest message. The rejected HTLC is
-// referenced by its unique HTLCKey ID. An HTLCAddReject message is bound to a
-// single active channel, referenced by a unique ChannelPoint. Additionally, the
-// HTLCKey of the rejected HTLC is present
-type HTLCAddReject struct {
-	// ChannelPoint references the particular active channel to which this
-	// HTLCAddReject message is binded to.
-	ChannelPoint *wire.OutPoint
-
-	// HTLCKey is used to identify which HTLC previously attempted to be
-	// added via an HTLCAddRequest message is being declined.
-	HTLCKey HTLCKey
-}
-
-// Decode deserializes a serialized HTLCAddReject message stored in the passed
-// io.Reader observing the specified protocol version.
-//
-// This is part of the lnwire.Message interface.
-func (c *HTLCAddReject) Decode(r io.Reader, pver uint32) error {
-	// ChannelPoint (8)
-	// HTLCKey   (8)
-	err := readElements(r,
-		&c.ChannelPoint,
-		&c.HTLCKey,
-	)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-// NewHTLCAddReject returns a new empty HTLCAddReject message.
-func NewHTLCAddReject() *HTLCAddReject {
-	return &HTLCAddReject{}
-}
-
-// A compile time check to ensure HTLCAddReject implements the lnwire.Message
-// interface.
-var _ Message = (*HTLCAddReject)(nil)
-
-// Encode serializes the target HTLCAddReject into the passed io.Writer observing
-// the protocol version specified.
-//
-// This is part of the lnwire.Message interface.
-func (c *HTLCAddReject) Encode(w io.Writer, pver uint32) error {
-	err := writeElements(w,
-		c.ChannelPoint,
-		c.HTLCKey,
-	)
-
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-// Command returns the integer uniquely identifying this message type on the
-// wire.
-//
-// This is part of the lnwire.Message interface.
-func (c *HTLCAddReject) Command() uint32 {
-	return CmdHTLCAddReject
-}
-
-// MaxPayloadLength returns the maximum allowed payload size for a HTLCAddReject
-// complete message observing the specified protocol version.
-//
-// This is part of the lnwire.Message interface.
-func (c *HTLCAddReject) MaxPayloadLength(uint32) uint32 {
-	// 36 + 8
-	return 44
-}
-
-// Validate performs any necessary sanity checks to ensure all fields present
-// on the HTLCAddReject are valid.
-//
-// This is part of the lnwire.Message interface.
-func (c *HTLCAddReject) Validate() error {
-	// We're good!
-	return nil
-}

lnwire/htlc_addreject_test.go

@@ -1,33 +0,0 @@
-package lnwire
-
-import (
-	"bytes"
-	"reflect"
-	"testing"
-)
-
-func TestHTLCAddRejectEncodeDecode(t *testing.T) {
-	// First create a new HTLCAR message.
-	rejectReq := &HTLCAddReject{
-		ChannelPoint: outpoint1,
-		HTLCKey:      22,
-	}
-
-	// Next encode the HTLCAR message into an empty bytes buffer.
-	var b bytes.Buffer
-	if err := rejectReq.Encode(&b, 0); err != nil {
-		t.Fatalf("unable to encode HTLCSettleRequest: %v", err)
-	}
-
-	// Deserialize the encoded HTLCAR message into a new empty struct.
-	rejectReq2 := &HTLCAddReject{}
-	if err := rejectReq2.Decode(&b, 0); err != nil {
-		t.Fatalf("unable to decode HTLCAddReject: %v", err)
-	}
-
-	// Assert equality of the two instances.
-	if !reflect.DeepEqual(rejectReq, rejectReq2) {
-		t.Fatalf("encode/decode error messages don't match %#v vs %#v",
-			rejectReq, rejectReq2)
-	}
-}

lnwire/lnwire.go

@@ -24,22 +24,6 @@ const MaxSliceLength = 65535
 // key script.
 type PkScript []byte
 
-// HTLCKey is an identifier used to uniquely identify any HTLCs transmitted
-// between Alice and Bob. In order to cancel, timeout, or settle HTLCs this
-// identifier should be used to allow either side to easily locate and modify
-// any staged or pending HTLCs.
-// TODO(roasbeef): change to HTLCIdentifier?
-type HTLCKey int64
-
-// CommitHeight is an integer which represents the highest HTLCKey seen by
-// either side within their commitment transaction. Any addition to the pending,
-// HTLC lists on either side will increment this height. As a result this value
-// should always be monotonically increasing. Any CommitSignature or
-// CommitRevocation messages will reference a value for the commitment height
-// up to which it covers. HTLCs are only explicitly excluded by sending
-// HTLCReject messages referencing a particular HTLCKey.
-type CommitHeight uint64
-
 // CreditsAmount are the native currency unit used within the Lightning Network.
 // Credits are denominated in sub-satoshi amounts, so micro-satoshis (1/1000).
 // This value is purposefully signed in order to allow the expression of negative
@@ -88,7 +72,7 @@ func writeElement(w io.Writer, element interface{}) error {
 		if _, err := w.Write(b[:]); err != nil {
 			return err
 		}
-	case CancelReason:
+	case FailCode:
 		var b [2]byte
 		binary.BigEndian.PutUint16(b[:], uint16(e))
 		if _, err := w.Write(b[:]); err != nil {
@@ -100,8 +84,10 @@ func writeElement(w io.Writer, element interface{}) error {
 		if _, err := w.Write(b[:]); err != nil {
 			return err
 		}
-	case CreditsAmount:
+	case btcutil.Amount:
-		if err := binary.Write(w, binary.BigEndian, int64(e)); err != nil {
+		var b [8]byte
+		binary.BigEndian.PutUint64(b[:], uint64(e))
+		if _, err := w.Write(b[:]); err != nil {
 			return err
 		}
 	case uint32:
@@ -116,14 +102,6 @@ func writeElement(w io.Writer, element interface{}) error {
 		if _, err := w.Write(b[:]); err != nil {
 			return err
 		}
-	case HTLCKey:
-		if err := binary.Write(w, binary.BigEndian, int64(e)); err != nil {
-			return err
-		}
-	case btcutil.Amount:
-		if err := binary.Write(w, binary.BigEndian, int64(e)); err != nil {
-			return err
-		}
 	case *btcec.PublicKey:
 		var b [33]byte
 		serializedPubkey := e.SerializeCompressed()
@@ -186,48 +164,14 @@ func writeElement(w io.Writer, element interface{}) error {
 		if _, err := w.Write(e[:]); err != nil {
 			return err
 		}
-	case [][32]byte:
-		// First write out the number of elements in the slice.
-		sliceSize := len(e)
-		if err := writeElement(w, uint16(sliceSize)); err != nil {
-			return err
-		}
-
-		// Then write each out sequentially.
-		for _, element := range e {
-			if err := writeElement(w, element); err != nil {
-				return err
-			}
-		}
-	case [32]byte:
-		// TODO(roasbeef): should be factor out to caller logic...
-		if _, err := w.Write(e[:]); err != nil {
-			return err
-		}
-	case [33]byte:
-		// TODO(roasbeef): should be factor out to caller logic...
-		if _, err := w.Write(e[:]); err != nil {
-			return err
-		}
 	case wire.BitcoinNet:
 		var b [4]byte
 		binary.BigEndian.PutUint32(b[:], uint32(e))
 		if _, err := w.Write(b[:]); err != nil {
 			return err
 		}
-	case [4]byte:
-		if _, err := w.Write(e[:]); err != nil {
-			return err
-		}
 	case []byte:
-		// Enforce the maxmium length of all slices used in the wire
+		if _, err := w.Write(e[:]); err != nil {
-		// protocol.
-		sliceLength := len(e)
-		if sliceLength > MaxSliceLength {
-			return fmt.Errorf("Slice length too long!")
-		}
-
-		if err := wire.WriteVarBytes(w, 0, e); err != nil {
 			return err
 		}
 	case PkScript:
@@ -360,7 +304,7 @@ func writeElement(w io.Writer, element interface{}) error {
 			return err
 		}
 	case Alias:
-		if err := writeElements(w, ([32]byte)(e.data)); err != nil {
+		if err := writeElements(w, e.data[:]); err != nil {
 			return err
 		}
 
@@ -394,12 +338,12 @@ func readElement(r io.Reader, element interface{}) error {
 			return err
 		}
 		*e = b[0]
-	case *CancelReason:
+	case *FailCode:
 		var b [2]byte
 		if _, err := io.ReadFull(r, b[:]); err != nil {
 			return err
 		}
-		*e = CancelReason(binary.BigEndian.Uint16(b[:]))
+		*e = FailCode(binary.BigEndian.Uint16(b[:]))
 	case *uint16:
 		var b [2]byte
 		if _, err := io.ReadFull(r, b[:]); err != nil {
@@ -412,12 +356,6 @@ func readElement(r io.Reader, element interface{}) error {
 			return err
 		}
 		*e = ErrorCode(binary.BigEndian.Uint16(b[:]))
-	case *CreditsAmount:
-		var b [8]byte
-		if _, err := io.ReadFull(r, b[:]); err != nil {
-			return err
-		}
-		*e = CreditsAmount(int64(binary.BigEndian.Uint64(b[:])))
 	case *uint32:
 		var b [4]byte
 		if _, err := io.ReadFull(r, b[:]); err != nil {
@@ -430,12 +368,6 @@ func readElement(r io.Reader, element interface{}) error {
 			return err
 		}
 		*e = binary.BigEndian.Uint64(b[:])
-	case *HTLCKey:
-		var b [8]byte
-		if _, err := io.ReadFull(r, b[:]); err != nil {
-			return err
-		}
-		*e = HTLCKey(int64(binary.BigEndian.Uint64(b[:])))
 	case *btcutil.Amount:
 		var b [8]byte
 		if _, err := io.ReadFull(r, b[:]); err != nil {
@@ -518,33 +450,6 @@ func readElement(r io.Reader, element interface{}) error {
 		if err != nil {
 			return err
 		}
-	case *[][32]byte:
-		// How many to read
-		var sliceSize uint16
-		err = readElement(r, &sliceSize)
-		if err != nil {
-			return err
-		}
-
-		data := make([][32]byte, 0, sliceSize)
-		// Append the actual
-		for i := uint16(0); i < sliceSize; i++ {
-			var element [32]byte
-			err = readElement(r, &element)
-			if err != nil {
-				return err
-			}
-			data = append(data, element)
-		}
-		*e = data
-	case *[32]byte:
-		if _, err = io.ReadFull(r, e[:]); err != nil {
-			return err
-		}
-	case *[33]byte:
-		if _, err = io.ReadFull(r, e[:]); err != nil {
-			return err
-		}
 	case *wire.BitcoinNet:
 		var b [4]byte
 		if _, err := io.ReadFull(r, b[:]); err != nil {
@@ -552,16 +457,10 @@ func readElement(r io.Reader, element interface{}) error {
 		}
 		*e = wire.BitcoinNet(binary.BigEndian.Uint32(b[:]))
 		return nil
-	case *[4]byte:
+	case []byte:
-		if _, err := io.ReadFull(r, e[:]); err != nil {
+		if _, err := io.ReadFull(r, e); err != nil {
 			return err
 		}
-	case *[]byte:
-		b, err := wire.ReadVarBytes(r, 0, MaxSliceLength, "byte slice")
-		if err != nil {
-			return err
-		}
-		*e = b
 	case *PkScript:
 		pkScript, err := wire.ReadVarBytes(r, 0, 25, "pkscript")
 		if err != nil {
@@ -687,7 +586,7 @@ func readElement(r io.Reader, element interface{}) error {
 		}
 	case *Alias:
 		var a [32]byte
-		if err := readElements(r, &a); err != nil {
+		if err := readElements(r, a[:]); err != nil {
 			return err
 		}
 

lnwire/message.go

@@ -37,10 +37,9 @@ const (
 	CmdCloseComplete = uint32(310)
 
 	// Commands for negotiating HTLCs.
-	CmdHTLCAddReject     = uint32(1020)
 	CmdHTLCSettleRequest = uint32(1100)
-	CmdCancelHTLC        = uint32(1300)
 	CmdUpdateAddHTLC    = uint32(1000)
+	CmdUpdateFailHTLC   = uint32(1020)
 
 	// Commands for modifying commitment transactions.
 	CmdCommitSig    = uint32(2000)
@@ -106,14 +105,12 @@ func makeEmptyMessage(command uint32) (Message, error) {
 		msg = &CloseRequest{}
 	case CmdCloseComplete:
 		msg = &CloseComplete{}
-	case CmdHTLCAddReject:
-		msg = &HTLCAddReject{}
 	case CmdHTLCSettleRequest:
 		msg = &HTLCSettleRequest{}
-	case CmdCancelHTLC:
-		msg = &CancelHTLC{}
 	case CmdUpdateAddHTLC:
 		msg = &UpdateAddHTLC{}
+	case CmdUpdateFailHTLC:
+		msg = &UpdateFailHTLC{}
 	case CmdCommitSig:
 		msg = &CommitSig{}
 	case CmdRevokeAndAck:

lnwire/update_fail_htlc.go

@@ -1,50 +1,51 @@
 package lnwire
 
 import (
+	"fmt"
 	"io"
 
 	"github.com/roasbeef/btcd/wire"
 )
 
-// CancelReason specifies the precise reason that an upstream HTLC was
+// FailReason specifies the precise reason that an upstream HTLC was cancelled.
-// cancelled. Each CancelHTLC message carries a CancelReason which is to be
+// Each UpdateFailHTLC message carries a FailCode which is to be passed back
-// passed back unaltered to the source of the HTLC within the route.
+// unaltered to the source of the HTLC within the route.
 //
 // TODO(roasbeef): implement proper encrypted error messages as defined in spec
 //  * these errors as it stands reveal the error cause to all links in the
 //    route and are horrible for privacy
-type CancelReason uint16
+type FailCode uint16
 
 const (
 	// InsufficientCapacity indicates that a payment failed due to a link
 	// in the ultimate route not having enough satoshi flow to successfully
 	// carry the payment.
-	InsufficientCapacity = 0
+	InsufficientCapacity FailCode = 0
 
 	// UpstreamTimeout indicates that an upstream link had to enforce the
 	// absolute HTLC timeout, removing the HTLC.
-	UpstreamTimeout = 1
+	UpstreamTimeout FailCode = 1
 
 	// UnknownPaymentHash indicates that the destination did not recognize
 	// the payment hash.
-	UnknownPaymentHash = 2
+	UnknownPaymentHash FailCode = 2
 
 	// UnknownDestination indicates that the specified next hop within the
 	// Sphinx packet at a point in the route contained an unknown or
 	// invalid "next hop".
-	UnknownDestination = 3
+	UnknownDestination FailCode = 3
 
 	// SphinxParseError indicates that an intermediate node was unable
 	// properly parse the HTLC.
-	SphinxParseError = 4
+	SphinxParseError FailCode = 4
 
 	// IncorrectValue indicates that the HTLC ultimately extended to the
 	// destination did not match the value that was expected.
-	IncorrectValue = 5
+	IncorrectValue FailCode = 5
 )
 
-// String returns a human-readable version of the CancelReason type.
+// String returns a human-readable version of the FailCode type.
-func (c CancelReason) String() string {
+func (c FailCode) String() string {
 	switch c {
 	case InsufficientCapacity:
 		return "InsufficientCapacity: next hop had insufficient " +
@@ -71,35 +72,42 @@ func (c CancelReason) String() string {
 	}
 }
 
-// CancelHTLC is sent by Alice to Bob in order to remove a previously added
+// UpdateFailHTLC is sent by Alice to Bob in order to remove a previously added
-// HTLC. Upon receipt of an CancelHTLC the HTLC should be removed from the next
+// HTLC. Upon receipt of an UpdateFailHTLC the HTLC should be removed from the
-// commitment transaction, with the CancelHTLC propagated backwards in the
+// next commitment transaction, with the UpdateFailHTLC propagated backwards in
-// route to fully un-clear the HTLC.
+// the route to fully undo the HTLC.
-type CancelHTLC struct {
+type UpdateFailHTLC struct {
-	// ChannelPoint is the particular active channel that this CancelHTLC
+	// ChannelPoint is the particular active channel that this UpdateFailHTLC
 	// is binded to.
-	ChannelPoint *wire.OutPoint
+	ChannelPoint wire.OutPoint
 
-	// HTLCKey references which HTLC on the remote node's commitment
+	// ID references which HTLC on the remote node's commitment transaction
-	// transaction has timed out.
+	// has timed out.
-	HTLCKey HTLCKey
+	ID uint64
 
-	// Reason described the event that caused the HTLC to be cancelled
+	// Reason is an onion-encrypted blob that details why the HTLC was
-	// within the route.
+	// failed. This blob is only fully decryptable by the initiator of the
-	Reason CancelReason
+	// HTLC message.
+	// TODO(roasbeef): properly format the encrypted failure reason
+	Reason []byte
 }
 
-// Decode deserializes a serialized CancelHTLC message stored in the passed
+// A compile time check to ensure UpdateFailHTLC implements the lnwire.Message
+// interface.
+var _ Message = (*UpdateFailHTLC)(nil)
+
+// Decode deserializes a serialized UpdateFailHTLC message stored in the passed
 // io.Reader observing the specified protocol version.
 //
 // This is part of the lnwire.Message interface.
-func (c *CancelHTLC) Decode(r io.Reader, pver uint32) error {
+func (c *UpdateFailHTLC) Decode(r io.Reader, pver uint32) error {
 	// ChannelPoint(8)
 	// HTLCKey(8)
+	// Reason(??)
 	err := readElements(r,
 		&c.ChannelPoint,
-		&c.HTLCKey,
+		&c.ID,
-		&c.Reason,
+		c.Reason[:],
 	)
 	if err != nil {
 		return err
@@ -108,24 +116,15 @@ func (c *CancelHTLC) Decode(r io.Reader, pver uint32) error {
 	return nil
 }
 
-// CancelHTLC creates a new CancelHTLC message.
+// Encode serializes the target UpdateFailHTLC into the passed io.Writer observing
-func NewHTLCTimeoutRequest() *CancelHTLC {
-	return &CancelHTLC{}
-}
-
-// A compile time check to ensure CancelHTLC implements the lnwire.Message
-// interface.
-var _ Message = (*CancelHTLC)(nil)
-
-// Encode serializes the target CancelHTLC into the passed io.Writer observing
 // the protocol version specified.
 //
 // This is part of the lnwire.Message interface.
-func (c *CancelHTLC) Encode(w io.Writer, pver uint32) error {
+func (c *UpdateFailHTLC) Encode(w io.Writer, pver uint32) error {
 	err := writeElements(w,
 		c.ChannelPoint,
-		c.HTLCKey,
+		c.ID,
-		c.Reason,
+		c.Reason[:],
 	)
 	if err != nil {
 		return err
@@ -138,24 +137,34 @@ func (c *CancelHTLC) Encode(w io.Writer, pver uint32) error {
 // wire.
 //
 // This is part of the lnwire.Message interface.
-func (c *CancelHTLC) Command() uint32 {
+func (c *UpdateFailHTLC) Command() uint32 {
-	return CmdCancelHTLC
+	return CmdUpdateFailHTLC
 }
 
-// MaxPayloadLength returns the maximum allowed payload size for a CancelHTLC
+// MaxPayloadLength returns the maximum allowed payload size for a UpdateFailHTLC
 // complete message observing the specified protocol version.
 //
 // This is part of the lnwire.Message interface.
-func (c *CancelHTLC) MaxPayloadLength(uint32) uint32 {
+func (c *UpdateFailHTLC) MaxPayloadLength(uint32) uint32 {
-	// 36 + 8 + 2
+	// 36 + 8 + 154
-	return 46
+	return 198
 }
 
 // Validate performs any necessary sanity checks to ensure all fields present
-// on the CancelHTLC are valid.
+// on the UpdateFailHTLC are valid.
 //
 // This is part of the lnwire.Message interface.
-func (c *CancelHTLC) Validate() error {
+func (c *UpdateFailHTLC) Validate() error {
 	// We're good!
 	return nil
 }
+
+// String returns the string representation of the target UpdateFailHTLC.  This is
+// part of the lnwire.Message interface.
+func (c *UpdateFailHTLC) String() string {
+	return fmt.Sprintf("\n--- Begin UpdateFailHTLC ---\n") +
+		fmt.Sprintf("ChannelPoint:\t%d\n", c.ChannelPoint) +
+		fmt.Sprintf("ID:\t%d\n", c.ID) +
+		fmt.Sprintf("Reason:\t\t%x\n", c.Reason) +
+		fmt.Sprintf("--- End UpdateFailHTLC ---\n")
+}

lnwire/update_fail_htlc_test.go

@@ -6,22 +6,22 @@ import (
 	"testing"
 )
 
-func TestCancelHTLCEncodeDecode(t *testing.T) {
+func TestUpdateFailHTLC(t *testing.T) {
-	// First create a new HTLCTR message.
+	// First create a new UFH message.
-	cancelMsg := &CancelHTLC{
+	cancelMsg := &UpdateFailHTLC{
-		ChannelPoint: outpoint1,
+		ChannelPoint: *outpoint1,
-		HTLCKey:      22,
+		ID:           22,
-		Reason:       UnknownPaymentHash,
 	}
+	copy(cancelMsg.Reason[:], bytes.Repeat([]byte{21}, 20))
 
-	// Next encode the HTLCTR message into an empty bytes buffer.
+	// Next encode the UFH message into an empty bytes buffer.
 	var b bytes.Buffer
 	if err := cancelMsg.Encode(&b, 0); err != nil {
 		t.Fatalf("unable to encode CancelHTLC: %v", err)
 	}
 
-	// Deserialize the encoded HTLCTR message into a new empty struct.
+	// Deserialize the encoded UFH message into a new empty struct.
-	cancelMsg2 := &CancelHTLC{}
+	cancelMsg2 := &UpdateFailHTLC{}
 	if err := cancelMsg2.Decode(&b, 0); err != nil {
 		t.Fatalf("unable to decode CancelHTLC: %v", err)
 	}