Merge #6103: backport: bitcoin#18638

1840c9441a fix: drop extra pings - follow up for #18638 (UdjinM6)
264e7f9e62 Merge #18638: net: Use mockable time for ping/pong, add tests (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  Split from https://github.com/dashpay/dash/pull/6102

  ## What was done?
  So far as bitcoin#19499 is backported, bitcoin#18638 can be finished and removed related workarounds.

  ## How Has This Been Tested?
  Run unit/functional tests

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK 1840c9441a
  PastaPastaPasta:
    utACK 1840c9441a

Tree-SHA512: f34657c14514d6ee596175b3faf9ee44e58e8b0339939a0708d58ab2d119786830c183f9b236ed87c08ef8e1dbd031a38fc596b5aa4d38e10521658df4330e79
This commit is contained in:
pasta 2024-07-15 10:55:43 -05:00
commit dad9ff1108
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
3 changed files with 5 additions and 14 deletions

View File

@ -785,8 +785,7 @@ void CNode::CopyStats(CNodeStats& stats)
bool CNode::ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete) bool CNode::ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete)
{ {
complete = false; complete = false;
// TODO: use mocktime here after bitcoin#19499 is backported const auto time = GetTime<std::chrono::microseconds>();
const auto time = std::chrono::microseconds(GetTimeMicros());
LOCK(cs_vRecv); LOCK(cs_vRecv);
m_last_recv = std::chrono::duration_cast<std::chrono::seconds>(time); m_last_recv = std::chrono::duration_cast<std::chrono::seconds>(time);
nRecvBytes += msg_bytes.size(); nRecvBytes += msg_bytes.size();

View File

@ -92,9 +92,7 @@ class PingPongTest(BitcoinTestFramework):
self.mock_forward(ping_delay) self.mock_forward(ping_delay)
no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message) no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message)
no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce)) no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce))
# TODO this check doesn't work due to partial 18638 self.check_peer_info(pingtime=ping_delay, minping=ping_delay, pingwait=None)
# re-enable it after #19499 is done
# self.check_peer_info(pingtime=ping_delay, minping=ping_delay, pingwait=None)
self.log.info('Check that minping is decreased after a fast roundtrip') self.log.info('Check that minping is decreased after a fast roundtrip')
# mock time PING_INTERVAL ahead to trigger node into sending a ping # mock time PING_INTERVAL ahead to trigger node into sending a ping
@ -104,9 +102,7 @@ class PingPongTest(BitcoinTestFramework):
self.mock_forward(ping_delay) self.mock_forward(ping_delay)
no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message) no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message)
no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce)) no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce))
# TODO this check doesn't work due to partial 18638 self.check_peer_info(pingtime=ping_delay, minping=ping_delay, pingwait=None)
# re-enable it after #19499 is done
# self.check_peer_info(pingtime=ping_delay, minping=ping_delay, pingwait=None)
self.log.info('Check that peer is disconnected after ping timeout') self.log.info('Check that peer is disconnected after ping timeout')
assert 'ping' not in no_pong_node.last_message assert 'ping' not in no_pong_node.last_message

View File

@ -50,10 +50,7 @@ class NetTest(DashTestFramework):
self.wallet.generate(1) self.wallet.generate(1)
# Get out of IBD for the getpeerinfo tests. # Get out of IBD for the getpeerinfo tests.
self.nodes[0].generate(101) self.nodes[0].generate(101)
# Wait for one ping/pong to finish so that we can be sure that there is no chatter between nodes for some time
# Especially the exchange of messages like getheaders and friends causes test failures here
self.nodes[0].ping()
self.wait_until(lambda: all(['pingtime' in n for n in self.nodes[0].getpeerinfo()]))
# By default, the test framework sets up an addnode connection from # By default, the test framework sets up an addnode connection from
# node 1 --> node0. By connecting node0 --> node 1, we're left with # node 1 --> node0. By connecting node0 --> node 1, we're left with
# the two nodes being connected both ways. # the two nodes being connected both ways.
@ -173,8 +170,7 @@ class NetTest(DashTestFramework):
# Connect nodes both ways. # Connect nodes both ways.
self.connect_nodes(0, 1) self.connect_nodes(0, 1)
self.connect_nodes(1, 0) self.connect_nodes(1, 0)
self.nodes[1].ping()
self.wait_until(lambda: all(['pingtime' in n for n in self.nodes[1].getpeerinfo()]))
assert_equal(self.nodes[1].getnetworkinfo()['connections'], 2) assert_equal(self.nodes[1].getnetworkinfo()['connections'], 2)
assert_equal(self.nodes[1].getnetworkinfo()['connections_in'], 1) assert_equal(self.nodes[1].getnetworkinfo()['connections_in'], 1)
assert_equal(self.nodes[1].getnetworkinfo()['connections_out'], 1) assert_equal(self.nodes[1].getnetworkinfo()['connections_out'], 1)