highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-08-28 17:18:36 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#32823: test: Fix wait_for_getheaders() call in test_outbound_eviction_blocks_relay_only()

Browse Source 
ec004cdb86 test: Use rehash() in outbound eviction block-relay (pablomartin4btc)
26598ed21e test: Clarify roles in outbound eviction comments (pablomartin4btc)

Pull request description:

  This change avoids relying on `tip_header.hash`, which is `None` when the header is deserialized from hex during `CBlockHeader()` construction.
  Instead, `tip_header.rehash()` explicitly computes the hash, making the test behavior more robust.

  Using the explicit `rehash()` avoids depending on `wait_for_getheaders()` falling back to any received message, thus making the test more deterministic.

  This is a follow-up to #32742.

  Also, as noted in a previous review [comment](https://github.com/bitcoin/bitcoin/pull/32742#pullrequestreview-2923802386), "_the hash field is wrong either way, simply due to being the wrong type (it is an optional hex string), as opposed to an optional int_".

  ---

  The first commit intention is to improve clarity around the tests purpose, helping reviewers follow what's being verified and why. What started as a small comment during review of #32742 led me reviewing and try to improve most relevant tests comments for consistency and correctness.

ACKs for top commit:
  achow101:
    ACK ec004cdb86
  theStack:
    lgtm ACK ec004cdb86 #️⃣
  yuvicc:
    ACK ec004cdb86
  danielabrozzoni:
    ACK ec004cdb86

Tree-SHA512: 6a14dedfdc425cd806f63443b3b9f79df69a7717452739f5d7fef1b2bdba23402670d63cf1d6b66c9f1a6b460d4d4a6f185426d0a4982fa95115a234cd6baef7
This commit is contained in:
Ava Chow
2025-07-02 13:44:20 -07:00
parent 35cae56a92 ec004cdb86
commit 68ca13e1f9
1 changed files with 15 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
test/functional/p2p_outbound_eviction.py
View File

@@ -55,7 +55,7 @@ class P2POutEvict(BitcoinTestFramework):
self.log.info("Test that the peer gets evicted")
peer.wait_for_disconnect()
self.log.info("Create an outbound connection and send header but never catch up")
self.log.info("Create an outbound connection and send header but the peer never catches up")
# Mimic a node that just falls behind for long enough
# This should also apply for a node doing IBD that does not catch up in time
# Connect a peer and make it send us headers ending in our tip's parent
@@ -75,7 +75,7 @@ class P2POutEvict(BitcoinTestFramework):
self.log.info("Create an outbound connection and keep lagging behind, but not too much")
# Test that if the peer never catches up with our current tip, but it does with the
# expected work that we set when setting the timer (that is, our tip at the time)
# we do not disconnect the peer
# the node does not disconnect the peer
peer = node.add_outbound_p2p_connection(P2PInterface(), p2p_idx=0, connection_type="outbound-full-relay")
self.log.info("Mine a block so our peer starts lagging")
@@ -83,7 +83,7 @@ class P2POutEvict(BitcoinTestFramework):
best_block_hash = self.generateblock(node, output="raw(42)", transactions=[])["hash"]
peer.sync_with_ping()
self.log.info("Keep catching up with the old tip and check that we are not evicted")
self.log.info("The peer keeps catching up with the old tip; check that the node does not evict the peer")
for i in range(10):
# Generate an additional block so the peers is 2 blocks behind
prev_header = from_hex(CBlockHeader(), node.getblockheader(best_block_hash, False))
@@ -96,21 +96,21 @@ class P2POutEvict(BitcoinTestFramework):
node.setmocktime(cur_mock_time)
peer.sync_with_ping()
# Wait until we get out last call (by receiving a getheaders)
# Make the peer wait until it gets node's last call (by receiving a getheaders)
peer.wait_for_getheaders(block_hash=prev_prev_hash)
# Send a header with the previous tip (so we go back to 1 block behind)
# The peer sends a header with the previous tip (so the peer goes back to 1 block behind)
peer.send_and_ping(msg_headers([prev_header]))
prev_prev_hash = tip_header.hashPrevBlock
self.log.info("Create an outbound connection and take some time to catch up, but do it in time")
# Check that if the peer manages to catch up within time, the timeouts are removed (and the peer is not disconnected)
# We are reusing the peer from the previous case which already sent us a valid (but old) block and whose timer is ticking
# We are reusing the peer from the previous case which already sent the node a valid (but old) block and whose timer is ticking
# Send an updated headers message matching our tip
# Make the peer send an updated headers message matching our tip
peer.send_and_ping(msg_headers([from_hex(CBlockHeader(), node.getblockheader(best_block_hash, False))]))
# Wait for long enough for the timeouts to have triggered and check that we are still connected
# Wait for long enough for the timeouts to have triggered and check that the peer is still connected
cur_mock_time += (CHAIN_SYNC_TIMEOUT + 1)
node.setmocktime(cur_mock_time)
peer.sync_with_ping()
@@ -123,8 +123,10 @@ class P2POutEvict(BitcoinTestFramework):
def test_outbound_eviction_protected(self):
# This tests the eviction logic for **protected** outbound peers (that is, PeerManagerImpl::ConsiderEviction)
# Outbound connections are flagged as protected as long as they have sent us a connecting block with at least as
# much work as our current tip and we have enough empty protected_peers slots.
# Outbound connections are flagged as protected if:
# - The peer sends a connecting block with at least as much work as our current tip.
# - There are still available slots in the node's protected_peers list.
# This test ensures that such protected outbound peers are not disconnected even after chain sync and headers timeouts.
node = self.nodes[0]
cur_mock_time = node.mocktime
tip_header = from_hex(CBlockHeader(), node.getblockheader(node.getbestblockhash(), False))
@@ -145,7 +147,7 @@ class P2POutEvict(BitcoinTestFramework):
peer.wait_for_getheaders(block_hash=tip_header.hashPrevBlock)
cur_mock_time += (HEADERS_RESPONSE_TIME + 1)
node.setmocktime(cur_mock_time)
self.log.info("Test that the node does not get evicted")
self.log.info("Test that the peer does not get evicted")
peer.sync_with_ping()
node.disconnect_p2ps()
@@ -205,7 +207,7 @@ class P2POutEvict(BitcoinTestFramework):
cur_mock_time += (HEADERS_RESPONSE_TIME + 1)
node.setmocktime(cur_mock_time)
self.log.info("Check how none of the honest nor protected peers was evicted but all the misbehaving unprotected were")
self.log.info("Check that none of the honest or protected peers were evicted, but all misbehaving unprotected peers were")
for peer in protected_peers + honest_unprotected_peers:
peer.sync_with_ping()
for peer in misbehaving_unprotected_peers:
@@ -233,7 +235,7 @@ class P2POutEvict(BitcoinTestFramework):
cur_mock_time += (CHAIN_SYNC_TIMEOUT + 1)
node.setmocktime(cur_mock_time)
peer.sync_with_ping()
peer.wait_for_getheaders(block_hash=tip_header.hash)
peer.wait_for_getheaders(block_hash=tip_header.rehash())
cur_mock_time += (HEADERS_RESPONSE_TIME + 1)
node.setmocktime(cur_mock_time)
self.log.info("Test that the peer gets evicted")