test: fix immediate tx relay in wallet_groups.py #26970
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
|
Good catch. I think that would be preferable to enable whitelisting for outbound peers rather than tackle each specific test case with workarounds like this one (the trickle mechanism is irrelevant for the large majority of tests). Without knowing the background story, it seems odd to only apply the whitelist flag to connections that weren't originated by the node. Supposedly, as the node picked it, outbound connections should be "more" trustworthy than inbound (so, disable the trickle mechanism if the address was whitelisted shouldn't be a problem). |
|
Got these results from my macOS 13 machine: master: pr branch: |
ab4efad test: fix immediate tx relay in wallet_groups.py (Sebastian Falbesoner) Pull request description: In the functional test wallet_groups.py we whitelist peers on all nodes (`[email protected]`) to enable immediate tx relay for fast mempool synchronization. However, considering that this setting only applies to inbound peers and the default test topology looks like this: ``` node0 <--- node1 <---- node2 <--- ... <-- nodeN ``` txs propagate fast only from lower- to higher-numbered nodes (i.e. "left to right" in the above diagram) and take long from higher- to lower-numbered nodes ("right to left") since in the latter direction we only have outbound peers, where the trickle relay is still active. As a consequence, if a tx is submitted from any node other than node0, the mempool synchronization can take quite long. This PR fixes this by simply adding another connection from node0 to the last node, leading to a ~2-3x speedup (5 runs measured via `time ./test/functional/wallet_groups.py` are shown): ``` master: 0m53.31s real 0m08.22s user 0m05.60s system 0m32.85s real 0m07.44s user 0m04.08s system 0m46.40s real 0m09.18s user 0m04.23s system 0m46.96s real 0m11.10s user 0m05.74s system 0m57.23s real 0m10.53s user 0m05.59s system PR: 0m19.64s real 0m09.58s user 0m05.50s system 0m18.05s real 0m07.77s user 0m04.03s system 0m18.99s real 0m07.90s user 0m04.25s system 0m17.49s real 0m07.56s user 0m03.92s system 0m18.11s real 0m07.74s user 0m03.88s system ``` Note that in most tests this is not a problem since txs very often originate from node0. ACKs for top commit: brunoerg: utACK ab4efad Tree-SHA512: 12675357e6eb5a18383f2bfe719a184c0790863b37a98749d8e757dd5dc3a36212e16a81f0a192340c11b793eda00db359c7011f46f7c27e3a093af4f5b62147
theStack
deleted the
202301-test-fix_wallet_groups_immediate_tx_relay
branch
5 participants
In the functional test wallet_groups.py we whitelist peers on all nodes (
[email protected]) to enable immediate tx relay for fast mempool synchronization. However, considering that this setting only applies to inbound peers and the default test topology looks like this:txs propagate fast only from lower- to higher-numbered nodes (i.e. "left to right" in the above diagram) and take long from higher- to lower-numbered nodes ("right to left") since in the latter direction we only have outbound peers, where the trickle relay is still active. As a consequence, if a tx is submitted from any node other than node0, the mempool synchronization can take quite long.
This PR fixes this by simply adding another connection from node0 to the last node, leading to a ~2-3x speedup (5 runs measured via
time ./test/functional/wallet_groups.pyare shown):Note that in most tests this is not a problem since txs very often originate from node0.