Lower default relay fees

ajtowns commented at 5:02 am on August 9, 2018: member

In the meeting of 2018-07-05, we discussed dropping the minimum fee rate below 1000 satoshi/kB. This patch set does only that, leaving related features for other PRs.

it changes the min tx fee to 200 s/kvB, and the incremental relay fee to 100 s/B
it leaves wallet min fee and incremental fee at 1000 s/kvB and 5000 s/kvB for compatibility while the network upgrades
it changes the functional tests that are written assuming 1000s/kvB as the min fee levels to explicitly set that via command line parameter

There’s a bit more of an explanation for some of the choices in the individual commit messages.

Related PRs:

#13987 – report minfee in getpeerinfo. useful to observe if your peers have lowered their min relay fees
#13988 – settxfee checks. this is useful since currently settxfee to the minrelaytxfee works fine, but with a lower minrelaytxfee, settxfee will be ignored for the range of values between minrelaytxfee and mintxfee which can be confusing
#13990 – fix fee estimation to work with lower fees; currently fee estimation won’t give estimates lower than 1000 satoshi per kilobyte.

fanquake added the label TX fees and policy on Aug 9, 2018

ken2812221 commented at 5:43 am on August 9, 2018: contributor

Concept ACK

ajtowns commented at 6:20 am on August 9, 2018: member

This was more complicated than I expected, so careful review would probably be a good idea. Possible things worth focusing on:

Maybe the default block min fee should be left as is, so miners have deliberately/voluntarily lower minimum fees?
I’ve not looked much at the estimation code, any maybe I’m misunderstanding how it works. And my code could have bugs.
There’s no tests for upgrading the fee estimates; “it works for me”, but that’s probably not really good enough…
Setting the default incremental relay fee higher than the minrelaytxfee causes init.cpp to automatically raise the minrelaytxfee to match, so I’ve set it as lower rather than the same, or changing that logic. I think lower makes sense, because an update to a tx probably imposes less cost on the network than an entire new tx. Not sure though.
Not a fan of all the hardcoding in the tests wrt min fee rates, but didn’t see a reasonable way of generalising them either.

DrahtBot commented at 7:19 am on August 9, 2018: member

#13804 (Transaction Pool Layer by MarcoFalke)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in src/policy/fees.cpp:217 in 9718bf41a9 outdated

212+    assert(txstatsSource.failAvg.size() == maxPeriods);
213+
214+    confAvg.resize(maxPeriods);
215+    for (unsigned int i = 0; i < maxPeriods; i++) {
216+        confAvg[i].resize(buckets.size());
217+    }

domob1812 commented at 9:49 am on August 9, 2018:

Nit (here and below): You could use a C++11 for-each loop.

ajtowns commented at 8:23 am on August 16, 2018:

Nit now applies to #13990

MarcoFalke added this to the milestone 0.18.0 on Aug 9, 2018

MarcoFalke added the label Feature on Aug 9, 2018

in src/test/mempool_tests.cpp:571 in 9718bf41a9 outdated

579+    // ... but feerate should never drop below the min incremental relay fee
580+
581+    SetMockTime(42 + 9*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4);
582     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), 0);
583-    // ... unless it has gone all the way to 0 (after getting past 1000/2)
584+    // ... unless it has gone all the way to 0 (after getting past minrelayfee/2)

MarcoFalke commented at 10:54 am on August 9, 2018:

Instead of changing all the constants in this test, couldn’t you set the global min relay fee at the start of the test to 1000 and then back to what it was initially at the end of the test?

ajtowns commented at 8:27 am on August 10, 2018:

I thought it made more sense to check the default behaviour works right, though that would be plausible too

luke-jr commented at 9:46 pm on August 27, 2018:

If tests don’t already run in parallel, it would be nice if they did someday…

promag commented at 11:09 pm on August 27, 2018:

@luke-jr just curious, why? Running in parallel makes it harder to debug and reason about.

in src/wallet/rpcwallet.cpp:2985 in 9718bf41a9 outdated

