From c60d47882b8130808051240f39518f8360070c3a Mon Sep 17 00:00:00 2001 From: Shane Howley Date: Wed, 14 Feb 2018 11:41:13 +0000 Subject: [PATCH 1/2] swarm/network: Fix for local node address not being set correctly This was causing nodes to share bad addresses during peer discovery. The Update(OverlayAddr) interface method returns an updated copy rather than mutating the existing object. --- swarm/network/protocol.go | 4 ++-- swarm/swarm.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/swarm/network/protocol.go b/swarm/network/protocol.go index 0cf682fb98..7a838b8d9f 100644 --- a/swarm/network/protocol.go +++ b/swarm/network/protocol.go @@ -124,10 +124,10 @@ func NewBzz(config *BzzConfig, kad Overlay, store StateStore) *Bzz { // UpdateLocalAddr updates underlayaddress of the running node func (b *Bzz) UpdateLocalAddr(byteaddr []byte) *BzzAddr { - b.localAddr.Update(&BzzAddr{ + b.localAddr = b.localAddr.Update(&BzzAddr{ UAddr: byteaddr, OAddr: b.localAddr.OAddr, - }) + }).(*BzzAddr) return b.localAddr } diff --git a/swarm/swarm.go b/swarm/swarm.go index 0ec85adadd..b28b5aaafa 100644 --- a/swarm/swarm.go +++ b/swarm/swarm.go @@ -209,7 +209,7 @@ func (self *Swarm) Start(srv *p2p.Server) error { // update uaddr to correct enode newaddr := self.bzz.UpdateLocalAddr([]byte(srv.Self().String())) - log.Warn("Updated bzz local addr", "oaddr", fmt.Sprintf("%x", newaddr.OAddr), "uaddr", fmt.Sprintf("%x", newaddr.UAddr)) + log.Warn("Updated bzz local addr", "oaddr", fmt.Sprintf("%x", newaddr.OAddr), "uaddr", fmt.Sprintf("%s", newaddr.UAddr)) // set chequebook if self.config.SwapEnabled { From dac45876c8f44c79d8f4ae91de1a66f1fc7385b3 Mon Sep 17 00:00:00 2001 From: Shane Howley Date: Tue, 20 Feb 2018 10:52:59 +0000 Subject: [PATCH 2/2] swarm/network: Send hive depth notification to new peer when added Fixed tests not failing on errors --- swarm/network/discovery_test.go | 34 +++++----- swarm/network/hive.go | 12 +++- swarm/network/protocol_test.go | 110 +++++++++++++++++++------------- 3 files changed, 91 insertions(+), 65 deletions(-) diff --git a/swarm/network/discovery_test.go b/swarm/network/discovery_test.go index 695f9fbcb4..4cdb341ce6 100644 --- a/swarm/network/discovery_test.go +++ b/swarm/network/discovery_test.go @@ -17,10 +17,8 @@ package network import ( - "fmt" "testing" - "github.com/ethereum/go-ethereum/log" p2ptest "github.com/ethereum/go-ethereum/p2p/testing" ) @@ -30,28 +28,30 @@ import ( * */ func TestDiscovery(t *testing.T) { - addr := RandomAddr() - to := NewKademlia(addr.OAddr, NewKadParams()) + params := NewHiveParams() + s, pp := newHiveTester(t, params) - run := func(p *BzzPeer) error { - dp := newDiscovery(p, to) - to.On(p) - defer to.Off(p) - log.Trace(fmt.Sprintf("kademlia on %v", p)) - return p.Run(dp.HandleMsg) - } + id := s.IDs[0] + raddr := NewAddrFromNodeID(id) + pp.Register([]OverlayAddr{OverlayAddr(raddr)}) - s := newBzzBaseTester(t, 1, addr, DiscoverySpec, run) - defer s.Stop() + // start the hive and wait for the connection + pp.Start(s.Server) + defer pp.Stop() - s.TestExchanges(p2ptest.Exchange{ - Label: "outgoing SubPeersMsg", + // send subPeersMsg to the peer + err := s.TestExchanges(p2ptest.Exchange{ + Label: "outgoing subPeersMsg", Expects: []p2ptest.Expect{ { - Code: 3, + Code: 1, Msg: &subPeersMsg{Depth: 0}, - Peer: s.ProtocolTester.IDs[0], + Peer: id, }, }, }) + + if err != nil { + t.Fatal(err) + } } diff --git a/swarm/network/hive.go b/swarm/network/hive.go index a309d93983..05004d1b8c 100644 --- a/swarm/network/hive.go +++ b/swarm/network/hive.go @@ -162,9 +162,15 @@ func (h *Hive) connect() { func (h *Hive) Run(p *BzzPeer) error { dp := newDiscovery(p, h) depth, changed := h.On(dp) - // if we want discovery, advertise changed depth of depth - if h.Discovery && changed { - NotifyDepth(depth, h) + // if we want discovery, advertise change of depth + if h.Discovery { + if changed { + // if depth changed, send to all peers + NotifyDepth(depth, h) + } else { + // otherwise just send depth to new peer + dp.NotifyDepth(depth) + } } NotifyPeer(p.Off(), h) defer h.Off(dp) diff --git a/swarm/network/protocol_test.go b/swarm/network/protocol_test.go index e90aa07318..19bc9bea7c 100644 --- a/swarm/network/protocol_test.go +++ b/swarm/network/protocol_test.go @@ -103,7 +103,7 @@ func newBzzBaseTester(t *testing.T, n int, addr *BzzAddr, spec *protocols.Spec, return run(p) } - protocall := func(p *p2p.Peer, rw p2p.MsgReadWriter) error { + protocol := func(p *p2p.Peer, rw p2p.MsgReadWriter) error { return srv(&BzzPeer{ Peer: protocols.NewPeer(p, rw, spec), localAddr: addr, @@ -111,7 +111,7 @@ func newBzzBaseTester(t *testing.T, n int, addr *BzzAddr, spec *protocols.Spec, }) } - s := p2ptest.NewProtocolTester(t, NewNodeIDFromAddr(addr), n, protocall) + s := p2ptest.NewProtocolTester(t, NewNodeIDFromAddr(addr), n, protocol) for _, id := range s.IDs { cs[id.String()] = make(chan bool) @@ -130,21 +130,25 @@ type bzzTester struct { cs map[string]chan bool } -func newBzzTester(t *testing.T, n int, addr *BzzAddr, pp *p2ptest.TestPeerPool, spec *protocols.Spec, services func(Peer) error) *bzzTester { - - extraservices := func(p *BzzPeer) error { - pp.Add(p) - defer pp.Remove(p) - if services == nil { - return nil - } - return services(p) +func newBzzHandshakeTester(t *testing.T, n int, addr *BzzAddr) *bzzTester { + config := &BzzConfig{ + OverlayAddr: addr.Over(), + UnderlayAddr: addr.Under(), + HiveParams: NewHiveParams(), + } + kad := NewKademlia(addr.OAddr, NewKadParams()) + bzz := NewBzz(config, kad, nil) + + s := p2ptest.NewProtocolTester(t, NewNodeIDFromAddr(addr), 1, bzz.runBzz) + + return &bzzTester{ + addr: addr, + ProtocolTester: s, } - return newBzzBaseTester(t, n, addr, spec, extraservices) } // should test handshakes in one exchange? parallelisation -func (s *bzzTester) testHandshake(lhs, rhs *HandshakeMsg, disconnects ...*p2ptest.Disconnect) { +func (s *bzzTester) testHandshake(lhs, rhs *HandshakeMsg, disconnects ...*p2ptest.Disconnect) error { var peers []discover.NodeID id := NewNodeIDFromAddr(rhs.Addr) if len(disconnects) > 0 { @@ -155,66 +159,82 @@ func (s *bzzTester) testHandshake(lhs, rhs *HandshakeMsg, disconnects ...*p2ptes peers = []discover.NodeID{id} } - s.TestExchanges(HandshakeMsgExchange(lhs, rhs, id)...) - s.TestDisconnected(disconnects...) -} - -func (s *bzzTester) runHandshakes(ids ...discover.NodeID) { - if len(ids) == 0 { - ids = s.IDs - } - for _, id := range ids { - s.testHandshake(correctBzzHandshake(s.addr), correctBzzHandshake(NewAddrFromNodeID(id))) - <-s.cs[id.String()] + if err := s.TestExchanges(HandshakeMsgExchange(lhs, rhs, id)...); err != nil { + return err } + if len(disconnects) > 0 { + return s.TestDisconnected(disconnects...) + } + + // If we don't expect disconnect, ensure peers remain connected + err := s.TestDisconnected(&p2ptest.Disconnect{ + Peer: s.IDs[0], + Error: nil, + }) + + if err == nil { + return fmt.Errorf("Unexpected peer disconnect") + } + + if err.Error() != "timed out waiting for peers to disconnect" { + return err + } + + return nil } func correctBzzHandshake(addr *BzzAddr) *HandshakeMsg { return &HandshakeMsg{ - Version: 0, + Version: 1, NetworkID: 322, Addr: addr, } } func TestBzzHandshakeNetworkIDMismatch(t *testing.T) { - pp := p2ptest.NewTestPeerPool() addr := RandomAddr() - s := newBzzTester(t, 1, addr, pp, nil, nil) - defer s.Stop() - + s := newBzzHandshakeTester(t, 1, addr) id := s.IDs[0] - s.testHandshake( + + err := s.testHandshake( correctBzzHandshake(addr), - &HandshakeMsg{Version: 0, NetworkID: 321, Addr: NewAddrFromNodeID(id)}, - &p2ptest.Disconnect{Peer: id, Error: fmt.Errorf("network id mismatch 321 (!= 322)")}, + &HandshakeMsg{Version: 1, NetworkID: 321, Addr: NewAddrFromNodeID(id)}, + &p2ptest.Disconnect{Peer: id, Error: fmt.Errorf("Handshake error: Message handler error: (msg code 0): network id mismatch 321 (!= 322)")}, ) + + if err != nil { + t.Fatal(err) + } } func TestBzzHandshakeVersionMismatch(t *testing.T) { - pp := p2ptest.NewTestPeerPool() addr := RandomAddr() - s := newBzzTester(t, 1, addr, pp, nil, nil) - defer s.Stop() - + s := newBzzHandshakeTester(t, 1, addr) id := s.IDs[0] - s.testHandshake( + + err := s.testHandshake( correctBzzHandshake(addr), - &HandshakeMsg{Version: 1, NetworkID: 322, Addr: NewAddrFromNodeID(id)}, - &p2ptest.Disconnect{Peer: id, Error: fmt.Errorf("version mismatch 1 (!= 0)")}, + &HandshakeMsg{Version: 0, NetworkID: 322, Addr: NewAddrFromNodeID(id)}, + &p2ptest.Disconnect{Peer: id, Error: fmt.Errorf("Handshake error: Message handler error: (msg code 0): version mismatch 0 (!= 1)")}, ) + + if err != nil { + t.Fatal(err) + } } func TestBzzHandshakeSuccess(t *testing.T) { - pp := p2ptest.NewTestPeerPool() addr := RandomAddr() - s := newBzzTester(t, 1, addr, pp, nil, nil) - defer s.Stop() - + s := newBzzHandshakeTester(t, 1, addr) id := s.IDs[0] - s.testHandshake( + + err := s.testHandshake( correctBzzHandshake(addr), - &HandshakeMsg{Version: 0, NetworkID: 322, Addr: NewAddrFromNodeID(id)}, + &HandshakeMsg{Version: 1, NetworkID: 322, Addr: NewAddrFromNodeID(id)}, ) + + if err != nil { + t.Fatal(err) + } }