test: remove unneeded -maxorphantx=1000 settings #30133

1. 
   theStack commented at 8:36 pm on May 17, 2024: contributor

   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).
2. 
   DrahtBot commented at 8:36 pm on May 17, 2024: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	maflcko, AngusP, edilmedeiros, glozow

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

3. DrahtBot added the label Tests on May 17, 2024
4. test: remove unneeded `-maxorphantx=1000` settings

   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).

   8950053636
5. theStack force-pushed on May 17, 2024
6. 
   maflcko commented at 8:32 am on May 18, 2024: member

   utACK 8950053636cb38ed85fe2d58b53e5d0acb35c390
7. in test/functional/mempool_packages.py:34 in 8950053636

   30@@ -31,10 +31,8 @@ def set_test_params(self):
   31         self.noban_tx_relay = True
   32         self.extra_args = [
   33             [
   34-                "-maxorphantx=1000",
   35             ],
   

   ---

   ---

   

   AngusP commented at 7:31 pm on May 18, 2024:

   nit:

   0        self.extra_args = [
   1            [],
   2            ...
   

   leaving

   0        self.extra_args = [
   1            [
   2            ],
   3            ...
   

   is a bit weird, practically any linter/autoformatter would complain 😆

   ---

   

   edilmedeiros commented at 8:05 pm on May 18, 2024:

   I don’t see this as a problem per se. test/functional/feature_rbf.py already had the same pattern.

   ---

   

   AngusP commented at 8:10 pm on May 18, 2024:

   Fair, it’s a stylistic nit so just an opinion, can be disregarded
8. AngusP approved
9. 
   AngusP commented at 7:35 pm on May 18, 2024: contributor

   tACK 8950053636cb38ed85fe2d58b53e5d0acb35c390

   I did wonder whether maxorphantx was zeroed or otherwise not the default 100 for functests which would perhaps explain it being set here, but I can’t see anything to indicate that being the case.

10. 
    edilmedeiros commented at 8:06 pm on May 18, 2024: contributor

    Tested ACK 8950053636cb38ed85fe2d58b53e5d0acb35c390

    The changes don’t break the tests. I took some time to think if the removed params would be essential, but can’t see how.

11. 
    glozow commented at 8:45 am on May 20, 2024: member

    ACK 8950053636cb38ed85fe2d58b53e5d0acb35c390 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

12. glozow merged this on May 20, 2024
13. glozow closed this on May 20, 2024

    ---

14. theStack deleted the branch on May 20, 2024

Contributors
theStack DrahtBot maflcko AngusP edilmedeiros glozow

Labels
Tests