p2p: NewMultiplexTransport takes an MConnConfig (#2869)

* p2p: NewMultiplexTransport takes an MConnConfig

* changelog

* move test func to test file

CHANGELOG_PENDING.md

@@ -39,3 +39,5 @@ program](https://hackerone.com/tendermint).
 - [p2p] \#2857 "Send failed" is logged at debug level instead of error.
 
 ### BUG FIXES:
+
+- [p2p] \#2869 Set connection config properly instead of always using default

node/node.go

@@ -371,7 +371,8 @@ func NewNode(config *cfg.Config,
 
 	// Setup Transport.
 	var (
-		transport   = p2p.NewMultiplexTransport(nodeInfo, *nodeKey)
+		mConnConfig = p2p.MConnConfig(config.P2P)
+		transport   = p2p.NewMultiplexTransport(nodeInfo, *nodeKey, mConnConfig)
 		connFilters = []p2p.ConnFilterFunc{}
 		peerFilters = []p2p.PeerFilterFunc{}
 	)

p2p/peer.go

@@ -8,7 +8,6 @@ import (
 	cmn "github.com/tendermint/tendermint/libs/common"
 	"github.com/tendermint/tendermint/libs/log"
 
-	"github.com/tendermint/tendermint/config"
 	tmconn "github.com/tendermint/tendermint/p2p/conn"
 )
 
@@ -42,7 +41,6 @@ type Peer interface {
 type peerConn struct {
 	outbound   bool
 	persistent bool
-	config     *config.P2PConfig
 	conn       net.Conn // source connection
 
 	originalAddr *NetAddress // nil for inbound connections
@@ -53,7 +51,6 @@ type peerConn struct {
 
 func newPeerConn(
 	outbound, persistent bool,
-	config *config.P2PConfig,
 	conn net.Conn,
 	originalAddr *NetAddress,
 ) peerConn {
@@ -61,7 +58,6 @@ func newPeerConn(
 	return peerConn{
 		outbound:     outbound,
 		persistent:   persistent,
-		config:       config,
 		conn:         conn,
 		originalAddr: originalAddr,
 	}

p2p/switch.go

@@ -27,6 +27,17 @@ const (
 	reconnectBackOffBaseSeconds = 3
 )
 
+// MConnConfig returns an MConnConfig with fields updated
+// from the P2PConfig.
+func MConnConfig(cfg *config.P2PConfig) conn.MConnConfig {
+	mConfig := conn.DefaultMConnConfig()
+	mConfig.FlushThrottle = cfg.FlushThrottleTimeout
+	mConfig.SendRate = cfg.SendRate
+	mConfig.RecvRate = cfg.RecvRate
+	mConfig.MaxPacketMsgPayloadSize = cfg.MaxPacketMsgPayloadSize
+	return mConfig
+}
+
 //-----------------------------------------------------------------------------
 
 // An AddrBook represents an address book from the pex package, which is used
@@ -70,8 +81,6 @@ type Switch struct {
 	filterTimeout time.Duration
 	peerFilters   []PeerFilterFunc
 
-	mConfig conn.MConnConfig
-
 	rng *cmn.Rand // seed for randomizing dial times and orders
 
 	metrics *Metrics
@@ -102,14 +111,6 @@ func NewSwitch(
 	// Ensure we have a completely undeterministic PRNG.
 	sw.rng = cmn.NewRand()
 
-	mConfig := conn.DefaultMConnConfig()
-	mConfig.FlushThrottle = cfg.FlushThrottleTimeout
-	mConfig.SendRate = cfg.SendRate
-	mConfig.RecvRate = cfg.RecvRate
-	mConfig.MaxPacketMsgPayloadSize = cfg.MaxPacketMsgPayloadSize
-
-	sw.mConfig = mConfig
-
 	sw.BaseService = *cmn.NewBaseService(nil, "P2P Switch", sw)
 
 	for _, option := range options {

p2p/test_util.go

@@ -135,7 +135,7 @@ func (sw *Switch) addPeerWithConnection(conn net.Conn) error {
 
 	p := newPeer(
 		pc,
-		sw.mConfig,
+		MConnConfig(sw.config),
 		ni,
 		sw.reactorsByCh,
 		sw.chDescs,
@@ -175,7 +175,7 @@ func MakeSwitch(
 	}
 	nodeInfo := testNodeInfo(nodeKey.ID(), fmt.Sprintf("node%d", i))
 
-	t := NewMultiplexTransport(nodeInfo, nodeKey)
+	t := NewMultiplexTransport(nodeInfo, nodeKey, MConnConfig(cfg))
 
 	addr := nodeInfo.NetAddress()
 	if err := t.Listen(*addr); err != nil {
@@ -232,7 +232,6 @@ func testPeerConn(
 
 	// Only the information we already have
 	return peerConn{
-		config:       cfg,
 		outbound:     outbound,
 		persistent:   persistent,
 		conn:         conn,

p2p/transport.go

@@ -6,7 +6,6 @@ import (
 	"net"
 	"time"
 
-	"github.com/tendermint/tendermint/config"
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/p2p/conn"
 )
@@ -129,11 +128,10 @@ type MultiplexTransport struct {
 	nodeKey          NodeKey
 	resolver         IPResolver
 
-	// TODO(xla): Those configs are still needed as we parameterise peerConn and
+	// TODO(xla): This config is still needed as we parameterise peerConn and
 	// peer currently. All relevant configuration should be refactored into options
 	// with sane defaults.
-	mConfig   conn.MConnConfig
+	mConfig conn.MConnConfig
-	p2pConfig config.P2PConfig
 }
 
 // Test multiplexTransport for interface completeness.
@@ -144,6 +142,7 @@ var _ transportLifecycle = (*MultiplexTransport)(nil)
 func NewMultiplexTransport(
 	nodeInfo NodeInfo,
 	nodeKey NodeKey,
+	mConfig conn.MConnConfig,
 ) *MultiplexTransport {
 	return &MultiplexTransport{
 		acceptc:          make(chan accept),
@@ -151,7 +150,7 @@ func NewMultiplexTransport(
 		dialTimeout:      defaultDialTimeout,
 		filterTimeout:    defaultFilterTimeout,
 		handshakeTimeout: defaultHandshakeTimeout,
-		mConfig:          conn.DefaultMConnConfig(),
+		mConfig:          mConfig,
 		nodeInfo:         nodeInfo,
 		nodeKey:          nodeKey,
 		conns:            NewConnSet(),
@@ -405,7 +404,6 @@ func (mt *MultiplexTransport) wrapPeer(
 	peerConn := newPeerConn(
 		cfg.outbound,
 		cfg.persistent,
-		&mt.p2pConfig,
 		c,
 		dialedAddr,
 	)

p2p/transport_test.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/tendermint/tendermint/crypto/ed25519"
+	"github.com/tendermint/tendermint/p2p/conn"
 )
 
 var defaultNodeName = "host_peer"
@@ -17,8 +18,20 @@ func emptyNodeInfo() NodeInfo {
 	return DefaultNodeInfo{}
 }
 
+// newMultiplexTransport returns a tcp connected multiplexed peer
+// using the default MConnConfig. It's a convenience function used
+// for testing.
+func newMultiplexTransport(
+	nodeInfo NodeInfo,
+	nodeKey NodeKey,
+) *MultiplexTransport {
+	return NewMultiplexTransport(
+		nodeInfo, nodeKey, conn.DefaultMConnConfig(),
+	)
+}
+
 func TestTransportMultiplexConnFilter(t *testing.T) {
-	mt := NewMultiplexTransport(
+	mt := newMultiplexTransport(
 		emptyNodeInfo(),
 		NodeKey{
 			PrivKey: ed25519.GenPrivKey(),
@@ -75,7 +88,7 @@ func TestTransportMultiplexConnFilter(t *testing.T) {
 }
 
 func TestTransportMultiplexConnFilterTimeout(t *testing.T) {
-	mt := NewMultiplexTransport(
+	mt := newMultiplexTransport(
 		emptyNodeInfo(),
 		NodeKey{
 			PrivKey: ed25519.GenPrivKey(),
@@ -140,7 +153,7 @@ func TestTransportMultiplexAcceptMultiple(t *testing.T) {
 		go func() {
 			var (
 				pv     = ed25519.GenPrivKey()
-				dialer = NewMultiplexTransport(
+				dialer = newMultiplexTransport(
 					testNodeInfo(PubKeyToID(pv.PubKey()), defaultNodeName),
 					NodeKey{
 						PrivKey: pv,
@@ -261,7 +274,7 @@ func TestTransportMultiplexAcceptNonBlocking(t *testing.T) {
 		<-slowc
 
 		var (
-			dialer = NewMultiplexTransport(
+			dialer = newMultiplexTransport(
 				fastNodeInfo,
 				NodeKey{
 					PrivKey: fastNodePV,
@@ -307,7 +320,7 @@ func TestTransportMultiplexValidateNodeInfo(t *testing.T) {
 	go func() {
 		var (
 			pv     = ed25519.GenPrivKey()
-			dialer = NewMultiplexTransport(
+			dialer = newMultiplexTransport(
 				testNodeInfo(PubKeyToID(pv.PubKey()), ""), // Should not be empty
 				NodeKey{
 					PrivKey: pv,
@@ -350,7 +363,7 @@ func TestTransportMultiplexRejectMissmatchID(t *testing.T) {
 	errc := make(chan error)
 
 	go func() {
-		dialer := NewMultiplexTransport(
+		dialer := newMultiplexTransport(
 			testNodeInfo(
 				PubKeyToID(ed25519.GenPrivKey().PubKey()), "dialer",
 			),
@@ -396,7 +409,7 @@ func TestTransportMultiplexRejectIncompatible(t *testing.T) {
 	go func() {
 		var (
 			pv     = ed25519.GenPrivKey()
-			dialer = NewMultiplexTransport(
+			dialer = newMultiplexTransport(
 				testNodeInfoWithNetwork(PubKeyToID(pv.PubKey()), "dialer", "incompatible-network"),
 				NodeKey{
 					PrivKey: pv,
@@ -553,7 +566,7 @@ func TestTransportHandshake(t *testing.T) {
 func testSetupMultiplexTransport(t *testing.T) *MultiplexTransport {
 	var (
 		pv = ed25519.GenPrivKey()
-		mt = NewMultiplexTransport(
+		mt = newMultiplexTransport(
 			testNodeInfo(
 				PubKeyToID(pv.PubKey()), "transport",
 			),