Fix #79 by adding a new node JSON-RPC command

* Gives node operators full control of peer connectivity
* RPC adds ability to disconnect all matching non-persistent peers,
  remove persistent peers, and connect to peers making them either
  temporary or persistent.

btcjson/v2/btcjson/btcdextcmds.go

@@ -7,6 +7,42 @@
 
 package btcjson
 
+// NodeSubCmd defines the type used in the addnode JSON-RPC command for the
+// sub command field.
+type NodeSubCmd string
+
+const (
+	// NConnect indicates the specified host that should be connected to.
+	NConnect NodeSubCmd = "connect"
+
+	// NRemove indicates the specified peer that should be removed as a
+	// persistent peer.
+	NRemove NodeSubCmd = "remove"
+
+	// NDisconnect indicates the specified peer should be disonnected.
+	NDisconnect NodeSubCmd = "disconnect"
+)
+
+// NodeCmd defines the dropnode JSON-RPC command.
+type NodeCmd struct {
+	SubCmd        NodeSubCmd `jsonrpcusage:"\"connect|remove|disconnect\""`
+	Target        string
+	ConnectSubCmd *string `jsonrpcusage:"\"perm|temp\""`
+}
+
+// NewNodeCmd returns a new instance which can be used to issue a `node`
+// JSON-RPC command.
+//
+// The parameters which are pointers indicate they are optional.  Passing nil
+// for optional parameters will use the default value.
+func NewNodeCmd(subCmd NodeSubCmd, target string, connectSubCmd *string) *NodeCmd {
+	return &NodeCmd{
+		SubCmd:        subCmd,
+		Target:        target,
+		ConnectSubCmd: connectSubCmd,
+	}
+}
+
 // DebugLevelCmd defines the debuglevel JSON-RPC command.  This command is not a
 // standard Bitcoin command.  It is an extension for btcd.
 type DebugLevelCmd struct {
@@ -45,6 +81,7 @@ func init() {
 	flags := UsageFlag(0)
 
 	MustRegisterCmd("debuglevel", (*DebugLevelCmd)(nil), flags)
+	MustRegisterCmd("node", (*NodeCmd)(nil), flags)
 	MustRegisterCmd("getbestblock", (*GetBestBlockCmd)(nil), flags)
 	MustRegisterCmd("getcurrentnet", (*GetCurrentNetCmd)(nil), flags)
 }

btcjson/v2/btcjson/btcdextcmds_test.go

@@ -42,6 +42,64 @@ func TestBtcdExtCmds(t *testing.T) {
 				LevelSpec: "trace",
 			},
 		},
