test: remove unneeded -maxorphantx=1000 settings
#30133
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. 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. |
It's unclear what the motivation for increasing the orphan pool is, and it seems that this not needed at all. None of these tests involve orphan transactions explicitly, and if they would occur occasionally, there is no good reason to prefer a value of 1000 over the default of 100 (see DEFAULT_MAX_ORPHAN_TRANSACTIONS).
theStack
force-pushed
the
test-remove_maxorphantx_extra_args
branch
from
75246ee
to
8950053
Compare
|
utACK 8950053 |
| [ | ||
| "-maxorphantx=1000", | ||
| ], |
There was a problem hiding this comment.
nit:
self.extra_args = [
[],
...leaving
self.extra_args = [
[
],
...is a bit weird, practically any linter/autoformatter would complain 😆
There was a problem hiding this comment.
I don't see this as a problem per se. test/functional/feature_rbf.py already had the same pattern.
There was a problem hiding this comment.
Fair, it's a stylistic nit so just an opinion, can be disregarded
|
Tested ACK 8950053 The changes don't break the tests. |
There was a problem hiding this comment.
ACK 8950053 From skimming the tests, it appears that none of these need a larger -maxorphantx.
It looks like the extra -maxorphantx=1000s have existed since each of the tests were introduced. My best guess is this was common when package limits were higher and/or tx relay and orphanage worked a bit differently, and then not cleaned up. For example, descendant packages of 1000 were allowed when mempool_packages.py was first added (along with package tracking): https://github.com/bitcoin/bitcoin/pull/6654/files#diff-4cee285ce55bc2b79f96cb2746cec9815129dd061a797d86ff4b84eda8ea0cdaR18.
feature_rbf.py: https://github.com/bitcoin/bitcoin/pull/6871/files#diff-181f65de910230cbedfa2da002451abab45b0c0baf7dd0fe4da56747a7ca07c8R75.
mempool_package_onemore.py: https://github.com/bitcoin/bitcoin/pull/15681/files#diff-e6c355d5d6411731bcc975121cae3a145896c8a0df5c210d32a6ba5fab4a104eR21
It's unclear what the motivation for increasing the orphan pool is here, and it seems that this not needed at all. None of these tests involve orphan transactions explicitly, and if they would occur occasionally, there is no good reason to prefer a value of 1000 over the default of 100 (see DEFAULT_MAX_ORPHAN_TRANSACTIONS).