Skip to content

Commit

Permalink
config, net, test: asmap feature refinements and functional tests
Browse files Browse the repository at this point in the history
Summary:
```
This PR builds on PR #16702 to add functional tests / sanity checks and
user-facing refinements for passing -asmap to configure ASN-based IP
bucketing in addrman. As per our review discussion in that PR, the idea
here is to handle aspects like functional tests and config arg handling
that can help the PR be merged while enabling the author to focus on the
bucketing itself.
```

Backport of core [[bitcoin/bitcoin#17812 | PR17812]].

Depends on D8198.

Test Plan:
  ninja all check-all

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D8200
  • Loading branch information
jonatack authored and Fabcien committed Oct 30, 2020
1 parent e4c59a2 commit 930510c
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 35 deletions.
8 changes: 4 additions & 4 deletions src/addrman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ int CAddrInfo::GetTriedBucket(const uint256 &nKey,
.GetCheapHash();
int tried_bucket = hash2 % ADDRMAN_TRIED_BUCKET_COUNT;
uint32_t mapped_as = GetMappedAS(asmap);
LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to tried bucket %i.\n",
LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to tried bucket %i\n",
ToStringIP(), mapped_as, tried_bucket);
return tried_bucket;
}
Expand All @@ -38,7 +38,7 @@ int CAddrInfo::GetNewBucket(const uint256 &nKey, const CNetAddr &src,
.GetCheapHash();
int new_bucket = hash2 % ADDRMAN_NEW_BUCKET_COUNT;
uint32_t mapped_as = GetMappedAS(asmap);
LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to new bucket %i.\n",
LogPrint(BCLog::NET, "IP %s mapped to AS%i belongs to new bucket %i\n",
ToStringIP(), mapped_as, new_bucket);
return new_bucket;
}
Expand Down Expand Up @@ -744,12 +744,12 @@ std::vector<bool> CAddrMan::DecodeAsmap(fs::path path) {
FILE *filestr = fsbridge::fopen(path, "rb");
CAutoFile file(filestr, SER_DISK, CLIENT_VERSION);
if (file.IsNull()) {
LogPrintf("Failed to open asmap file from disk.\n");
LogPrintf("Failed to open asmap file from disk\n");
return bits;
}
fseek(filestr, 0, SEEK_END);
int length = ftell(filestr);
LogPrintf("Opened asmap file %s (%d bytes) from disk.\n", path, length);
LogPrintf("Opened asmap file %s (%d bytes) from disk\n", path, length);
fseek(filestr, 0, SEEK_SET);
char cur_byte;
for (int i = 0; i < length; ++i) {
Expand Down
59 changes: 33 additions & 26 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,12 @@ void SetupServerArgs() {
"open (see the `addnode` RPC command help for more info)",
ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY,
OptionsCategory::CONNECTION);
gArgs.AddArg("-asmap=<file>",
strprintf("Specify asn mapping used for bucketing of the "
"peers (default: %s). Relative paths will be "
"prefixed by the net-specific datadir location.",
DEFAULT_ASMAP_FILENAME),
ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
gArgs.AddArg("-banscore=<n>",
strprintf("Threshold for disconnecting and discouraging "
"misbehaving peers (default: %u)",
Expand Down Expand Up @@ -714,10 +720,6 @@ void SetupServerArgs() {
"Tor control port password (default: empty)",
ArgsManager::ALLOW_ANY | ArgsManager::SENSITIVE,
OptionsCategory::CONNECTION);
gArgs.AddArg("-asmap=<file>",
"Specify asn mapping used for bucketing of the peers. Path "
"should be relative to the -datadir path.",
ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
#ifdef USE_UPNP
#if USE_UPNP
gArgs.AddArg("-upnp",
Expand Down Expand Up @@ -2308,6 +2310,33 @@ bool AppInitMain(Config &config, RPCServer &rpcServer,
}
}

// Read asmap file if configured
if (gArgs.IsArgSet("-asmap")) {
fs::path asmap_path = fs::path(gArgs.GetArg("-asmap", ""));
if (asmap_path.empty()) {
asmap_path = DEFAULT_ASMAP_FILENAME;
}
if (!asmap_path.is_absolute()) {
asmap_path = GetDataDir() / asmap_path;
}
if (!fs::exists(asmap_path)) {
InitError(strprintf(_("Could not find asmap file %s"), asmap_path));
return false;
}
std::vector<bool> asmap = CAddrMan::DecodeAsmap(asmap_path);
if (asmap.size() == 0) {
InitError(
strprintf(_("Could not parse asmap file %s"), asmap_path));
return false;
}
const uint256 asmap_version = SerializeHash(asmap);
node.connman->SetAsmap(std::move(asmap));
LogPrintf("Using asmap version %s for IP bucketing\n",
asmap_version.ToString());
} else {
LogPrintf("Using /16 prefix for IP bucketing\n");
}

#if ENABLE_ZMQ
g_zmq_notification_interface = CZMQNotificationInterface::Create();

Expand Down Expand Up @@ -2773,28 +2802,6 @@ bool AppInitMain(Config &config, RPCServer &rpcServer,
return false;
}

// Read asmap file if configured
if (gArgs.IsArgSet("-asmap")) {
std::string asmap_file = gArgs.GetArg("-asmap", "");
if (asmap_file.empty()) {
asmap_file = DEFAULT_ASMAP_FILENAME;
}
const fs::path asmap_path = GetDataDir() / asmap_file;
std::vector<bool> asmap = CAddrMan::DecodeAsmap(asmap_path);
if (asmap.size() == 0) {
InitError(
strprintf(_("Could not find or parse specified asmap: '%s'"),
asmap_path));
return false;
}
const uint256 asmap_version = SerializeHash(asmap);
node.connman->SetAsmap(std::move(asmap));
LogPrintf("Using asmap version %s for IP bucketing.\n",
asmap_version.ToString());
} else {
LogPrintf("Using /16 prefix for IP bucketing.\n");
}

// Step 13: finished

SetRPCWarmupFinished();
Expand Down
8 changes: 6 additions & 2 deletions src/netaddress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@ bool CNetAddr::IsRFC7343() const {
(GetByte(12) & 0xF0) == 0x20);
}

bool CNetAddr::IsHeNet() const {
return (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x04 &&
GetByte(12) == 0x70);
}

/**
* @returns Whether or not this is a dummy address that maps an onion address
* into IPv6.
Expand Down Expand Up @@ -524,8 +529,7 @@ std::vector<uint8_t> CNetAddr::GetGroup(const std::vector<bool> &asmap) const {
} else if (IsTor()) {
nStartByte = 6;
nBits = 4;
} else if (GetByte(15) == 0x20 && GetByte(14) == 0x01 &&
GetByte(13) == 0x04 && GetByte(12) == 0x70) {
} else if (IsHeNet()) {
// for he.net, use /36 groups
nBits = 36;
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/netaddress.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,12 @@ class CNetAddr {
explicit CNetAddr(const struct in_addr &ipv4Addr);
void SetIP(const CNetAddr &ip);

private:
/**
* Set raw IPv4 or IPv6 address (in network byte order)
* @note Only NET_IPV4 and NET_IPV6 are allowed for network.
*/
void SetRaw(Network network, const uint8_t *data);

public:
bool SetInternal(const std::string &name);

// for Tor addresses
Expand Down Expand Up @@ -87,6 +85,8 @@ class CNetAddr {
// IPv6 IPv4-translated address (::FFFF:0:0:0/96) (actually defined in
// RFC2765)
bool IsRFC6145() const;
// IPv6 Hurricane Electric - https://he.net (2001:0470::/36)
bool IsHeNet() const;
bool IsTor() const;
bool IsLocal() const;
bool IsRoutable() const;
Expand Down
4 changes: 3 additions & 1 deletion src/rpc/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ static UniValue getpeerinfo(const Config &config,
"(ip:port) Local address as reported by the peer"},
{RPCResult::Type::NUM, "mapped_as",
"The AS in the BGP route to the peer used for "
"diversifying peer selection\n"},
"diversifying\n"
"peer selection (only available if the asmap config flag "
"is set)\n"},
{RPCResult::Type::STR_HEX, "services",
"The services offered"},
{RPCResult::Type::ARR,
Expand Down
115 changes: 115 additions & 0 deletions test/functional/feature_asmap.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
#!/usr/bin/env python3
# Copyright (c) 2020 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test asmap config argument for ASN-based IP bucketing.
Verify node behaviour and debug log when launching bitcoind in these cases:
1. `bitcoind` with no -asmap arg, using /16 prefix for IP bucketing
2. `bitcoind -asmap=<absolute path>`, using the unit test skeleton asmap
3. `bitcoind -asmap=<relative path>`, using the unit test skeleton asmap
4. `bitcoind -asmap/-asmap=` with no file specified, using the default asmap
5. `bitcoind -asmap` with no file specified and a missing default asmap file
6. `bitcoind -asmap` with an empty (unparsable) default asmap file
The tests are order-independent.
"""
import os
import shutil

from test_framework.test_framework import BitcoinTestFramework

DEFAULT_ASMAP_FILENAME = 'ip_asn.map' # defined in src/init.cpp
ASMAP = '../../src/test/data/asmap.raw' # path to unit test skeleton asmap
VERSION = 'fec61fa21a9f46f3b17bdcd660d7f4cd90b966aad3aec593c99b35f0aca15853'


def expected_messages(filename):
return ['Opened asmap file "{}" (59 bytes) from disk'.format(filename),
'Using asmap version {} for IP bucketing'.format(VERSION)]


class AsmapTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = False
self.num_nodes = 1

def test_without_asmap_arg(self):
self.log.info('Test bitcoind with no -asmap arg passed')
self.stop_node(0)
with self.node.assert_debug_log(['Using /16 prefix for IP bucketing']):
self.start_node(0)

def test_asmap_with_absolute_path(self):
self.log.info('Test bitcoind -asmap=<absolute path>')
self.stop_node(0)
filename = os.path.join(self.datadir, 'my-map-file.map')
shutil.copyfile(self.asmap_raw, filename)
with self.node.assert_debug_log(expected_messages(filename)):
self.start_node(0, ['-asmap={}'.format(filename)])
os.remove(filename)

def test_asmap_with_relative_path(self):
self.log.info('Test bitcoind -asmap=<relative path>')
self.stop_node(0)
name = 'ASN_map'
filename = os.path.join(self.datadir, name)
shutil.copyfile(self.asmap_raw, filename)
with self.node.assert_debug_log(expected_messages(filename)):
self.start_node(0, ['-asmap={}'.format(name)])
os.remove(filename)

def test_default_asmap(self):
shutil.copyfile(self.asmap_raw, self.default_asmap)
for arg in ['-asmap', '-asmap=']:
self.log.info(
'Test bitcoind {} (using default map file)'.format(arg))
self.stop_node(0)
with self.node.assert_debug_log(expected_messages(self.default_asmap)):
self.start_node(0, [arg])
os.remove(self.default_asmap)

def test_default_asmap_with_missing_file(self):
self.log.info('Test bitcoind -asmap with missing default map file')
self.stop_node(0)
msg = "Error: Could not find asmap file \"{}\"".format(
self.default_asmap)
self.node.assert_start_raises_init_error(
extra_args=['-asmap'], expected_msg=msg)

def test_empty_asmap(self):
self.log.info('Test bitcoind -asmap with empty map file')
self.stop_node(0)
with open(self.default_asmap, "w", encoding="utf-8") as f:
f.write("")
msg = "Error: Could not parse asmap file \"{}\"".format(
self.default_asmap)
self.node.assert_start_raises_init_error(
extra_args=['-asmap'], expected_msg=msg)
os.remove(self.default_asmap)

def run_test(self):
self.node = self.nodes[0]
self.datadir = os.path.join(self.node.datadir, self.chain)
self.default_asmap = os.path.join(self.datadir, DEFAULT_ASMAP_FILENAME)
self.asmap_raw = os.path.join(
os.path.dirname(
os.path.realpath(__file__)), ASMAP)

self.test_without_asmap_arg()
self.test_asmap_with_absolute_path()
self.test_asmap_with_relative_path()
self.test_default_asmap()
self.test_default_asmap_with_missing_file()
self.test_empty_asmap()


if __name__ == '__main__':
AsmapTest().main()

0 comments on commit 930510c

Please sign in to comment.