+		{
+			name: "node",
+			newCmd: func() (interface{}, error) {
+				return btcjson.NewCmd("node", btcjson.NRemove, "1.1.1.1")
+			},
+			staticCmd: func() interface{} {
+				return btcjson.NewNodeCmd("remove", "1.1.1.1", nil)
+			},
+			marshalled: `{"jsonrpc":"1.0","method":"node","params":["remove","1.1.1.1"],"id":1}`,
+			unmarshalled: &btcjson.NodeCmd{
+				SubCmd: btcjson.NRemove,
+				Target: "1.1.1.1",
+			},
+		},
+		{
+			name: "node",
+			newCmd: func() (interface{}, error) {
+				return btcjson.NewCmd("node", btcjson.NDisconnect, "1.1.1.1")
+			},
+			staticCmd: func() interface{} {
+				return btcjson.NewNodeCmd("disconnect", "1.1.1.1", nil)
+			},
+			marshalled: `{"jsonrpc":"1.0","method":"node","params":["disconnect","1.1.1.1"],"id":1}`,
+			unmarshalled: &btcjson.NodeCmd{
+				SubCmd: btcjson.NDisconnect,
+				Target: "1.1.1.1",
+			},
+		},
+		{
+			name: "node",
+			newCmd: func() (interface{}, error) {
+				return btcjson.NewCmd("node", btcjson.NConnect, "1.1.1.1", "perm")
+			},
+			staticCmd: func() interface{} {
+				return btcjson.NewNodeCmd("connect", "1.1.1.1", btcjson.String("perm"))
+			},
+			marshalled: `{"jsonrpc":"1.0","method":"node","params":["connect","1.1.1.1","perm"],"id":1}`,
+			unmarshalled: &btcjson.NodeCmd{
+				SubCmd:        btcjson.NConnect,
+				Target:        "1.1.1.1",
+				ConnectSubCmd: btcjson.String("perm"),
+			},
+		},
+		{
+			name: "node",
+			newCmd: func() (interface{}, error) {
+				return btcjson.NewCmd("node", btcjson.NConnect, "1.1.1.1", "temp")
+			},
+			staticCmd: func() interface{} {
+				return btcjson.NewNodeCmd("connect", "1.1.1.1", btcjson.String("temp"))
+			},
+			marshalled: `{"jsonrpc":"1.0","method":"node","params":["connect","1.1.1.1","temp"],"id":1}`,
+			unmarshalled: &btcjson.NodeCmd{
+				SubCmd:        btcjson.NConnect,
+				Target:        "1.1.1.1",
+				ConnectSubCmd: btcjson.String("temp"),
+			},
+		},
 		{
 			name: "getbestblock",
 			newCmd: func() (interface{}, error) {

docs/json_rpc_api.md

@@ -186,7 +186,7 @@ the method name for further details such as parameter and return information.
 |   |   |
 |---|---|
 |Method|addnode|
-|Parameters|1. peer (string, required) - ip address and port of the peer tooperate on<br />2. command (string, required) - `add` to add a persistent peer, `remove` to remove a persistent peer, or `onetry` to try a single connection to a peer|
+|Parameters|1. peer (string, required) - ip address and port of the peer to operate on<br />2. command (string, required) - `add` to add a persistent peer, `remove` to remove a persistent peer, or `onetry` to try a single connection to a peer|
 |Description|Attempts to add or remove a persistent peer.|
 |Returns|Nothing|
 [Return to Overview](#MethodOverview)<br />
@@ -553,6 +553,8 @@ The following is an overview of the RPC methods which are implemented by btcd, b
 |2|[getbestblock](#getbestblock)|Y|Get block height and hash of best block in the main chain.|None|
 |3|[getcurrentnet](#getcurrentnet)|Y|Get bitcoin network btcd is running on.|None|
 |4|[searchrawtransactions](#searchrawtransactions)|Y|Query for transactions related to a particular address.|None|
+|5|[node](#node)|N|Attempts to add or remove a peer. |None|
+
 
 <a name="ExtMethodDetails" />
 **6.2 Method Details**<br />
@@ -609,6 +611,18 @@ The following is an overview of the RPC methods which are implemented by btcd, b
 
 ***
 
+<a name="node"/>
+
+|   |   |
+|---|---|
+|Method|node|
+|Parameters|1. command (string, required) - `connect` to add a peer (defaults to temporary), `remove` to remove a persistent peer, or `disconnect` to remove all matching non-persistent peers <br /> 2. peer (string, required) - ip address and port, or ID of the peer to operate on<br /> 3. connection type (string, optional) - `perm` indicates the peer should be added as a permanent peer, `temp` indicates a connection should only be attempted once. |
+|Description|Attempts to add or remove a peer.|
+|Returns|Nothing|
+[Return to Overview](#MethodOverview)<br />
+
+***
+
 <a name="WSExtMethods" />
 ### 7. Websocket Extension Methods (Websocket-specific)
 

rpcserver.go

@@ -155,6 +155,7 @@ var rpcHandlersBeforeInit = map[string]commandHandler{
 	"gettxout":              handleGetTxOut,
 	"getwork":               handleGetWork,
 	"help":                  handleHelp,
+	"node":                  handleNode,
 	"ping":                  handlePing,
 	"searchrawtransactions": handleSearchRawTransactions,
 	"sendrawtransaction":    handleSendRawTransaction,
@@ -369,17 +370,125 @@ func handleAddNode(s *rpcServer, cmd interface{}, closeChan <-chan struct{}) (in
 	case "onetry":
 		err = s.server.AddAddr(addr, false)
 	default:
-		err = errors.New("invalid subcommand for addnode")
+		return nil, &btcjson.RPCError{
+			Code:    btcjson.ErrRPCInvalidParameter,
+			Message: "invalid subcommand for addnode",
+		}
 	}
 
 	if err != nil {
-		return nil, internalRPCError(err.Error(), "")
+		return nil, &btcjson.RPCError{
+			Code:    btcjson.ErrRPCInvalidParameter,
+			Message: err.Error(),
+		}
 	}
 
 	// no data returned unless an error.
 	return nil, nil
 }
 
+// handleNode handles node commands.
+func handleNode(s *rpcServer, cmd interface{}, closeChan <-chan struct{}) (interface{}, error) {
+	c := cmd.(*btcjson.NodeCmd)
+
+	var addr string
+	var nodeId uint64
+	var errN, err error
+	switch c.SubCmd {
+	case "disconnect":
+		// If we have a valid uint disconnect by node id. Otherwise,
+		// attempt to disconnect by address, returning an error if a
+		// valid IP address is not supplied.
+		if nodeId, errN = strconv.ParseUint(c.Target, 10, 32); errN == nil {
+			err = s.server.DisconnectNodeById(int32(nodeId))
+		} else {
+			if _, _, errP := net.SplitHostPort(c.Target); errP == nil || net.ParseIP(c.Target) != nil {
+				addr = normalizeAddress(c.Target, activeNetParams.DefaultPort)
+				err = s.server.DisconnectNodeByAddr(addr)
+			} else {
+				return nil, &btcjson.RPCError{
+					Code:    btcjson.ErrRPCInvalidParameter,
+					Message: "invalid address or node ID",
+				}
+			}
+		}
+		if err != nil && peerExists(s.server.PeerInfo(), addr, int32(nodeId)) {
+			return nil, &btcjson.RPCError{
+				Code:    btcjson.ErrRPCMisc,
+				Message: "can't disconnect a permanent peer, use remove",
+			}
+		}
+	case "remove":
+		// If we have a valid uint disconnect by node id. Otherwise,
+		// attempt to disconnect by address, returning an error if a
+		// valid IP address is not supplied.
+		if nodeId, errN = strconv.ParseUint(c.Target, 10, 32); errN == nil {
+			err = s.server.RemoveNodeById(int32(nodeId))
+		} else {
+			if _, _, errP := net.SplitHostPort(c.Target); errP == nil || net.ParseIP(c.Target) != nil {
+				addr = normalizeAddress(c.Target, activeNetParams.DefaultPort)
+				err = s.server.RemoveNodeByAddr(addr)
+			} else {
+				return nil, &btcjson.RPCError{
+					Code:    btcjson.ErrRPCInvalidParameter,
+					Message: "invalid address or node ID",
+				}
+			}
+		}
+		if err != nil && peerExists(s.server.PeerInfo(), addr, int32(nodeId)) {
+			return nil, &btcjson.RPCError{
+				Code:    btcjson.ErrRPCMisc,
+				Message: "can't remove a temporary peer, use disconnect",
+			}
+		}
+	case "connect":
+		addr = normalizeAddress(c.Target, activeNetParams.DefaultPort)
+
+		// Default to temporary connections.
+		subCmd := "temp"
+		if c.ConnectSubCmd != nil {
+			subCmd = *c.ConnectSubCmd
+		}
+
+		switch subCmd {
+		case "perm", "temp":
+			err = s.server.ConnectNode(addr, subCmd == "perm")
+		default:
+			return nil, &btcjson.RPCError{
+				Code:    btcjson.ErrRPCInvalidParameter,
+				Message: "invalid subcommand for node connect",
+			}
+		}
+	default:
+		return nil, &btcjson.RPCError{
+			Code:    btcjson.ErrRPCInvalidParameter,
+			Message: "invalid subcommand for node",
+		}
+	}
+
+	if err != nil {
+		return nil, &btcjson.RPCError{
+			Code:    btcjson.ErrRPCInvalidParameter,
+			Message: err.Error(),
+		}
+	}
+
+	// no data returned unless an error.
+	return nil, nil
+}
+
+// peerExists determines if a certain peer is currently connected given
+// information about all currently connected peers. Peer existence is
+// determined using either a target address or node id.
+func peerExists(peerInfos []*btcjson.GetPeerInfoResult, addr string, nodeId int32) bool {
+	for _, peerInfo := range peerInfos {
+		if peerInfo.ID == nodeId || peerInfo.Addr == addr {
+			return true
+		}
+	}
+	return false
+}
+
 // messageToHex serializes a message to the wire protocol encoding using the
 // latest protocol version and returns a hex-encoded string of the result.
 func messageToHex(msg wire.Message) (string, error) {

rpcserverhelp.go

@@ -33,6 +33,12 @@ var helpDescsEnUS = map[string]string{
 	"addnode-addr":      "IP address and port of the peer to operate on",
 	"addnode-subcmd":    "'add' to add a persistent peer, 'remove' to remove a persistent peer, or 'onetry' to try a single connection to a peer",
 
+	// NodeCmd help.
+	"node--synopsis":     "Attempts to add or remove a peer.",
+	"node-subcmd":        "'disconnect' to remove all matching non-persistent peers, 'remove' to remove a persistent peer, or 'connect' to connect to a peer",
+	"node-target":        "Either the IP address and port of the peer to operate on, or a valid peer ID.",
+	"node-connectsubcmd": "'perm' to make the connected peer a permanent one, 'temp' to try a single connect to a peer",
+
 	// TransactionInput help.
 	"transactioninput-txid": "The hash of the input transaction",
 	"transactioninput-vout": "The specific output of the input transaction to redeem",
@@ -521,6 +527,7 @@ var rpcResultTypes = map[string][]interface{}{
 	"getrawtransaction":     []interface{}{(*string)(nil), (*btcjson.TxRawResult)(nil)},
 	"gettxout":              []interface{}{(*btcjson.GetTxOutResult)(nil)},
 	"getwork":               []interface{}{(*btcjson.GetWorkResult)(nil), (*bool)(nil)},
+	"node":                  nil,
 	"help":                  []interface{}{(*string)(nil), (*string)(nil)},
 	"ping":                  nil,
 	"searchrawtransactions": []interface{}{(*string)(nil), (*[]btcjson.TxRawResult)(nil)},

server.go

@@ -411,7 +411,7 @@ type addNodeMsg struct {
 }
 
 type delNodeMsg struct {
-	addr  string
+	cmp   func(*peer) bool
 	reply chan error
 }
 
@@ -419,6 +419,22 @@ type getAddedNodesMsg struct {
 	reply chan []*peer
 }
 
+type disconnectNodeMsg struct {
+	cmp   func(*peer) bool
+	reply chan error
+}
+
+type connectNodeMsg struct {
+	addr      string
+	permanent bool
+	reply     chan error
+}
+
+type removeNodeMsg struct {
+	cmp   func(*peer) bool
+	reply chan error
+}
+
 // handleQuery is the central handler for all queries and commands from other
 // goroutines related to peer state.
 func (s *server) handleQuery(querymsg interface{}, state *peerState) {
@@ -475,16 +491,20 @@ func (s *server) handleQuery(querymsg interface{}, state *peerState) {
 		msg.reply <- infos
 
 	case addNodeMsg:
+	case connectNodeMsg:
 		// XXX(oga) duplicate oneshots?
-		if msg.permanent {
+		for e := state.persistentPeers.Front(); e != nil; e = e.Next() {
-			for e := state.persistentPeers.Front(); e != nil; e = e.Next() {
+			peer := e.Value.(*peer)
-				peer := e.Value.(*peer)
+			if peer.addr == msg.addr {
-				if peer.addr == msg.addr {
+				if msg.permanent {
 					msg.reply <- errors.New("peer already connected")
-					return
+				} else {
+					msg.reply <- errors.New("peer exists as a permanent peer")
 				}
+				return
 			}
 		}
+
 		// TODO(oga) if too many, nuke a non-perm peer.
 		if s.handleAddPeerMsg(state,
 			newOutboundPeer(s, msg.addr, msg.permanent, 0)) {
@@ -494,21 +514,12 @@ func (s *server) handleQuery(querymsg interface{}, state *peerState) {
 		}
 
 	case delNodeMsg:
-		found := false
+	case removeNodeMsg:
-		for e := state.persistentPeers.Front(); e != nil; e = e.Next() {
+		found := disconnectPeer(state.persistentPeers, msg.cmp, func(p *peer) {
-			peer := e.Value.(*peer)
+			// Keep group counts ok since we remove from
-			if peer.addr == msg.addr {
+			// the list now.
-				// Keep group counts ok since we remove from
+			state.outboundGroups[addrmgr.GroupKey(p.na)]--
-				// the list now.
+		})
-				state.outboundGroups[addrmgr.GroupKey(peer.na)]--
-				// This is ok because we are not continuing
-				// to iterate so won't corrupt the loop.
-				state.persistentPeers.Remove(e)
-				peer.Disconnect()
-				found = true
-				break
-			}
-		}
 
 		if found {
 			msg.reply <- nil
@@ -525,9 +536,63 @@ func (s *server) handleQuery(querymsg interface{}, state *peerState) {
 			peers = append(peers, peer)
 		}
 		msg.reply <- peers
+	case disconnectNodeMsg:
+		// Check inbound peers. We pass a nil callback since we don't
+		// require any additional actions on disconnect for inbound peers.
+		found := disconnectPeer(state.peers, msg.cmp, nil)
+		if found {
+			msg.reply <- nil
+			return
+		}
+
+		// Check outbound peers.
+		found = disconnectPeer(state.outboundPeers, msg.cmp, func(p *peer) {
+			// Keep group counts ok since we remove from
+			// the list now.
+			state.outboundGroups[addrmgr.GroupKey(p.na)]--
+		})
+		if found {
+			// If there are multiple outbound connections to the same
+			// ip:port, continue disconnecting them all until no such
+			// peers are found.
+			for found {
+				found = disconnectPeer(state.outboundPeers, msg.cmp, func(p *peer) {
+					state.outboundGroups[addrmgr.GroupKey(p.na)]--
+				})
+			}
+			msg.reply <- nil
+			return
+		}
+
+		msg.reply <- errors.New("peer not found")
 	}
 }
 
+// disconnectPeer attempts to drop the connection of a tageted peer in the
+// passed peer list. Targets are identified via usage of the passed
+// `compareFunc`, which should return `true` if the passed peer is the target
+// peer. This function returns true on success and false if the peer is unable
+// to be located. If the peer is found, and the passed callback: `whenFound'
+// isn't nil, we call it with the peer as the argument before it is removed
+// from the peerList, and is disconnected from the server.
+func disconnectPeer(peerList *list.List, compareFunc func(*peer) bool, whenFound func(*peer)) bool {
+	for e := peerList.Front(); e != nil; e = e.Next() {
+		peer := e.Value.(*peer)
+		if compareFunc(peer) {
+			if whenFound != nil {
+				whenFound(peer)
+			}
+
+			// This is ok because we are not continuing
+			// to iterate so won't corrupt the loop.
+			peerList.Remove(e)
+			peer.Disconnect()
+			return true
+		}
+	}
+	return false
+}
+
 // listenHandler is the main listener which accepts incoming connections for the
 // server.  It must be run as a goroutine.
 func (s *server) listenHandler(listener net.Listener) {
@@ -831,7 +896,75 @@ func (s *server) AddAddr(addr string, permanent bool) error {
 func (s *server) RemoveAddr(addr string) error {
 	replyChan := make(chan error)
 
-	s.query <- delNodeMsg{addr: addr, reply: replyChan}
+	s.query <- delNodeMsg{
+		cmp:   func(p *peer) bool { return p.addr == addr },
+		reply: replyChan,
+	}
+
+	return <-replyChan
+}
+
+// DisconnectNodeByAddr disconnects a peer by target address. Both outbound and
+// inbound nodes will be searched for the target node. An error message will
+// be returned if the peer was not found.
+func (s *server) DisconnectNodeByAddr(addr string) error {
+	replyChan := make(chan error)
+
+	s.query <- disconnectNodeMsg{
+		cmp:   func(p *peer) bool { return p.addr == addr },
+		reply: replyChan,
+	}
+
+	return <-replyChan
+}
+
+// DisconnectNodeByID disconnects a peer by target node id. Both outbound and
+// inbound nodes will be searched for the target node. An error message will be
+// returned if the peer was not found.
+func (s *server) DisconnectNodeById(id int32) error {
+	replyChan := make(chan error)
+
+	s.query <- disconnectNodeMsg{
+		cmp:   func(p *peer) bool { return p.id == id },
+		reply: replyChan,
+	}
+
+	return <-replyChan
+}
+
+// RemoveNodeByAddr removes a peer from the list of persistent peers if
+// present. An error will be returned if the peer was not found.
+func (s *server) RemoveNodeByAddr(addr string) error {
+	replyChan := make(chan error)
+
+	s.query <- removeNodeMsg{
+		cmp:   func(p *peer) bool { return p.addr == addr },
+		reply: replyChan,
+	}
+
+	return <-replyChan
+}
+
+// RemoveNodeById removes a peer by node ID from the list of persistent peers
+// if present. An error will be returned if the peer was not found.
+func (s *server) RemoveNodeById(id int32) error {
+	replyChan := make(chan error)
+
+	s.query <- removeNodeMsg{
+		cmp:   func(p *peer) bool { return p.id == id },
+		reply: replyChan,
+	}
+
+	return <-replyChan
+}
+
+// ConnectNode adds `addr' as a new outbound peer. If permanent is true then the
+// peer will be persistent and reconnect if the connection is lost.
+// It is an error to call this with an already existing peer.
+func (s *server) ConnectNode(addr string, permanent bool) error {
+	replyChan := make(chan error)
+
+	s.query <- connectNodeMsg{addr: addr, permanent: permanent, reply: replyChan}
 
 	return <-replyChan
 }