Skip to content

Commit

Permalink
[init] Remove -addrmantest command line arg
Browse files Browse the repository at this point in the history
-addrmantest is only used in `p2p_node_network_limited.py` test to
test if the node self-advertises a hard-coded local address
(which wouldn't be advertised in the tests because it's unroutable
without the test-only code path) to check pruning-related services
are correct in that addr.

Remove -addrmantest because the self advertisement happens because
of hard coded test path logic, and expected services are nominal
due to how easily the test-only code could diverge from mainnet
logic. It's also being used only in 1 test.
  • Loading branch information
stratospher committed Dec 22, 2023
1 parent 35df609 commit 5d27993
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 19 deletions.
1 change: 0 additions & 1 deletion src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,6 @@ void SetupServerArgs(ArgsManager& argsman)
argsman.AddArg("-limitancestorsize=<n>", strprintf("Do not accept transactions whose size with all in-mempool ancestors exceeds <n> kilobytes (default: %u)", DEFAULT_ANCESTOR_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-limitdescendantcount=<n>", strprintf("Do not accept transactions if any ancestor would have <n> or more in-mempool descendants (default: %u)", DEFAULT_DESCENDANT_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-limitdescendantsize=<n>", strprintf("Do not accept transactions if any ancestor would have more than <n> kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT_KVB), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-addrmantest", "Allows to test address relay on localhost", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-test=<option>", "Pass a test-only option. Options include : " + Join(TEST_OPTIONS_DOC, ", ") + ".", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-capturemessages", "Capture all P2P messages to disk", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
argsman.AddArg("-mocktime=<n>", "Replace actual time with " + UNIX_EPOCH_TIME + " (default: 0)", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
Expand Down
7 changes: 1 addition & 6 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,6 @@ static int GetnScore(const CService& addr)
std::optional<CService> GetLocalAddrForPeer(CNode& node)
{
CService addrLocal{GetLocalAddress(node)};
if (gArgs.GetBoolArg("-addrmantest", false)) {
// use IPv4 loopback during addrmantest
addrLocal = CService(LookupNumeric("127.0.0.1", GetListenPort()));
}
// If discovery is enabled, sometimes give our peer the address it
// tells us that it sees us as in case it has a better idea of our
// address than we do.
Expand All @@ -260,8 +256,7 @@ std::optional<CService> GetLocalAddrForPeer(CNode& node)
addrLocal.SetIP(node.GetAddrLocal());
}
}
if (addrLocal.IsRoutable() || gArgs.GetBoolArg("-addrmantest", false))
{
if (addrLocal.IsRoutable()) {
LogPrint(BCLog::NET, "Advertising address %s to peer=%d\n", addrLocal.ToStringAddrPort(), node.GetId());
return addrLocal;
}
Expand Down
13 changes: 1 addition & 12 deletions test/functional/p2p_node_network_limited.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
NODE_P2P_V2,
NODE_WITNESS,
msg_getdata,
msg_verack,
)
from test_framework.p2p import P2PInterface
from test_framework.test_framework import BitcoinTestFramework
Expand Down Expand Up @@ -43,7 +42,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
self.num_nodes = 3
self.extra_args = [['-prune=550', '-addrmantest'], [], []]
self.extra_args = [['-prune=550'], [], []]

def disconnect_all(self):
self.disconnect_nodes(0, 1)
Expand Down Expand Up @@ -78,16 +77,6 @@ def run_test(self):
self.log.info("Requesting block at height 2 (tip-289) must fail (ignored).")
node.send_getdata_for_block(blocks[0]) # first block outside of the 288+2 limit
node.wait_for_disconnect(5)

self.log.info("Check local address relay, do a fresh connection.")
self.nodes[0].disconnect_p2ps()
node1 = self.nodes[0].add_p2p_connection(P2PIgnoreInv())
node1.send_message(msg_verack())

node1.wait_for_addr()
#must relay address with NODE_NETWORK_LIMITED
assert_equal(node1.firstAddrnServices, expected_services)

self.nodes[0].disconnect_p2ps()

# connect unsynced node 2 with pruned NODE_NETWORK_LIMITED peer
Expand Down

0 comments on commit 5d27993

Please sign in to comment.