estimatefee / estimatepriority RPC methods #3959

gavinandresen · 2014-03-25T13:18:47Z

New RPC methods: return an estimate of the fee (or priority) a transaction needs to be likely to confirm in a given number of blocks.

Fee-per-kilobyte estimates for 1..10 confirmations from this code:

gavin$ for i in {1..10}; do ./bitcoind estimatefee $i; done
0.00044643
0.00044248
0.00044053
0.00029499
0.00020492
0.00014970
0.00012981
0.00012190
0.00011769
0.00011217

Mike Hearn created the first version of this method for estimating fees.
It works as follows:

For transactions that took 1 to N (I picked N=25) blocks to confirm, keep N buckets with at most 100 entries in each recording the fees-per-kilobyte paid by those transactions.

(separate buckets are kept for transactions that confirmed because they are high-priority)

The buckets are filled as blocks are found, and are saved/restored as part of the mempool.dat file.

A few variations on Mike's initial scheme:

To estimate the fee needed for a transaction to confirm in X buckets, all of the samples in all of the buckets are used and a median of all of the data is used to make the estimate. For example, imagine
25 buckets each containing the full 100 entries. Those 2,500 samples are sorted, and the estimate of the fee needed to confirm in the very next block is the 50'th-highest-fee-entry in that sorted list; the estimate of the fee needed to confirm in the next two blocks is the 150'th-highest-fee-entry, etc.

That algorithm has the nice property that estimates of how much fee you need to pay to get confirmed in block N will always be greater than or equal to the estimate for block N+1. It would clearly be wrong
to say "pay 11 uBTC and you'll get confirmed in 3 blocks, but pay 12 uBTC and it will take LONGER".

A single block will not contribute more than 10 entries to any one bucket, so a single miner and a large block cannot overwhelm the estimates.

mikehearn · 2014-03-27T16:49:05Z

qa/rpc-tests/smartfees.py

+            return i
+    return -1
+
+def gather_inputs(from_node, amount_needed):


A lot of this stuff looks like it should be in a utility library (the main function too), otherwise it's getting copy/pasted everywhere.

Moving useful functions to util.py, but am going to hold off on moving main; there's another pull request that bundles everything up into a framework...

gavinandresen · 2014-03-31T18:00:39Z

Rebased on master, and tweaked to address Mike's comments (thanks for the review!).

gavinandresen · 2014-04-01T13:58:27Z

Pull-tester errors were with the regression test:

pull-tester is running python 2.6 (fixed by using subprocess.check_call instead of python2.7 subprocess.check_output).

mikehearn · 2014-05-07T17:23:54Z

What's the status of this work? Does it need more review or testing or what?

laanwj · 2014-05-07T17:34:56Z

It's queued to be merged for 0.10, so after the 0.9.2 branch split-off.
But more review and testing is always welcome.

laanwj · 2014-05-21T14:02:26Z

src/qt/coincontroldialog.cpp

@@ -610,19 +595,19 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)

    // tool tips
    QString toolTip1 = tr("This label turns red, if the transaction size is greater than 1000 bytes.") + "<br /><br />";
-    toolTip1 += tr("This means a fee of at least %1 per kB is required.").arg(BitcoinUnits::formatWithUnit(nDisplayUnit, CTransaction::nMinTxFee)) + "<br /><br />";
+    toolTip1 += tr("This means a fee of at least %1 per kB is required.").arg(BitcoinUnits::formatWithUnit(nDisplayUnit, CTransaction::minTxFee.GetFee(1000))) + "<br /><br />";


Nit: There are a lot of getFee(1000)'s. I don't have an objection to them, although on first reading I was confused ('what is this magic 1000 number'). But instead of suggesting defining a constant BYTES_PER_KB :p I think it would be nicer to have a function getFeePerKB() that's simply an inline for getFee(1000).

gavinandresen · 2014-05-23T16:33:50Z

Nit-picked : replace GetFee(1000) with inline GetFeePerK() method.

gavinandresen · 2014-05-29T16:35:45Z

Rebased on master.

