From c5751b75a9dadf4e5bc780fa812eb4ee9d161936 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 | 22 ++++++++++++---------- addrmgr/network_test.go | 15 ++------------- connmgr/seed.go | 15 +++++++-------- peer/peer.go | 5 +---- wire/netaddress.go | 28 ++++++++++++++++------------ 7 files changed, 51 insertions(+), 81 deletions(-) diff --git a/addrmgr/addrmanager.go b/addrmgr/addrmanager.go index 1f6d869a8..0ee2b6d25 100644 --- a/addrmgr/addrmanager.go +++ b/addrmgr/addrmanager.go @@ -596,18 +596,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 } @@ -1069,16 +1067,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 c384b0156..5e4e55c96 100644 --- a/addrmgr/addrmanager_test.go +++ b/addrmgr/addrmanager_test.go @@ -93,12 +93,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) } @@ -245,7 +240,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) @@ -262,7 +258,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++ { @@ -273,12 +268,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() @@ -296,7 +286,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++ { @@ -307,12 +296,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 79503df33..a289d5a38 100644 --- a/addrmgr/knownaddress_test.go +++ b/addrmgr/knownaddress_test.go @@ -14,33 +14,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, }, @@ -56,11 +57,12 @@ 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) + 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} diff --git a/addrmgr/network_test.go b/addrmgr/network_test.go index 98dc054e4..8af3369f5 100644 --- a/addrmgr/network_test.go +++ b/addrmgr/network_test.go @@ -7,7 +7,6 @@ package addrmgr_test import ( "net" "testing" - "time" "github.com/btcsuite/btcd/addrmgr" "github.com/btcsuite/btcd/wire" @@ -40,12 +39,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 @@ -198,12 +192,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 8c7a0e0d6..063b546ab 100644 --- a/connmgr/seed.go +++ b/connmgr/seed.go @@ -60,14 +60,13 @@ func SeedFromDNS(chainParams *chaincfg.Params, reqServices wire.ServiceFlag, // 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 291c5ec9b..44a0f56ac 100644 --- a/peer/peer.go +++ b/peer/peer.go @@ -775,10 +775,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 05b335442..5a2610bcc 100644 --- a/wire/netaddress.go +++ b/wire/netaddress.go @@ -57,20 +57,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, @@ -88,7 +89,6 @@ func NewNetAddress(addr *net.TCPAddr, services ServiceFlag) *NetAddress { // 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 bitcoin protocol uses a uint32 for the timestamp so it will @@ -101,7 +101,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 } @@ -111,8 +111,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 }