test: Remove MiniWallet mempool_valid option #25356

1. 
   MarcoFalke commented at 12:08 pm on June 13, 2022: member

   It seems an unnecessary burden to force MiniWallet call-sites to figure out for each tx whether it is mempool valid or not. The result will only be used for internal sanity checks. So remove the option:

   • Replace the vsize sanity check with a call to get_vsize().
   • Drop the fee check. Hopefully any future bug here will be caught by code-review or otherwise.
2. fanquake added the label Tests on Jun 13, 2022
3. fanquake requested review from instagibbs on Jun 13, 2022
4. instagibbs approved
5. 
   instagibbs commented at 1:52 pm on June 13, 2022: member

   ACK fac709456a94eec72e5b75df147e9a76f61a63b0

   It’s a confusing interface as-is, and these hidden checks imo are more likely to make tests less understandable.

6. test: Remove MiniWallet mempool_valid option fa779de665
7. MarcoFalke force-pushed on Jun 13, 2022
8. theStack approved
9. 
   theStack commented at 5:20 pm on June 13, 2022: member

   Code-review ACK fa779de665227cf15a4056959e9753618cc013ac

   Agree that this creates more confusion than it helps and can be removed. The journey of this parameter’s logic went from “always test mempool validity” (fa085b470a9647f3b261f506b46f4e7ca2baf0b0, introduction of the method) to “only test mempool validity if set to True” (a498acce4514d83d8dafcebaad522a89b6dc70fa, for performance reasons) to “never test mempool validity” (this PR).

10. MarcoFalke merged this on Jun 13, 2022
11. MarcoFalke closed this on Jun 13, 2022

    ---

12. MarcoFalke deleted the branch on Jun 13, 2022
13. sidhujag referenced this in commit 546e0832fd on Jun 15, 2022
14. Fabcien referenced this in commit 493af7ae84 on Dec 2, 2022
15. DrahtBot locked this on Jun 13, 2023

Contributors
MarcoFalke instagibbs theStack

Labels
Tests