Merge pull request #2485 from halseth/error-codes-dont-send

[lnwire+funding] Don't send ErrorCode on wire

fundingmanager.go

@@ -26,7 +26,6 @@ import (
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/routing"
 	"golang.org/x/crypto/salsa20"
-	"google.golang.org/grpc"
 )
 
 const (
@@ -858,19 +857,18 @@ func (f *fundingManager) failFundingFlow(peer lnpeer.Peer, tempChanID [32]byte,
 	}
 
 	// We only send the exact error if it is part of out whitelisted set of
-	// errors (lnwire.ErrorCode or lnwallet.ReservationError).
+	// errors (lnwire.FundingError or lnwallet.ReservationError).
 	var msg lnwire.ErrorData
 	switch e := fundingErr.(type) {
 
-	// Let the actual error message be sent to the remote.
+	// Let the actual error message be sent to the remote for the
+	// whitelisted types.
 	case lnwallet.ReservationError:
 		msg = lnwire.ErrorData(e.Error())
+	case lnwire.FundingError:
+		msg = lnwire.ErrorData(e.Error())
 
-	// Send the status code.
+	// For all other error types we just send a generic error.
-	case lnwire.ErrorCode:
-		msg = lnwire.ErrorData{byte(e)}
-
-	// We just send a generic error.
 	default:
 		msg = lnwire.ErrorData("funding failed due to internal error")
 	}
@@ -2957,31 +2955,18 @@ func (f *fundingManager) handleErrorMsg(fmsg *fundingErrorMsg) {
 	resCtx, err := f.cancelReservationCtx(fmsg.peerKey, chanID)
 	if err != nil {
 		fndgLog.Warnf("Received error for non-existent funding "+
-			"flow: %v (%v)", err, spew.Sdump(protocolErr))
+			"flow: %v (%v)", err, protocolErr.Error())
 		return
 	}
 
 	// If we did indeed find the funding workflow, then we'll return the
 	// error back to the caller (if any), and cancel the workflow itself.
-	lnErr := lnwire.ErrorCode(protocolErr.Data[0])
+	fundingErr := fmt.Errorf("received funding error from %x: %v",
-	fndgLog.Errorf("Received funding error from %x: %v",
+		fmsg.peerKey.SerializeCompressed(), protocolErr.Error(),
-		fmsg.peerKey.SerializeCompressed(), string(protocolErr.Data),
 	)
+	fndgLog.Errorf(fundingErr.Error())
 
-	// If this isn't a simple error code, then we'll display the entire
+	resCtx.err <- fundingErr
-	// thing.
-	if len(protocolErr.Data) > 1 {
-		err = grpc.Errorf(
-			lnErr.ToGrpcCode(), string(protocolErr.Data),
-		)
-	} else {
-		// Otherwise, we'll attempt to display just the error code
-		// itself.
-		err = grpc.Errorf(
-			lnErr.ToGrpcCode(), lnErr.String(),
-		)
-	}
-	resCtx.err <- err
 }
 
 // pruneZombieReservations loops through all pending reservations and fails the

fundingmanager_test.go

@@ -602,7 +602,7 @@ func fundChannel(t *testing.T, alice, bob *testNode, localFundingAmt,
 		if gotError {
 			t.Fatalf("expected OpenChannel to be sent "+
 				"from bob, instead got error: %v",
-				lnwire.ErrorCode(errorMsg.Data[0]))
+				errorMsg.Error())
 		}
 		t.Fatalf("expected OpenChannel to be sent from "+
 			"alice, instead got %T", aliceMsg)
@@ -728,7 +728,7 @@ func assertFundingMsgSent(t *testing.T, msgChan chan lnwire.Message,
 		errorMsg, gotError := msg.(*lnwire.Error)
 		if gotError {
 			t.Fatalf("expected %s to be sent, instead got error: %v",
-				msgType, lnwire.ErrorCode(errorMsg.Data[0]))
+				msgType, errorMsg.Error())
 		}
 
 		_, _, line, _ := runtime.Caller(1)
@@ -1469,7 +1469,7 @@ func TestFundingManagerPeerTimeoutAfterInitFunding(t *testing.T) {
 		if gotError {
 			t.Fatalf("expected OpenChannel to be sent "+
 				"from bob, instead got error: %v",
-				lnwire.ErrorCode(errorMsg.Data[0]))
+				errorMsg.Error())
 		}
 		t.Fatalf("expected OpenChannel to be sent from "+
 			"alice, instead got %T", aliceMsg)
@@ -1531,7 +1531,7 @@ func TestFundingManagerPeerTimeoutAfterFundingOpen(t *testing.T) {
 		if gotError {
 			t.Fatalf("expected OpenChannel to be sent "+
 				"from bob, instead got error: %v",
-				lnwire.ErrorCode(errorMsg.Data[0]))
+				errorMsg.Error())
 		}
 		t.Fatalf("expected OpenChannel to be sent from "+
 			"alice, instead got %T", aliceMsg)
@@ -1602,7 +1602,7 @@ func TestFundingManagerPeerTimeoutAfterFundingAccept(t *testing.T) {
 		if gotError {
 			t.Fatalf("expected OpenChannel to be sent "+
 				"from bob, instead got error: %v",
-				lnwire.ErrorCode(errorMsg.Data[0]))
+				errorMsg.Error())
 		}
 		t.Fatalf("expected OpenChannel to be sent from "+
 			"alice, instead got %T", aliceMsg)
@@ -2326,7 +2326,7 @@ func TestFundingManagerCustomChannelParameters(t *testing.T) {
 		if gotError {
 			t.Fatalf("expected OpenChannel to be sent "+
 				"from bob, instead got error: %v",
-				lnwire.ErrorCode(errorMsg.Data[0]))
+				errorMsg.Error())
 		}
 		t.Fatalf("expected OpenChannel to be sent from "+
 			"alice, instead got %T", aliceMsg)
@@ -2561,7 +2561,7 @@ func TestFundingManagerMaxPendingChannels(t *testing.T) {
 			if gotError {
 				t.Fatalf("expected OpenChannel to be sent "+
 					"from bob, instead got error: %v",
-					lnwire.ErrorCode(errorMsg.Data[0]))
+					errorMsg.Error())
 			}
 			t.Fatalf("expected OpenChannel to be sent from "+
 				"alice, instead got %T", aliceMsg)
@@ -2725,7 +2725,7 @@ func TestFundingManagerRejectPush(t *testing.T) {
 		if gotError {
 			t.Fatalf("expected OpenChannel to be sent "+
 				"from bob, instead got error: %v",
-				lnwire.ErrorCode(errorMsg.Data[0]))
+				errorMsg.Error())
 		}
 		t.Fatalf("expected OpenChannel to be sent from "+
 			"alice, instead got %T", aliceMsg)
@@ -2736,9 +2736,9 @@ func TestFundingManagerRejectPush(t *testing.T) {
 
 	// Assert Bob responded with an ErrNonZeroPushAmount error.
 	err := assertFundingMsgSent(t, bob.msgChan, "Error").(*lnwire.Error)
-	if "Non-zero push amounts are disabled" != string(err.Data) {
+	if !strings.Contains(err.Error(), "Non-zero push amounts are disabled") {
 		t.Fatalf("expected ErrNonZeroPushAmount error, got \"%v\"",
-			string(err.Data))
+			err.Error())
 	}
 }
 
@@ -2782,7 +2782,7 @@ func TestFundingManagerMaxConfs(t *testing.T) {
 		if gotError {
 			t.Fatalf("expected OpenChannel to be sent "+
 				"from bob, instead got error: %v",
-				lnwire.ErrorCode(errorMsg.Data[0]))
+				errorMsg.Error())
 		}
 		t.Fatalf("expected OpenChannel to be sent from "+
 			"alice, instead got %T", aliceMsg)
@@ -2805,9 +2805,9 @@ func TestFundingManagerMaxConfs(t *testing.T) {
 	// Alice should respond back with an error indicating MinAcceptDepth is
 	// too large.
 	err := assertFundingMsgSent(t, alice.msgChan, "Error").(*lnwire.Error)
-	if !strings.Contains(string(err.Data), "minimum depth") {
+	if !strings.Contains(err.Error(), "minimum depth") {
 		t.Fatalf("expected ErrNumConfsTooLarge, got \"%v\"",
-			string(err.Data))
+			err.Error())
 	}
 }
 

htlcswitch/link.go

@@ -1882,13 +1882,9 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) {
 		// Error received from remote, MUST fail channel, but should
 		// only print the contents of the error message if all
 		// characters are printable ASCII.
-		errMsg := "non-ascii data"
-		if isASCII(msg.Data) {
-			errMsg = string(msg.Data)
-		}
 		l.fail(LinkFailureError{code: ErrRemoteError},
 			"ChannelPoint(%v): received error from peer: %v",
-			l.channel.ChannelPoint(), errMsg)
+			l.channel.ChannelPoint(), msg.Error())
 	default:
 		log.Warnf("ChannelPoint(%v): received unknown message of type %T",
 			l.channel.ChannelPoint(), msg)
