[NOD-798] Disconnect from a peer if a block received from it gets rejected (#648)

* [NOD-798] Disconnect from a peer if its block gets rejected.

* [NOD-798] Make a comment less ambiguous.

cmd/kaspactl/config.go

@@ -7,9 +7,9 @@ package main
 import (
 	"fmt"
 	"github.com/kaspanet/kaspad/config"
+	"github.com/kaspanet/kaspad/version"
 	"github.com/pkg/errors"
 	"io/ioutil"
-	"net"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -103,28 +103,6 @@ type ConfigFlags struct {
 	config.NetworkFlags
 }
 
-// normalizeAddress returns addr with the passed default port appended if
-// there is not already a port specified.
-func normalizeAddress(addr string, useTestnet, useSimnet, useDevnet bool) string {
-	_, _, err := net.SplitHostPort(addr)
-	if err != nil {
-		var defaultPort string
-		switch {
-		case useDevnet:
-			defaultPort = "16610"
-		case useTestnet:
-			defaultPort = "16210"
-		case useSimnet:
-			defaultPort = "16510"
-		default:
-			defaultPort = "16110"
-		}
-
-		return net.JoinHostPort(addr, defaultPort)
-	}
-	return addr
-}
-
 // cleanAndExpandPath expands environement variables and leading ~ in the
 // passed path, cleans the result, and returns it.
 func cleanAndExpandPath(path string) string {
@@ -184,7 +162,7 @@ func loadConfig() (*ConfigFlags, []string, error) {
 	appName = strings.TrimSuffix(appName, filepath.Ext(appName))
 	usageMessage := fmt.Sprintf("Use %s -h to show options", appName)
 	if preCfg.ShowVersion {
-		fmt.Println(appName, "version", version())
+		fmt.Println(appName, "version", version.Version())
 		os.Exit(0)
 	}
 
@@ -212,7 +190,7 @@ func loadConfig() (*ConfigFlags, []string, error) {
 	parser := flags.NewParser(activeConfig, flags.Default)
 	err = flags.NewIniParser(parser).ParseFile(preCfg.ConfigFile)
 	if err != nil {
-		if pErr := &(os.PathError{}); !errors.As(err, pErr) {
+		if pErr := &(os.PathError{}); !errors.As(err, &pErr) {
 			fmt.Fprintf(os.Stderr, "Error parsing config file: %s\n",
 				err)
 			fmt.Fprintln(os.Stderr, usageMessage)
@@ -239,8 +217,10 @@ func loadConfig() (*ConfigFlags, []string, error) {
 
 	// Add default port to RPC server based on --testnet and --simnet flags
 	// if needed.
-	activeConfig.RPCServer = normalizeAddress(activeConfig.RPCServer, activeConfig.Testnet,
-		activeConfig.Simnet, activeConfig.Devnet)
+	activeConfig.RPCServer, err = activeConfig.NetParams().NormalizeRPCServerAddress(activeConfig.RPCServer)
+	if err != nil {
+		return nil, nil, err
+	}
 
 	return activeConfig, remainingArgs, nil
 }

cmd/kaspactl/version.go

@@ -1,75 +0,0 @@
-// Copyright (c) 2013 The btcsuite developers
-// Use of this source code is governed by an ISC
-// license that can be found in the LICENSE file.
-
-package main
-
-import (
-	"bytes"
-	"fmt"
-	"strings"
-)
-
-// semanticAlphabet
-const semanticAlphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-"
-
-// These constants define the application version and follow the semantic
-// versioning 2.0.0 spec (http://semver.org/).
-const (
-	appMajor uint = 0
-	appMinor uint = 12
-	appPatch uint = 0
-
-	// appPreRelease MUST only contain characters from semanticAlphabet
-	// per the semantic versioning spec.
-	appPreRelease = "beta"
-)
-
-// appBuild is defined as a variable so it can be overridden during the build
-// process with '-ldflags "-X main.appBuild foo' if needed. It MUST only
-// contain characters from semanticAlphabet per the semantic versioning spec.
-var appBuild string
-
-// version returns the application version as a properly formed string per the
-// semantic versioning 2.0.0 spec (http://semver.org/).
-func version() string {
-	// Start with the major, minor, and patch versions.
-	version := fmt.Sprintf("%d.%d.%d", appMajor, appMinor, appPatch)
-
-	// Append pre-release version if there is one. The hyphen called for
-	// by the semantic versioning spec is automatically appended and should
-	// not be contained in the pre-release string. The pre-release version
-	// is not appended if it contains invalid characters.
-	preRelease := normalizeVerString(appPreRelease)
-	if preRelease != "" {
-		version = fmt.Sprintf("%s-%s", version, preRelease)
-	}
-
-	// Append build metadata if there is any. The plus called for
-	// by the semantic versioning spec is automatically appended and should
-	// not be contained in the build metadata string. The build metadata
-	// string is not appended if it contains invalid characters.
-	build := normalizeVerString(appBuild)
-	if build != "" {
-		version = fmt.Sprintf("%s+%s", version, build)
-	}
-
-	return version
-}
-
-// normalizeVerString returns the passed string stripped of all characters which
-// are not valid according to the semantic versioning guidelines for pre-release
-// version and build metadata strings. In particular they MUST only contain
-// characters in semanticAlphabet.
-func normalizeVerString(str string) string {
-	var result bytes.Buffer
-	for _, r := range str {
-		if strings.ContainsRune(semanticAlphabet, r) {
-			// Ignoring the error here since it can only fail if
-			// the the system is out of memory and there are much
-			// bigger issues at that point.
-			_, _ = result.WriteRune(r)
-		}
-	}
-	return result.String()
-}

cmd/kaspaminer/client.go

@@ -6,7 +6,6 @@ import (
 	"github.com/kaspanet/kaspad/wire"
 	"github.com/pkg/errors"
 	"io/ioutil"
-	"net"
 	"time"
 )
 
@@ -43,8 +42,13 @@ func connectToServer(cfg *configFlags) (*minerClient, error) {
 		return nil, err
 	}
 
+	rpcAddr, err := cfg.NetParams().NormalizeRPCServerAddress(cfg.RPCServer)
+	if err != nil {
+		return nil, err
+	}
+
 	connCfg := &rpcclient.ConnConfig{
-		Host:           normalizeRPCServerAddress(cfg.RPCServer, cfg),
+		Host:           rpcAddr,
 		Endpoint:       "ws",
 		User:           cfg.RPCUser,
 		Pass:           cfg.RPCPassword,
@@ -63,16 +67,6 @@ func connectToServer(cfg *configFlags) (*minerClient, error) {
 	return client, nil
 }
 
-// normalizeRPCServerAddress returns addr with the current network default
-// port appended if there is not already a port specified.
-func normalizeRPCServerAddress(addr string, cfg *configFlags) string {
-	_, _, err := net.SplitHostPort(addr)
-	if err != nil {
-		return net.JoinHostPort(addr, cfg.NetParams().RPCPort)
-	}
-	return addr
-}
-
 func readCert(cfg *configFlags) ([]byte, error) {
 	if cfg.DisableTLS {
 		return nil, nil

cmd/kaspaminer/config.go

@@ -11,7 +11,7 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/jessevdk/go-flags"
-	"github.com/kaspanet/kaspad/cmd/kaspaminer/version"
+	"github.com/kaspanet/kaspad/version"
 )
 
 const (

cmd/kaspaminer/main.go

@@ -2,11 +2,11 @@ package main
 
 import (
 	"fmt"
+	"github.com/kaspanet/kaspad/version"
 	"os"
 
 	"github.com/pkg/errors"
 
-	"github.com/kaspanet/kaspad/cmd/kaspaminer/version"
 	"github.com/kaspanet/kaspad/signal"
 	"github.com/kaspanet/kaspad/util/panics"
 )

cmd/kaspaminer/version/version.go

@@ -1,50 +0,0 @@
-package version
-
-import (
-	"fmt"
-	"strings"
-)
-
-// validCharacters  is a list of characters valid in the appBuild string
-const validCharacters = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-"
-
-const (
-	appMajor uint = 0
-	appMinor uint = 1
-	appPatch uint = 0
-)
-
-// appBuild is defined as a variable so it can be overridden during the build
-// process with '-ldflags "-X github.com/kaspanet/kaspad/cmd/kaspaminer/version.appBuild=foo"' if needed.
-// It MUST only contain characters from validCharacters.
-var appBuild string
-
-var version = "" // string used for memoization of version
-
-// Version returns the application version as a properly formed string
-func Version() string {
-	if version == "" {
-		// Start with the major, minor, and patch versions.
-		version = fmt.Sprintf("%d.%d.%d", appMajor, appMinor, appPatch)
-
-		// Append build metadata if there is any.
-		// Panic if any invalid characters are encountered
-		if appBuild != "" {
-			checkAppBuild(appBuild)
-
-			version = fmt.Sprintf("%s-%s", version, appBuild)
-		}
-	}
-
-	return version
-}
-
-// checkAppBuild verifies that appBuild does not contain any characters outside of validCharacters.
-// In case of any invalid characters checkAppBuild panics
-func checkAppBuild(appBuild string) {
-	for _, r := range appBuild {
-		if !strings.ContainsRune(validCharacters, r) {
-			panic(fmt.Errorf("appBuild string (%s) contains forbidden characters. Only alphanumeric characters and dashes are allowed", appBuild))
-		}
-	}
-}

config/config.go

@@ -311,7 +311,7 @@ func loadConfig() (*Config, []string, error) {
 
 		err := flags.NewIniParser(parser).ParseFile(preCfg.ConfigFile)
 		if err != nil {
-			if pErr := &(os.PathError{}); !errors.As(err, pErr) {
+			if pErr := &(os.PathError{}); !errors.As(err, &pErr) {
 				fmt.Fprintf(os.Stderr, "Error parsing config "+
 					"file: %s\n", err)
 				fmt.Fprintln(os.Stderr, usageMessage)
@@ -705,13 +705,19 @@ func loadConfig() (*Config, []string, error) {
 
 	// Add default port to all listener addresses if needed and remove
 	// duplicate addresses.
-	activeConfig.Listeners = network.NormalizeAddresses(activeConfig.Listeners,
+	activeConfig.Listeners, err = network.NormalizeAddresses(activeConfig.Listeners,
 		activeConfig.NetParams().DefaultPort)
+	if err != nil {
+		return nil, nil, err
+	}
 
 	// Add default port to all rpc listener addresses if needed and remove
 	// duplicate addresses.
-	activeConfig.RPCListeners = network.NormalizeAddresses(activeConfig.RPCListeners,
+	activeConfig.RPCListeners, err = network.NormalizeAddresses(activeConfig.RPCListeners,
 		activeConfig.NetParams().RPCPort)
+	if err != nil {
+		return nil, nil, err
+	}
 
 	// Only allow TLS to be disabled if the RPC is bound to localhost
 	// addresses.
@@ -745,10 +751,17 @@ func loadConfig() (*Config, []string, error) {
 
 	// Add default port to all added peer addresses if needed and remove
 	// duplicate addresses.
-	activeConfig.AddPeers = network.NormalizeAddresses(activeConfig.AddPeers,
+	activeConfig.AddPeers, err = network.NormalizeAddresses(activeConfig.AddPeers,
 		activeConfig.NetParams().DefaultPort)
-	activeConfig.ConnectPeers = network.NormalizeAddresses(activeConfig.ConnectPeers,
+	if err != nil {
+		return nil, nil, err
+	}
+
+	activeConfig.ConnectPeers, err = network.NormalizeAddresses(activeConfig.ConnectPeers,
 		activeConfig.NetParams().DefaultPort)
+	if err != nil {
+		return nil, nil, err
+	}
 
 	// Setup dial and DNS resolution (lookup) functions depending on the
 	// specified options. The default is to use the standard

dagconfig/params.go

@@ -5,6 +5,7 @@
 package dagconfig
 
 import (
+	"github.com/kaspanet/kaspad/util/network"
 	"math"
 	"math/big"
 	"time"
@@ -185,6 +186,12 @@ type Params struct {
 	HDCoinType uint32
 }
 
+// NormalizeRPCServerAddress returns addr with the current network default
+// port appended if there is not already a port specified.
+func (p *Params) NormalizeRPCServerAddress(addr string) (string, error) {
+	return network.NormalizeAddress(addr, p.RPCPort)
+}
+
 // MainnetParams defines the network parameters for the main Kaspa network.
 var MainnetParams = Params{
 	K:           ghostdagK,

database/cmd/dbtool/main.go

@@ -70,7 +70,7 @@ func realMain() error {
 	defer os.Stdout.Sync()
 	log = backendLogger.Logger("MAIN")
 	spawn = panics.GoroutineWrapperFunc(log)
-	dbLog, _ := logger.Get(logger.SubsystemTags.BCDB)
+	dbLog, _ := logger.Get(logger.SubsystemTags.KSDB)
 	dbLog.SetLevel(logs.LevelDebug)
 
 	// Setup the parser options and commands.

database/ffldb/log.go

@@ -8,4 +8,4 @@ import (
 	"github.com/kaspanet/kaspad/logger"
 )
 
-var log, _ = logger.Get(logger.SubsystemTags.BCDB)
+var log, _ = logger.Get(logger.SubsystemTags.KSDB)

logger/logger.go

@@ -31,7 +31,7 @@ var (
 	adxrLog = BackendLog.Logger("ADXR")
 	amgrLog = BackendLog.Logger("AMGR")
 	cmgrLog = BackendLog.Logger("CMGR")
-	bcdbLog = BackendLog.Logger("BCDB")
+	ksdbLog = BackendLog.Logger("KSDB")
 	kasdLog = BackendLog.Logger("KASD")
 	bdagLog = BackendLog.Logger("BDAG")
 	cnfgLog = BackendLog.Logger("CNFG")
@@ -52,7 +52,7 @@ var SubsystemTags = struct {
 	ADXR,
 	AMGR,
 	CMGR,
-	BCDB,
+	KSDB,
 	KASD,
 	BDAG,
 	CNFG,
@@ -70,7 +70,7 @@ var SubsystemTags = struct {
 	ADXR: "ADXR",
 	AMGR: "AMGR",
 	CMGR: "CMGR",
-	BCDB: "BCDB",
+	KSDB: "KSDB",
 	KASD: "KASD",
 	BDAG: "BDAG",
 	CNFG: "CNFG",
@@ -91,7 +91,7 @@ var subsystemLoggers = map[string]logs.Logger{
 	SubsystemTags.ADXR: adxrLog,
 	SubsystemTags.AMGR: amgrLog,
 	SubsystemTags.CMGR: cmgrLog,
-	SubsystemTags.BCDB: bcdbLog,
+	SubsystemTags.KSDB: ksdbLog,
 	SubsystemTags.KASD: kasdLog,
 	SubsystemTags.BDAG: bdagLog,
 	SubsystemTags.CNFG: cnfgLog,

netsync/manager.go

@@ -207,12 +207,13 @@ func (sm *SyncManager) startSync() {
 		return
 	}
 
-	log.Warnf("No sync peer candidates available")
 	if sm.shouldQueryPeerSelectedTips() {
+		hasSyncCandidates := false
 		for peer, state := range sm.peerStates {
 			if !state.syncCandidate {
 				continue
 			}
+			hasSyncCandidates = true
 
 			if time.Since(state.lastSelectedTipRequest) < minGetSelectedTipInterval {
 				continue
@@ -220,6 +221,9 @@ func (sm *SyncManager) startSync() {
 
 			queueMsgGetSelectedTip(peer, state)
 		}
+		if !hasSyncCandidates {
+			log.Warnf("No sync peer candidates available")
+		}
 	}
 }
 
@@ -508,6 +512,9 @@ func (sm *SyncManager) handleBlockMsg(bmsg *blockMsg) {
 		// send it.
 		code, reason := mempool.ErrToRejectErr(err)
 		peer.PushRejectMsg(wire.CmdBlock, code, reason, blockHash, false)
+
+		// Disconnect from the misbehaving peer.
+		peer.Disconnect()
 		return
 	}
 

rpcmodel/rpc_results.go

@@ -22,7 +22,7 @@ type GetBlockHeaderVerboseResult struct {
 	Difficulty           float64  `json:"difficulty"`
 	ParentHashes         []string `json:"parentHashes,omitempty"`
 	SelectedParentHash   string   `json:"selectedParentHash"`
-	NextHashes           []string `json:"nextHashes,omitempty"`
+	ChildHashes          []string `json:"childHashes,omitempty"`
 }
 
 // GetBlockVerboseResult models the data from the getblock command when the
@@ -47,7 +47,7 @@ type GetBlockVerboseResult struct {
 	Difficulty           float64       `json:"difficulty"`
 	ParentHashes         []string      `json:"parentHashes"`
 	SelectedParentHash   string        `json:"selectedParentHash,omitempty"`
-	NextHashes           []string      `json:"nextHashes,omitempty"`
+	ChildHashes          []string      `json:"childHashes,omitempty"`
 }
 
 // CreateMultiSigResult models the data returned from the createmultisig

server/rpc/common.go

@@ -264,7 +264,7 @@ func buildGetBlockVerboseResult(s *Server, block *util.Block, isVerboseTx bool)
 		Size:                 int32(block.MsgBlock().SerializeSize()),
 		Bits:                 strconv.FormatInt(int64(blockHeader.Bits), 16),
 		Difficulty:           getDifficultyRatio(blockHeader.Bits, params),
-		NextHashes:           childHashStrings,
+		ChildHashes:          childHashStrings,
 	}
 
 	if isVerboseTx {

server/rpc/handle_add_manual_node.go

@@ -11,8 +11,14 @@ func handleAddManualNode(s *Server, cmd interface{}, closeChan <-chan struct{})
 
 	oneTry := c.OneTry != nil && *c.OneTry
 
-	addr := network.NormalizeAddress(c.Addr, s.cfg.DAGParams.DefaultPort)
-	var err error
+	addr, err := network.NormalizeAddress(c.Addr, s.cfg.DAGParams.DefaultPort)
+	if err != nil {
+		return nil, &rpcmodel.RPCError{
+			Code:    rpcmodel.ErrRPCInvalidParameter,
+			Message: err.Error(),
+		}
+	}
+
 	if oneTry {
 		err = s.cfg.ConnMgr.Connect(addr, false)
 	} else {

server/rpc/handle_get_block_header.go

@@ -68,7 +68,7 @@ func handleGetBlockHeader(s *Server, cmd interface{}, closeChan <-chan struct{})
 		VersionHex:           fmt.Sprintf("%08x", blockHeader.Version),
 		HashMerkleRoot:       blockHeader.HashMerkleRoot.String(),
 		AcceptedIDMerkleRoot: blockHeader.AcceptedIDMerkleRoot.String(),
-		NextHashes:           childHashStrings,
+		ChildHashes:          childHashStrings,
 		ParentHashes:         daghash.Strings(blockHeader.ParentHashes),
 		SelectedParentHash:   selectedParentHash.String(),
 		Nonce:                blockHeader.Nonce,

server/rpc/handle_node.go

@@ -24,7 +24,11 @@ func handleNode(s *Server, cmd interface{}, closeChan <-chan struct{}) (interfac
 			err = s.cfg.ConnMgr.DisconnectByID(int32(nodeID))
 		} else {
 			if _, _, errP := net.SplitHostPort(c.Target); errP == nil || net.ParseIP(c.Target) != nil {
-				addr = network.NormalizeAddress(c.Target, params.DefaultPort)
+				addr, err = network.NormalizeAddress(c.Target, params.DefaultPort)
+				if err != nil {
+					break
+				}
+
 				err = s.cfg.ConnMgr.DisconnectByAddr(addr)
 			} else {
 				return nil, &rpcmodel.RPCError{
@@ -49,7 +53,11 @@ func handleNode(s *Server, cmd interface{}, closeChan <-chan struct{}) (interfac
 			err = s.cfg.ConnMgr.RemoveByID(int32(nodeID))
 		} else {
 			if _, _, errP := net.SplitHostPort(c.Target); errP == nil || net.ParseIP(c.Target) != nil {
-				addr = network.NormalizeAddress(c.Target, params.DefaultPort)
+				addr, err = network.NormalizeAddress(c.Target, params.DefaultPort)
+				if err != nil {
+					break
+				}
+
 				err = s.cfg.ConnMgr.RemoveByAddr(addr)
 			} else {
 				return nil, &rpcmodel.RPCError{
@@ -66,7 +74,10 @@ func handleNode(s *Server, cmd interface{}, closeChan <-chan struct{}) (interfac
 		}
 
 	case "connect":
-		addr = network.NormalizeAddress(c.Target, params.DefaultPort)
+		addr, err = network.NormalizeAddress(c.Target, params.DefaultPort)
+		if err != nil {
+			break
+		}
 
 		// Default to temporary connections.
 		subCmd := "temp"

server/rpc/handle_remove_manual_node.go

@@ -9,9 +9,15 @@ import (
 func handleRemoveManualNode(s *Server, cmd interface{}, closeChan <-chan struct{}) (interface{}, error) {
 	c := cmd.(*rpcmodel.RemoveManualNodeCmd)
 
-	addr := network.NormalizeAddress(c.Addr, s.cfg.DAGParams.DefaultPort)
-	err := s.cfg.ConnMgr.RemoveByAddr(addr)
+	addr, err := network.NormalizeAddress(c.Addr, s.cfg.DAGParams.DefaultPort)
+	if err != nil {
+		return nil, &rpcmodel.RPCError{
+			Code:    rpcmodel.ErrRPCInvalidParameter,
+			Message: err.Error(),
+		}
+	}
 
+	err = s.cfg.ConnMgr.RemoveByAddr(addr)
 	if err != nil {
 		return nil, &rpcmodel.RPCError{
 			Code:    rpcmodel.ErrRPCInvalidParameter,

server/rpc/rpcserverhelp.go

@@ -21,7 +21,7 @@ var helpDescsEnUS = map[string]string{
 		"The levelspec can either a debug level or of the form:\n" +
 		"<subsystem>=<level>,<subsystem2>=<level2>,...\n" +
 		"The valid debug levels are trace, debug, info, warn, error, and critical.\n" +
-		"The valid subsystems are AMGR, ADXR, BCDB, BMGR, KASD, BDAG, DISC, PEER, RPCS, SCRP, SRVR, and TXMP.\n" +
+		"The valid subsystems are AMGR, ADXR, KSDB, BMGR, KASD, BDAG, DISC, PEER, RPCS, SCRP, SRVR, and TXMP.\n" +
 		"Finally the keyword 'show' will return a list of the available subsystems.",
 	"debugLevel-levelSpec":   "The debug level(s) to use or the keyword 'show'",
 	"debugLevel--condition0": "levelspec!=show",
@@ -267,7 +267,7 @@ var helpDescsEnUS = map[string]string{
 	"getBlockVerboseResult-difficulty":           "The proof-of-work difficulty as a multiple of the minimum difficulty",
 	"getBlockVerboseResult-parentHashes":         "The hashes of the parent blocks",
 	"getBlockVerboseResult-selectedParentHash":   "The selected parent hash",
-	"getBlockVerboseResult-nextHashes":           "The hashes of the next blocks (only if there are any)",
+	"getBlockVerboseResult-childHashes":          "The hashes of the child blocks (only if there are any)",
 
 	// GetBlockCountCmd help.
 	"getBlockCount--synopsis": "Returns the number of blocks in the block DAG.",
@@ -295,7 +295,7 @@ var helpDescsEnUS = map[string]string{
 	"getBlockHeaderVerboseResult-difficulty":           "The proof-of-work difficulty as a multiple of the minimum difficulty",
 	"getBlockHeaderVerboseResult-parentHashes":         "The hashes of the parent blocks",
 	"getBlockHeaderVerboseResult-selectedParentHash":   "The selected parent hash",
-	"getBlockHeaderVerboseResult-nextHashes":           "The hashes of the next blocks (only if there are any)",
+	"getBlockHeaderVerboseResult-childHashes":          "The hashes of the child blocks (only if there are any)",
 
 	// TemplateRequest help.
 	"templateRequest-mode":         "This is 'template', 'proposal', or omitted",

util/network/network.go

@@ -6,22 +6,35 @@ import (
 
 // NormalizeAddresses returns a new slice with all the passed peer addresses
 // normalized with the given default port, and all duplicates removed.
-func NormalizeAddresses(addrs []string, defaultPort string) []string {
+func NormalizeAddresses(addrs []string, defaultPort string) ([]string, error) {
 	for i, addr := range addrs {
-		addrs[i] = NormalizeAddress(addr, defaultPort)
+		var err error
+		addrs[i], err = NormalizeAddress(addr, defaultPort)
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	return removeDuplicateAddresses(addrs)
+	return removeDuplicateAddresses(addrs), nil
 }
 
 // NormalizeAddress returns addr with the passed default port appended if
 // there is not already a port specified.
-func NormalizeAddress(addr, defaultPort string) string {
+func NormalizeAddress(addr, defaultPort string) (string, error) {
 	_, _, err := net.SplitHostPort(addr)
+	// net.SplitHostPort returns an error if the given host is missing a
+	// port, but theoretically it can return an error for other reasons,
+	// and this is why we check addrWithPort for validity.
 	if err != nil {
-		return net.JoinHostPort(addr, defaultPort)
+		addrWithPort := net.JoinHostPort(addr, defaultPort)
+		_, _, err := net.SplitHostPort(addrWithPort)
+		if err != nil {
+			return "", err
+		}
+
+		return addrWithPort, nil
 	}
-	return addr
+	return addr, nil
 }
 
 // removeDuplicateAddresses returns a new slice with all duplicate entries in