Separate different uses of minimum fees

morcos commented at 9:03 pm on December 19, 2016: member

I’ve been meaning to do this for a while. There is no behavior change here, but there were unintended consequences of having these all of these different notions linked to minRelayTxFee.

blockmintxfee should be set by miners to reflect their marginal cost of transmitting extra bytes.
incrementalrelayfee was previously hard-coded to the default minRelayTxFee, but it should be configurable so it can be adjusted to reflect the cost of network wide relay of transactions.
dustrelayfee should not be accidentally changed when someone wants to limit their mempool to higher fee transactions.

I plan a future change to make the wallet smarter about not generating change outputs near the dust limit at all. I also think we should consider splitting dust into 2 dust limits if we ever raise it again and first raise the creation limit and second the standard limit a version later. Since it is not clear we’ll ever need to increase the dust limit, this change isn’t introduced now.

fanquake added the label Refactoring on Dec 19, 2016

fanquake added the label TX fees and policy on Dec 19, 2016

gmaxwell commented at 10:38 pm on December 24, 2016: contributor

utACK. My comment on reading the code was that we need to split the wallet and non-wallet behavior (esp for dust) but I see that your PR comment mentions an intention to do that later.

Introduce -blockmintxfee daec955fd6

morcos force-pushed on Jan 4, 2017

morcos commented at 6:33 pm on January 4, 2017: member

rebased

laanwj added this to the milestone 0.14.0 on Jan 12, 2017

jtimon commented at 11:33 pm on January 12, 2017: contributor

utACK 947b63a If it is decided to hide some or all of the new options and only expose them after 0.14 branches for translation reasons that’s fine with me too (although I don’t see how no documentation is better than documentation in English).

instagibbs commented at 5:08 pm on January 13, 2017: member

utACK 947b63a06043480a3a9f490b0d5485486f73e5bd

in src/init.cpp: in 3b09f89d77 outdated

464@@ -465,8 +465,10 @@ std::string HelpMessage(HelpMessageMode mode)
465     AppendParamsHelpMessages(strUsage, showDebug);
466 
467     strUsage += HelpMessageGroup(_("Node relay options:"));
468-    if (showDebug)
469+    if (showDebug) {
470         strUsage += HelpMessageOpt("-acceptnonstdtxn", strprintf("Relay and mine \"non-standard\" transactions (%sdefault: %u)", "testnet/regtest only; ", !Params(CBaseChainParams::TESTNET).RequireStandard()));
471+        strUsage += HelpMessageOpt("-incrementalrelayfee=<amt>", strprintf(_("Fee rate (in %s/kB) used to define cost of free relay, used for mempool limiting and BIP 125 replacement. (default: %s)"), CURRENCY_UNIT, FormatMoney(DEFAULT_INCREMENTAL_RELAY_FEE)));

sdaftuar commented at 9:18 pm on January 13, 2017:

Is this not supposed to be translated? Also, “cost of free relay” is a very confusing phrase. Perhaps “cost of relay”?

in src/txmempool.h: in 3b09f89d77 outdated

585@@ -591,7 +586,7 @@ class CTxMemPool
586 
587     /** The minimum fee to get into the mempool, which may itself not be enough
588       *  for larger-sized transactions.
589-      *  The minReasonableRelayFee constructor arg is used to bound the time it
590+      *  The incrementalRelayFee policy variable is used to bound the time it

sdaftuar commented at 9:24 pm on January 13, 2017:

Should still mention what minReasonableRelayFee constructor arg is for – I guess fee estimation wants the min relay fee?

morcos commented at 11:58 pm on January 13, 2017:

Removed in #9548 actually, but I will address your other points…

in src/init.cpp: in 947b63a060 outdated

467@@ -468,6 +468,7 @@ std::string HelpMessage(HelpMessageMode mode)
468     if (showDebug) {
469         strUsage += HelpMessageOpt("-acceptnonstdtxn", strprintf("Relay and mine \"non-standard\" transactions (%sdefault: %u)", "testnet/regtest only; ", !Params(CBaseChainParams::TESTNET).RequireStandard()));
470         strUsage += HelpMessageOpt("-incrementalrelayfee=<amt>", strprintf(_("Fee rate (in %s/kB) used to define cost of free relay, used for mempool limiting and BIP 125 replacement. (default: %s)"), CURRENCY_UNIT, FormatMoney(DEFAULT_INCREMENTAL_RELAY_FEE)));
471+        strUsage += HelpMessageOpt("-dustrelayfee=<amt>", strprintf(_("Fee rate (in %s/kB) used to defined dust, the size of an output such that it will cost about 1/3 of its value in fees at this fee rate to spend it. (default: %s)"), CURRENCY_UNIT, FormatMoney(DUST_RELAY_TX_FEE)));

sdaftuar commented at 9:26 pm on January 13, 2017:

This should not be translated? Also, “size of an output” -> “value of an output”?

MarcoFalke commented at 11:09 am on January 14, 2017:

This should not be called dustrelayfee, but dustfee or dustrate. My reasoning is that the option has more implications than just setting a relay fee rate.

sdaftuar changes_requested

sdaftuar commented at 9:34 pm on January 13, 2017: member

Couple nits, concept ACK, haven’t tested yet (but plan to).

sipa commented at 11:09 pm on January 13, 2017: member

Concept ACK

MarcoFalke commented at 11:10 am on January 14, 2017: member

ACK, but needs all debug strings removed from the translations.

Introduce -incrementalrelayfee 7b1add3c28

Introduce -dustrelayfee eb30d1a5b2

morcos force-pushed on Jan 16, 2017

morcos commented at 1:44 pm on January 16, 2017: member

Removed translation strings for -debug options.

Ideally this would get merged in time to keep the translation of -blockmintxfee. I think it is ready for merge other than any disagreement over the name of -dustrelayfee. I’m happy to change it if we agree on something else. -dustminfee, -dustdefinefee, -dustfee or just leave it?

laanwj commented at 6:31 pm on January 16, 2017: member

utACK eb30d1a

laanwj merged this on Jan 16, 2017

laanwj closed this on Jan 16, 2017

laanwj referenced this in commit dd98f04538 on Jan 16, 2017

codablock referenced this in commit 9e61302f3d on Jan 19, 2018

codablock referenced this in commit 9ffe7c23ac on Jan 20, 2018

codablock referenced this in commit 40dff103de on Jan 21, 2018

andvgal referenced this in commit 251f797ef2 on Jan 6, 2019

CryptoCentric referenced this in commit c0d2550fb1 on Feb 27, 2019

CryptoCentric referenced this in commit e11d1ae195 on Feb 27, 2019

random-zebra referenced this in commit 2d50b6e3b9 on Jun 9, 2021

MarcoFalke locked this on Sep 8, 2021

Separate different uses of minimum fees #9380