multi: add custom error messages to channel acceptor

This commit adds an optional error message to the channel acceptor's
reponse to allow operators to inform (or insult) unsuccessful channel
initiators as to the reason for their rejection.

This field is added in addition to the existing accept field to maintain
backwards compatibity. If we were to deprecate accept and interpret a
non-nil error as rejecting the channel, then received a response with
accept=false and a nil error, the server cannot tell whether this is a
legacy rejection or new mesage type acceptance (due to nil error),
so we keep both fields.

chanacceptor/acceptor_test.go

@@ -120,7 +120,7 @@ func (c *channelAcceptorCtx) stop() {
 // queryAndAssert takes a map of open channel requests which we want to call
 // Accept for to the outcome we expect from the acceptor, dispatches each
 // request in a goroutine and then asserts that we get the outcome we expect.
-func (c *channelAcceptorCtx) queryAndAssert(queries map[*lnwire.OpenChannel]bool) {
+func (c *channelAcceptorCtx) queryAndAssert(queries map[*lnwire.OpenChannel]*ChannelAcceptResponse) {
 	var (
 		node = &btcec.PublicKey{
 			X: big.NewInt(1),
@@ -168,12 +168,14 @@ func TestMultipleAcceptClients(t *testing.T) {
 			PendingChannelID: [32]byte{3},
 		}
 
+		customError = errors.New("go away")
+
 		// Queries is a map of the channel IDs we will query Accept
 		// with, and the set of outcomes we expect.
-		queries = map[*lnwire.OpenChannel]bool{
-			chan1: true,
-			chan2: false,
-			chan3: false,
+		queries = map[*lnwire.OpenChannel]*ChannelAcceptResponse{
+			chan1: NewChannelAcceptResponse(true, nil),
+			chan2: NewChannelAcceptResponse(false, errChannelRejected),
+			chan3: NewChannelAcceptResponse(false, customError),
 		}
 
 		// Responses is a mocked set of responses from the remote
@@ -190,6 +192,7 @@ func TestMultipleAcceptClients(t *testing.T) {
 			chan3.PendingChannelID: {
 				PendingChanId: chan3.PendingChannelID[:],
 				Accept:        false,
+				Error:         customError.Error(),
 			},
 		}
 	)
@@ -205,3 +208,41 @@ func TestMultipleAcceptClients(t *testing.T) {
 	// Shutdown our acceptor.
 	testCtx.stop()
 }
+
+// TestInvalidResponse tests the case where our remote channel acceptor sends us
+// an invalid response, so the channel acceptor stream terminates.
+func TestInvalidResponse(t *testing.T) {
+	var (
+		chan1 = [32]byte{1}
+
+		// We make a single query, and expect it to fail with our
+		// generic error because our response is invalid.
+		queries = map[*lnwire.OpenChannel]*ChannelAcceptResponse{
+			{
+				PendingChannelID: chan1,
+			}: NewChannelAcceptResponse(
+				false, errChannelRejected,
+			),
+		}
+
+		// Create a single response which is invalid because it accepts
+		// the channel but also contains an error message.
+		responses = map[[32]byte]*lnrpc.ChannelAcceptResponse{
+			chan1: {
+				PendingChanId: chan1[:],
+				Accept:        true,
+				Error:         "has an error as well",
+			},
+		}
+	)
+
+	// Create and start our channel acceptor.
+	testCtx := newChanAcceptorCtx(t, len(queries), responses)
+	testCtx.start()
+
+	testCtx.queryAndAssert(queries)
+
+	// We do not expect our channel acceptor to exit because of one invalid
+	// response, so we shutdown and assert here.
+	testCtx.stop()
+}

chanacceptor/chainedacceptor.go

@@ -46,18 +46,26 @@ func (c *ChainedAcceptor) RemoveAcceptor(id uint64) {
 // and returns the conjunction of all these predicates.
 //
 // NOTE: Part of the ChannelAcceptor interface.
-func (c *ChainedAcceptor) Accept(req *ChannelAcceptRequest) bool {
-	result := true
-
+func (c *ChainedAcceptor) Accept(req *ChannelAcceptRequest) *ChannelAcceptResponse {
 	c.acceptorsMtx.RLock()
-	for _, acceptor := range c.acceptors {
-		// We call Accept first in case any acceptor (perhaps an RPCAcceptor)
-		// wishes to be notified about ChannelAcceptRequest.
-		result = acceptor.Accept(req) && result
-	}
-	c.acceptorsMtx.RUnlock()
+	defer c.acceptorsMtx.RUnlock()
 
-	return result
+	for _, acceptor := range c.acceptors {
+		// Call our acceptor to determine whether we want to accept this
+		// channel.
+		acceptorResponse := acceptor.Accept(req)
+
+		// If we should reject the channel, we can just exit early. This
+		// has the effect of returning the error belonging to our first
+		// failed acceptor.
+		if acceptorResponse.RejectChannel() {
+			return acceptorResponse
+		}
+	}
+
+	// If we have gone through all of our acceptors with no objections, we
+	// can return an acceptor with a nil error.
+	return NewChannelAcceptResponse(true, nil)
 }
 
 // A compile-time constraint to ensure ChainedAcceptor implements the

chanacceptor/errors.go

@@ -0,0 +1,8 @@
+package chanacceptor
+
+// ChanAcceptError is an error that it returned when an external channel
+// acceptor rejects a channel. Note that this error type is whitelisted and will
+// be delivered to the peer initiating a channel.
+type ChanAcceptError struct {
+	error
+}

chanacceptor/interface.go

@@ -1,10 +1,19 @@
 package chanacceptor
 
 import (
+	"errors"
+
 	"github.com/btcsuite/btcd/btcec"
 	"github.com/lightningnetwork/lnd/lnwire"
 )
 
+var (
+	// errChannelRejected is returned when the rpc channel acceptor rejects
+	// a channel due to acceptor timeout, shutdown, or because no custom
+	// error value is available when the channel was rejected.
+	errChannelRejected = errors.New("channel rejected")
+)
+
 // ChannelAcceptRequest is a struct containing the requesting node's public key
 // along with the lnwire.OpenChannel message that they sent when requesting an
 // inbound channel. This information is provided to each acceptor so that they
@@ -18,8 +27,48 @@ type ChannelAcceptRequest struct {
 	OpenChanMsg *lnwire.OpenChannel
 }
 
-// ChannelAcceptor is an interface that represents a predicate on the data
+// ChannelAcceptResponse is a struct containing the response to a request to
+// open an inbound channel.
+type ChannelAcceptResponse struct {
+	// ChanAcceptError the error returned by the channel acceptor. If the
+	// channel was accepted, this value will be nil.
+	ChanAcceptError
+}
+
+// NewChannelAcceptResponse is a constructor for a channel accept response,
+// which creates a response with an appropriately wrapped error (in the case of
+// a rejection) so that the error will be whitelisted and delivered to the
+// initiating peer. Accepted channels simply return a response containing a nil
+// error.
+func NewChannelAcceptResponse(accept bool,
+	acceptErr error) *ChannelAcceptResponse {
+
+	// If we want to accept the channel, we return a response with a nil
+	// error.
+	if accept {
+		return &ChannelAcceptResponse{}
+	}
+
+	// Use a generic error when no custom error is provided.
+	if acceptErr == nil {
+		acceptErr = errChannelRejected
+	}
+
+	return &ChannelAcceptResponse{
+		ChanAcceptError: ChanAcceptError{
+			error: acceptErr,
+		},
+	}
+}
+
+// RejectChannel returns a boolean that indicates whether we should reject the
+// channel.
+func (c *ChannelAcceptResponse) RejectChannel() bool {
+	return c.error != nil
+}
+
+// ChannelAcceptor is an interface that represents  a predicate on the data
 // contained in ChannelAcceptRequest.
 type ChannelAcceptor interface {
-	Accept(req *ChannelAcceptRequest) bool
+	Accept(req *ChannelAcceptRequest) *ChannelAcceptResponse
 }

chanacceptor/rpcacceptor.go

@@ -2,19 +2,36 @@ package chanacceptor
 
 import (
 	"errors"
+	"fmt"
 	"sync"
 	"time"
 
 	"github.com/lightningnetwork/lnd/lnrpc"
 )
 
-var errShuttingDown = errors.New("server shutting down")
+var (
+	errShuttingDown = errors.New("server shutting down")
+
+	// errCustomLength is returned when our custom error's length exceeds
+	// our maximum.
+	errCustomLength = fmt.Errorf("custom error message exceeds length "+
+		"limit: %v", maxErrorLength)
+
+	// errAcceptWithError is returned when we get a response which accepts
+	// a channel but ambiguously also sets a custom error message.
+	errAcceptWithError = errors.New("channel acceptor response accepts " +
+		"channel, but also includes custom error")
+
+	// maxErrorLength is the maximum error length we allow the error we
+	// send to our peer to be.
+	maxErrorLength = 500
+)
 
 // chanAcceptInfo contains a request for a channel acceptor decision, and a
 // channel that the response should be sent on.
 type chanAcceptInfo struct {
 	request  *ChannelAcceptRequest
-	response chan bool
+	response chan *ChannelAcceptResponse
 }
 
 // RPCAcceptor represents the RPC-controlled variant of the ChannelAcceptor.
@@ -52,8 +69,8 @@ type RPCAcceptor struct {
 // receives, failing the request if the timeout elapses.
 //
 // NOTE: Part of the ChannelAcceptor interface.
-func (r *RPCAcceptor) Accept(req *ChannelAcceptRequest) bool {
-	respChan := make(chan bool, 1)
+func (r *RPCAcceptor) Accept(req *ChannelAcceptRequest) *ChannelAcceptResponse {
+	respChan := make(chan *ChannelAcceptResponse, 1)
 
 	newRequest := &chanAcceptInfo{
 		request:  req,
@@ -63,6 +80,12 @@ func (r *RPCAcceptor) Accept(req *ChannelAcceptRequest) bool {
 	// timeout is the time after which ChannelAcceptRequests expire.
 	timeout := time.After(r.timeout)
 
+	// Create a rejection response which we can use for the cases where we
+	// reject the channel.
+	rejectChannel := NewChannelAcceptResponse(
+		false, errChannelRejected,
+	)
+
 	// Send the request to the newRequests channel.
 	select {
 	case r.requests <- newRequest:
@@ -70,13 +93,13 @@ func (r *RPCAcceptor) Accept(req *ChannelAcceptRequest) bool {
 	case <-timeout:
 		log.Errorf("RPCAcceptor returned false - reached timeout of %v",
 			r.timeout)
-		return false
+		return rejectChannel
 
 	case <-r.done:
-		return false
+		return rejectChannel
 
 	case <-r.quit:
-		return false
+		return rejectChannel
 	}
 
 	// Receive the response and return it. If no response has been received
@@ -88,13 +111,13 @@ func (r *RPCAcceptor) Accept(req *ChannelAcceptRequest) bool {
 	case <-timeout:
 		log.Errorf("RPCAcceptor returned false - reached timeout of %v",
 			r.timeout)
-		return false
+		return rejectChannel
 
 	case <-r.done:
-		return false
+		return rejectChannel
 
 	case <-r.quit:
-		return false
+		return rejectChannel
 	}
 }
 
@@ -160,6 +183,7 @@ func (r *RPCAcceptor) receiveResponses(errChan chan error,
 		openChanResp := lnrpc.ChannelAcceptResponse{
 			Accept:        resp.Accept,
 			PendingChanId: pendingID[:],
+			Error:         resp.Error,
 		}
 
 		// We have received a decision for one of our channel
@@ -186,7 +210,7 @@ func (r *RPCAcceptor) sendAcceptRequests(errChan chan error,
 	// listening and any in-progress requests should be terminated.
 	defer close(r.done)
 
-	acceptRequests := make(map[[32]byte]chan bool)
+	acceptRequests := make(map[[32]byte]chan *ChannelAcceptResponse)
 
 	for {
 		select {
@@ -234,9 +258,17 @@ func (r *RPCAcceptor) sendAcceptRequests(errChan chan error,
 				continue
 			}
 
+			// Validate the response we have received. If it is not
+			// valid, we log our error and proceed to deliver the
+			// rejection.
+			accept, acceptErr, err := validateAcceptorResponse(resp)
+			if err != nil {
+				log.Errorf("Invalid acceptor response: %v", err)
+			}
+
 			// Send the response boolean over the buffered response
 			// channel.
-			respChan <- resp.Accept
+			respChan <- NewChannelAcceptResponse(accept, acceptErr)
 
 			// Delete the channel from the acceptRequests map.
 			delete(acceptRequests, pendingID)
@@ -253,6 +285,41 @@ func (r *RPCAcceptor) sendAcceptRequests(errChan chan error,
 	}
 }
 
+// validateAcceptorResponse validates the response we get from the channel
+// acceptor, returning a boolean indicating whether to accept the channel, an
+// error to send to the peer, and any validation errors that occurred.
+func validateAcceptorResponse(req lnrpc.ChannelAcceptResponse) (bool, error,
+	error) {
+
+	// Check that the custom error provided is valid.
+	if len(req.Error) > maxErrorLength {
+		return false, errChannelRejected, errCustomLength
+	}
+
+	var haveCustomError = len(req.Error) != 0
+
+	switch {
+	// If accept is true, but we also have an error specified, we fail
+	// because this result is ambiguous.
+	case req.Accept && haveCustomError:
+		return false, errChannelRejected, errAcceptWithError
+
+	// If we accept without an error message, we can just return a nil
+	// error.
+	case req.Accept:
+		return true, nil, nil
+
+	// If we reject the channel, and have a custom error, then we use it.
+	case haveCustomError:
+		return false, fmt.Errorf(req.Error), nil
+
+	// Otherwise, we have rejected the channel with no custom error, so we
+	// just use a generic error to fail the channel.
+	default:
+		return false, errChannelRejected, nil
+	}
+}
+
 // A compile-time constraint to ensure RPCAcceptor implements the ChannelAcceptor
 // interface.
 var _ ChannelAcceptor = (*RPCAcceptor)(nil)

chanacceptor/rpcacceptor_test.go

@@ -0,0 +1,85 @@
+package chanacceptor
+
+import (
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/lightningnetwork/lnd/lnrpc"
+	"github.com/stretchr/testify/require"
+)
+
+// TestValidateAcceptorResponse test validation of acceptor responses.
+func TestValidateAcceptorResponse(t *testing.T) {
+	customError := errors.New("custom error")
+
+	tests := []struct {
+		name        string
+		response    lnrpc.ChannelAcceptResponse
+		accept      bool
+		acceptorErr error
+		error       error
+	}{
+		{
+			name: "accepted with error",
+			response: lnrpc.ChannelAcceptResponse{
+				Accept: true,
+				Error:  customError.Error(),
+			},
+			accept:      false,
+			acceptorErr: errChannelRejected,
+			error:       errAcceptWithError,
+		},
+		{
+			name: "custom error too long",
+			response: lnrpc.ChannelAcceptResponse{
+				Accept: false,
+				Error:  strings.Repeat(" ", maxErrorLength+1),
+			},
+			accept:      false,
+			acceptorErr: errChannelRejected,
+			error:       errCustomLength,
+		},
+		{
+			name: "accepted",
+			response: lnrpc.ChannelAcceptResponse{
+				Accept: true,
+			},
+			accept:      true,
+			acceptorErr: nil,
+			error:       nil,
+		},
+		{
+			name: "rejected with error",
+			response: lnrpc.ChannelAcceptResponse{
+				Accept: false,
+				Error:  customError.Error(),
+			},
+			accept:      false,
+			acceptorErr: customError,
+			error:       nil,
+		},
+		{
+			name: "rejected with no error",
+			response: lnrpc.ChannelAcceptResponse{
+				Accept: false,
+			},
+			accept:      false,
+			acceptorErr: errChannelRejected,
+			error:       nil,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+
+		t.Run(test.name, func(t *testing.T) {
+			accept, acceptErr, err := validateAcceptorResponse(
+				test.response,
+			)
+			require.Equal(t, test.accept, accept)
+			require.Equal(t, test.acceptorErr, acceptErr)
+			require.Equal(t, test.error, err)
+		})
+	}
+}

fundingmanager.go

@@ -746,6 +746,8 @@ func (f *fundingManager) failFundingFlow(peer lnpeer.Peer, tempChanID [32]byte,
 		msg = lnwire.ErrorData(e.Error())
 	case lnwire.FundingError:
 		msg = lnwire.ErrorData(e.Error())
+	case chanacceptor.ChanAcceptError:
+		msg = lnwire.ErrorData(e.Error())
 
 	// For all other error types we just send a generic error.
 	default:
@@ -1282,10 +1284,13 @@ func (f *fundingManager) handleFundingOpen(peer lnpeer.Peer,
 		OpenChanMsg: msg,
 	}
 
-	if !f.cfg.OpenChannelPredicate.Accept(chanReq) {
+	// Query our channel acceptor to determine whether we should reject
+	// the channel.
+	acceptorResp := f.cfg.OpenChannelPredicate.Accept(chanReq)
+	if acceptorResp.RejectChannel() {
 		f.failFundingFlow(
 			peer, msg.PendingChannelID,
-			fmt.Errorf("open channel request rejected"),
+			acceptorResp.ChanAcceptError,
 		)
 		return
 	}

lnrpc/rpc.proto

@@ -791,6 +791,16 @@ message ChannelAcceptResponse {
 
     // The pending channel id to which this response applies.
     bytes pending_chan_id = 2;
+
+    /*
+    An optional error to send the initiating party to indicate why the channel
+    was rejected. This field *should not* contain sensitive information, it will
+    be sent to the initiating party. This field should only be set if accept is
+    false, the channel will be rejected if an error is set with accept=true
+    because the meaning of this response is ambiguous. Limited to 500
+    characters.
+    */
+    string error = 3;
 }
 
 message ChannelPoint {