test: Set maxfeerate=0 in MiniWallet sendrawtransaction() #25262
Conversation
|
Concept ACK, Approach ACK Leaving other tests (e.g. feature_dbcrash.py) that use MiniWallet and have a maxfeerate check for a follow up PR? Or include in this PR? |
|
I think I covered all cases. Can you point to a line where MiniWallet::sendrawtransaction is called that passes the option? |
theStack
left a comment
theStack
left a comment
There was a problem hiding this comment.
Code-review ACK fafaad9
Leaving other tests (e.g. feature_dbcrash.py) that use MiniWallet and have a maxfeerate check for a follow up PR? Or include in this PR?
Note that other tests that pass the maxfeerate parameter explicitly directly call the node's sendrawtransaction RPC (e.g. self.nodes[0].sendrawtransaction(...)), not the MiniWallet method with the same name. This PR tackles all instances of MiniWallet.sendrawtransaction (verified that the two in mempool_accept.py are the only ones).
|
Sorry for the noise ACK fafaad9 |
|
Sure, no worries. |
…saction() fafaad9 test: Set maxfeerate=0 in MiniWallet sendrawtransaction() (MacroFake) Pull request description: It should be safe to set, because MiniWallet will only ever deal with test transactions, so loss-of-funds is not a reason to keep the feerate check. It is beneficial to set, as it makes tests less verbose to write. Also, it may speed up tests, as the fee-check can be skipped: bitcoin#25087 (comment) ACKs for top commit: michaelfolkson: ACK fafaad9 theStack: Code-review ACK fafaad9 Tree-SHA512: 94c5c163595207a295c7b21f0127d669a9307f6f8b1de5e043d43c52a6714076e2fdce65f2644308a2b90c679642c94f771dab1ff8bc5c0c8b1f5013324b3902
4 participants
It should be safe to set, because MiniWallet will only ever deal with test transactions, so loss-of-funds is not a reason to keep the feerate check.
It is beneficial to set, as it makes tests less verbose to write. Also, it may speed up tests, as the fee-check can be skipped: #25087 (comment)