From 82a43f7abd74e71463f8daac78641fb8e0524c25 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 16 Aug 2021 13:19:55 +0200 Subject: [PATCH] remove deprecated basichost.New constructor --- config/muxer_test.go | 7 +- p2p/discovery/mdns_test.go | 21 +++--- p2p/host/basic/basic_host.go | 55 +-------------- p2p/host/basic/basic_host_test.go | 82 +++++++++++++--------- p2p/protocol/ping/ping_test.go | 22 +++--- p2p/test/backpressure/backpressure_test.go | 10 +-- p2p/test/reconnects/reconnect_test.go | 31 ++++---- 7 files changed, 98 insertions(+), 130 deletions(-) diff --git a/config/muxer_test.go b/config/muxer_test.go index 0a86092053..5a86dd5b4f 100644 --- a/config/muxer_test.go +++ b/config/muxer_test.go @@ -9,7 +9,7 @@ import ( swarmt "github.com/libp2p/go-libp2p-swarm/testing" bhost "github.com/libp2p/go-libp2p/p2p/host/basic" - mux "github.com/libp2p/go-libp2p-core/mux" + "github.com/libp2p/go-libp2p-core/mux" yamux "github.com/libp2p/go-libp2p-yamux" ) @@ -60,7 +60,10 @@ func TestMuxerBadTypes(t *testing.T) { func TestCatchDuplicateTransportsMuxer(t *testing.T) { ctx := context.Background() - h := bhost.New(swarmt.GenSwarm(t, ctx)) + h, err := bhost.NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + if err != nil { + t.Fatal(err) + } yamuxMuxer, err := MuxerConstructor(yamux.DefaultTransport) if err != nil { t.Fatal(err) diff --git a/p2p/discovery/mdns_test.go b/p2p/discovery/mdns_test.go index 8f1e5bec91..8307eb0db4 100644 --- a/p2p/discovery/mdns_test.go +++ b/p2p/discovery/mdns_test.go @@ -5,9 +5,10 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + "github.com/libp2p/go-libp2p-core/host" "github.com/libp2p/go-libp2p-core/peer" - swarmt "github.com/libp2p/go-libp2p-swarm/testing" bhost "github.com/libp2p/go-libp2p/p2p/host/basic" ) @@ -27,19 +28,16 @@ func TestMdnsDiscovery(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - a := bhost.New(swarmt.GenSwarm(t, ctx)) - b := bhost.New(swarmt.GenSwarm(t, ctx)) + a, err := bhost.NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) + b, err := bhost.NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) sa, err := NewMdnsService(ctx, a, time.Second, "someTag") - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) sb, err := NewMdnsService(ctx, b, time.Second, "someTag") - if err != nil { - t.Fatal(err) - } - + require.NoError(t, err) _ = sb n := &DiscoveryNotifee{a} @@ -48,8 +46,7 @@ func TestMdnsDiscovery(t *testing.T) { time.Sleep(time.Second * 2) - err = a.Connect(ctx, peer.AddrInfo{ID: b.ID()}) - if err != nil { + if err := a.Connect(ctx, peer.AddrInfo{ID: b.ID()}); err != nil { t.Fatal(err) } } diff --git a/p2p/host/basic/basic_host.go b/p2p/host/basic/basic_host.go index 77c1ab1687..580ef28999 100644 --- a/p2p/host/basic/basic_host.go +++ b/p2p/host/basic/basic_host.go @@ -56,20 +56,6 @@ var ( // addresses returned by Addrs. type AddrsFactory func([]ma.Multiaddr) []ma.Multiaddr -// Option is a type used to pass in options to the host. -// -// Deprecated in favor of HostOpts and NewHost. -type Option int - -// NATPortMap makes the host attempt to open port-mapping in NAT devices -// for all its listeners. Pass in this option in the constructor to -// asynchronously a) find a gateway, b) open port mappings, c) republish -// port mappings periodically. The NATed addresses are included in the -// Host's Addrs() list. -// -// This option is deprecated in favor of HostOpts and NewHost. -const NATPortMap Option = iota - // BasicHost is the basic implementation of the host.Host interface. This // particular host implementation: // * uses a protocol muxer to mux per-protocol streams @@ -155,6 +141,9 @@ type HostOpts struct { // NewHost constructs a new *BasicHost and activates it by attaching its stream and connection handlers to the given inet.Network. func NewHost(ctx context.Context, n network.Network, opts *HostOpts) (*BasicHost, error) { hostCtx, cancel := context.WithCancel(ctx) + if opts == nil { + opts = &HostOpts{} + } h := &BasicHost{ network: n, @@ -329,44 +318,6 @@ func (h *BasicHost) updateLocalIpAddr() { } } -// New constructs and sets up a new *BasicHost with given Network and options. -// The following options can be passed: -// * NATPortMap -// * AddrsFactory -// * connmgr.ConnManager -// * madns.Resolver -// -// This function is deprecated in favor of NewHost and HostOpts. -func New(net network.Network, opts ...interface{}) *BasicHost { - hostopts := &HostOpts{} - - for _, o := range opts { - switch o := o.(type) { - case Option: - switch o { - case NATPortMap: - hostopts.NATManager = NewNATManager - } - case AddrsFactory: - hostopts.AddrsFactory = o - case connmgr.ConnManager: - hostopts.ConnManager = o - case *madns.Resolver: - hostopts.MultiaddrResolver = o - } - } - - h, err := NewHost(context.Background(), net, hostopts) - if err != nil { - // this cannot happen with legacy options - // plus we want to keep the (deprecated) legacy interface unchanged - panic(err) - } - h.Start() - - return h -} - // Start starts background tasks in the host func (h *BasicHost) Start() { h.refCount.Add(1) diff --git a/p2p/host/basic/basic_host_test.go b/p2p/host/basic/basic_host_test.go index be52a26b19..5cb21954c8 100644 --- a/p2p/host/basic/basic_host_test.go +++ b/p2p/host/basic/basic_host_test.go @@ -11,6 +11,8 @@ import ( "testing" "time" + "github.com/libp2p/go-eventbus" + autonat "github.com/libp2p/go-libp2p-autonat" "github.com/libp2p/go-libp2p-core/event" "github.com/libp2p/go-libp2p-core/helpers" "github.com/libp2p/go-libp2p-core/host" @@ -20,30 +22,31 @@ import ( "github.com/libp2p/go-libp2p-core/protocol" "github.com/libp2p/go-libp2p-core/record" "github.com/libp2p/go-libp2p-core/test" - - "github.com/libp2p/go-eventbus" - autonat "github.com/libp2p/go-libp2p-autonat" swarmt "github.com/libp2p/go-libp2p-swarm/testing" "github.com/libp2p/go-libp2p/p2p/protocol/identify" ma "github.com/multiformats/go-multiaddr" madns "github.com/multiformats/go-multiaddr-dns" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestHostDoubleClose(t *testing.T) { ctx := context.Background() - h1 := New(swarmt.GenSwarm(t, ctx)) + h1, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) h1.Close() h1.Close() } func TestHostSimple(t *testing.T) { ctx := context.Background() - h1 := New(swarmt.GenSwarm(t, ctx)) - h2 := New(swarmt.GenSwarm(t, ctx)) + h1, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) defer h1.Close() + h2, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) defer h2.Close() h2pi := h2.Peerstore().PeerInfo(h2.ID()) @@ -90,26 +93,27 @@ func TestHostSimple(t *testing.T) { func TestMultipleClose(t *testing.T) { ctx := context.Background() - h := New(swarmt.GenSwarm(t, ctx)) + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) require.NoError(t, h.Close()) require.NoError(t, h.Close()) require.NoError(t, h.Close()) - } func TestSignedPeerRecordWithNoListenAddrs(t *testing.T) { ctx := context.Background() - h := New(swarmt.GenSwarm(t, ctx, swarmt.OptDialOnly)) + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx, swarmt.OptDialOnly), nil) + require.NoError(t, err) + h.Start() if len(h.Addrs()) != 0 { t.Errorf("expected no listen addrs, got %d", len(h.Addrs())) } // now add a listen addr - err := h.Network().Listen(ma.StringCast("/ip4/0.0.0.0/tcp/0")) - if err != nil { + if err := h.Network().Listen(ma.StringCast("/ip4/0.0.0.0/tcp/0")); err != nil { t.Fatal(err) } if len(h.Addrs()) < 1 { @@ -124,15 +128,15 @@ func TestSignedPeerRecordWithNoListenAddrs(t *testing.T) { if !ok { t.Fatalf("peerstore doesn't support certified addrs") } - rec := cab.GetPeerRecord(h.ID()) - if rec == nil { + if cab.GetPeerRecord(h.ID()) == nil { t.Fatalf("no signed peer record in peerstore for new host %s", h.ID()) } } func TestProtocolHandlerEvents(t *testing.T) { ctx := context.Background() - h := New(swarmt.GenSwarm(t, ctx)) + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) defer h.Close() sub, err := h.EventBus().Subscribe(&event.EvtLocalProtocolsUpdated{}, eventbus.BufSize(16)) @@ -193,7 +197,8 @@ func TestHostAddrsFactory(t *testing.T) { } ctx := context.Background() - h := New(swarmt.GenSwarm(t, ctx), AddrsFactory(addrsFactory)) + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), &HostOpts{AddrsFactory: addrsFactory}) + require.NoError(t, err) defer h.Close() addrs := h.Addrs() @@ -219,7 +224,9 @@ func TestLocalIPChangesWhenListenAddrChanges(t *testing.T) { ctx := context.Background() // no listen addrs - h := New(swarmt.GenSwarm(t, ctx, swarmt.OptDialOnly)) + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx, swarmt.OptDialOnly), nil) + require.NoError(t, err) + h.Start() defer h.Close() h.addrMu.Lock() @@ -227,7 +234,7 @@ func TestLocalIPChangesWhenListenAddrChanges(t *testing.T) { h.allInterfaceAddrs = nil h.addrMu.Unlock() - // change listen addrs and veify local IP addr is not nil again + // change listen addrs and verify local IP addr is not nil again require.NoError(t, h.Network().Listen(ma.StringCast("/ip4/0.0.0.0/tcp/0"))) h.SignalAddressChange() time.Sleep(1 * time.Second) @@ -242,7 +249,8 @@ func TestAllAddrs(t *testing.T) { ctx := context.Background() // no listen addrs - h := New(swarmt.GenSwarm(t, ctx, swarmt.OptDialOnly)) + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx, swarmt.OptDialOnly), nil) + require.NoError(t, err) defer h.Close() require.Nil(t, h.AllAddrs()) @@ -269,8 +277,10 @@ func TestAllAddrs(t *testing.T) { func getHostPair(ctx context.Context, t *testing.T) (host.Host, host.Host) { t.Helper() - h1 := New(swarmt.GenSwarm(t, ctx)) - h2 := New(swarmt.GenSwarm(t, ctx)) + h1, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) + h2, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) h2pi := h2.Peerstore().PeerInfo(h2.ID()) if err := h1.Connect(ctx, h2pi); err != nil { @@ -392,8 +402,10 @@ func TestHostProtoPreknowledge(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - h1 := New(swarmt.GenSwarm(t, ctx)) - h2 := New(swarmt.GenSwarm(t, ctx)) + h1, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) + h2, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) conn := make(chan protocol.ID) handler := func(s network.Stream) { @@ -601,7 +613,8 @@ func TestAddrResolution(t *testing.T) { t.Fatal(err) } - h := New(swarmt.GenSwarm(t, ctx), resolver) + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), &HostOpts{MultiaddrResolver: resolver}) + require.NoError(t, err) defer h.Close() pi, err := peer.AddrInfoFromP2pAddr(p2paddr1) @@ -658,7 +671,8 @@ func TestAddrResolutionRecursive(t *testing.T) { t.Fatal(err) } - h := New(swarmt.GenSwarm(t, ctx), resolver) + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), &HostOpts{MultiaddrResolver: resolver}) + require.NoError(t, err) defer h.Close() pi1, err := peer.AddrInfoFromP2pAddr(p2paddr1) @@ -692,10 +706,12 @@ func TestAddrChangeImmediatelyIfAddressNonEmpty(t *testing.T) { taddrs := []ma.Multiaddr{ma.StringCast("/ip4/1.2.3.4/tcp/1234")} starting := make(chan struct{}) - h := New(swarmt.GenSwarm(t, ctx), AddrsFactory(func(addrs []ma.Multiaddr) []ma.Multiaddr { + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), &HostOpts{AddrsFactory: func(addrs []ma.Multiaddr) []ma.Multiaddr { <-starting return taddrs - })) + }}) + require.NoError(t, err) + h.Start() defer h.Close() sub, err := h.EventBus().Subscribe(&event.EvtLocalAddressesUpdated{}) @@ -731,7 +747,9 @@ func TestAddrChangeImmediatelyIfAddressNonEmpty(t *testing.T) { func TestStatefulAddrEvents(t *testing.T) { ctx := context.Background() - h := New(swarmt.GenSwarm(t, ctx)) + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) + h.Start() defer h.Close() sub, err := h.EventBus().Subscribe(&event.EvtLocalAddressesUpdated{}, eventbus.BufSize(10)) @@ -743,7 +761,6 @@ func TestStatefulAddrEvents(t *testing.T) { select { case v := <-sub.Out(): assert.NotNil(t, v) - case <-time.After(time.Second * 5): t.Error("timed out waiting for event") } @@ -799,13 +816,13 @@ func TestHostAddrChangeDetection(t *testing.T) { } ctx := context.Background() - h := New(swarmt.GenSwarm(t, ctx), AddrsFactory(addrsFactory)) + h, err := NewHost(ctx, swarmt.GenSwarm(t, ctx), &HostOpts{AddrsFactory: addrsFactory}) + require.NoError(t, err) + h.Start() defer h.Close() sub, err := h.EventBus().Subscribe(&event.EvtLocalAddressesUpdated{}, eventbus.BufSize(10)) - if err != nil { - t.Error(err) - } + require.NoError(t, err) defer sub.Close() // wait for the host background thread to start @@ -883,6 +900,7 @@ func TestNegotiationCancel(t *testing.T) { } func waitForAddrChangeEvent(ctx context.Context, sub event.Subscription, t *testing.T) event.EvtLocalAddressesUpdated { + t.Helper() for { select { case evt, more := <-sub.Out(): diff --git a/p2p/protocol/ping/ping_test.go b/p2p/protocol/ping/ping_test.go index 942fe13252..dff2f5eff4 100644 --- a/p2p/protocol/ping/ping_test.go +++ b/p2p/protocol/ping/ping_test.go @@ -5,27 +5,27 @@ import ( "testing" "time" - "github.com/libp2p/go-libp2p-core/peer" + "github.com/stretchr/testify/require" + "github.com/libp2p/go-libp2p-core/peer" swarmt "github.com/libp2p/go-libp2p-swarm/testing" bhost "github.com/libp2p/go-libp2p/p2p/host/basic" - ping "github.com/libp2p/go-libp2p/p2p/protocol/ping" + "github.com/libp2p/go-libp2p/p2p/protocol/ping" ) func TestPing(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - h1 := bhost.New(swarmt.GenSwarm(t, ctx)) - h2 := bhost.New(swarmt.GenSwarm(t, ctx)) + h1, err := bhost.NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) + h2, err := bhost.NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) - err := h1.Connect(ctx, peer.AddrInfo{ + err = h1.Connect(ctx, peer.AddrInfo{ ID: h2.ID(), Addrs: h2.Addrs(), }) - - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) ps1 := ping.NewPingService(h1) ps2 := ping.NewPingService(h2) @@ -42,9 +42,7 @@ func testPing(t *testing.T, ps *ping.PingService, p peer.ID) { for i := 0; i < 5; i++ { select { case res := <-ts: - if res.Error != nil { - t.Fatal(res.Error) - } + require.NoError(t, res.Error) t.Log("ping took: ", res.RTT) case <-time.After(time.Second * 4): t.Fatal("failed to receive ping") diff --git a/p2p/test/backpressure/backpressure_test.go b/p2p/test/backpressure/backpressure_test.go index 218f3ff15c..d3e8e69622 100644 --- a/p2p/test/backpressure/backpressure_test.go +++ b/p2p/test/backpressure/backpressure_test.go @@ -6,13 +6,13 @@ import ( "testing" "time" - bhost "github.com/libp2p/go-libp2p/p2p/host/basic" "github.com/stretchr/testify/require" logging "github.com/ipfs/go-log/v2" "github.com/libp2p/go-libp2p-core/network" - protocol "github.com/libp2p/go-libp2p-core/protocol" + "github.com/libp2p/go-libp2p-core/protocol" swarmt "github.com/libp2p/go-libp2p-swarm/testing" + bhost "github.com/libp2p/go-libp2p/p2p/host/basic" ) var log = logging.Logger("backpressure") @@ -23,8 +23,10 @@ func TestStBackpressureStreamWrite(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - h1 := bhost.New(swarmt.GenSwarm(t, ctx)) - h2 := bhost.New(swarmt.GenSwarm(t, ctx)) + h1, err := bhost.NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) + h2, err := bhost.NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) // setup sender handler on 1 h1.SetStreamHandler(protocol.TestingID, func(s network.Stream) { diff --git a/p2p/test/reconnects/reconnect_test.go b/p2p/test/reconnects/reconnect_test.go index 9ec16ba43d..ecc989b665 100644 --- a/p2p/test/reconnects/reconnect_test.go +++ b/p2p/test/reconnects/reconnect_test.go @@ -8,14 +8,15 @@ import ( "testing" "time" - bhost "github.com/libp2p/go-libp2p/p2p/host/basic" + "github.com/stretchr/testify/require" u "github.com/ipfs/go-ipfs-util" logging "github.com/ipfs/go-log/v2" "github.com/libp2p/go-libp2p-core/host" "github.com/libp2p/go-libp2p-core/network" - protocol "github.com/libp2p/go-libp2p-core/protocol" + "github.com/libp2p/go-libp2p-core/protocol" swarmt "github.com/libp2p/go-libp2p-swarm/testing" + bhost "github.com/libp2p/go-libp2p/p2p/host/basic" ) var log = logging.Logger("reconnect") @@ -102,8 +103,10 @@ func newSender() (chan sendChans, func(s network.Stream)) { // TestReconnect tests whether hosts are able to disconnect and reconnect. func TestReconnect2(t *testing.T) { ctx := context.Background() - h1 := bhost.New(swarmt.GenSwarm(t, ctx)) - h2 := bhost.New(swarmt.GenSwarm(t, ctx)) + h1, err := bhost.NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) + h2, err := bhost.NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) hosts := []host.Host{h1, h2} h1.SetStreamHandler(protocol.TestingID, EchoStreamHandler) @@ -121,19 +124,15 @@ func TestReconnect2(t *testing.T) { // TestReconnect tests whether hosts are able to disconnect and reconnect. func TestReconnect5(t *testing.T) { + const num = 5 ctx := context.Background() - h1 := bhost.New(swarmt.GenSwarm(t, ctx)) - h2 := bhost.New(swarmt.GenSwarm(t, ctx)) - h3 := bhost.New(swarmt.GenSwarm(t, ctx)) - h4 := bhost.New(swarmt.GenSwarm(t, ctx)) - h5 := bhost.New(swarmt.GenSwarm(t, ctx)) - hosts := []host.Host{h1, h2, h3, h4, h5} - - h1.SetStreamHandler(protocol.TestingID, EchoStreamHandler) - h2.SetStreamHandler(protocol.TestingID, EchoStreamHandler) - h3.SetStreamHandler(protocol.TestingID, EchoStreamHandler) - h4.SetStreamHandler(protocol.TestingID, EchoStreamHandler) - h5.SetStreamHandler(protocol.TestingID, EchoStreamHandler) + hosts := make([]host.Host, 0, num) + for i := 0; i < num; i++ { + h, err := bhost.NewHost(ctx, swarmt.GenSwarm(t, ctx), nil) + require.NoError(t, err) + h.SetStreamHandler(protocol.TestingID, EchoStreamHandler) + hosts = append(hosts, h) + } rounds := 4 if testing.Short() {