I'm very happy with how quickly this generates good estimates; after just three or four blocks it has a good idea of what priority or fee is needed to get into the next couple of blocks.

sipa · 2014-05-29T17:18:38Z

Save/restore of the mempool seems quite dangerous in making some unconfirmed transactions potentially live forever in the network...

jgarzik · 2014-05-29T17:22:02Z

Nod -- you only want to save/restore once we have transactions expiring from the mempool (janitor etc.) after X blocks.

petertodd · 2014-05-30T07:06:26Z

@sipa Yup. Even with expiration it makes many DoS attacks and similar vulnerabilities much more effective. For instance OOM crashes are hard to exploit right now because the txs are hard to propagate; not true with save restore.

jgarzik · 2014-05-30T07:23:21Z

In general, lacking other tools, clearing the mempool [implicitly] at startup is a valuable tool.

gavinandresen · 2014-05-30T13:22:57Z

Consensus is saving/restoring memory pool is dangerous, so I've removed that functionality.

Fee estimates ARE saved/restored, to datadir fee_estimates.dat file.

Use CFeeRate instead of an int64_t for quantities that are fee-per-size. Helps prevent unit-conversion mismatches between the wallet, relaying, and mining code.

Choose ports at startup based on PID, so multiple regression tests can run on the same system at the same time.

New RPC methods: return an estimate of the fee (or priority) a transaction needs to be likely to confirm in a given number of blocks. Mike Hearn created the first version of this method for estimating fees. It works as follows: For transactions that took 1 to N (I picked N=25) blocks to confirm, keep N buckets with at most 100 entries in each recording the fees-per-kilobyte paid by those transactions. (separate buckets are kept for transactions that confirmed because they are high-priority) The buckets are filled as blocks are found, and are saved/restored in a new fee_estiamtes.dat file in the data directory. A few variations on Mike's initial scheme: To estimate the fee needed for a transaction to confirm in X buckets, all of the samples in all of the buckets are used and a median of all of the data is used to make the estimate. For example, imagine 25 buckets each containing the full 100 entries. Those 2,500 samples are sorted, and the estimate of the fee needed to confirm in the very next block is the 50'th-highest-fee-entry in that sorted list; the estimate of the fee needed to confirm in the next two blocks is the 150'th-highest-fee-entry, etc. That algorithm has the nice property that estimates of how much fee you need to pay to get confirmed in block N will always be greater than or equal to the estimate for block N+1. It would clearly be wrong to say "pay 11 uBTC and you'll get confirmed in 3 blocks, but pay 12 uBTC and it will take LONGER". A single block will not contribute more than 10 entries to any one bucket, so a single miner and a large block cannot overwhelm the estimates.

jgarzik · 2014-06-06T15:00:23Z

ACK. Speaking conservatively, appears to be low impact as implemented at first glance.

The largest impact is to existing operations is the hook that is executed when a block appears, necessitating the removal of TXs from the mempool. At a quick glance, it was difficult to discern the big-O() of this operation, but at least it is all in CPU/RAM, and only executed once per incoming new block.

One nit (perhaps for post merge): I don't like the mempool itself doing the reading. Would prefer a helper function outside the mempool class that perform the necessary I/O. Our normal pattern is a pure implement-serialize chain until actual file I/O is needed. This seems to mix metaphors a bit more than usual.

sipa · 2014-06-06T15:08:12Z

src/qt/coincontroldialog.cpp

@@ -536,26 +536,11 @@ void CoinControlDialog::updateLabels(WalletModel *model, QDialog* dialog)
        {
            nChange = nAmount - nPayFee - nPayAmount;

-            // if sub-cent change is required, the fee must be raised to at least CTransaction::nMinTxFee


Wouldn't we want to keep this block, but do it to prevent dust change rather than subcent change?

Dust change is handled at line 543.

BitcoinPullTester · 2014-06-06T16:50:51Z

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/171ca7745e77c9f78f26556457fe64e5b2004a75 for binaries and test log.

This could happen for one of several reasons:

It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in qa/pull-tester)
It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
It does not build on either Linux i386 or Win32 (via MinGW cross compile)
The test suite fails on either Linux i386 or Win32
The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

