Fixes tendermint/tendermint#3439

* make sure we create valid private keys:

 - genPrivKey samples and rejects invalid fieldelems (like libsecp256k1)
 - GenPrivKeySecp256k1 uses `(sha(secret) mod (n − 1)) + 1`
 - fix typo, rename test file: s/secpk256k1/secp256k1/

* Update crypto/secp256k1/secp256k1.go

consensus/state_test.go

@@ -14,7 +14,7 @@ import (
 	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/libs/log"
 	tmpubsub "github.com/tendermint/tendermint/libs/pubsub"
-	p2pdummy "github.com/tendermint/tendermint/p2p/dummy"
+	p2pmock "github.com/tendermint/tendermint/p2p/mock"
 	"github.com/tendermint/tendermint/types"
 )
 
@@ -1548,7 +1548,7 @@ func TestStateHalt1(t *testing.T) {
 func TestStateOutputsBlockPartsStats(t *testing.T) {
 	// create dummy peer
 	cs, _ := randConsensusState(1)
-	peer := p2pdummy.NewPeer()
+	peer := p2pmock.NewPeer(nil)
 
 	// 1) new block part
 	parts := types.NewPartSetFromData(cmn.RandBytes(100), 10)
@@ -1591,7 +1591,7 @@ func TestStateOutputsBlockPartsStats(t *testing.T) {
 func TestStateOutputVoteStats(t *testing.T) {
 	cs, vss := randConsensusState(2)
 	// create dummy peer
-	peer := p2pdummy.NewPeer()
+	peer := p2pmock.NewPeer(nil)
 
 	vote := signVote(vss[1], types.PrecommitType, []byte("test"), types.PartSetHeader{})
 

crypto/secp256k1/secp256k1.go

@@ -6,6 +6,7 @@ import (
 	"crypto/subtle"
 	"fmt"
 	"io"
+	"math/big"
 
 	"golang.org/x/crypto/ripemd160"
 
@@ -65,32 +66,61 @@ func (privKey PrivKeySecp256k1) Equals(other crypto.PrivKey) bool {
 }
 
 // GenPrivKey generates a new ECDSA private key on curve secp256k1 private key.
-// It uses OS randomness in conjunction with the current global random seed
+// It uses OS randomness to generate the private key.
-// in tendermint/libs/common to generate the private key.
 func GenPrivKey() PrivKeySecp256k1 {
 	return genPrivKey(crypto.CReader())
 }
 
 // genPrivKey generates a new secp256k1 private key using the provided reader.
 func genPrivKey(rand io.Reader) PrivKeySecp256k1 {
-	privKeyBytes := [32]byte{}
+	var privKeyBytes [32]byte
+	d := new(big.Int)
+	for {
+		privKeyBytes = [32]byte{}
 		_, err := io.ReadFull(rand, privKeyBytes[:])
 		if err != nil {
 			panic(err)
 		}
-	// crypto.CRandBytes is guaranteed to be 32 bytes long, so it can be
+
-	// casted to PrivKeySecp256k1.
+		d.SetBytes(privKeyBytes[:])
+		// break if we found a valid point (i.e. > 0 and < N == curverOrder)
+		isValidFieldElement := 0 < d.Sign() && d.Cmp(secp256k1.S256().N) < 0
+		if isValidFieldElement {
+			break
+		}
+	}
+
 	return PrivKeySecp256k1(privKeyBytes)
 }
 
+var one = new(big.Int).SetInt64(1)
+
 // GenPrivKeySecp256k1 hashes the secret with SHA2, and uses
 // that 32 byte output to create the private key.
+//
+// It makes sure the private key is a valid field element by setting:
+//
+// c = sha256(secret)
+// k = (c mod (n − 1)) + 1, where n = curve order.
+//
 // NOTE: secret should be the output of a KDF like bcrypt,
 // if it's derived from user input.
 func GenPrivKeySecp256k1(secret []byte) PrivKeySecp256k1 {
-	privKey32 := sha256.Sum256(secret)
+	secHash := sha256.Sum256(secret)
-	// sha256.Sum256() is guaranteed to be 32 bytes long, so it can be
+	// to guarantee that we have a valid field element, we use the approach of:
-	// casted to PrivKeySecp256k1.
+	// "Suite B Implementer’s Guide to FIPS 186-3", A.2.1
+	// https://apps.nsa.gov/iaarchive/library/ia-guidance/ia-solutions-for-classified/algorithm-guidance/suite-b-implementers-guide-to-fips-186-3-ecdsa.cfm
+	// see also https://github.com/golang/go/blob/0380c9ad38843d523d9c9804fe300cb7edd7cd3c/src/crypto/ecdsa/ecdsa.go#L89-L101
+	fe := new(big.Int).SetBytes(secHash[:])
+	n := new(big.Int).Sub(secp256k1.S256().N, one)
+	fe.Mod(fe, n)
+	fe.Add(fe, one)
+
+	feB := fe.Bytes()
+	var privKey32 [32]byte
+	// copy feB over to fixed 32 byte privKey32 and pad (if necessary)
+	copy(privKey32[32-len(feB):32], feB)
+
 	return PrivKeySecp256k1(privKey32)
 }
 

crypto/secp256k1/secp256k1_cgo_test.go

@@ -0,0 +1,39 @@
+// +build libsecp256k1
+
+package secp256k1
+
+import (
+	"github.com/magiconair/properties/assert"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestPrivKeySecp256k1SignVerify(t *testing.T) {
+	msg := []byte("A.1.2 ECC Key Pair Generation by Testing Candidates")
+	priv := GenPrivKey()
+	tests := []struct {
+		name             string
+		privKey          PrivKeySecp256k1
+		wantSignErr      bool
+		wantVerifyPasses bool
+	}{
+		{name: "valid sign-verify round", privKey: priv, wantSignErr: false, wantVerifyPasses: true},
+		{name: "invalid private key", privKey: [32]byte{}, wantSignErr: true, wantVerifyPasses: false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := tt.privKey.Sign(msg)
+			if tt.wantSignErr {
+				require.Error(t, err)
+				t.Logf("Got error: %s", err)
+				return
+			}
+			require.NoError(t, err)
+			require.NotNil(t, got)
+
+			pub := tt.privKey.PubKey()
+			assert.Equal(t, tt.wantVerifyPasses, pub.VerifyBytes(msg, got))
+		})
+	}
+}

crypto/secp256k1/secp256k1_internal_test.go

@@ -0,0 +1,45 @@
+package secp256k1
+
+import (
+	"bytes"
+	"math/big"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	underlyingSecp256k1 "github.com/btcsuite/btcd/btcec"
+)
+
+func Test_genPrivKey(t *testing.T) {
+
+	empty := make([]byte, 32)
+	oneB := big.NewInt(1).Bytes()
+	onePadded := make([]byte, 32)
+	copy(onePadded[32-len(oneB):32], oneB)
+	t.Logf("one padded: %v, len=%v", onePadded, len(onePadded))
+
+	validOne := append(empty, onePadded...)
+	tests := []struct {
+		name        string
+		notSoRand   []byte
+		shouldPanic bool
+	}{
+		{"empty bytes (panics because 1st 32 bytes are zero and 0 is not a valid field element)", empty, true},
+		{"curve order: N", underlyingSecp256k1.S256().N.Bytes(), true},
+		{"valid because 0 < 1 < N", validOne, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.shouldPanic {
+				require.Panics(t, func() {
+					genPrivKey(bytes.NewReader(tt.notSoRand))
+				})
+				return
+			}
+			got := genPrivKey(bytes.NewReader(tt.notSoRand))
+			fe := new(big.Int).SetBytes(got[:])
+			require.True(t, fe.Cmp(underlyingSecp256k1.S256().N) < 0)
+			require.True(t, fe.Sign() > 0)
+		})
+	}
+}

crypto/secp256k1/secp256k1_nocgo_test.go

@@ -6,7 +6,6 @@ import (
 	"testing"
 
 	secp256k1 "github.com/btcsuite/btcd/btcec"
-
 	"github.com/stretchr/testify/require"
 )
 

crypto/secp256k1/secp256k1_test.go

@@ -2,6 +2,7 @@ package secp256k1_test
 
 import (
 	"encoding/hex"
+	"math/big"
 	"testing"
 
 	"github.com/btcsuite/btcutil/base58"
@@ -84,3 +85,28 @@ func TestSecp256k1LoadPrivkeyAndSerializeIsIdentity(t *testing.T) {
 		require.Equal(t, privKeyBytes[:], serializedBytes)
 	}
 }
+
+func TestGenPrivKeySecp256k1(t *testing.T) {
+	// curve oder N
+	N := underlyingSecp256k1.S256().N
+	tests := []struct {
+		name   string
+		secret []byte
+	}{
+		{"empty secret", []byte{}},
+		{"some long secret", []byte("We live in a society exquisitely dependent on science and technology, in which hardly anyone knows anything about science and technology.")},
+		{"another seed used in cosmos tests #1", []byte{0}},
+		{"another seed used in cosmos tests #2", []byte("mySecret")},
+		{"another seed used in cosmos tests #3", []byte("")},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotPrivKey := secp256k1.GenPrivKeySecp256k1(tt.secret)
+			require.NotNil(t, gotPrivKey)
+			// interpret as a big.Int and make sure it is a valid field element:
+			fe := new(big.Int).SetBytes(gotPrivKey[:])
+			require.True(t, fe.Cmp(N) < 0)
+			require.True(t, fe.Sign() > 0)
+		})
+	}
+}

node/node.go

@@ -488,7 +488,7 @@ func NewNode(config *cfg.Config,
 	addrBook := pex.NewAddrBook(config.P2P.AddrBookFile(), config.P2P.AddrBookStrict)
 
 	// Add ourselves to addrbook to prevent dialing ourselves
-	addrBook.AddOurAddress(nodeInfo.NetAddress())
+	addrBook.AddOurAddress(sw.NetAddress())
 
 	addrBook.SetLogger(p2pLogger.With("book", config.P2P.AddrBookFile()))
 	if config.P2P.PexReactor {

p2p/dummy/peer.go

@@ -1,100 +0,0 @@
-package dummy
-
-import (
-	"net"
-
-	cmn "github.com/tendermint/tendermint/libs/common"
-	p2p "github.com/tendermint/tendermint/p2p"
-	tmconn "github.com/tendermint/tendermint/p2p/conn"
-)
-
-type peer struct {
-	cmn.BaseService
-	kv map[string]interface{}
-}
-
-var _ p2p.Peer = (*peer)(nil)
-
-// NewPeer creates new dummy peer.
-func NewPeer() *peer {
-	p := &peer{
-		kv: make(map[string]interface{}),
-	}
-	p.BaseService = *cmn.NewBaseService(nil, "peer", p)
-
-	return p
-}
-
-// FlushStop just calls Stop.
-func (p *peer) FlushStop() {
-	p.Stop()
-}
-
-// ID always returns dummy.
-func (p *peer) ID() p2p.ID {
-	return p2p.ID("dummy")
-}
-
-// IsOutbound always returns false.
-func (p *peer) IsOutbound() bool {
-	return false
-}
-
-// IsPersistent always returns false.
-func (p *peer) IsPersistent() bool {
-	return false
-}
-
-// NodeInfo always returns empty node info.
-func (p *peer) NodeInfo() p2p.NodeInfo {
-	return p2p.DefaultNodeInfo{}
-}
-
-// RemoteIP always returns localhost.
-func (p *peer) RemoteIP() net.IP {
-	return net.ParseIP("127.0.0.1")
-}
-
-// Addr always returns tcp://localhost:8800.
-func (p *peer) RemoteAddr() net.Addr {
-	return &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 8800}
-}
-
-// CloseConn always returns nil.
-func (p *peer) CloseConn() error {
-	return nil
-}
-
-// Status always returns empry connection status.
-func (p *peer) Status() tmconn.ConnectionStatus {
-	return tmconn.ConnectionStatus{}
-}
-
-// Send does not do anything and just returns true.
-func (p *peer) Send(byte, []byte) bool {
-	return true
-}
-
-// TrySend does not do anything and just returns true.
-func (p *peer) TrySend(byte, []byte) bool {
-	return true
-}
-
-// Set records value under key specified in the map.
-func (p *peer) Set(key string, value interface{}) {
-	p.kv[key] = value
-}
-
-// Get returns a value associated with the key. Nil is returned if no value
-// found.
-func (p *peer) Get(key string) interface{} {
-	if value, ok := p.kv[key]; ok {
-		return value
-	}
-	return nil
-}
-
-// OriginalAddr always returns nil.
-func (p *peer) OriginalAddr() *p2p.NetAddress {
-	return nil
-}

p2p/mock/peer.go

@@ -0,0 +1,68 @@
+package mock
+
+import (
+	"net"
+
+	"github.com/tendermint/tendermint/crypto/ed25519"
+	cmn "github.com/tendermint/tendermint/libs/common"
+	"github.com/tendermint/tendermint/p2p"
+	"github.com/tendermint/tendermint/p2p/conn"
+)
+
+type Peer struct {
+	*cmn.BaseService
+	ip                   net.IP
+	id                   p2p.ID
+	addr                 *p2p.NetAddress
+	kv                   map[string]interface{}
+	Outbound, Persistent bool
+}
+
+// NewPeer creates and starts a new mock peer. If the ip
+// is nil, random routable address is used.
+func NewPeer(ip net.IP) *Peer {
+	var netAddr *p2p.NetAddress
+	if ip == nil {
+		_, netAddr = p2p.CreateRoutableAddr()
+	} else {
+		netAddr = p2p.NewNetAddressIPPort(ip, 26656)
+	}
+	nodeKey := p2p.NodeKey{PrivKey: ed25519.GenPrivKey()}
+	netAddr.ID = nodeKey.ID()
+	mp := &Peer{
+		ip:   ip,
+		id:   nodeKey.ID(),
+		addr: netAddr,
+		kv:   make(map[string]interface{}),
+	}
+	mp.BaseService = cmn.NewBaseService(nil, "MockPeer", mp)
+	mp.Start()
+	return mp
+}
+
+func (mp *Peer) FlushStop()                              { mp.Stop() }
+func (mp *Peer) TrySend(chID byte, msgBytes []byte) bool { return true }
+func (mp *Peer) Send(chID byte, msgBytes []byte) bool    { return true }
+func (mp *Peer) NodeInfo() p2p.NodeInfo {
+	return p2p.DefaultNodeInfo{
+		ID_:        mp.addr.ID,
+		ListenAddr: mp.addr.DialString(),
+	}
+}
+func (mp *Peer) Status() conn.ConnectionStatus { return conn.ConnectionStatus{} }
+func (mp *Peer) ID() p2p.ID                    { return mp.id }
+func (mp *Peer) IsOutbound() bool              { return mp.Outbound }
+func (mp *Peer) IsPersistent() bool            { return mp.Persistent }
+func (mp *Peer) Get(key string) interface{} {
+	if value, ok := mp.kv[key]; ok {
+		return value
+	}
+	return nil
+}
+func (mp *Peer) Set(key string, value interface{}) {
+	mp.kv[key] = value
+}
+func (mp *Peer) RemoteIP() net.IP            { return mp.ip }
+func (mp *Peer) SocketAddr() *p2p.NetAddress { return mp.addr }
+func (mp *Peer) RemoteAddr() net.Addr        { return &net.TCPAddr{IP: mp.ip, Port: 8800} }
+func (mp *Peer) CloseConn() error            { return nil }

p2p/node_info.go

@@ -23,14 +23,8 @@ func MaxNodeInfoSize() int {
 // NodeInfo exposes basic info of a node
 // and determines if we're compatible.
 type NodeInfo interface {
-	nodeInfoAddress
-	nodeInfoTransport
-}
-
-// nodeInfoAddress exposes just the core info of a node.
-type nodeInfoAddress interface {
 	ID() ID
-	NetAddress() *NetAddress
+	nodeInfoTransport
 }
 
 // nodeInfoTransport validates a nodeInfo and checks

p2p/peer.go

@@ -29,7 +29,7 @@ type Peer interface {
 
 	NodeInfo() NodeInfo // peer's info
 	Status() tmconn.ConnectionStatus
-	OriginalAddr() *NetAddress // original address for outbound peers
+	SocketAddr() *NetAddress // actual address of the socket
 
 	Send(byte, []byte) bool
 	TrySend(byte, []byte) bool
@@ -46,7 +46,7 @@ type peerConn struct {
 	persistent bool
 	conn       net.Conn // source connection
 
-	originalAddr *NetAddress // nil for inbound connections
+	socketAddr *NetAddress
 
 	// cached RemoteIP()
 	ip net.IP
@@ -55,14 +55,14 @@ type peerConn struct {
 func newPeerConn(
 	outbound, persistent bool,
 	conn net.Conn,
-	originalAddr *NetAddress,
+	socketAddr *NetAddress,
 ) peerConn {
 
 	return peerConn{
 		outbound:   outbound,
 		persistent: persistent,
 		conn:       conn,
-		originalAddr: originalAddr,
+		socketAddr: socketAddr,
 	}
 }
 
@@ -223,13 +223,12 @@ func (p *peer) NodeInfo() NodeInfo {
 	return p.nodeInfo
 }
 
-// OriginalAddr returns the original address, which was used to connect with
+// SocketAddr returns the address of the socket.
-// the peer. Returns nil for inbound peers.
+// For outbound peers, it's the address dialed (after DNS resolution).
-func (p *peer) OriginalAddr() *NetAddress {
+// For inbound peers, it's the address returned by the underlying connection
-	if p.peerConn.outbound {
+// (not what's reported in the peer's NodeInfo).
-		return p.peerConn.originalAddr
+func (p *peer) SocketAddr() *NetAddress {
-	}
+	return p.peerConn.socketAddr
-	return nil
 }
 
 // Status returns the peer's ConnectionStatus.

p2p/peer_set_test.go

@@ -29,7 +29,7 @@ func (mp *mockPeer) IsPersistent() bool                      { return true }
 func (mp *mockPeer) Get(s string) interface{}                { return s }
 func (mp *mockPeer) Set(string, interface{})                 {}
 func (mp *mockPeer) RemoteIP() net.IP                        { return mp.ip }
-func (mp *mockPeer) OriginalAddr() *NetAddress               { return nil }
+func (mp *mockPeer) SocketAddr() *NetAddress                 { return nil }
 func (mp *mockPeer) RemoteAddr() net.Addr                    { return &net.TCPAddr{IP: mp.ip, Port: 8800} }
 func (mp *mockPeer) CloseConn() error                        { return nil }
 

p2p/peer_test.go

@@ -109,25 +109,27 @@ func testOutboundPeerConn(
 	persistent bool,
 	ourNodePrivKey crypto.PrivKey,
 ) (peerConn, error) {
+
+	var pc peerConn
 	conn, err := testDial(addr, config)
 	if err != nil {
-		return peerConn{}, cmn.ErrorWrap(err, "Error creating peer")
+		return pc, cmn.ErrorWrap(err, "Error creating peer")
 	}
 
-	pc, err := testPeerConn(conn, config, true, persistent, ourNodePrivKey, addr)
+	pc, err = testPeerConn(conn, config, true, persistent, ourNodePrivKey, addr)
 	if err != nil {
 		if cerr := conn.Close(); cerr != nil {
-			return peerConn{}, cmn.ErrorWrap(err, cerr.Error())
+			return pc, cmn.ErrorWrap(err, cerr.Error())
 		}
-		return peerConn{}, err
+		return pc, err
 	}
 
 	// ensure dialed ID matches connection ID
 	if addr.ID != pc.ID() {
 		if cerr := conn.Close(); cerr != nil {
-			return peerConn{}, cmn.ErrorWrap(err, cerr.Error())
+			return pc, cmn.ErrorWrap(err, cerr.Error())
 		}
-		return peerConn{}, ErrSwitchAuthenticationFailure{addr, pc.ID()}
+		return pc, ErrSwitchAuthenticationFailure{addr, pc.ID()}
 	}
 
 	return pc, nil

p2p/pex/pex_reactor.go

@@ -167,7 +167,7 @@ func (r *PEXReactor) AddPeer(p Peer) {
 		}
 	} else {
 		// inbound peer is its own source
-		addr := p.NodeInfo().NetAddress()
+		addr := p.SocketAddr()
 		src := addr
 
 		// add to book. dont RequestAddrs right away because
@@ -309,7 +309,7 @@ func (r *PEXReactor) ReceiveAddrs(addrs []*p2p.NetAddress, src Peer) error {
 	}
 	r.requestsSent.Delete(id)
 
-	srcAddr := src.NodeInfo().NetAddress()
+	srcAddr := src.SocketAddr()
 	for _, netAddr := range addrs {
 		// Validate netAddr. Disconnect from a peer if it sends us invalid data.
 		if netAddr == nil {

p2p/pex/pex_reactor_test.go

@@ -2,8 +2,8 @@ package pex
 
 import (
 	"fmt"
+
 	"io/ioutil"
-	"net"
 	"os"
 	"path/filepath"
 	"testing"
@@ -12,14 +12,11 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"github.com/tendermint/tendermint/crypto"
-	"github.com/tendermint/tendermint/crypto/ed25519"
-	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/libs/log"
+	"github.com/tendermint/tendermint/p2p/mock"
 
 	"github.com/tendermint/tendermint/config"
 	"github.com/tendermint/tendermint/p2p"
-	"github.com/tendermint/tendermint/p2p/conn"
 )
 
 var (
@@ -101,7 +98,7 @@ func TestPEXReactorRunning(t *testing.T) {
 	}
 
 	addOtherNodeAddrToAddrBook := func(switchIndex, otherSwitchIndex int) {
-		addr := switches[otherSwitchIndex].NodeInfo().NetAddress()
+		addr := switches[otherSwitchIndex].NetAddress()
 		books[switchIndex].AddAddress(addr, addr)
 	}
 
@@ -132,7 +129,7 @@ func TestPEXReactorReceive(t *testing.T) {
 	r.RequestAddrs(peer)
 
 	size := book.Size()
-	addrs := []*p2p.NetAddress{peer.NodeInfo().NetAddress()}
+	addrs := []*p2p.NetAddress{peer.SocketAddr()}
 	msg := cdc.MustMarshalBinaryBare(&pexAddrsMessage{Addrs: addrs})
 	r.Receive(PexChannel, peer, msg)
 	assert.Equal(t, size+1, book.Size())
@@ -148,7 +145,7 @@ func TestPEXReactorRequestMessageAbuse(t *testing.T) {
 	sw := createSwitchAndAddReactors(r)
 	sw.SetAddrBook(book)
 
-	peer := newMockPeer()
+	peer := mock.NewPeer(nil)
 	p2p.AddPeerToSwitch(sw, peer)
 	assert.True(t, sw.Peers().Has(peer.ID()))
 
@@ -178,7 +175,7 @@ func TestPEXReactorAddrsMessageAbuse(t *testing.T) {
 	sw := createSwitchAndAddReactors(r)
 	sw.SetAddrBook(book)
 
-	peer := newMockPeer()
+	peer := mock.NewPeer(nil)
 	p2p.AddPeerToSwitch(sw, peer)
 	assert.True(t, sw.Peers().Has(peer.ID()))
 
@@ -189,7 +186,7 @@ func TestPEXReactorAddrsMessageAbuse(t *testing.T) {
 	assert.True(t, r.requestsSent.Has(id))
 	assert.True(t, sw.Peers().Has(peer.ID()))
 
-	addrs := []*p2p.NetAddress{peer.NodeInfo().NetAddress()}
+	addrs := []*p2p.NetAddress{peer.SocketAddr()}
 	msg := cdc.MustMarshalBinaryBare(&pexAddrsMessage{Addrs: addrs})
 
 	// receive some addrs. should clear the request
@@ -234,7 +231,7 @@ func TestCheckSeeds(t *testing.T) {
 	badPeerConfig = &PEXReactorConfig{
 		Seeds: []string{"ed3dfd27bfc4af18f67a49862f04cc100696e84d@bad.network.addr:26657",
 			"d824b13cb5d40fa1d8a614e089357c7eff31b670@anotherbad.network.addr:26657",
-			seed.NodeInfo().NetAddress().String()},
+			seed.NetAddress().String()},
 	}
 	peer = testCreatePeerWithConfig(dir, 2, badPeerConfig)
 	require.Nil(t, peer.Start())
@@ -268,12 +265,13 @@ func TestConnectionSpeedForPeerReceivedFromSeed(t *testing.T) {
 	defer os.RemoveAll(dir) // nolint: errcheck
 
 	// 1. create peer
-	peer := testCreateDefaultPeer(dir, 1)
+	peerSwitch := testCreateDefaultPeer(dir, 1)
-	require.Nil(t, peer.Start())
+	require.Nil(t, peerSwitch.Start())
-	defer peer.Stop()
+	defer peerSwitch.Stop()
 
 	// 2. Create seed which knows about the peer
-	seed := testCreateSeed(dir, 2, []*p2p.NetAddress{peer.NodeInfo().NetAddress()}, []*p2p.NetAddress{peer.NodeInfo().NetAddress()})
+	peerAddr := peerSwitch.NetAddress()
+	seed := testCreateSeed(dir, 2, []*p2p.NetAddress{peerAddr}, []*p2p.NetAddress{peerAddr})
 	require.Nil(t, seed.Start())
 	defer seed.Stop()
 
@@ -300,7 +298,7 @@ func TestPEXReactorCrawlStatus(t *testing.T) {
 	// Create a peer, add it to the peer set and the addrbook.
 	peer := p2p.CreateRandomPeer(false)
 	p2p.AddPeerToSwitch(pexR.Switch, peer)
-	addr1 := peer.NodeInfo().NetAddress()
+	addr1 := peer.SocketAddr()
 	pexR.book.AddAddress(addr1, addr1)
 
 	// Add a non-connected address to the book.
@@ -364,7 +362,7 @@ func TestPEXReactorSeedModeFlushStop(t *testing.T) {
 	reactor := switches[0].Reactors()["pex"].(*PEXReactor)
 	peerID := switches[1].NodeInfo().ID()
 
-	err = switches[1].DialPeerWithAddress(switches[0].NodeInfo().NetAddress(), false)
+	err = switches[1].DialPeerWithAddress(switches[0].NetAddress(), false)
 	assert.NoError(t, err)
 
 	// sleep up to a second while waiting for the peer to send us a message.
@@ -402,7 +400,7 @@ func TestPEXReactorDoesNotAddPrivatePeersToAddrBook(t *testing.T) {
 	pexR.RequestAddrs(peer)
 
 	size := book.Size()
-	addrs := []*p2p.NetAddress{peer.NodeInfo().NetAddress()}
+	addrs := []*p2p.NetAddress{peer.SocketAddr()}
 	msg := cdc.MustMarshalBinaryBare(&pexAddrsMessage{Addrs: addrs})
 	pexR.Receive(PexChannel, peer, msg)
 	assert.Equal(t, size, book.Size())
@@ -418,8 +416,8 @@ func TestPEXReactorDialPeer(t *testing.T) {
 	sw := createSwitchAndAddReactors(pexR)
 	sw.SetAddrBook(book)
 
-	peer := newMockPeer()
+	peer := mock.NewPeer(nil)
-	addr := peer.NodeInfo().NetAddress()
+	addr := peer.SocketAddr()
 
 	assert.Equal(t, 0, pexR.AttemptsToDial(addr))
 
@@ -444,44 +442,6 @@ func TestPEXReactorDialPeer(t *testing.T) {
 	}
 }
 
-type mockPeer struct {
-	*cmn.BaseService
-	pubKey               crypto.PubKey
-	addr                 *p2p.NetAddress
-	outbound, persistent bool
-}
-
-func newMockPeer() mockPeer {
-	_, netAddr := p2p.CreateRoutableAddr()
-	mp := mockPeer{
-		addr:   netAddr,
-		pubKey: ed25519.GenPrivKey().PubKey(),
-	}
-	mp.BaseService = cmn.NewBaseService(nil, "MockPeer", mp)
-	mp.Start()
-	return mp
-}
-
-func (mp mockPeer) FlushStop()         { mp.Stop() }
-func (mp mockPeer) ID() p2p.ID         { return mp.addr.ID }
-func (mp mockPeer) IsOutbound() bool   { return mp.outbound }
-func (mp mockPeer) IsPersistent() bool { return mp.persistent }
-func (mp mockPeer) NodeInfo() p2p.NodeInfo {
-	return p2p.DefaultNodeInfo{
-		ID_:        mp.addr.ID,
-		ListenAddr: mp.addr.DialString(),
-	}
-}
-func (mockPeer) RemoteIP() net.IP              { return net.ParseIP("127.0.0.1") }
-func (mockPeer) Status() conn.ConnectionStatus { return conn.ConnectionStatus{} }
-func (mockPeer) Send(byte, []byte) bool        { return false }
-func (mockPeer) TrySend(byte, []byte) bool     { return false }
-func (mockPeer) Set(string, interface{})       {}
-func (mockPeer) Get(string) interface{}        { return nil }
-func (mockPeer) OriginalAddr() *p2p.NetAddress { return nil }
-func (mockPeer) RemoteAddr() net.Addr          { return &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 8800} }
-func (mockPeer) CloseConn() error              { return nil }
-
 func assertPeersWithTimeout(
 	t *testing.T,
 	switches []*p2p.Switch,
@@ -590,7 +550,7 @@ func testCreateSeed(dir string, id int, knownAddrs, srcAddrs []*p2p.NetAddress)
 // Starting and stopping the peer is left to the caller
 func testCreatePeerWithSeed(dir string, id int, seed *p2p.Switch) *p2p.Switch {
 	conf := &PEXReactorConfig{
-		Seeds: []string{seed.NodeInfo().NetAddress().String()},
+		Seeds: []string{seed.NetAddress().String()},
 	}
 	return testCreatePeerWithConfig(dir, id, conf)
 }

p2p/switch.go

@@ -86,6 +86,12 @@ type Switch struct {
 	metrics *Metrics
 }
 
+// NetAddress returns the address the switch is listening on.
+func (sw *Switch) NetAddress() *NetAddress {
+	addr := sw.transport.NetAddress()
+	return &addr
+}
+
 // SwitchOption sets an optional parameter on the Switch.
 type SwitchOption func(*Switch)
 
@@ -284,13 +290,7 @@ func (sw *Switch) StopPeerForError(peer Peer, reason interface{}) {
 	sw.stopAndRemovePeer(peer, reason)
 
 	if peer.IsPersistent() {
-		addr := peer.OriginalAddr()
+		go sw.reconnectToPeer(peer.SocketAddr())
-		if addr == nil {
-			// FIXME: persistent peers can't be inbound right now.
-			// self-reported address for inbound persistent peers
-			addr = peer.NodeInfo().NetAddress()
-		}
-		go sw.reconnectToPeer(addr)
 	}
 }
 
@@ -378,7 +378,7 @@ func (sw *Switch) SetAddrBook(addrBook AddrBook) {
 // like contributed to consensus.
 func (sw *Switch) MarkPeerAsGood(peer Peer) {
 	if sw.addrBook != nil {
-		sw.addrBook.MarkGood(peer.NodeInfo().NetAddress())
+		sw.addrBook.MarkGood(peer.SocketAddr())
 	}
 }
 
@@ -395,7 +395,7 @@ func (sw *Switch) DialPeersAsync(addrBook AddrBook, peers []string, persistent b
 		sw.Logger.Error("Error in peer's address", "err", err)
 	}
 
-	ourAddr := sw.nodeInfo.NetAddress()
+	ourAddr := sw.NetAddress()
 
 	// TODO: this code feels like it's in the wrong place.
 	// The integration tests depend on the addrBook being saved
@@ -524,7 +524,7 @@ func (sw *Switch) acceptRoutine() {
 		if in >= sw.config.MaxNumInboundPeers {
 			sw.Logger.Info(
 				"Ignoring inbound connection: already have enough inbound peers",
-				"address", p.NodeInfo().NetAddress().String(),
+				"address", p.SocketAddr(),
 				"have", in,
 				"max", sw.config.MaxNumInboundPeers,
 			)
@@ -641,7 +641,7 @@ func (sw *Switch) addPeer(p Peer) error {
 		return err
 	}
 
-	p.SetLogger(sw.Logger.With("peer", p.NodeInfo().NetAddress()))
+	p.SetLogger(sw.Logger.With("peer", p.SocketAddr()))
 
 	// Handle the shut down case where the switch has stopped but we're
 	// concurrently trying to add a peer.

p2p/switch_test.go

@@ -160,10 +160,6 @@ func assertMsgReceivedWithTimeout(t *testing.T, msgBytes []byte, channel byte, r
 
 func TestSwitchFiltersOutItself(t *testing.T) {
 	s1 := MakeSwitch(cfg, 1, "127.0.0.1", "123.123.123", initSwitchFunc)
-	// addr := s1.NodeInfo().NetAddress()
-
-	// // add ourselves like we do in node.go#427
-	// s1.addrBook.AddOurAddress(addr)
 
 	// simulate s1 having a public IP by creating a remote peer with the same ID
 	rp := &remotePeer{PrivKey: s1.nodeKey.PrivKey, Config: cfg}
@@ -495,7 +491,7 @@ func TestSwitchAcceptRoutine(t *testing.T) {
 		rp := &remotePeer{PrivKey: ed25519.GenPrivKey(), Config: cfg}
 		remotePeers = append(remotePeers, rp)
 		rp.Start()
-		c, err := rp.Dial(sw.NodeInfo().NetAddress())
+		c, err := rp.Dial(sw.NetAddress())
 		require.NoError(t, err)
 		// spawn a reading routine to prevent connection from closing
 		go func(c net.Conn) {
@@ -514,7 +510,7 @@ func TestSwitchAcceptRoutine(t *testing.T) {
 	// 2. check we close new connections if we already have MaxNumInboundPeers peers
 	rp := &remotePeer{PrivKey: ed25519.GenPrivKey(), Config: cfg}
 	rp.Start()
-	conn, err := rp.Dial(sw.NodeInfo().NetAddress())
+	conn, err := rp.Dial(sw.NetAddress())
 	require.NoError(t, err)
 	// check conn is closed
 	one := make([]byte, 1)

p2p/test_util.go

@@ -36,6 +36,7 @@ func CreateRandomPeer(outbound bool) *peer {
 	p := &peer{
 		peerConn: peerConn{
 			outbound:   outbound,
+			socketAddr: netAddr,
 		},
 		nodeInfo: mockNodeInfo{netAddr},
 		mconn:    &conn.MConnection{},
@@ -174,10 +175,15 @@ func MakeSwitch(
 		PrivKey: ed25519.GenPrivKey(),
 	}
 	nodeInfo := testNodeInfo(nodeKey.ID(), fmt.Sprintf("node%d", i))
+	addr, err := NewNetAddressString(
+		IDAddressString(nodeKey.ID(), nodeInfo.(DefaultNodeInfo).ListenAddr),
+	)
+	if err != nil {
+		panic(err)
+	}
 
 	t := NewMultiplexTransport(nodeInfo, nodeKey, MConnConfig(cfg))
 
-	addr := nodeInfo.NetAddress()
 	if err := t.Listen(*addr); err != nil {
 		panic(err)
 	}
@@ -214,7 +220,7 @@ func testPeerConn(
 	cfg *config.P2PConfig,
 	outbound, persistent bool,
 	ourNodePrivKey crypto.PrivKey,
-	originalAddr *NetAddress,
+	socketAddr *NetAddress,
 ) (pc peerConn, err error) {
 	conn := rawConn
 
@@ -231,12 +237,7 @@ func testPeerConn(
 	}
 
 	// Only the information we already have
-	return peerConn{
+	return newPeerConn(outbound, persistent, conn, socketAddr), nil
-		outbound:     outbound,
-		persistent:   persistent,
-		conn:         conn,
-		originalAddr: originalAddr,
-	}, nil
 }
 
 //----------------------------------------------------------------

p2p/transport.go

@@ -24,6 +24,7 @@ type IPResolver interface {
 // accept is the container to carry the upgraded connection and NodeInfo from an
 // asynchronously running routine to the Accept method.
 type accept struct {
+	netAddr  *NetAddress
 	conn     net.Conn
 	nodeInfo NodeInfo
 	err      error
@@ -47,6 +48,9 @@ type peerConfig struct {
 // the transport. Each transport is also responsible to filter establishing
 // peers specific to its domain.
 type Transport interface {
+	// Listening address.
+	NetAddress() NetAddress
+
 	// Accept returns a newly connected Peer.
 	Accept(peerConfig) (Peer, error)
 
@@ -115,6 +119,7 @@ func MultiplexTransportResolver(resolver IPResolver) MultiplexTransportOption {
 // MultiplexTransport accepts and dials tcp connections and upgrades them to
 // multiplexed peers.
 type MultiplexTransport struct {
+	netAddr  NetAddress
 	listener net.Listener
 
 	acceptc chan accept
@@ -161,6 +166,11 @@ func NewMultiplexTransport(
 	}
 }
 
+// NetAddress implements Transport.
+func (mt *MultiplexTransport) NetAddress() NetAddress {
+	return mt.netAddr
+}
+
 // Accept implements Transport.
 func (mt *MultiplexTransport) Accept(cfg peerConfig) (Peer, error) {
 	select {
@@ -173,7 +183,7 @@ func (mt *MultiplexTransport) Accept(cfg peerConfig) (Peer, error) {
 
 		cfg.outbound = false
 
-		return mt.wrapPeer(a.conn, a.nodeInfo, cfg, nil), nil
+		return mt.wrapPeer(a.conn, a.nodeInfo, cfg, a.netAddr), nil
 	case <-mt.closec:
 		return nil, &ErrTransportClosed{}
 	}
@@ -224,6 +234,7 @@ func (mt *MultiplexTransport) Listen(addr NetAddress) error {
 		return err
 	}
 
+	mt.netAddr = addr
 	mt.listener = ln
 
 	go mt.acceptPeers()
@@ -258,15 +269,21 @@ func (mt *MultiplexTransport) acceptPeers() {
 			var (
 				nodeInfo   NodeInfo
 				secretConn *conn.SecretConnection
+				netAddr    *NetAddress
 			)
 
 			err := mt.filterConn(c)
 			if err == nil {
 				secretConn, nodeInfo, err = mt.upgrade(c, nil)
+				if err == nil {
+					addr := c.RemoteAddr()
+					id := PubKeyToID(secretConn.RemotePubKey())
+					netAddr = NewNetAddress(id, addr)
+				}
 			}
 
 			select {
-			case mt.acceptc <- accept{secretConn, nodeInfo, err}:
+			case mt.acceptc <- accept{netAddr, secretConn, nodeInfo, err}:
 				// Make the upgraded peer available.
 			case <-mt.closec:
 				// Give up if the transport was closed.
@@ -426,14 +443,14 @@ func (mt *MultiplexTransport) wrapPeer(
 	c net.Conn,
 	ni NodeInfo,
 	cfg peerConfig,
-	dialedAddr *NetAddress,
+	socketAddr *NetAddress,
 ) Peer {
 
 	peerConn := newPeerConn(
 		cfg.outbound,
 		cfg.persistent,
 		c,
-		dialedAddr,
+		socketAddr,
 	)
 
 	p := newPeer(

p2p/transport_test.go

@@ -8,6 +8,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/require"
+
 	"github.com/tendermint/tendermint/crypto/ed25519"
 	"github.com/tendermint/tendermint/p2p/conn"
 )
@@ -142,43 +144,23 @@ func TestTransportMultiplexConnFilterTimeout(t *testing.T) {
 
 func TestTransportMultiplexAcceptMultiple(t *testing.T) {
 	mt := testSetupMultiplexTransport(t)
+	id, addr := mt.nodeKey.ID(), mt.listener.Addr().String()
+	laddr, err := NewNetAddressStringWithOptionalID(IDAddressString(id, addr))
+	require.NoError(t, err)
 
 	var (
 		seed     = rand.New(rand.NewSource(time.Now().UnixNano()))
-		errc = make(chan error, seed.Intn(64)+64)
+		nDialers = seed.Intn(64) + 64
+		errc     = make(chan error, nDialers)
 	)
 
 	// Setup dialers.
-	for i := 0; i < cap(errc); i++ {
+	for i := 0; i < nDialers; i++ {
-		go func() {
+		go testDialer(*laddr, errc)
-			var (
-				pv     = ed25519.GenPrivKey()
-				dialer = newMultiplexTransport(
-					testNodeInfo(PubKeyToID(pv.PubKey()), defaultNodeName),
-					NodeKey{
-						PrivKey: pv,
-					},
-				)
-			)
-			addr, err := NewNetAddressStringWithOptionalID(IDAddressString(mt.nodeKey.ID(), mt.listener.Addr().String()))
-			if err != nil {
-				errc <- err
-				return
-			}
-
-			_, err = dialer.Dial(*addr, peerConfig{})
-			if err != nil {
-				errc <- err
-				return
-			}
-
-			// Signal that the connection was established.
-			errc <- nil
-		}()
 	}
 
 	// Catch connection errors.
-	for i := 0; i < cap(errc); i++ {
+	for i := 0; i < nDialers; i++ {
 		if err := <-errc; err != nil {
 			t.Fatal(err)
 		}
@@ -216,6 +198,27 @@ func TestTransportMultiplexAcceptMultiple(t *testing.T) {
 	}
 }
 
+func testDialer(dialAddr NetAddress, errc chan error) {
+	var (
+		pv     = ed25519.GenPrivKey()
+		dialer = newMultiplexTransport(
+			testNodeInfo(PubKeyToID(pv.PubKey()), defaultNodeName),
+			NodeKey{
+				PrivKey: pv,
+			},
+		)
+	)
+
+	_, err := dialer.Dial(dialAddr, peerConfig{})
+	if err != nil {
+		errc <- err
+		return
+	}
+
+	// Signal that the connection was established.
+	errc <- nil
+}
+
 func TestTransportMultiplexAcceptNonBlocking(t *testing.T) {
 	mt := testSetupMultiplexTransport(t)
 
@@ -591,6 +594,7 @@ func TestTransportHandshake(t *testing.T) {
 	}
 }
 
+// create listener
 func testSetupMultiplexTransport(t *testing.T) *MultiplexTransport {
 	var (
 		pv = ed25519.GenPrivKey()

rpc/core/consensus.go

@@ -215,7 +215,7 @@ func DumpConsensusState() (*ctypes.ResultDumpConsensusState, error) {
 		}
 		peerStates[i] = ctypes.PeerStateInfo{
 			// Peer basic info.
-			NodeAddress: peer.NodeInfo().NetAddress().String(),
+			NodeAddress: peer.SocketAddr().String(),
 			// Peer consensus state.
 			PeerState: peerStateJSON,
 		}