AcceptToMemoryPool: Don’t fee-check wallet-created transactions #6726

1. 
   MarcoFalke commented at 9:17 am on September 26, 2015: member

   This fixes #6725 by limiting the scope of REJECT_HIGHFEE back to raw transactions (as intended by it’s original author).
2. 
   laanwj commented at 9:26 am on September 26, 2015: member

   Concept-level:

   a) Should we be disabling sanity checks for our own transactions? It provides a level of fallback safety. HIGHFEE check exists for a good reason

   b) We shouldn’t be generating transactions that our own mempool doesn’s accept: good chance that other nodes won’t accept them either

3. laanwj added the label Wallet on Sep 26, 2015
4. 
   MarcoFalke commented at 9:34 am on September 26, 2015: member

   @laanwj “sanity checks for our own [wallet created] transactions …”

   … should be executed by the wallet code. (Which it already does)

   @laanwj “good chance that other nodes won’t accept them either”.

   This PR only fixes the fee-sanity-check. And other nodes accepting txs over the network don’t care about (high) fees when adding them to their own mempool. (I can add test cases to this PR if you want)

5. 
   sipa commented at 12:35 pm on September 26, 2015: member

   @laanwj The reason the fee check in AcceptToMemoryPool exists is because there is no other way to sanity check the fee of raw transactions. Wallet transactions are always created with a known fee anyway.
6. 
   jonasschnelli commented at 6:52 am on September 28, 2015: contributor

   I understand the motivation of this PR. Does it make sense to have this absurde-fee-checks in the mempool logic? I think the mempool should not enforce a such policy.

   Conceptual i think having a function that could check if a transaction would be accepted to the mempool (or AcceptToMemoryPool() with a dontAdd boolean).

   IMO the wallets send functions as well as the sendrawtransaction RPC call should do the necessary checks.

7. 
   MarcoFalke commented at 8:50 am on September 28, 2015: member

   @jonasschnelli Does it make sense to have this absurde-fee-checks in the mempool logic?

   No, see OP of #2949: The only reason to do this is “because the inputs may not be known”.

   Though, I’d rather not bloat this PR and leave the dry-run logic (or whatever is better) to a follow up PR.

8. 
   morcos commented at 0:31 am on October 19, 2015: member

   @MarcoFalke I don’t understand this:

   @laanwj “sanity checks for our own [wallet created] transactions …”

   … should be executed by the wallet code. (Which it already does)

   It looks to me like you’re removing any sanity checking at all from the wallet code. Although there is still some from the GUI. see #5202.

   As the author of the fee estimation code, I can tell you I would not feel comfortable if there was not a hard coded (or better yet configurable) high fee check.

9. AcceptToMemoryPool: Don't fee-check wallet-created transactions 18f5df4740
10. MarcoFalke force-pushed on Oct 22, 2015
11. 
    laanwj commented at 11:11 am on November 5, 2015: member

    Nack - see discussion in #6887, I don’t like removing highfee checks
12. laanwj closed this on Nov 5, 2015

    ---

13. MarcoFalke deleted the branch on Nov 5, 2015
14. MarcoFalke locked this on Sep 8, 2021

Contributors
MarcoFalke laanwj sipa jonasschnelli morcos

Labels
Wallet