From fdb479f1213f892fc52a17dde311d5b441a4cbf8 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 11 Mar 2021 18:24:13 -0800 Subject: [PATCH] peer: allow external testing of peer.Peer The previous use of allowSelfConns prevented this, as users aren't able to invoke peer.TstAllowSelfConns themselves due to being part of a test file, which aren't exported at the library level, leading to a "disconnecting peer connected to self" error upon establishing a mock connection between two peers. By including the option at the config level instead (false by default, prevents connections to self) we enable users of the peer library to properly test the behavior of the peer.Peer struct externally. --- peer/example_test.go | 2 ++ peer/export_test.go | 18 ------------------ peer/peer.go | 12 ++++++------ peer/peer_test.go | 12 +++++++----- 4 files changed, 15 insertions(+), 29 deletions(-) delete mode 100644 peer/export_test.go diff --git a/peer/example_test.go b/peer/example_test.go index cb67683b..d4662a2b 100644 --- a/peer/example_test.go +++ b/peer/example_test.go @@ -25,6 +25,7 @@ func mockRemotePeer() error { UserAgentVersion: "1.0.0", // User agent version to advertise. ChainParams: &chaincfg.SimNetParams, TrickleInterval: time.Second * 10, + AllowSelfConns: true, } // Accept connections on the simnet port. @@ -81,6 +82,7 @@ func Example_newOutboundPeer() { verack <- struct{}{} }, }, + AllowSelfConns: true, } p, err := peer.NewOutboundPeer(peerCfg, "127.0.0.1:18555") if err != nil { diff --git a/peer/export_test.go b/peer/export_test.go deleted file mode 100644 index 06ec78a1..00000000 --- a/peer/export_test.go +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright (c) 2015 The btcsuite developers -// Use of this source code is governed by an ISC -// license that can be found in the LICENSE file. - -/* -This test file is part of the peer package rather than than the peer_test -package so it can bridge access to the internals to properly test cases which -are either not possible or can't reliably be tested via the public interface. -The functions are only exported while the tests are being run. -*/ - -package peer - -// TstAllowSelfConns allows the test package to allow self connections by -// disabling the detection logic. -func TstAllowSelfConns() { - allowSelfConns = true -} diff --git a/peer/peer.go b/peer/peer.go index 5b083a74..92ac3d27 100644 --- a/peer/peer.go +++ b/peer/peer.go @@ -84,11 +84,6 @@ var ( // sentNonces houses the unique nonces that are generated when pushing // version messages that are used to detect self connections. sentNonces = lru.NewCache(50) - - // allowSelfConns is only used to allow the tests to bypass the self - // connection detecting and disconnect logic since they intentionally - // do so for testing purposes. - allowSelfConns bool ) // MessageListeners defines callback function pointers to invoke with message @@ -276,6 +271,11 @@ type Config struct { // TrickleInterval is the duration of the ticker which trickles down the // inventory to a peer. TrickleInterval time.Duration + + // AllowSelfConns is only used to allow the tests to bypass the self + // connection detecting and disconnect logic since they intentionally + // do so for testing purposes. + AllowSelfConns bool } // minUint32 is a helper function to return the minimum of two uint32s. @@ -1896,7 +1896,7 @@ func (p *Peer) readRemoteVersionMsg() error { } // Detect self connections. - if !allowSelfConns && sentNonces.Contains(msg.Nonce) { + if !p.cfg.AllowSelfConns && sentNonces.Contains(msg.Nonce) { return errors.New("disconnecting peer connected to self") } diff --git a/peer/peer_test.go b/peer/peer_test.go index 6cc3113d..dd7f36aa 100644 --- a/peer/peer_test.go +++ b/peer/peer_test.go @@ -238,6 +238,7 @@ func TestPeerConnection(t *testing.T) { ProtocolVersion: wire.RejectVersion, // Configure with older version Services: 0, TrickleInterval: time.Second * 10, + AllowSelfConns: true, } peer2Cfg := &peer.Config{ Listeners: peer1Cfg.Listeners, @@ -247,6 +248,7 @@ func TestPeerConnection(t *testing.T) { ChainParams: &chaincfg.MainNetParams, Services: wire.SFNodeNetwork | wire.SFNodeWitness, TrickleInterval: time.Second * 10, + AllowSelfConns: true, } wantStats1 := peerStats{ @@ -452,6 +454,7 @@ func TestPeerListeners(t *testing.T) { ChainParams: &chaincfg.MainNetParams, Services: wire.SFNodeBloom, TrickleInterval: time.Second * 10, + AllowSelfConns: true, } inConn, outConn := pipe( &conn{raddr: "10.0.0.1:8333"}, @@ -623,6 +626,7 @@ func TestOutboundPeer(t *testing.T) { ChainParams: &chaincfg.MainNetParams, Services: 0, TrickleInterval: time.Second * 10, + AllowSelfConns: true, } r, w := io.Pipe() @@ -764,6 +768,7 @@ func TestUnsupportedVersionPeer(t *testing.T) { ChainParams: &chaincfg.MainNetParams, Services: 0, TrickleInterval: time.Second * 10, + AllowSelfConns: true, } localNA := wire.NewNetAddressIPPort( @@ -874,6 +879,7 @@ func TestDuplicateVersionMsg(t *testing.T) { UserAgentVersion: "1.0", ChainParams: &chaincfg.MainNetParams, Services: 0, + AllowSelfConns: true, } inConn, outConn := pipe( &conn{laddr: "10.0.0.1:9108", raddr: "10.0.0.2:9108"}, @@ -935,6 +941,7 @@ func TestUpdateLastBlockHeight(t *testing.T) { UserAgentVersion: "1.0", ChainParams: &chaincfg.MainNetParams, Services: 0, + AllowSelfConns: true, } remotePeerCfg := peerCfg remotePeerCfg.NewestBlock = func() (*chainhash.Hash, int32, error) { @@ -982,8 +989,3 @@ func TestUpdateLastBlockHeight(t *testing.T) { remotePeerHeight+1) } } - -func init() { - // Allow self connection when running the tests. - peer.TstAllowSelfConns() -}