Skip to content

Commit

Permalink
addrmgr: Remove DNS lookups from address manager.
Browse files Browse the repository at this point in the history
This removes the need to perform DNS lookups in the address manager. The
direction forward is similar to that of BIP 155: addrv2. The address
manager stores encoded versions of each address type, and doesn't need
to know how to dial each type.
  • Loading branch information
matthawkins90 authored and davecgh committed Aug 6, 2024
1 parent 4a6b0f5 commit fe43331
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 151 deletions.
35 changes: 1 addition & 34 deletions addrmgr/addrmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,6 @@ type AddrManager struct {
// is saved to and loaded from.
peersFile string

// lookupFunc is a function provided to the address manager that is used to
// perform DNS lookups for a given hostname.
// The provided function MUST be safe for concurrent access.
lookupFunc func(string) ([]net.IP, error)

// key is a random seed used to map addresses to new and tried buckets.
key [32]byte

Expand Down Expand Up @@ -762,32 +757,6 @@ func EncodeHost(host string) (NetAddressType, []byte) {
return UnknownAddressType, nil
}

// HostToNetAddress parses and returns a network address given a hostname in a
// supported format (IPv4, IPv6). If the hostname cannot be immediately
// converted from a known address format, it will be resolved using the lookup
// function provided to the address manager. If it cannot be resolved, an error
// is returned.
//
// This function is safe for concurrent access.
func (a *AddrManager) HostToNetAddress(host string, port uint16, services wire.ServiceFlag) (*NetAddress, error) {
addrType, addrBytes := EncodeHost(host)
if addrType != UnknownAddressType {
// Since the host type has been successfully recognized and encoded,
// there is no need to perform a DNS lookup.
now := time.Unix(time.Now().Unix(), 0)
return NewNetAddressFromParams(addrType, addrBytes, port, now, services)
}
// Cannot determine the host address type. Must use DNS.
ips, err := a.lookupFunc(host)
if err != nil {
return nil, err
}
if len(ips) == 0 {
return nil, fmt.Errorf("no addresses found for %s", host)
}
return NewNetAddressFromIPPort(ips[0], port, services), nil
}

// GetAddress returns a single address that should be routable. It picks a
// random one from the possible addresses with preference given to ones that
// have not been used recently and should not pick 'close' addresses
Expand Down Expand Up @@ -1246,11 +1215,9 @@ func (a *AddrManager) IsExternalAddrCandidate(localAddr, remoteAddr *NetAddress)

// New constructs a new address manager instance.
// Use Start to begin processing asynchronous address updates.
// The address manager uses lookupFunc for necessary DNS lookups.
func New(dataDir string, lookupFunc func(string) ([]net.IP, error)) *AddrManager {
func New(dataDir string) *AddrManager {
am := AddrManager{
peersFile: filepath.Join(dataDir, peersFilename),
lookupFunc: lookupFunc,
quit: make(chan struct{}),
localAddresses: make(map[string]*localAddress),
triedBucketSize: defaultTriedBucketSize,
Expand Down
126 changes: 19 additions & 107 deletions addrmgr/addrmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package addrmgr

import (
"errors"
"fmt"
"net"
"os"
Expand Down Expand Up @@ -36,10 +35,6 @@ func natfOnlyIPv6(addrType NetAddressType) bool {
return addrType == IPv6Address
}

func lookupFunc(host string) ([]net.IP, error) {
return nil, errors.New("not implemented")
}

// addAddressByIP is a convenience function that adds an address to the
// address manager given a valid string representation of an ip address and
// a port.
Expand All @@ -53,7 +48,7 @@ func (a *AddrManager) addAddressByIP(addr string, port uint16) {
// that addresses are correctly added and/or updated, and that tried addresses
// are not re-added.
func TestAddOrUpdateAddress(t *testing.T) {
amgr := New("testaddaddressupdate", nil)
amgr := New("testaddaddressupdate")
amgr.Start()
if ka := amgr.GetAddress(); ka != nil {
t.Fatal("address manager should contain no addresses")
Expand Down Expand Up @@ -139,7 +134,7 @@ func TestAddOrUpdateAddress(t *testing.T) {

// TestExpireNew ensures that expireNew will correctly throw away bad addresses.
func TestExpireNew(t *testing.T) {
n := New("testexpirenew", nil)
n := New("testexpirenew")

now := time.Now()

Expand Down Expand Up @@ -195,7 +190,7 @@ func TestLoadPeersWithCorruptPeersFile(t *testing.T) {
if err := fp.Close(); err != nil {
t.Fatalf("Could not write empty peers file: %s", peersFile)
}
amgr := New(dir, nil)
amgr := New(dir)
amgr.Start()
amgr.Stop()
if _, err := os.Stat(peersFile); err != nil {
Expand All @@ -214,7 +209,7 @@ func TestStartStop(t *testing.T) {
t.Fatalf("peers file exists though it should not: %s", peersFile)
}

amgr := New(dir, nil)
amgr := New(dir)
amgr.Start()

// Add single network address to the address manager.
Expand All @@ -232,7 +227,7 @@ func TestStartStop(t *testing.T) {
}

// Start a new address manager, which initializes it from the peers file.
amgr = New(dir, nil)
amgr = New(dir)
amgr.Start()

knownAddress := amgr.GetAddress()
Expand All @@ -257,7 +252,7 @@ func TestStartStop(t *testing.T) {
// TestNeedMoreAddresses adds 1000 addresses and then checks to see if
// NeedMoreAddresses correctly determines that no more addresses are needed.
func TestNeedMoreAddresses(t *testing.T) {
n := New("testneedmoreaddresses", lookupFunc)
n := New("testneedmoreaddresses")
addrsToAdd := needAddressThreshold
b := n.NeedMoreAddresses()
if !b {
Expand Down Expand Up @@ -289,14 +284,14 @@ func TestNeedMoreAddresses(t *testing.T) {
// never-attempted addresses, or addresses which don't match the given filter.
func TestAddressCache(t *testing.T) {
// Test that the randomized subset is nil if no addresses are known.
n := New("testaddresscacheisempty", nil)
n := New("testaddresscacheisempty")
addrList := n.AddressCache(natfAny)
if addrList != nil {
t.Fatalf("expected empty AddressCache. Got %v", addrList)
}

// Test that bad and never-attempted addresses aren't shared.
n = New("testaddresscachewithbad", nil)
n = New("testaddresscachewithbad")
srcAddr := NewNetAddressFromIPPort(net.ParseIP("173.144.173.111"), 8333, 0)
// Create and add a bad address from the future.
futureTime := time.Now().AddDate(0, 1, 0)
Expand All @@ -321,7 +316,7 @@ func TestAddressCache(t *testing.T) {
}

// Test that a filter will prevent certain addresses from being shared.
n = New("testaddresscachewithfilter", nil)
n = New("testaddresscachewithfilter")
goodIPv4 := NewNetAddressFromIPPort(net.ParseIP(routableIPv4Addr), 0, wire.SFNodeNetwork)
goodIPv6 := NewNetAddressFromIPPort(net.ParseIP(routableIPv6Addr), 0, wire.SFNodeNetwork)
n.addOrUpdateAddress(goodIPv4, srcAddr)
Expand All @@ -339,91 +334,8 @@ func TestAddressCache(t *testing.T) {
}
}

// TestHostToNetAddress ensures that HostToNetAddress behaves as expected
// given valid and invalid host name arguments.
func TestHostToNetAddress(t *testing.T) {
// Define a hostname that will cause a lookup to be performed using the
// lookupFunc provided to the address manager instance for each test.
const hostnameForLookup = "hostname.test"
const services = wire.SFNodeNetwork

tests := []struct {
name string
host string
port uint16
lookupFunc func(host string) ([]net.IP, error)
wantErr bool
want *NetAddress
}{{
// name: "valid onion address",
// host: "a5ccbdkubbr2jlcp.onion",
// port: 8333,
// lookupFunc: nil,
// wantErr: false,
// want: NewNetAddressFromIPPort(
// net.ParseIP("fd87:d87e:eb43:744:208d:5408:63a4:ac4f"), 8333,
// services),
// }, {
// name: "invalid onion address",
// host: "0000000000000000.onion",
// port: 8333,
// lookupFunc: nil,
// wantErr: true,
// want: nil,
// }, {
name: "unresolvable host name",
host: hostnameForLookup,
port: 8333,
lookupFunc: func(host string) ([]net.IP, error) {
return nil, fmt.Errorf("unresolvable host %v", host)
},
wantErr: true,
want: nil,
}, {
name: "not resolved host name",
host: hostnameForLookup,
port: 8333,
lookupFunc: func(host string) ([]net.IP, error) {
return nil, nil
},
wantErr: true,
want: nil,
}, {
name: "resolved host name",
host: hostnameForLookup,
port: 8333,
lookupFunc: func(host string) ([]net.IP, error) {
return []net.IP{net.ParseIP("127.0.0.1")}, nil
},
wantErr: false,
want: NewNetAddressFromIPPort(net.ParseIP("127.0.0.1"), 8333,
services),
}, {
name: "valid ip address",
host: "12.1.2.3",
port: 8333,
lookupFunc: nil,
wantErr: false,
want: NewNetAddressFromIPPort(net.ParseIP("12.1.2.3"), 8333,
services),
}}

for _, test := range tests {
addrManager := New("testHostToNetAddress", test.lookupFunc)
result, err := addrManager.HostToNetAddress(test.host, test.port,
services)
if test.wantErr == true && err == nil {
t.Errorf("%q: expected error but one was not returned", test.name)
}
if !reflect.DeepEqual(result, test.want) {
t.Errorf("%q: unexpected result - got %v, want %v", test.name,
result, test.want)
}
}
}

func TestGetAddress(t *testing.T) {
n := New("testgetaddress", lookupFunc)
n := New("testgetaddress")

// Get an address from an empty set (should error).
if rv := n.GetAddress(); rv != nil {
Expand Down Expand Up @@ -478,7 +390,7 @@ func TestGetAddress(t *testing.T) {

// TestAttempt ensures that Attempt will correctly update the lastAttempt time.
func TestAttempt(t *testing.T) {
n := New("testattempt", lookupFunc)
n := New("testattempt")

// Add a new address and get it.
n.addAddressByIP(routableIPv4Addr, 8333)
Expand Down Expand Up @@ -509,7 +421,7 @@ func TestAttempt(t *testing.T) {

// TestConnected ensures that Connected will correctly update the connected timestamp.
func TestConnected(t *testing.T) {
n := New("testconnected", lookupFunc)
n := New("testconnected")

// Add a new address and get it
n.addAddressByIP(routableIPv4Addr, 8333)
Expand Down Expand Up @@ -549,7 +461,7 @@ func TestConnected(t *testing.T) {
// as good if it hasn't first been added to a new bucket.
func TestGood(t *testing.T) {
// Phase 1: test adding addresses and marking them good.
n := New("testgood", lookupFunc)
n := New("testgood")
addrsToAdd := 64 * 64
addrs := make([]*NetAddress, addrsToAdd)

Expand Down Expand Up @@ -583,7 +495,7 @@ func TestGood(t *testing.T) {
// enter the new bucket, and when marked good it should move to the tried
// bucket. If the tried bucket is full then it should make room for the
// newly tried address by moving the old one back to the new bucket.
n = New("testgood_tried_overflow", lookupFunc)
n = New("testgood_tried_overflow")
n.triedBucketSize = 1
n.getNewBucket = func(netAddr, srcAddr *NetAddress) int {
return 0
Expand Down Expand Up @@ -681,7 +593,7 @@ func TestGood(t *testing.T) {

// Phase 3: Test trying to mark an address as good without first adding it
// to the new bucket
n = New("testgood_not_new", lookupFunc)
n = New("testgood_not_new")
// Directly add an address to the address index without adding it to New
n.addrIndex[addrAKey] = &KnownAddress{na: addrA, srcAddr: srcAddr, tried: false}
// Try to mark the address as good
Expand All @@ -701,7 +613,7 @@ func TestGood(t *testing.T) {
// expected and that the services field is not mutated when new services are
// added.
func TestSetServices(t *testing.T) {
addressManager := New("testSetServices", nil)
addressManager := New("testSetServices")
const services = wire.SFNodeNetwork

// Attempt to set services for an address not known to the address manager.
Expand Down Expand Up @@ -789,7 +701,7 @@ func TestAddLocalAddress(t *testing.T) {
const testPort = 8333
const testServices = wire.SFNodeNetwork

amgr := New("testaddlocaladdress", nil)
amgr := New("testaddlocaladdress")
validLocalAddresses := make(map[string]struct{})
for _, test := range tests {
netAddr := NewNetAddressFromIPPort(test.ip, testPort, testServices)
Expand Down Expand Up @@ -877,7 +789,7 @@ func TestGetBestLocalAddress(t *testing.T) {
newAddressFromIP(net.ParseIP("2001:470::1")),
}}

amgr := New("testgetbestlocaladdress", nil)
amgr := New("testgetbestlocaladdress")

// Test0: Default response (non-routable) when there's no stored addresses.
for x, test := range tests {
Expand Down Expand Up @@ -1072,7 +984,7 @@ func TestIsExternalAddrCandidate(t *testing.T) {
expectedReach: Ipv6Weak,
}}

addressManager := New("TestIsExternalAddrCandidate", nil)
addressManager := New("TestIsExternalAddrCandidate")
for _, test := range tests {
localIP := net.ParseIP(test.localAddr)
remoteIP := net.ParseIP(test.remoteAddr)
Expand Down
2 changes: 1 addition & 1 deletion addrmgr/netaddress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func TestNewNetAddressFromParams(t *testing.T) {
// TestNewNetAddressFromString verifies that the newNetAddressFromString
// constructor correctly creates a network address with expected field values.
func TestNewNetAddressFromString(t *testing.T) {
amgr := New("TestNewNetAddressFromString", nil)
amgr := New("TestNewNetAddressFromString")
tests := []struct {
name string
addrString string
Expand Down
2 changes: 1 addition & 1 deletion config.go
Original file line number Diff line number Diff line change
Expand Up @@ -1372,7 +1372,7 @@ func dcrdDial(ctx context.Context, network, addr string) (net.Conn, error) {
return cfg.dial(ctx, network, addr)
}

// dcrdLookup returns the correct DNS lookup function to use depending on the
// dcrdLookup invokes the correct DNS lookup function to use depending on the
// passed host and configuration options. For example, .onion addresses will be
// resolved using the onion specific proxy if one was specified, but will
// otherwise treat the normal proxy as tor unless --noonion was specified in
Expand Down
Loading

0 comments on commit fe43331

Please sign in to comment.