policy: Remove promiscuousmempoolflags #13527

1. 
   MarcoFalke commented at 9:44 pm on June 23, 2018: member

   It seems odd to clutter validation code with features that can only ever be used for testing (testnet or regtest). Removing that test-only code makes the mempool logic less painful to understand and easier to reason about when changed or refactored in the future.
2. policy: Remove promiscuousmempoolflags faa24441ec
3. MarcoFalke added the label TX fees and policy on Jun 23, 2018
4. MarcoFalke added the label Validation on Jun 23, 2018
5. 
   MarcoFalke commented at 10:18 pm on June 23, 2018: member

   As a side note: The current implementation is broken anyway because it returns a dirty state in some cases, but doesn’t fail to add the transaction to the mempool. See explanation in the first comment of #13407#issue-193085240
6. 
   laanwj commented at 12:58 pm on June 24, 2018: member

   Concept ACK. Though I wonder why this was the case. Does removing this reduce test coverage?
7. 
   MarcoFalke commented at 1:10 pm on June 24, 2018: member

   I believe this increases overall test coverage.

   Clearly by removing (partly untested) code in ATMP the coverage increases there. Though, to the best of my knowledge the runtime error of (“TestBlockValidity failed”) in miner is no longer covered. If you think this coverage is important, I am happy to volunteer to write a unit test for this, but I think introducing bugs in other modules just so that we can abuse functional tests for an increase in line coverage is not the way to go.

8. 
   sipa commented at 0:51 am on June 27, 2018: member

   @morcos You introduced this in 4f7ff004978. Any comments?
9. 
   morcos commented at 1:47 am on June 27, 2018: member

   I’m a bit rusty on all this code, but in principle I’m fine with no longer having that flag.

   Concept ACK

10. 
    sipa commented at 3:06 am on July 14, 2018: member

    utACK faa24441ec047ec336b86f586016b9d318c1c0ad. I didn’t review the test changes.
11. 
    fanquake commented at 1:14 pm on July 15, 2018: member

    After merging this there are still a couple mentions of -promiscuousmempoolflags: https://github.com/bitcoin/bitcoin/blob/8803c9132a78d8182bd828a29f7051fc7688f934/src/init.cpp#L349 https://github.com/bitcoin/bitcoin/blob/8803c9132a78d8182bd828a29f7051fc7688f934/test/lint/check-doc.py#L25
12. 
    MarcoFalke commented at 3:51 pm on July 15, 2018: member

    @fanquake This is wanted. We could remove those in the version that follows the version which includes this pull request.
13. 
    DrahtBot commented at 5:33 pm on July 25, 2018: member

    • #13868 (Remove unused fScriptChecks parameter from CheckInputs by Empact)
    • #13407 ([refactor, move-only-ish] Refactor mempool accept/reject logic by skeees)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

14. 
    laanwj commented at 1:38 pm on August 7, 2018: member

    utACK 870bd4c73ddf494dc23c658bf0fb672ee0109158
15. laanwj merged this on Aug 7, 2018
16. laanwj closed this on Aug 7, 2018

    ---

17. laanwj referenced this in commit e7ea858729 on Aug 7, 2018
18. MarcoFalke deleted the branch on Aug 7, 2018
19. MarcoFalke referenced this in commit 75795603dd on Oct 21, 2018
20. UdjinM6 referenced this in commit c70cb311a4 on Sep 8, 2019
21. PastaPastaPasta referenced this in commit 6fd35565a3 on Sep 9, 2019
22. barrystyle referenced this in commit 5cb846b051 on Jan 22, 2020
23. DrahtBot locked this on Sep 8, 2021

Contributors
MarcoFalke laanwj sipa morcos fanquake DrahtBot

Labels
TX fees and policy Validation