From 64a414c64fd59706460f47c2aef5930a9e86dce4 Mon Sep 17 00:00:00 2001 From: David de Kloet Date: Tue, 6 Dec 2016 20:48:02 +0100 Subject: [PATCH] wire: Treat NetAddress more like immutable Replace assignments to individual fields of wire.NetAddress with creating the entire object at once, as one would do if the type was immutable. In some places this replaces the creation of a NetAddress with a high-precision timestamp with a call to a 'constructor' that converts the timestamp to single second precision. For consistency, the tests have also been changed to use single-precision timestamps. Lastly, the number of allocations in readNetAddress have been reduced by reading the services directly into the NetAddress instead of first into a temporary variable. --- addrmgr/addrmanager.go | 21 ++++++++------------- addrmgr/addrmanager_test.go | 26 +++++--------------------- addrmgr/knownaddress_test.go | 24 +++++++++++++----------- addrmgr/network_test.go | 15 ++------------- connmgr/seed.go | 15 +++++++-------- peer/peer.go | 5 +---- wire/netaddress.go | 28 ++++++++++++++++------------ 7 files changed, 52 insertions(+), 82 deletions(-) diff --git a/addrmgr/addrmanager.go b/addrmgr/addrmanager.go index 0bd18c9d60..29ffbb6f1a 100644 --- a/addrmgr/addrmanager.go +++ b/addrmgr/addrmanager.go @@ -601,18 +601,16 @@ func (a *AddrManager) AddAddressByIP(addrIP string) error { return err } // Put it in wire.Netaddress - var na wire.NetAddress - na.Timestamp = time.Now() - na.IP = net.ParseIP(addr) - if na.IP == nil { + ip := net.ParseIP(addr) + if ip == nil { return fmt.Errorf("invalid ip address %s", addr) } port, err := strconv.ParseUint(portStr, 10, 0) if err != nil { return fmt.Errorf("invalid port %s: %v", portStr, err) } - na.Port = uint16(port) - a.AddAddress(&na, &na) // XXX use correct src address + na := wire.NewNetAddressIPPort(ip, uint16(port), 0) + a.AddAddress(na, na) // XXX use correct src address return nil } @@ -1074,16 +1072,13 @@ func (a *AddrManager) GetBestLocalAddress(remoteAddr *wire.NetAddress) *wire.Net remoteAddr.Port) // Send something unroutable if nothing suitable. - bestAddress = &wire.NetAddress{ - Timestamp: time.Now(), - Services: wire.SFNodeNetwork, - Port: 0, - } + var ip net.IP if !IsIPv4(remoteAddr) && !IsOnionCatTor(remoteAddr) { - bestAddress.IP = net.IPv6zero + ip = net.IPv6zero } else { - bestAddress.IP = net.IPv4zero + ip = net.IPv4zero } + bestAddress = wire.NewNetAddressIPPort(ip, 0, wire.SFNodeNetwork) } return bestAddress diff --git a/addrmgr/addrmanager_test.go b/addrmgr/addrmanager_test.go index e9a1f21d93..e2a6004f8f 100644 --- a/addrmgr/addrmanager_test.go +++ b/addrmgr/addrmanager_test.go @@ -94,12 +94,7 @@ func addNaTests() { func addNaTest(ip string, port uint16, want string) { nip := net.ParseIP(ip) - na := wire.NetAddress{ - Timestamp: time.Now(), - Services: wire.SFNodeNetwork, - IP: nip, - Port: port, - } + na := *wire.NewNetAddressIPPort(nip, port, wire.SFNodeNetwork) test := naTest{na, want} naTests = append(naTests, test) } @@ -246,7 +241,8 @@ func TestConnected(t *testing.T) { } ka := n.GetAddress() na := ka.NetAddress() - na.Timestamp = time.Now().Add(time.Hour * -1) // make it an hour ago + // make it an hour ago + na.Timestamp = time.Unix(time.Now().Add(time.Hour*-1).Unix(), 0) n.Connected(na) @@ -263,7 +259,6 @@ func TestNeedMoreAddresses(t *testing.T) { t.Errorf("Expected that we need more addresses") } addrs := make([]*wire.NetAddress, addrsToAdd) - now := time.Now() var err error for i := 0; i < addrsToAdd; i++ { @@ -274,12 +269,7 @@ func TestNeedMoreAddresses(t *testing.T) { } } - srcAddr := &wire.NetAddress{ - Timestamp: now, - Services: 0, - IP: net.IPv4(173, 144, 173, 111), - Port: 8333, - } + srcAddr := wire.NewNetAddressIPPort(net.IPv4(173, 144, 173, 111), 8333, 0) n.AddAddresses(addrs, srcAddr) numAddrs := n.NumAddresses() @@ -297,7 +287,6 @@ func TestGood(t *testing.T) { n := addrmgr.New("testgood", lookupFunc) addrsToAdd := 64 * 64 addrs := make([]*wire.NetAddress, addrsToAdd) - now := time.Now() var err error for i := 0; i < addrsToAdd; i++ { @@ -308,12 +297,7 @@ func TestGood(t *testing.T) { } } - srcAddr := &wire.NetAddress{ - Timestamp: now, - Services: 0, - IP: net.IPv4(173, 144, 173, 111), - Port: 8333, - } + srcAddr := wire.NewNetAddressIPPort(net.IPv4(173, 144, 173, 111), 8333, 0) n.AddAddresses(addrs, srcAddr) for _, addr := range addrs { diff --git a/addrmgr/knownaddress_test.go b/addrmgr/knownaddress_test.go index 45f6a41bbd..8b9ff3c0c9 100644 --- a/addrmgr/knownaddress_test.go +++ b/addrmgr/knownaddress_test.go @@ -15,33 +15,34 @@ import ( ) func TestChance(t *testing.T) { + now := time.Unix(time.Now().Unix(), 0) var tests = []struct { addr *addrmgr.KnownAddress expected float64 }{ { //Test normal case - addrmgr.TstNewKnownAddress(&wire.NetAddress{Timestamp: time.Now().Add(-35 * time.Second)}, + addrmgr.TstNewKnownAddress(&wire.NetAddress{Timestamp: now.Add(-35 * time.Second)}, 0, time.Now().Add(-30*time.Minute), time.Now(), false, 0), 1.0, }, { //Test case in which lastseen < 0 - addrmgr.TstNewKnownAddress(&wire.NetAddress{Timestamp: time.Now().Add(20 * time.Second)}, + addrmgr.TstNewKnownAddress(&wire.NetAddress{Timestamp: now.Add(20 * time.Second)}, 0, time.Now().Add(-30*time.Minute), time.Now(), false, 0), 1.0, }, { //Test case in which lastattempt < 0 - addrmgr.TstNewKnownAddress(&wire.NetAddress{Timestamp: time.Now().Add(-35 * time.Second)}, + addrmgr.TstNewKnownAddress(&wire.NetAddress{Timestamp: now.Add(-35 * time.Second)}, 0, time.Now().Add(30*time.Minute), time.Now(), false, 0), 1.0 * .01, }, { //Test case in which lastattempt < ten minutes - addrmgr.TstNewKnownAddress(&wire.NetAddress{Timestamp: time.Now().Add(-35 * time.Second)}, + addrmgr.TstNewKnownAddress(&wire.NetAddress{Timestamp: now.Add(-35 * time.Second)}, 0, time.Now().Add(-5*time.Minute), time.Now(), false, 0), 1.0 * .01, }, { //Test case with several failed attempts. - addrmgr.TstNewKnownAddress(&wire.NetAddress{Timestamp: time.Now().Add(-35 * time.Second)}, + addrmgr.TstNewKnownAddress(&wire.NetAddress{Timestamp: now.Add(-35 * time.Second)}, 2, time.Now().Add(-30*time.Minute), time.Now(), false, 0), 1 / 1.5 / 1.5, }, @@ -57,12 +58,13 @@ func TestChance(t *testing.T) { } func TestIsBad(t *testing.T) { - future := time.Now().Add(35 * time.Minute) - monthOld := time.Now().Add(-43 * time.Hour * 24) - secondsOld := time.Now().Add(-2 * time.Second) - minutesOld := time.Now().Add(-27 * time.Minute) - hoursOld := time.Now().Add(-5 * time.Hour) - zeroTime, _ := time.Parse("Jan 1, 1970 at 0:00am (GMT)", "Jan 1, 1970 at 0:00am (GMT)") + now := time.Unix(time.Now().Unix(), 0) + future := now.Add(35 * time.Minute) + monthOld := now.Add(-43 * time.Hour * 24) + secondsOld := now.Add(-2 * time.Second) + minutesOld := now.Add(-27 * time.Minute) + hoursOld := now.Add(-5 * time.Hour) + zeroTime := time.Time{} futureNa := &wire.NetAddress{Timestamp: future} minutesOldNa := &wire.NetAddress{Timestamp: minutesOld} diff --git a/addrmgr/network_test.go b/addrmgr/network_test.go index eebb5fc0f3..26664c6a53 100644 --- a/addrmgr/network_test.go +++ b/addrmgr/network_test.go @@ -8,7 +8,6 @@ package addrmgr_test import ( "net" "testing" - "time" "github.com/decred/dcrd/addrmgr" "github.com/decred/dcrd/wire" @@ -41,12 +40,7 @@ func TestIPTypes(t *testing.T) { rfc4193, rfc4380, rfc4843, rfc4862, rfc5737, rfc6052, rfc6145, rfc6598, local, valid, routable bool) ipTest { nip := net.ParseIP(ip) - na := wire.NetAddress{ - Timestamp: time.Now(), - Services: wire.SFNodeNetwork, - IP: nip, - Port: 8333, - } + na := *wire.NewNetAddressIPPort(nip, 8333, wire.SFNodeNetwork) test := ipTest{na, rfc1918, rfc2544, rfc3849, rfc3927, rfc3964, rfc4193, rfc4380, rfc4843, rfc4862, rfc5737, rfc6052, rfc6145, rfc6598, local, valid, routable} return test @@ -199,12 +193,7 @@ func TestGroupKey(t *testing.T) { for i, test := range tests { nip := net.ParseIP(test.ip) - na := wire.NetAddress{ - Timestamp: time.Now(), - Services: wire.SFNodeNetwork, - IP: nip, - Port: 8333, - } + na := *wire.NewNetAddressIPPort(nip, 8333, wire.SFNodeNetwork) if key := addrmgr.GroupKey(&na); key != test.expected { t.Errorf("TestGroupKey #%d (%s): unexpected group key "+ "- got '%s', want '%s'", i, test.name, diff --git a/connmgr/seed.go b/connmgr/seed.go index 71d7717161..36b73d7f59 100644 --- a/connmgr/seed.go +++ b/connmgr/seed.go @@ -50,14 +50,13 @@ func SeedFromDNS(chainParams *chaincfg.Params, lookupFn LookupFunc, seedFn OnSee // if this errors then we have *real* problems intPort, _ := strconv.Atoi(chainParams.DefaultPort) for i, peer := range seedpeers { - addresses[i] = new(wire.NetAddress) - addresses[i].SetAddress(peer, uint16(intPort)) - // bitcoind seeds with addresses from - // a time randomly selected between 3 - // and 7 days ago. - addresses[i].Timestamp = time.Now().Add(-1 * - time.Second * time.Duration(secondsIn3Days+ - randSource.Int31n(secondsIn4Days))) + addresses[i] = wire.NewNetAddressTimestamp( + // bitcoind seeds with addresses from + // a time randomly selected between 3 + // and 7 days ago. + time.Now().Add(-1*time.Second*time.Duration(secondsIn3Days+ + randSource.Int31n(secondsIn4Days))), + 0, peer, uint16(intPort)) } seedFn(addresses) diff --git a/peer/peer.go b/peer/peer.go index d15d26b0c1..173c026c67 100644 --- a/peer/peer.go +++ b/peer/peer.go @@ -761,10 +761,7 @@ func (p *Peer) localVersionMsg() (*wire.MsgVersion, error) { proxyaddress, _, err := net.SplitHostPort(p.cfg.Proxy) // invalid proxy means poorly configured, be on the safe side. if err != nil || p.na.IP.String() == proxyaddress { - theirNA = &wire.NetAddress{ - Timestamp: time.Now(), - IP: net.IP([]byte{0, 0, 0, 0}), - } + theirNA = wire.NewNetAddressIPPort(net.IP([]byte{0, 0, 0, 0}), 0, 0) } } diff --git a/wire/netaddress.go b/wire/netaddress.go index c80eacb305..c672977b75 100644 --- a/wire/netaddress.go +++ b/wire/netaddress.go @@ -60,20 +60,21 @@ func (na *NetAddress) AddService(service ServiceFlag) { na.Services |= service } -// SetAddress is a convenience function to set the IP address and port in one -// call. -func (na *NetAddress) SetAddress(ip net.IP, port uint16) { - na.IP = ip - na.Port = port -} - // NewNetAddressIPPort returns a new NetAddress using the provided IP, port, and // supported services with defaults for the remaining fields. func NewNetAddressIPPort(ip net.IP, port uint16, services ServiceFlag) *NetAddress { + return NewNetAddressTimestamp(time.Now(), services, ip, port) +} + +// NewNetAddressTimestamp returns a new NetAddress using the provided +// timestamp, IP, port, and supported services. The timestamp is rounded to +// single second precision. +func NewNetAddressTimestamp( + timestamp time.Time, services ServiceFlag, ip net.IP, port uint16) *NetAddress { // Limit the timestamp to one second precision since the protocol // doesn't support better. na := NetAddress{ - Timestamp: time.Unix(time.Now().Unix(), 0), + Timestamp: time.Unix(timestamp.Unix(), 0), Services: services, IP: ip, Port: port, @@ -100,7 +101,6 @@ func NewNetAddress(addr net.Addr, services ServiceFlag) (*NetAddress, error) { // version and whether or not the timestamp is included per ts. Some messages // like version do not include the timestamp. func readNetAddress(r io.Reader, pver uint32, na *NetAddress, ts bool) error { - var services ServiceFlag var ip [16]byte // NOTE: The decred protocol uses a uint32 for the timestamp so it will @@ -113,7 +113,7 @@ func readNetAddress(r io.Reader, pver uint32, na *NetAddress, ts bool) error { } } - err := readElements(r, &services, &ip) + err := readElements(r, &na.Services, &ip) if err != nil { return err } @@ -123,8 +123,12 @@ func readNetAddress(r io.Reader, pver uint32, na *NetAddress, ts bool) error { return err } - na.Services = services - na.SetAddress(net.IP(ip[:]), port) + *na = NetAddress{ + Timestamp: na.Timestamp, + Services: na.Services, + IP: net.IP(ip[:]), + Port: port, + } return nil }