2981@@ -2982,6 +2982,14 @@ static UniValue settxfee(const JSONRPCRequest& request)
2982     LOCK2(cs_main, pwallet->cs_wallet);
2983 
2984     CAmount nAmount = AmountFromValue(request.params[0]);
2985+    CFeeRate nFeeRate(nAmount, 1000);

MarcoFalke commented at 10:58 am on August 9, 2018:

nit: Should be snake_case: tx_fee_rate.

Also, could submit this feature independent of this pull request, i.e. create a separate pull request.

ajtowns commented at 8:23 am on August 16, 2018:

Fixed in independent PR #13988

in src/rpc/net.cpp:173 in 9718bf41a9 outdated

169@@ -169,6 +170,7 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
170             obj.pushKV("inflight", heights);
171         }
172         obj.pushKV("whitelisted", stats.fWhitelisted);
173+        obj.pushKV("minfee", ValueFromAmount(stats.minFeeFilter));

MarcoFalke commented at 10:58 am on August 9, 2018:

Missing test. Also, this feature should probably be submitted in separate pull request.

ajtowns commented at 8:24 am on August 16, 2018:

Test added in separate #13987

Xekyo commented at 7:23 pm on August 9, 2018: member

“it changes the min tx fee to 200 s/B, and the incremental relay fee to 100 s/B”

I think you mean kvB instead of B, here.

in src/policy/fees.cpp:209 in 9718bf41a9 outdated

201@@ -195,6 +202,45 @@ TxConfirmStats::TxConfirmStats(const std::vector<double>& defaultBuckets,
202     resizeInMemoryCounters(buckets.size());
203 }
204 
205+TxConfirmStats::TxConfirmStats(const std::vector<double>& defaultBuckets, const std::map<double, unsigned int>& defaultBucketMap, const TxConfirmStats& txstatsSource)
206+    : buckets(defaultBuckets), bucketMap(defaultBucketMap)
207+{
208+    decay = txstatsSource.decay;
209+    scale = txstatsSource.scale;

Empact commented at 3:59 pm on August 10, 2018:

nit: these can go in the initializer list

ajtowns commented at 5:46 am on August 31, 2018:

this comment applies to #13990 now

in src/policy/fees.cpp:1008 in 9718bf41a9 outdated

1010-                bucketMap[buckets[i]] = i;
1011-            }
1012+LogPrintf("%s: loaded a bunch of fee estimation data, version %d, buckets %d (target: %d\n", __func__, nVersionThatWrote, numBuckets, buckets.size());
1013+
1014+LogPrintf("%s: buckets.size %d, buckets[0] %f, buckets[1] %f\n", __func__, buckets.size(), buckets[0], buckets[1]);
1015+LogPrintf("%s: fileBuckets.size %d, fileBuckets[0] %f, fileBuckets[1] %f\n", __func__, fileBuckets.size(), fileBuckets[0], fileBuckets[1]);

Empact commented at 3:59 pm on August 10, 2018:

nit: whitespace

ajtowns commented at 2:47 am on August 16, 2018:

Ugh, those weren’t meant to be committed in the first place…

ajtowns force-pushed on Aug 16, 2018

promag commented at 11:11 pm on August 27, 2018: member

Concept ACK.

Mind adding a release note?

DrahtBot added the label Needs rebase on Aug 29, 2018

ajtowns force-pushed on Aug 31, 2018

DrahtBot removed the label Needs rebase on Aug 31, 2018

ajtowns commented at 5:46 am on August 31, 2018: member

Rebased and release note added

in doc/release-notes-13922.md:14 in 42e2af57e5 outdated

 9+satoshis/kB). The previous default for all these values was 10 bits
10+per kilobyte. These defaults can be overridden using the minrelaytxfee,
11+blockmintxfee and incrementalrelayfee options.
12+
13+Note that until these lower defaults are widely adopted across the
14+network, transactions created with lower fee rates may not propogate

practicalswift commented at 9:09 pm on September 1, 2018:

Typo found by codespell: propogate

ajtowns force-pushed on Sep 7, 2018

in test/functional/mempool_limit.py:16 in 56251ceb4b outdated

