Merge bitcoin/bitcoin#24662: addrman: Use system time instead of adjusted network time

fadd8b2676 addrman: Use system time instead of adjusted network time (MarcoFalke)

Pull request description:

  This changes addrman to use system time for address relay instead of the network adjusted time.

  This is an improvement, because network time has multiple issues:

  * It is non-monotonic, even if the system time is monotonic.
  * It may be wrong, even if the system time is correct.
  * It may be wrong, if the system time is wrong. For example, when the node has limited number of connections (`4`), or the system time is wrong by too much (more than +-70 minutes), or the system time only got wrong after timedata collected more than half of the entries while the time was correct, ...)

  This may slightly degrade addr relay for nodes where timedata successfully adjusted the time. Addr relay can already deal with minor offsets of up to 10 minutes. Offsets larger than this should still allow addr relay and not result in a DoS.

ACKs for top commit:
  dergoegge:
    Code review ACK fadd8b2676

Tree-SHA512: b6c178fa01161544e5bc76c4cb23e11bcc30391f7b7a64accce864923766647bcfce2e8ae21d36fb1ffc1afa07bc46415aca612405bd8d4cc1f319c92a08498f
This commit is contained in:
fanquake
2022-08-05 08:58:09 +01:00
9 changed files with 28 additions and 29 deletions

View File

@@ -225,7 +225,7 @@ BOOST_AUTO_TEST_CASE(addrman_new_multiplicity)
{
auto addrman = std::make_unique<AddrMan>(EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node));
CAddress addr{CAddress(ResolveService("253.3.3.3", 8333), NODE_NONE)};
const auto start_time{AdjustedTime()};
const auto start_time{Now<NodeSeconds>()};
addr.nTime = start_time;
// test that multiplicity stays at 1 if nTime doesn't increase
@@ -295,15 +295,15 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
BOOST_CHECK_EQUAL(vAddr1.size(), 0U);
CAddress addr1 = CAddress(ResolveService("250.250.2.1", 8333), NODE_NONE);
addr1.nTime = AdjustedTime(); // Set time so isTerrible = false
addr1.nTime = Now<NodeSeconds>(); // Set time so isTerrible = false
CAddress addr2 = CAddress(ResolveService("250.251.2.2", 9999), NODE_NONE);
addr2.nTime = AdjustedTime();
addr2.nTime = Now<NodeSeconds>();
CAddress addr3 = CAddress(ResolveService("251.252.2.3", 8333), NODE_NONE);
addr3.nTime = AdjustedTime();
addr3.nTime = Now<NodeSeconds>();
CAddress addr4 = CAddress(ResolveService("252.253.3.4", 8333), NODE_NONE);
addr4.nTime = AdjustedTime();
addr4.nTime = Now<NodeSeconds>();
CAddress addr5 = CAddress(ResolveService("252.254.4.5", 8333), NODE_NONE);
addr5.nTime = AdjustedTime();
addr5.nTime = Now<NodeSeconds>();
CNetAddr source1 = ResolveIP("250.1.2.1");
CNetAddr source2 = ResolveIP("250.2.3.3");
@@ -329,7 +329,7 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr)
CAddress addr = CAddress(ResolveService(strAddr), NODE_NONE);
// Ensure that for all addrs in addrman, isTerrible == false.
addr.nTime = AdjustedTime();
addr.nTime = Now<NodeSeconds>();
addrman->Add({addr}, ResolveIP(strAddr));
if (i % 8 == 0)
addrman->Good(addr);
@@ -822,7 +822,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
// Ensure test of address fails, so that it is evicted.
// Update entry in tried by setting last good connection in the deep past.
BOOST_CHECK(!addrman->Good(info, NodeSeconds{1s}));
addrman->Attempt(info, /*fCountFailure=*/false, AdjustedTime() - 61s);
addrman->Attempt(info, /*fCountFailure=*/false, Now<NodeSeconds>() - 61s);
// Should swap 36 for 19.
addrman->ResolveCollisions();
@@ -966,7 +966,7 @@ BOOST_AUTO_TEST_CASE(addrman_update_address)
CNetAddr source{ResolveIP("252.2.2.2")};
CAddress addr{CAddress(ResolveService("250.1.1.1", 8333), NODE_NONE)};
const auto start_time{AdjustedTime() - 10000s};
const auto start_time{Now<NodeSeconds>() - 10000s};
addr.nTime = start_time;
BOOST_CHECK(addrman->Add({addr}, source));
BOOST_CHECK_EQUAL(addrman->size(), 1U);