From 8d9bc83214bb73a33b50b18d82ae3c6ebcce387a Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Mon, 9 Sep 2019 17:54:38 +0300 Subject: [PATCH] util: drop Endpoint structure, fix #321 I think it's useless, buggy and hides parsing errors for no good reason. --- pkg/network/helper_test.go | 10 +++--- pkg/network/payload/address.go | 20 +++++++----- pkg/network/payload/address_test.go | 11 ++++--- pkg/network/peer.go | 5 +-- pkg/network/server.go | 10 +++--- pkg/network/server_test.go | 8 +++-- pkg/network/tcp_peer.go | 20 ++++++------ pkg/rpc/server.go | 2 +- pkg/util/endpoint.go | 49 ----------------------------- 9 files changed, 50 insertions(+), 85 deletions(-) delete mode 100644 pkg/util/endpoint.go diff --git a/pkg/network/helper_test.go b/pkg/network/helper_test.go index bdf6a9c83..c231f96dd 100644 --- a/pkg/network/helper_test.go +++ b/pkg/network/helper_test.go @@ -2,6 +2,7 @@ package network import ( "math/rand" + "net" "testing" "time" @@ -111,22 +112,23 @@ func (t localTransport) Close() {} var defaultMessageHandler = func(t *testing.T, msg *Message) {} type localPeer struct { - endpoint util.Endpoint + netaddr net.TCPAddr version *payload.Version t *testing.T messageHandler func(t *testing.T, msg *Message) } func newLocalPeer(t *testing.T) *localPeer { + naddr, _ := net.ResolveTCPAddr("tcp", "0.0.0.0:0") return &localPeer{ t: t, - endpoint: util.NewEndpoint("0.0.0.0:0"), + netaddr: *naddr, messageHandler: defaultMessageHandler, } } -func (p *localPeer) Endpoint() util.Endpoint { - return p.endpoint +func (p *localPeer) NetAddr() *net.TCPAddr { + return &p.netaddr } func (p *localPeer) Disconnect(err error) {} func (p *localPeer) WriteMsg(msg *Message) error { diff --git a/pkg/network/payload/address.go b/pkg/network/payload/address.go index 723275338..c891753e5 100644 --- a/pkg/network/payload/address.go +++ b/pkg/network/payload/address.go @@ -2,6 +2,7 @@ package payload import ( "io" + "net" "time" "github.com/CityOfZion/neo-go/pkg/util" @@ -11,16 +12,19 @@ import ( type AddressAndTime struct { Timestamp uint32 Services uint64 - Endpoint util.Endpoint + IP [16]byte + Port uint16 } // NewAddressAndTime creates a new AddressAndTime object. -func NewAddressAndTime(e util.Endpoint, t time.Time) *AddressAndTime { - return &AddressAndTime{ +func NewAddressAndTime(e *net.TCPAddr, t time.Time) *AddressAndTime { + aat := AddressAndTime{ Timestamp: uint32(t.UTC().Unix()), Services: 1, - Endpoint: e, + Port: uint16(e.Port), } + copy(aat.IP[:], e.IP) + return &aat } // DecodeBinary implements the Payload interface. @@ -28,8 +32,8 @@ func (p *AddressAndTime) DecodeBinary(r io.Reader) error { br := util.BinReader{R: r} br.ReadLE(&p.Timestamp) br.ReadLE(&p.Services) - br.ReadBE(&p.Endpoint.IP) - br.ReadBE(&p.Endpoint.Port) + br.ReadBE(&p.IP) + br.ReadBE(&p.Port) return br.Err } @@ -38,8 +42,8 @@ func (p *AddressAndTime) EncodeBinary(w io.Writer) error { bw := util.BinWriter{W: w} bw.WriteLE(p.Timestamp) bw.WriteLE(p.Services) - bw.WriteBE(p.Endpoint.IP) - bw.WriteBE(p.Endpoint.Port) + bw.WriteBE(p.IP) + bw.WriteBE(p.Port) return bw.Err } diff --git a/pkg/network/payload/address_test.go b/pkg/network/payload/address_test.go index d8593dbba..154bf017d 100644 --- a/pkg/network/payload/address_test.go +++ b/pkg/network/payload/address_test.go @@ -3,23 +3,26 @@ package payload import ( "bytes" "fmt" + "net" "testing" "time" - "github.com/CityOfZion/neo-go/pkg/util" "github.com/stretchr/testify/assert" ) func TestEncodeDecodeAddress(t *testing.T) { var ( - e = util.NewEndpoint("127.0.0.1:2000") + e, _ = net.ResolveTCPAddr("tcp", "127.0.0.1:2000") ts = time.Now() addr = NewAddressAndTime(e, ts) buf = new(bytes.Buffer) ) assert.Equal(t, ts.UTC().Unix(), int64(addr.Timestamp)) - assert.Equal(t, e, addr.Endpoint) + aatip := make(net.IP, 16) + copy(aatip, addr.IP[:]) + assert.Equal(t, e.IP, aatip) + assert.Equal(t, e.Port, int(addr.Port)) err := addr.EncodeBinary(buf) assert.Nil(t, err) @@ -34,7 +37,7 @@ func TestEncodeDecodeAddressList(t *testing.T) { var lenList uint8 = 4 addrList := &AddressList{make([]*AddressAndTime, lenList)} for i := 0; i < int(lenList); i++ { - e := util.NewEndpoint(fmt.Sprintf("127.0.0.1:200%d", i)) + e, _ := net.ResolveTCPAddr("tcp", fmt.Sprintf("127.0.0.1:200%d", i)) addrList.Addrs[i] = NewAddressAndTime(e, time.Now()) } diff --git a/pkg/network/peer.go b/pkg/network/peer.go index 6d698a11e..1298d615a 100644 --- a/pkg/network/peer.go +++ b/pkg/network/peer.go @@ -1,13 +1,14 @@ package network import ( + "net" + "github.com/CityOfZion/neo-go/pkg/network/payload" - "github.com/CityOfZion/neo-go/pkg/util" ) // Peer represents a network node neo-go is connected to. type Peer interface { - Endpoint() util.Endpoint + NetAddr() *net.TCPAddr Disconnect(error) WriteMsg(msg *Message) error Done() chan error diff --git a/pkg/network/server.go b/pkg/network/server.go index 21b305616..84d0f7ab3 100644 --- a/pkg/network/server.go +++ b/pkg/network/server.go @@ -133,17 +133,17 @@ func (s *Server) run() { // When a new peer is connected we send out our version immediately. if err := s.sendVersion(p); err != nil { log.WithFields(log.Fields{ - "endpoint": p.Endpoint(), + "addr": p.NetAddr(), }).Error(err) } s.peers[p] = true log.WithFields(log.Fields{ - "endpoint": p.Endpoint(), + "addr": p.NetAddr(), }).Info("new peer connected") case drop := <-s.unregister: delete(s.peers, drop.peer) log.WithFields(log.Fields{ - "endpoint": drop.peer.Endpoint(), + "addr": drop.peer.NetAddr(), "reason": drop.reason, "peerCount": s.PeerCount(), }).Warn("peer disconnected") @@ -168,7 +168,7 @@ func (s *Server) PeerCount() int { // every ProtoTickInterval with the peer. func (s *Server) startProtocol(p Peer) { log.WithFields(log.Fields{ - "endpoint": p.Endpoint(), + "addr": p.NetAddr(), "userAgent": string(p.Version().UserAgent), "startHeight": p.Version().StartHeight, "id": p.Version().Nonce, @@ -207,7 +207,7 @@ func (s *Server) sendVersion(p Peer) error { // When a peer sends out his version we reply with verack after validating // the version. func (s *Server) handleVersionCmd(p Peer, version *payload.Version) error { - if p.Endpoint().Port != version.Port { + if p.NetAddr().Port != int(version.Port) { return errPortMismatch } if s.id == version.Nonce { diff --git a/pkg/network/server_test.go b/pkg/network/server_test.go index 9bbceeb44..0d4e1238b 100644 --- a/pkg/network/server_test.go +++ b/pkg/network/server_test.go @@ -1,10 +1,10 @@ package network import ( + "net" "testing" "github.com/CityOfZion/neo-go/pkg/network/payload" - "github.com/CityOfZion/neo-go/pkg/util" "github.com/stretchr/testify/assert" ) @@ -39,7 +39,8 @@ func TestVerackAfterHandleVersionCmd(t *testing.T) { s = newTestServer() p = newLocalPeer(t) ) - p.endpoint = util.NewEndpoint("0.0.0.0:3000") + na, _ := net.ResolveTCPAddr("tcp", "0.0.0.0:3000") + p.netaddr = *na // Should have a verack p.messageHandler = func(t *testing.T, msg *Message) { @@ -62,7 +63,8 @@ func TestServerNotSendsVerack(t *testing.T) { s.id = 1 go s.run() - p.endpoint = util.NewEndpoint("0.0.0.0:3000") + na, _ := net.ResolveTCPAddr("tcp", "0.0.0.0:3000") + p.netaddr = *na s.register <- p // Port should mismatch diff --git a/pkg/network/tcp_peer.go b/pkg/network/tcp_peer.go index 1a63fea7e..e9e81ffa9 100644 --- a/pkg/network/tcp_peer.go +++ b/pkg/network/tcp_peer.go @@ -5,15 +5,14 @@ import ( "sync" "github.com/CityOfZion/neo-go/pkg/network/payload" - "github.com/CityOfZion/neo-go/pkg/util" ) // TCPPeer represents a connected remote node in the // network over TCP. type TCPPeer struct { // underlying TCP connection. - conn net.Conn - endpoint util.Endpoint + conn net.Conn + addr net.TCPAddr // The version of the peer. version *payload.Version @@ -25,10 +24,13 @@ type TCPPeer struct { // NewTCPPeer returns a TCPPeer structure based on the given connection. func NewTCPPeer(conn net.Conn) *TCPPeer { + raddr := conn.RemoteAddr() + // can't fail because raddr is a real connection + tcpaddr, _ := net.ResolveTCPAddr(raddr.Network(), raddr.String()) return &TCPPeer{ - conn: conn, - done: make(chan error, 1), - endpoint: util.NewEndpoint(conn.RemoteAddr().String()), + conn: conn, + done: make(chan error, 1), + addr: *tcpaddr, } } @@ -43,9 +45,9 @@ func (p *TCPPeer) WriteMsg(msg *Message) error { } } -// Endpoint implements the Peer interface. -func (p *TCPPeer) Endpoint() util.Endpoint { - return p.endpoint +// NetAddr implements the Peer interface. +func (p *TCPPeer) NetAddr() *net.TCPAddr { + return &p.addr } // Done implements the Peer interface and notifies diff --git a/pkg/rpc/server.go b/pkg/rpc/server.go index c6a1a2669..f8fcb836b 100644 --- a/pkg/rpc/server.go +++ b/pkg/rpc/server.go @@ -180,7 +180,7 @@ Methods: } for addr := range s.coreServer.Peers() { - peers.AddPeer("connected", addr.Endpoint().String()) + peers.AddPeer("connected", addr.NetAddr().String()) } results = peers diff --git a/pkg/util/endpoint.go b/pkg/util/endpoint.go deleted file mode 100644 index afdf320e0..000000000 --- a/pkg/util/endpoint.go +++ /dev/null @@ -1,49 +0,0 @@ -package util - -import ( - "fmt" - "strconv" - "strings" -) - -// Endpoint host + port of a node, compatible with net.Addr. -type Endpoint struct { - IP [16]byte // TODO: make a uint128 type - Port uint16 -} - -// NewEndpoint creates an Endpoint from the given string. -func NewEndpoint(s string) (e Endpoint) { - hostPort := strings.Split(s, ":") - if len(hostPort) != 2 { - return e - } - host := hostPort[0] - port := hostPort[1] - - ch := strings.Split(host, ".") - - buf := [16]byte{} - var n int - for i := 0; i < len(ch); i++ { - n = 12 + i - nn, _ := strconv.Atoi(ch[i]) - buf[n] = byte(nn) - } - - p, _ := strconv.Atoi(port) - - return Endpoint{buf, uint16(p)} -} - -// Network implements the net.Addr interface. -func (e Endpoint) Network() string { return "tcp" } - -// String implements the net.Addr interface. -func (e Endpoint) String() string { - b := make([]uint8, 4) - for i := 0; i < 4; i++ { - b[i] = e.IP[len(e.IP)-4+i] - } - return fmt.Sprintf("%d.%d.%d.%d:%d", b[0], b[1], b[2], b[3], e.Port) -}