mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee

MarcoFalke commented at 7:36 pm on November 23, 2015: member

maxTxFee is the absolute upper bound on fees of transactions created by the wallet.

10000*minRelayTxFee_default = 0.1 BTC/kB
maxTxFee_default = 0.1 BTC

Further info: This check was initially only added to the mempool to reject high fee raw transactions by default (sendrawtransaction) but was later extended to also check wallet transactions. minRelayTxFee may not be the best config param to adjust fee behavior so switching to the already used maxTxFee could make sense.

This fixes the issue:

0-> settxfee 0.1
1<- true
2
3-> sendtoaddress mwEHWzBhaPYiErWWhmWW7QCjGoKNPaQQax 16
4<- Error: The transaction was rejected! This might happen if some of the coins in your wallet were already spent, such as if you used a copy of wallet.dat and coins were spent in the copy but not marked as spent here. (code -4)

MarcoFalke force-pushed on Nov 23, 2015

dcousens commented at 1:37 am on November 24, 2015: contributor

utACK

jtimon commented at 4:27 pm on November 27, 2015: contributor

Maybe you prefer to pass it to AcceptToMemoryPool explicitly directly. That would be a little bit more disruptive though.

Anyway, better is better, utACK.

EDIT: never mind the bike-shedding nit, the name maxTxFee has to be preserved because it’s currently used like that in the wallet.

MarcoFalke closed this on Dec 1, 2015

MarcoFalke deleted the branch on Dec 1, 2015

MarcoFalke restored the branch on Dec 1, 2015

MarcoFalke reopened this on Dec 1, 2015

MarcoFalke force-pushed on Dec 4, 2015

MarcoFalke commented at 5:59 pm on December 10, 2015: member

Rebased (trivial)

in src/wallet/wallet.cpp: in ffff08a86c outdated

2171@@ -2173,6 +2172,7 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey)
2172             if (!wtxNew.AcceptToMemoryPool(false))
2173             {
2174                 // This must not fail. The transaction has already been signed and recorded.
2175+                // Also the fee never exceeds maxTxFee which forces mempool rejection

morcos commented at 6:43 pm on December 10, 2015:

nit: this comment didn’t really help me. maybe // Wallet uses same maxTxFee as ATMP so tx should not be rejected for fee but maybe its just unnecessary.

jtimon commented at 7:11 pm on December 10, 2015:

But, please, don’t use acronyms for the name functions in the comments: the full name of the function is something that can be seen in grep.

MarcoFalke commented at 7:55 pm on December 10, 2015:

Guess I will drop this because it is already mentioned in wallet.h: https://github.com/bitcoin/bitcoin/pull/7084/files#diff-12635a58447c65585f51d32b7e04075bR205

morcos commented at 6:43 pm on December 10, 2015: member

utACK

MarcoFalke force-pushed on Dec 11, 2015

MarcoFalke commented at 4:16 pm on January 7, 2016: member

@laanwj Anything holding this back?

laanwj added the label Mempool on Jan 29, 2016

laanwj commented at 12:25 pm on January 29, 2016: member

You should also move -maxtxfee in init; it’s still under the wallet options, whereas it now applies to transactions submitted over RPC too.

dcousens commented at 1:30 am on January 30, 2016: contributor

utACK faa1b1f, but waiting on fix for init option as specified by @laanwj above.

mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee fa331db68b

MarcoFalke force-pushed on Jan 30, 2016

MarcoFalke commented at 10:56 am on January 30, 2016: member

I have also fixed the doxygen comments in main.h. Let me know if this is not wanted.

in src/main.h: in fa331db68b outdated

143@@ -138,8 +144,12 @@ extern unsigned int nBytesPerSigOp;
144 extern bool fCheckBlockIndex;
145 extern bool fCheckpointsEnabled;
146 extern size_t nCoinCacheUsage;
147+/** A fee rate smaller than this is considered zero fee (for relaying, mining and transaction creation) */
148 extern CFeeRate minRelayTxFee;
149+/** Absolute maximum transaction fee (in satoshis) used by wallet and mempool (rejects high fee in sendrawtransaction) */
150+extern CAmount maxTxFee;
151 extern bool fAlerts;
152+/* If the tip is older than this (in seconds), the node is considered to be in initial block download. */

laanwj commented at 12:34 pm on February 2, 2016:

Nit: /**, otherwise it won’t show up in doxygen at all

MarcoFalke commented at 12:42 pm on February 2, 2016:

Good catch! Need to bookmark https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#doxygen-comments I guess.

laanwj commented at 12:36 pm on February 2, 2016: member

I have also fixed the doxygen comments in main.h. Let me know if this is not wanted.

I’m ok with that, but let’s limit the amount of new changes from here on, as they keep people busy with reviewing and commenting :) I’m trying to get this ready for merge.

[doxygen] Actually display comment fa1193e254

jtimon commented at 12:53 pm on February 2, 2016: contributor

re-utACK fa1193e25440671300f428670c14dd15110f7714

laanwj merged this on Feb 2, 2016

laanwj closed this on Feb 2, 2016

laanwj referenced this in commit 11d74f6a6b on Feb 2, 2016

MarcoFalke deleted the branch on Feb 2, 2016

codablock referenced this in commit 9a9e2ca223 on Sep 16, 2017

codablock referenced this in commit dd0c6248fc on Sep 19, 2017

codablock referenced this in commit f79541b51d on Dec 9, 2017

codablock referenced this in commit d27ee5af78 on Dec 9, 2017

codablock referenced this in commit 0afbd1c3de on Dec 11, 2017

zkbot referenced this in commit 8713d73daf on Dec 18, 2019

zkbot referenced this in commit 2da77edbfe on Dec 18, 2019

zkbot referenced this in commit 577f7ef72a on Dec 18, 2019

MarcoFalke locked this on Sep 8, 2021

mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee #7084