@@ -3100,16 +3096,3 @@ func (l *channelLink) tracef(format string, a ...interface{}) {
 	msg := fmt.Sprintf(format, a...)
 	log.Tracef("ChannelLink(%s) %s", l.ShortChanID(), msg)
 }
-
-// isASCII is a helper method that checks whether all bytes in `data` would be
-// printable ASCII characters if interpreted as a string.
-func isASCII(data []byte) bool {
-	isASCII := true
-	for _, c := range data {
-		if c < 32 || c > 126 {
-			isASCII = false
-			break
-		}
-	}
-	return isASCII
-}

lntest/itest/lnd_test.go

@@ -42,7 +42,6 @@ import (
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/routing"
 	"golang.org/x/net/context"
-	"google.golang.org/grpc"
 )
 
 var (
@@ -6187,7 +6186,9 @@ func testMaxPendingChannels(net *lntest.NetworkHarness, t *harnessTest) {
 
 	if err == nil {
 		t.Fatalf("error wasn't received")
-	} else if grpc.Code(err) != lnwire.ErrMaxPendingChannels.ToGrpcCode() {
+	} else if !strings.Contains(
+		err.Error(), lnwire.ErrMaxPendingChannels.Error(),
+	) {
 		t.Fatalf("not expected error was received: %v", err)
 	}
 

lnwire/error.go

@@ -1,41 +1,32 @@
 package lnwire
 
 import (
+	"fmt"
 	"io"
-
-	"google.golang.org/grpc/codes"
 )
 
-// ErrorCode represents the short error code for each of the defined errors
+// FundingError represents a set of errors that can be encountered and sent
-// within the Lightning Network protocol spec.
+// during the funding workflow.
-type ErrorCode uint8
+type FundingError uint8
-
-// ToGrpcCode is used to generate gRPC specific code which will be propagated
-// to the ln rpc client. This code is used to have more detailed view of what
-// goes wrong and also in order to have the ability pragmatically determine the
-// error and take specific actions on the client side.
-func (e ErrorCode) ToGrpcCode() codes.Code {
-	return (codes.Code)(e) + 100
-}
 
 const (
 	// ErrMaxPendingChannels is returned by remote peer when the number of
 	// active pending channels exceeds their maximum policy limit.
-	ErrMaxPendingChannels ErrorCode = 1
+	ErrMaxPendingChannels FundingError = 1
 
 	// ErrSynchronizingChain is returned by a remote peer that receives a
 	// channel update or a funding request while their still syncing to the
 	// latest state of the blockchain.
-	ErrSynchronizingChain ErrorCode = 2
+	ErrSynchronizingChain FundingError = 2
 
 	// ErrChanTooLarge is returned by a remote peer that receives a
 	// FundingOpen request for a channel that is above their current
 	// soft-limit.
-	ErrChanTooLarge ErrorCode = 3
+	ErrChanTooLarge FundingError = 3
 )
 
-// String returns a human readable version of the target ErrorCode.
+// String returns a human readable version of the target FundingError.
-func (e ErrorCode) String() string {
+func (e FundingError) String() string {
 	switch e {
 	case ErrMaxPendingChannels:
 		return "Number of pending channels exceed maximum"
@@ -48,10 +39,10 @@ func (e ErrorCode) String() string {
 	}
 }
 
-// Error returns the human redable version of the target ErrorCode.
+// Error returns the human redable version of the target FundingError.
 //
-// Satisfies the Error interface.
+// NOTE: Satisfies the Error interface.
-func (e ErrorCode) Error() string {
+func (e FundingError) Error() string {
 	return e.String()
 }
 
@@ -65,8 +56,6 @@ type ErrorData []byte
 // format is purposefully general in order to allow expression of a wide array
 // of possible errors. Each Error message is directed at a particular open
 // channel referenced by ChannelPoint.
-//
-// TODO(roasbeef): remove the error code
 type Error struct {
 	// ChanID references the active channel in which the error occurred
 	// within. If the ChanID is all zeros, then this error applies to the
@@ -87,6 +76,18 @@ func NewError() *Error {
 // interface.
 var _ Message = (*Error)(nil)
 
+// Error returns the string representation to Error.
+//
+// NOTE: Satisfies the error interface.
+func (c *Error) Error() string {
+	errMsg := "non-ascii data"
+	if isASCII(c.Data) {
+		errMsg = string(c.Data)
+	}
+
+	return fmt.Sprintf("chan_id=%v, err=%v", c.ChanID, errMsg)
+}
+
 // Decode deserializes a serialized Error message stored in the passed
 // io.Reader observing the specified protocol version.
 //
@@ -125,3 +126,14 @@ func (c *Error) MaxPayloadLength(uint32) uint32 {
 	// 32 + 2 + 65501
 	return 65535
 }
+
+// isASCII is a helper method that checks whether all bytes in `data` would be
+// printable ASCII characters if interpreted as a string.
+func isASCII(data []byte) bool {
+	for _, c := range data {
+		if c < 32 || c > 126 {
+			return false
+		}
+	}
+	return true
+}

lnwire/lnwire.go

@@ -117,12 +117,6 @@ func WriteElement(w io.Writer, element interface{}) error {
 		if _, err := w.Write(b[:]); err != nil {
 			return err
 		}
-	case ErrorCode:
-		var b [2]byte
-		binary.BigEndian.PutUint16(b[:], uint16(e))
-		if _, err := w.Write(b[:]); err != nil {
-			return err
-		}
 	case MilliSatoshi:
 		var b [8]byte
 		binary.BigEndian.PutUint64(b[:], uint64(e))
@@ -506,12 +500,6 @@ func ReadElement(r io.Reader, element interface{}) error {
 			return err
 		}
 		*e = ChanUpdateChanFlags(b[0])
-	case *ErrorCode:
-		var b [2]byte
-		if _, err := io.ReadFull(r, b[:]); err != nil {
-			return err
-		}
-		*e = ErrorCode(binary.BigEndian.Uint16(b[:]))
 	case *uint32:
 		var b [4]byte
 		if _, err := io.ReadFull(r, b[:]); err != nil {

peer.go

@@ -1250,7 +1250,7 @@ func messageSummary(msg lnwire.Message) string {
 			msg.ChanID, msg.ID, msg.FailureCode)
 
 	case *lnwire.Error:
-		return fmt.Sprintf("chan_id=%v, err=%v", msg.ChanID, string(msg.Data))
+		return fmt.Sprintf("%v", msg.Error())
 
 	case *lnwire.AnnounceSignatures:
 		return fmt.Sprintf("chan_id=%v, short_chan_id=%v", msg.ChannelID,