12@@ -13,7 +13,7 @@ class MempoolLimitTest(BitcoinTestFramework):
13     def set_test_params(self):
14         self.setup_clean_chain = True
15         self.num_nodes = 1
16-        self.extra_args = [["-maxmempool=5", "-spendzeroconfchange=0"]]
17+        self.extra_args = [["-maxmempool=5", "-spendzeroconfchange=0","-minrelaytxfee=0.00001"]]

practicalswift commented at 12:14 pm on September 11, 2018:

0./test/functional/mempool_limit.py:16:70: E231 missing whitespace after ','

DrahtBot added the label Needs rebase on Sep 13, 2018

Set minrelaytxfee=0.00001 in tests

Some tests assume a minrelaytxfee of 1000 satoshi/kB, so explicitly set
that in preparation for lowering the default.

036bac83d2

Lower default fees

This changes the fee defaults to:

  BLOCK_MIN_TX_FEE = 200
  DEFAULT_MIN_RELAY_TX_FEE = 200

  DEFAULT_INCREMENTAL_RELAY_FEE = 100

  DEFAULT_TRANSACTION_MINFEE = 1000
  WALLET_INCREMENTAL_RELAY_FEE = 5000

These reduce default minimum network fees by a factor of 5 (from 1000s/kB
to 200s/kB), which matches previous decreases in lowering the price of
block data in USD to about 1c/kB:

  2013-05: 50,000 to 10,000 at $100 USD/BTC: 5c/kB to 1c/kB
  2014-11: 10,000 to 1,000 at $700 USD/BTC: 7c/kB to 0.7c/kB
  2015-10: 1,000 to 5,000 and back to 1,000 at $250 USD/BTC:
           0.25c/kB to 1.25c/kB to 0.25c/kB
  2018-08: 1,000 to 200 at $6000 USD/BTC: 6c/kB to 1.2c/kB

(Note that on a per-transaction basis, the witness discount generally
decreases fees by about a further 50%, so for individual's a better
comparison might be 3c/kB to 0.6c/kB)

The incremental relay fee is lowered further, to allow cheaper updates
of transactions, which makes better use of blockspace.

Because it will take time for the network to broadly support these lower
mining and relay fees, the wallet defaults are left unchanged at 1000s/kB
and 5000s/kB.

a04a0146a2

ajtowns force-pushed on Sep 18, 2018

ajtowns commented at 1:59 pm on September 18, 2018: member

Rebased due to conflict in python tests; fixed formatting nit; fixed amount typo in changes to mempool test (7000/5 is 1400 not 1500).

DrahtBot removed the label Needs rebase on Sep 18, 2018

ajtowns commented at 1:38 am on October 10, 2018: member

Repeating some in-person comments:

reducing the min incremental relay fee allows for cheap spamming, might be better to change the code so that the min incremental relay fee can be higher than the min relay fee, and leaving it higher (perhaps 200s/kB for relay, but 1000s/kB or 500s/kB for incremental relay/fee-bumping). Note that wallet defaults already have a higher incremental fee, so this would improve consistency.
(long term, not for this PR) it might be nice if min incremental relay fees were dynamically determined based on how much traffic there is – if there’s no traffic, updating a tx doesn’t need to cost anything significant; if there’s lots of traffic, or a spam attack, updating a tx should become as expensive as issuing a new tx (ie, incremental relay fee should be as high as mempool minfee, which may be well above the min relay fee)

ajtowns commented at 1:50 am on October 10, 2018: member

Closing for now pending further progress on #13990, intending to reopen or refile later

ajtowns closed this on Oct 10, 2018

brianddk commented at 2:49 am on August 21, 2020: contributor

@ajtowns It’s been two years, and minrelaytxfee is still a lot higher than it probably needs to be. Would you be willing to refile? Someone needs to get this change through.

The mempool has been emptying pretty much every week, and often every day. With LN interest catching on there is a larger need for “next-day” ultra-low fee on-chain TXNs.

MarcoFalke locked this on Feb 15, 2022

Lower default relay fees #13922