diff --git a/src/Makefile.test.include b/src/Makefile.test.include index a3b6c4d9b..0fe8ea42f 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -36,6 +36,7 @@ GENERATED_TEST_FILES = $(JSON_TEST_FILES:.json=.json.h) $(RAW_TEST_FILES:.raw=.r BITCOIN_TESTS =\ test/arith_uint256_tests.cpp \ test/bignum.h \ + test/addrman_tests.cpp \ test/alert_tests.cpp \ test/allocator_tests.cpp \ test/base32_tests.cpp \ diff --git a/src/addrman.cpp b/src/addrman.cpp index ff1f7e918..078b9e168 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -329,13 +329,17 @@ void CAddrMan::Attempt_(const CService& addr, int64_t nTime) info.nAttempts++; } -CAddrInfo CAddrMan::Select_() +CAddrInfo CAddrMan::Select_(bool newOnly) { if (size() == 0) return CAddrInfo(); + if (newOnly && nNew == 0) + return CAddrInfo(); + // Use a 50% chance for choosing between tried and new table entries. - if (nTried > 0 && (nNew == 0 || GetRandInt(2) == 0)) { + if (!newOnly && + (nTried > 0 && (nNew == 0 || GetRandInt(2) == 0))) { // use a tried node double fChanceFactor = 1.0; while (1) { diff --git a/src/addrman.h b/src/addrman.h index 384b6cfdb..1123caabf 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -22,6 +22,8 @@ */ class CAddrInfo : public CAddress { + + public: //! last try whatsoever by us (memory only) int64_t nLastTry; @@ -230,8 +232,8 @@ protected: //! Mark an entry as attempted to connect. void Attempt_(const CService &addr, int64_t nTime); - //! Select an address to connect to. - CAddrInfo Select_(); + //! Select an address to connect to, if newOnly is set to true, only the new table is selected from. + CAddrInfo Select_(bool newOnly); #ifdef DEBUG_ADDRMAN //! Perform consistency check. Returns an error code or zero. @@ -532,13 +534,13 @@ public: /** * Choose an address to connect to. */ - CAddrInfo Select() + CAddrInfo Select(bool newOnly = false) { CAddrInfo addrRet; { LOCK(cs); Check(); - addrRet = Select_(); + addrRet = Select_(newOnly); Check(); } return addrRet; @@ -567,6 +569,12 @@ public: Check(); } } + + //! Ensure that bucket placement is always the same for testing purposes. + void MakeDeterministic(){ + nKey.SetNull(); //Do not use outside of tests. + } + }; #endif // BITCOIN_ADDRMAN_H diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp new file mode 100644 index 000000000..cfcdd9abb --- /dev/null +++ b/src/test/addrman_tests.cpp @@ -0,0 +1,180 @@ +// Copyright (c) 2012-2013 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include "addrman.h" +#include "test/test_bitcoin.h" +#include +#include + +#include "random.h" + +using namespace std; + +class CAddrManTest : public CAddrMan{}; + +BOOST_FIXTURE_TEST_SUITE(addrman_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(addrman_simple) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + CNetAddr source = CNetAddr("252.2.2.2:8333"); + + // Test 1: Does Addrman respond correctly when empty. + BOOST_CHECK(addrman.size() == 0); + CAddrInfo addr_null = addrman.Select(); + BOOST_CHECK(addr_null.ToString() == "[::]:0"); + + // Test 2: Does Addrman::Add work as expected. + CService addr1 = CService("250.1.1.1:8333"); + addrman.Add(CAddress(addr1), source); + BOOST_CHECK(addrman.size() == 1); + CAddrInfo addr_ret1 = addrman.Select(); + BOOST_CHECK(addr_ret1.ToString() == "250.1.1.1:8333"); + + // Test 3: Does IP address deduplication work correctly. + // Expected dup IP should not be added. + CService addr1_dup = CService("250.1.1.1:8333"); + addrman.Add(CAddress(addr1_dup), source); + BOOST_CHECK(addrman.size() == 1); + + + // Test 5: New table has one addr and we add a diff addr we should + // have two addrs. + CService addr2 = CService("250.1.1.2:8333"); + addrman.Add(CAddress(addr2), source); + BOOST_CHECK(addrman.size() == 2); + + // Test 6: AddrMan::Clear() should empty the new table. + addrman.Clear(); + BOOST_CHECK(addrman.size() == 0); + CAddrInfo addr_null2 = addrman.Select(); + BOOST_CHECK(addr_null2.ToString() == "[::]:0"); +} + +BOOST_AUTO_TEST_CASE(addrman_ports) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + CNetAddr source = CNetAddr("252.2.2.2:8333"); + + BOOST_CHECK(addrman.size() == 0); + + // Test 7; Addr with same IP but diff port does not replace existing addr. + CService addr1 = CService("250.1.1.1:8333"); + addrman.Add(CAddress(addr1), source); + BOOST_CHECK(addrman.size() == 1); + + CService addr1_port = CService("250.1.1.1:8334"); + addrman.Add(CAddress(addr1_port), source); + BOOST_CHECK(addrman.size() == 1); + CAddrInfo addr_ret2 = addrman.Select(); + BOOST_CHECK(addr_ret2.ToString() == "250.1.1.1:8333"); + + // Test 8: Add same IP but diff port to tried table, it doesn't get added. + // Perhaps this is not ideal behavior but it is the current behavior. + addrman.Good(CAddress(addr1_port)); + BOOST_CHECK(addrman.size() == 1); + bool newOnly = true; + CAddrInfo addr_ret3 = addrman.Select(newOnly); + BOOST_CHECK(addr_ret3.ToString() == "250.1.1.1:8333"); +} + + +BOOST_AUTO_TEST_CASE(addrman_select) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + CNetAddr source = CNetAddr("252.2.2.2:8333"); + + // Test 9: Select from new with 1 addr in new. + CService addr1 = CService("250.1.1.1:8333"); + addrman.Add(CAddress(addr1), source); + BOOST_CHECK(addrman.size() == 1); + + bool newOnly = true; + CAddrInfo addr_ret1 = addrman.Select(newOnly); + BOOST_CHECK(addr_ret1.ToString() == "250.1.1.1:8333"); + + + // Test 10: move addr to tried, select from new expected nothing returned. + addrman.Good(CAddress(addr1)); + BOOST_CHECK(addrman.size() == 1); + CAddrInfo addr_ret2 = addrman.Select(newOnly); + BOOST_CHECK(addr_ret2.ToString() == "[::]:0"); + + CAddrInfo addr_ret3 = addrman.Select(); + BOOST_CHECK(addr_ret3.ToString() == "250.1.1.1:8333"); +} + +BOOST_AUTO_TEST_CASE(addrman_new_collisions) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + CNetAddr source = CNetAddr("252.2.2.2:8333"); + + BOOST_CHECK(addrman.size() == 0); + + for (unsigned int i = 1; i < 4; i++){ + CService addr = CService("250.1.1."+boost::to_string(i)); + addrman.Add(CAddress(addr), source); + + //Test 11: No collision in new table yet. + BOOST_CHECK(addrman.size() == i); + } + + //Test 12: new table collision! + CService addr1 = CService("250.1.1.4"); + addrman.Add(CAddress(addr1), source); + BOOST_CHECK(addrman.size() == 3); + + CService addr2 = CService("250.1.1.5"); + addrman.Add(CAddress(addr2), source); + BOOST_CHECK(addrman.size() == 4); +} + +BOOST_AUTO_TEST_CASE(addrman_tried_collisions) +{ + CAddrManTest addrman; + + // Set addrman addr placement to be deterministic. + addrman.MakeDeterministic(); + + CNetAddr source = CNetAddr("252.2.2.2:8333"); + + BOOST_CHECK(addrman.size() == 0); + + for (unsigned int i = 1; i < 75; i++){ + CService addr = CService("250.1.1."+boost::to_string(i)); + addrman.Add(CAddress(addr), source); + addrman.Good(CAddress(addr)); + + //Test 13: No collision in tried table yet. + BOOST_TEST_MESSAGE(addrman.size()); + BOOST_CHECK(addrman.size() == i); + } + + //Test 14: tried table collision! + CService addr1 = CService("250.1.1.76"); + addrman.Add(CAddress(addr1), source); + BOOST_CHECK(addrman.size() == 74); + + CService addr2 = CService("250.1.1.77"); + addrman.Add(CAddress(addr2), source); + BOOST_CHECK(addrman.size() == 75); +} + + +BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file