From 6168b404a773a9b18fa9c0940d1606c40378f26c Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Sat, 17 Nov 2018 03:16:49 -0500 Subject: [PATCH] p2p: NewMultiplexTransport takes an MConnConfig (#2869) * p2p: NewMultiplexTransport takes an MConnConfig * changelog * move test func to test file --- CHANGELOG_PENDING.md | 2 ++ node/node.go | 3 ++- p2p/peer.go | 4 ---- p2p/switch.go | 21 +++++++++++---------- p2p/test_util.go | 5 ++--- p2p/transport.go | 10 ++++------ p2p/transport_test.go | 29 +++++++++++++++++++++-------- 7 files changed, 42 insertions(+), 32 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index f49ddd725..cb3d76068 100644 --- a/CHANGELOG_PENDING.md +++ b/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 diff --git a/node/node.go b/node/node.go index f1da1df0e..bfd8d02e2 100644 --- a/node/node.go +++ b/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{} ) diff --git a/p2p/peer.go b/p2p/peer.go index 6417948d1..da301d497 100644 --- a/p2p/peer.go +++ b/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, } diff --git a/p2p/switch.go b/p2p/switch.go index b70900ea9..4996ebd91 100644 --- a/p2p/switch.go +++ b/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 { diff --git a/p2p/test_util.go b/p2p/test_util.go index d72c0c760..b8a34600c 100644 --- a/p2p/test_util.go +++ b/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, diff --git a/p2p/transport.go b/p2p/transport.go index 0b9b436f0..b16db54db 100644 --- a/p2p/transport.go +++ b/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 - p2pConfig config.P2PConfig + mConfig conn.MConnConfig } // 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, ) diff --git a/p2p/transport_test.go b/p2p/transport_test.go index 8a5c06bc3..182b28899 100644 --- a/p2p/transport_test.go +++ b/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", ),