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

maflcko · 2015-11-23T19:36:38Z

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:

-> settxfee 0.1
<- true

-> sendtoaddress mwEHWzBhaPYiErWWhmWW7QCjGoKNPaQQax 16
<- 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)

dcousens · 2015-11-24T01:37:49Z

utACK

jtimon · 2015-11-27T16:27:54Z

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

Anyway, better is better, utACK.

Bike-shedding: Can the global be named globalMaxTxFee ? You could also add the following to the beginning of AcceptToMemoryPool() ``` const CAmount& maxTxFee = globalMaxTxFee; ``` as a preparation for making it a explicit parameter to AcceptToMemoryPool.

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

maflcko · 2015-12-10T17:59:54Z

Rebased (trivial)

morcos · 2015-12-10T18:43:15Z

src/wallet/wallet.cpp

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


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.

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.

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

morcos · 2015-12-10T18:43:44Z

utACK

maflcko · 2016-01-07T16:16:53Z

@laanwj Anything holding this back?

laanwj · 2016-01-29T12:25:54Z

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 · 2016-01-30T01:30:29Z

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

maflcko · 2016-01-30T10:56:33Z

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

laanwj · 2016-02-02T12:34:21Z

src/main.h

 extern bool fAlerts;
+/* If the tip is older than this (in seconds), the node is considered to be in initial block download. */


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

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

laanwj · 2016-02-02T12:36:26Z

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.

jtimon · 2016-02-02T12:53:49Z

re-utACK fa1193e

…maxTxFee fa1193e [doxygen] Actually display comment (MarcoFalke) fa331db mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee (MarcoFalke)

…e with maxTxFee fa1193e [doxygen] Actually display comment (MarcoFalke) fa331db mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee (MarcoFalke)

Bitcoin 0.12 cleanup PRs 2 Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#6631 - bitcoin/bitcoin#6664 - Only the first commit (we already had the second through bitcoin/bitcoin#6825). - bitcoin/bitcoin#6669 - bitcoin/bitcoin#6887 - Only the non-QT parts. - bitcoin/bitcoin#6962 - bitcoin/bitcoin#6822 - Only first and third commits (we already had the second through an earlier PR). - bitcoin/bitcoin#7136 - Excludes Travis CI changes, and fixes to documents we don't have anymore. - bitcoin/bitcoin#7084 - bitcoin/bitcoin#7509 - bitcoin/bitcoin#7617 - bitcoin/bitcoin#7726 Part of #2074.

maflcko force-pushed the MarcoFalke-2015-mempoolMaxTxFee branch 2 times, most recently from fa4ee78 to faeb997 Compare November 23, 2015 21:50

jtimon mentioned this pull request Nov 27, 2015

Move maxTxFee out of mempool #7070

Merged

maflcko closed this Dec 1, 2015

maflcko deleted the MarcoFalke-2015-mempoolMaxTxFee branch December 1, 2015 19:30

maflcko restored the MarcoFalke-2015-mempoolMaxTxFee branch December 1, 2015 19:30

maflcko reopened this Dec 1, 2015

maflcko force-pushed the MarcoFalke-2015-mempoolMaxTxFee branch from faeb997 to ffff08a Compare December 4, 2015 16:18

morcos reviewed Dec 10, 2015
View reviewed changes

maflcko force-pushed the MarcoFalke-2015-mempoolMaxTxFee branch from ffff08a to faa1b1f Compare December 11, 2015 07:59

maflcko mentioned this pull request Jan 7, 2016

[0.12] Update release-notes.md #7309

Merged

laanwj added the Mempool label Jan 29, 2016

mempool: Replace maxFeeRate of 10000*minRelayTxFee with maxTxFee

fa331db

maflcko force-pushed the MarcoFalke-2015-mempoolMaxTxFee branch from faa1b1f to fa331db Compare January 30, 2016 10:29

laanwj reviewed Feb 2, 2016
View reviewed changes

[doxygen] Actually display comment

fa1193e

laanwj merged commit fa1193e into bitcoin:master Feb 2, 2016

maflcko deleted the MarcoFalke-2015-mempoolMaxTxFee branch February 2, 2016 18:27

maflcko mentioned this pull request Feb 2, 2016

High fee warnings need to be reworked #5202

Closed

This was referenced Mar 14, 2016

AcceptToMemoryPool: Use -maxtxfee when rejecting high fee txs #6723

Closed

Mempool should not reject wallet created transactions with sufficient fee #6725

Closed

str4d mentioned this pull request Dec 17, 2019

Bitcoin 0.12 cleanup PRs 2 zcash/zcash#4268

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

maflcko commented Nov 23, 2015

Uh oh!

dcousens commented Nov 24, 2015

Uh oh!

jtimon commented Nov 27, 2015

Uh oh!

maflcko commented Dec 10, 2015

Uh oh!

morcos Dec 10, 2015

Uh oh!

jtimon Dec 10, 2015

Uh oh!

maflcko Dec 10, 2015

Uh oh!

morcos commented Dec 10, 2015

Uh oh!

maflcko commented Jan 7, 2016

Uh oh!

laanwj commented Jan 29, 2016

Uh oh!

dcousens commented Jan 30, 2016

Uh oh!

maflcko commented Jan 30, 2016

Uh oh!

laanwj Feb 2, 2016

Uh oh!

maflcko Feb 2, 2016

Uh oh!

laanwj commented Feb 2, 2016

Uh oh!

jtimon commented Feb 2, 2016

Uh oh!

Uh oh!

		extern bool fAlerts;
		/* If the tip is older than this (in seconds), the node is considered to be in initial block download. */

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

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

Uh oh!

Conversation

maflcko commented Nov 23, 2015

Uh oh!

dcousens commented Nov 24, 2015

Uh oh!

jtimon commented Nov 27, 2015

Uh oh!

maflcko commented Dec 10, 2015

Uh oh!

morcos Dec 10, 2015

Choose a reason for hiding this comment

Uh oh!

jtimon Dec 10, 2015

Choose a reason for hiding this comment

Uh oh!

maflcko Dec 10, 2015

Choose a reason for hiding this comment

Uh oh!

morcos commented Dec 10, 2015

Uh oh!

maflcko commented Jan 7, 2016

Uh oh!

laanwj commented Jan 29, 2016

Uh oh!

dcousens commented Jan 30, 2016

Uh oh!

maflcko commented Jan 30, 2016

Uh oh!

laanwj Feb 2, 2016

Choose a reason for hiding this comment

Uh oh!

maflcko Feb 2, 2016

Choose a reason for hiding this comment

Uh oh!

laanwj commented Feb 2, 2016

Uh oh!

jtimon commented Feb 2, 2016

Uh oh!

Uh oh!