Commit e5f9218
committed
Merge bitcoin/bitcoin#32742: test: fix catchup loop in outbound eviction functional test
dd8447f test: fix catchup loop in outbound eviction functional test (Sebastian Falbesoner)
Pull request description:
In the course of working on an equivalent of #32421 for the `CBlockHeader` class, I noticed that the [catchup loop in the outbound eviction functional test](https://github.com/bitcoin/bitcoin/blob/19765dca197a6334e5ad3985a30c7836374d5c26/test/functional/p2p_outbound_eviction.py#L86-L103) currently has a small flaw: the contained waiting for a `getheaders` message
https://github.com/bitcoin/bitcoin/blob/19765dca197a6334e5ad3985a30c7836374d5c26/test/functional/p2p_outbound_eviction.py#L98-L99
only waits for _any_ such message instead of one with the intended block hash after the first iteration. The reason is that the `prev_prev_hash` variable is set incorrectly, since the `tip_header` instance is not updated and its field `.hash` is None [1]. Fix that by updating `tip_header` after generating a new block and also use the correct field on it -- we want the tip header's previous hash (`.hashPrevBlock`), which will be the previous-previous hash in the next iteration as intended.
Can be demonstrated by adding a debug output for `prev_prev_hash`, e.g.
```diff
diff --git a/test/functional/p2p_outbound_eviction.py b/test/functional/p2p_outbound_eviction.py
index 30ac85e..9886a49512 100755
--- a/test/functional/p2p_outbound_eviction.py
+++ b/test/functional/p2p_outbound_eviction.py
@@ -85,6 +85,7 @@ class P2POutEvict(BitcoinTestFramework):
self.log.info("Keep catching up with the old tip and check that we are not evicted")
for i in range(10):
+ print(f"i={i}, prev_prev_hash={prev_prev_hash}")
# Generate an additional block so the peers is 2 blocks behind
prev_header = from_hex(CBlockHeader(), node.getblockheader(best_block_hash, False))
best_block_hash = self.generateblock(node, output="raw(42)", transactions=[])["hash"]
```
master branch
```
...
i=0, prev_prev_hash=21722572577213525620063947414919931742473663114977483853465070858884938201585
i=1, prev_prev_hash=None
i=2, prev_prev_hash=None
i=3, prev_prev_hash=None
i=4, prev_prev_hash=None
i=5, prev_prev_hash=None
i=6, prev_prev_hash=None
i=7, prev_prev_hash=None
i=8, prev_prev_hash=None
i=9, prev_prev_hash=None
...
```
PR branch
```
...
i=0, prev_prev_hash=21722572577213525620063947414919931742473663114977483853465070858884938201585
i=1, prev_prev_hash=23204083306104595181276643925327085197417756603258684897360269464191973063397
i=2, prev_prev_hash=18117007775254206852722585270408843074799046031613422902091537272077477361634
i=3, prev_prev_hash=30556804635951812756130312631227721973553160707632138130845362630877961299882
i=4, prev_prev_hash=16476515948153779819467376247405243058769281687868039119037064816106574626111
i=5, prev_prev_hash=14965506521435221774966695805624206855826023174786191695076697927307467053159
i=6, prev_prev_hash=14510815979277079515923749862202324542606166669768865640616202929053689167149
i=7, prev_prev_hash=15360268707191667685151951417759114642582372006627142890517655217275478262166
i=8, prev_prev_hash=55984929479619644661389829786223559362979512070332438490054115824374865094074
i=9, prev_prev_hash=6591573629906616262191232272909118561529534571119028248829355592878183757083
...
```
[1] that's in my opinion another example how caching hashes is confusing and easy to be misused; it's better to remove it and just compute the hash on-the-fly, so returning None is not even possible anymore
ACKs for top commit:
maflcko:
lgtm ACK dd8447f
mzumsande:
Code Review ACK dd8447f
pablomartin4btc:
cr-ACK dd8447f
Tree-SHA512: bd8e786b52e3e96661453006140d6b8fad5a35f1c8d38243c61df52b19c97cd3800404745a2f9603bcdf0006e9780b4f15f8f7e4fa34ff07d52dba04d87b68d01 file changed
+2
-1
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
88 | 88 | | |
89 | 89 | | |
90 | 90 | | |
| 91 | + | |
91 | 92 | | |
92 | 93 | | |
93 | 94 | | |
| |||
100 | 101 | | |
101 | 102 | | |
102 | 103 | | |
103 | | - | |
| 104 | + | |
104 | 105 | | |
105 | 106 | | |
106 | 107 | | |
| |||
0 commit comments