Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Commit

Permalink
wire: Treat NetAddress more like immutable
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dskloetg authored and dajohi committed Mar 24, 2017
1 parent c58b757 commit 64a414c
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 82 deletions.
21 changes: 8 additions & 13 deletions addrmgr/addrmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down
26 changes: 5 additions & 21 deletions addrmgr/addrmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)

Expand All @@ -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++ {
Expand All @@ -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()
Expand All @@ -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++ {
Expand All @@ -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 {
Expand Down
24 changes: 13 additions & 11 deletions addrmgr/knownaddress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand All @@ -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}
Expand Down
15 changes: 2 additions & 13 deletions addrmgr/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package addrmgr_test
import (
"net"
"testing"
"time"

"github.com/decred/dcrd/addrmgr"
"github.com/decred/dcrd/wire"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 7 additions & 8 deletions connmgr/seed.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 1 addition & 4 deletions peer/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
28 changes: 16 additions & 12 deletions wire/netaddress.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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
}

Expand Down

0 comments on commit 64a414c

Please sign in to comment.