Merge bitcoin/bitcoin#29704: test: make p2p_handshake robust against timeoffset warnings

032a597482 test: make p2p_handshake robust against timeoffset warnings (stickies-v)

Pull request description:

  The new `p2p_handshake` test requires that limited nodes are not peered with when the node's system time exceeds ~ 24h of the node's chaintip timestamp, as per [`PeerManagerImpl::GetDesirableServiceFlags`](2ffaa92702/src/net_processing.cpp (L1717)).

  By patching this test to modify the timestamp of the chaintip as opposed to mocking the node's system time, we make it resilient to future commits where the node raises a warning if it detects its system time is too much out of sync with its outbound peers.

  Resolves a silent merge conflict in https://github.com/bitcoin/bitcoin/pull/29623, that is changing the warning behaviour when significant time differences with outbound peers are detected, [failing the test as it's currently in master](https://cirrus-ci.com/task/6553996884705280?logs=ci#L4666).

  Considerations/alternatives I've thought of:
  - could add `self.setup_clean_chain = True` to `self.set_test_params()`, to avoid creating a new tip with a (much) older date, but it doesn't seem to matter?
  - could avoid using `setmocktime` altogether and instead use [`create_block`](2ffaa92702/test/functional/test_framework/blocktools.py (L68)) instead, but that seems like it'll be a lot more verbose and I don't think it's worth it?

  Big thanks to theStack for his time in discussing this with me offline.

ACKs for top commit:
  maflcko:
    lgtm ACK 032a597482
  theStack:
    ACK 032a597482
  brunoerg:
    crACK 032a597482
  BrandonOdiwuor:
    Code Review ACK 032a597482

Tree-SHA512: 407564754a100bc9252f5737182de2e111993944ec9a0463a4a43195ce98cd1119de982c8fe5f7531ddb56603043812bf7bf2163a780d30b6df03a072c3308c3
pull/29712/head
fanquake 1 month ago
commit a175efe768
No known key found for this signature in database
GPG Key ID: 2EEB9F5CC09526C1

@ -62,6 +62,11 @@ class P2PHandshakeTest(BitcoinTestFramework):
assert (services & desirable_service_flags) == desirable_service_flags
self.add_outbound_connection(node, conn_type, services, wait_for_disconnect=False)
def generate_at_mocktime(self, time):
self.nodes[0].setmocktime(time)
self.generate(self.nodes[0], 1)
self.nodes[0].setmocktime(0)
def run_test(self):
node = self.nodes[0]
self.log.info("Check that lacking desired service flags leads to disconnect (non-pruned peers)")
@ -71,10 +76,10 @@ class P2PHandshakeTest(BitcoinTestFramework):
DESIRABLE_SERVICE_FLAGS_FULL, expect_disconnect=False)
self.log.info("Check that limited peers are only desired if the local chain is close to the tip (<24h)")
node.setmocktime(int(time.time()) + 25 * 3600) # tip outside the 24h window, should fail
self.generate_at_mocktime(int(time.time()) - 25 * 3600) # tip outside the 24h window, should fail
self.test_desirable_service_flags(node, [NODE_NETWORK_LIMITED | NODE_WITNESS],
DESIRABLE_SERVICE_FLAGS_FULL, expect_disconnect=True)
node.setmocktime(int(time.time()) + 23 * 3600) # tip inside the 24h window, should succeed
self.generate_at_mocktime(int(time.time()) - 23 * 3600) # tip inside the 24h window, should succeed
self.test_desirable_service_flags(node, [NODE_NETWORK_LIMITED | NODE_WITNESS],
DESIRABLE_SERVICE_FLAGS_PRUNED, expect_disconnect=False)

Loading…
Cancel
Save