171ca77 estimatefee / estimatepriority RPC methods (Gavin Andresen) 0193fb8 Allow multiple regression tests to run at once (Gavin Andresen) c6cb21d Type-safe CFeeRate class (Gavin Andresen)

gmaxwell · 2014-11-20T17:28:36Z

If mempool saving ever comes back as a proposal we should keep in mind that doing so presents a pretty strong watermarking attack. (... not that we don't have many other watermarking attacks.)

mikehearn reviewed Mar 27, 2014
View reviewed changes

laanwj added this to the 0.10.0 milestone May 6, 2014

laanwj reviewed May 21, 2014
View reviewed changes

This was referenced May 23, 2014

Encapsulate coin balances within a new CMoney type. #4067

Closed

Smart fees for wallet CreateTransaction #4250

Merged

gavinandresen added 3 commits June 6, 2014 10:34

Type-safe CFeeRate class

c6cb21d

Use CFeeRate instead of an int64_t for quantities that are fee-per-size. Helps prevent unit-conversion mismatches between the wallet, relaying, and mining code.

Allow multiple regression tests to run at once

0193fb8

Choose ports at startup based on PID, so multiple regression tests can run on the same system at the same time.

sipa reviewed Jun 6, 2014
View reviewed changes

laanwj merged commit 171ca77 into bitcoin:master Jun 6, 2014

This was referenced Jun 8, 2014

Fix CreateTransaction in the voluntary fee case #4308

Closed

[Qt] Coin control cleanup because of non-rounding fees #4314

Merged

christophebiocca mentioned this pull request Aug 26, 2014

Durable transaction fee model #4146

Closed

rnicoll mentioned this pull request Feb 1, 2015

estimatefee / estimatepriority RPC methods dogecoin/dogecoin#833

Merged

laanwj mentioned this pull request Mar 11, 2016

paytxfee behavior changed without warning #7633

Closed

DanielKrawisz mentioned this pull request Aug 13, 2016

rpc method estimatefee btcsuite/btcd#735

Closed

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

estimatefee / estimatepriority RPC methods #3959

estimatefee / estimatepriority RPC methods #3959

gavinandresen commented Mar 25, 2014

mikehearn Mar 27, 2014

gavinandresen Mar 31, 2014

gavinandresen commented Mar 31, 2014

gavinandresen commented Apr 1, 2014

mikehearn commented May 7, 2014

laanwj commented May 7, 2014

laanwj May 21, 2014

gavinandresen commented May 23, 2014

gavinandresen commented May 29, 2014

sipa commented May 29, 2014

jgarzik commented May 29, 2014

petertodd commented May 30, 2014

jgarzik commented May 30, 2014

gavinandresen commented May 30, 2014

jgarzik commented Jun 6, 2014

sipa Jun 6, 2014

gavinandresen Jun 6, 2014

BitcoinPullTester commented Jun 6, 2014

gmaxwell commented Nov 20, 2014

estimatefee / estimatepriority RPC methods #3959

estimatefee / estimatepriority RPC methods #3959

Conversation

gavinandresen commented Mar 25, 2014

mikehearn Mar 27, 2014

Choose a reason for hiding this comment

gavinandresen Mar 31, 2014

Choose a reason for hiding this comment

gavinandresen commented Mar 31, 2014

gavinandresen commented Apr 1, 2014

mikehearn commented May 7, 2014

laanwj commented May 7, 2014

laanwj May 21, 2014

Choose a reason for hiding this comment

gavinandresen commented May 23, 2014

gavinandresen commented May 29, 2014

sipa commented May 29, 2014

jgarzik commented May 29, 2014

petertodd commented May 30, 2014

jgarzik commented May 30, 2014

gavinandresen commented May 30, 2014

jgarzik commented Jun 6, 2014

sipa Jun 6, 2014

Choose a reason for hiding this comment

gavinandresen Jun 6, 2014

Choose a reason for hiding this comment

BitcoinPullTester commented Jun 6, 2014

gmaxwell commented Nov 20, 2014