multi: Implement smart fee estimation. #1434

matheusd · 2018-09-03T12:05:27Z

Initial release of the fee estimator.

matheusd · 2018-09-03T12:25:31Z

Sending for initial review and to discuss some last points that might require working before merging.

As a preliminary, I'm sending attached the fee database collected over the last few days in mainnet, and the dump of the estimator state for this same database:

feesdb-mainnet.tar.gz
fees.txt

Results of the estimation:

$ ./dcrctl.sh estimatefee 1
-32603: no bucket with the minimum required success percentage found
$ ./dcrctl.sh estimatefee 2
0.00112764
$ ./dcrctl.sh estimatefee 3
0.00112764
$ ./dcrctl.sh estimatefee 4
0.00087775
$ ./dcrctl.sh estimatefee 6
0.00087775

A few notes for discussion:

I've hooked into the estimatefee instead of the estimatesmartfee because there are no parameters to the estimator right now (eg: conservative vs economical) and the handler was already there, I just had to hook it. Can rework into a brand new handler if that is preferable.
Those txs with lower fee are already bringing down the estimates vs the current default wallet fee. Eyeballing from logs, they seem to be all revocations, so unsure whether we'll need to special case them.
The 0.00108347 bucket is mostly filled with tickets, which due to hard coded size estimates end up paying fees slightly larger than strictly required. Hopefully fixing Improve size/fee estimation for tickets dcrwallet#1272 improves this.
Due to how tickets are managed in the mempool and the fact that they are sent along with split txs (but are only eligible to be included in the block after the split has been mined and validated), they also severely skew the 1 confirmation bucket. Ticket handling in mempool would have to be seriously tweaked to fix this. Specifically, tickets would need to be tracked as out of the mempool (either as orphans or as some other classification) and could only enter the mempool after their respective split had been mined.

dnldd · 2018-09-10T10:47:32Z

Nicely done @matheusd, numblocks parameter description for estimatefee will have to be updated if the current rpc setup is maintained. Leaving this here as a reminder.

matheusd · 2018-09-14T17:34:05Z

Current stats after running in mainnet for ~2 weeks.

Internal estimator state

$ ./dcrctl estimatefee 1
-32603: no bucket with the minimum required success percentage found
$ ./dcrctl estimatefee 2
0.00136971
$ ./dcrctl estimatefee 3
0.00114035
$ ./dcrctl estimatefee 4
0.00114035
$ ./dcrctl estimatefee 5
0.00114035
$ ./dcrctl estimatefee 6
0.00087921
$ ./dcrctl estimatefee 7
0.00087921
$ ./dcrctl estimatefee 16
0.00087921

fees/estimator.go

fees/cmd/dumpfeedb/dumpfeedb.go

log.go

fees/estimator.go

server.go

davecgh

The EstimateSmartFee command in dcrjson needs to be updated to make the second parameter optional as well. Currently it is required.

fees/estimator.go

davecgh · 2018-12-05T03:28:19Z

fees/estimator.go

+	bucketFees := make([]feeRate, 0)
+	prevF := 0.0
+	for f := float64(cfg.MinBucketFee); f < max; f *= cfg.FeeRateStep {
+		if (f > 1e5) && (prevF < 1e5) {


Seems like this should be using a value passed in from elsewhere (perhaps policy.DefaultMinRelayTxFee), especially since wallet should be using 1e4 now. Otherwise this will certainly get out of date.

Slight difficulty on this one. I've moved the constant out of the fees package to the server.go file, passing it as an ExtraBucketFee to be inserted.

The main issue with using DefaultMinRelayTxFee is that this is not supposed to be current fee, but rather the one that the majority of the wallets are using (ie, the previous DefaultMinRelayTxFee).

So, one alternative would be to explicitly add a mempool.PreviousDefaultMinRelayTxFee and add comments detailing that it should be bumped once DefaultMinRelayTxFee is also bumped.

Another one, to make this less "magical" would be to remove this constant altogether and add explicit tracking of all order-of-magnitude bucket fees in the range (1e3, 1e4, 1e5, etc).

rpcserver.go

fees/estimator.go

dcrjson/chainsvrcmds.go

This adds the first version of the fees package, responsible for performing fee estimation of network transactions. The main goal of fee estimation is to allow the usage of dynamic fees by wallets, contingent on block contention and the desired confirmation range for a given transaction. This version was based on bitcoin core fee estimation.

This commit performs the necessary modifications to hook the fee estimation facility added by the previous commit into the node software. The block manager is modified to notify the estimator of all new blocks, such that their transactions can be accounted for, and the mempool is modified to relay transactions entering and leaving it, so that those transactions can be tracked. The results of the estimator can be queried by issuing estimatesmartfee rpc commands to the node.

matheusd · 2018-12-06T21:55:42Z

Squashed. I removed some unnecessary bumps from go.mod and go.sum, but other than that the commits are identical to the previous ones.

matheusd force-pushed the fee-estimator branch 3 times, most recently from 46cf163 to 2922e7c Compare September 3, 2018 14:35

davecgh added this to the 1.4.0 milestone Sep 4, 2018

matheusd force-pushed the fee-estimator branch from 2922e7c to 4264608 Compare September 13, 2018 12:22

dajohi reviewed Sep 17, 2018

View reviewed changes

fees/estimator.go Show resolved Hide resolved

dajohi reviewed Sep 17, 2018

View reviewed changes

fees/estimator.go Show resolved Hide resolved

matheusd force-pushed the fee-estimator branch 2 times, most recently from 15ef7ab to b0cc3e0 Compare October 11, 2018 15:52

matheusd force-pushed the fee-estimator branch 2 times, most recently from 1ff111b to 23cf919 Compare November 14, 2018 14:33

davecgh requested changes Dec 4, 2018

View reviewed changes

matheusd force-pushed the fee-estimator branch 2 times, most recently from 5188286 to 863ab8a Compare December 4, 2018 16:11

davecgh requested changes Dec 6, 2018

View reviewed changes

fees/estimator.go Outdated Show resolved Hide resolved

fees/estimator.go Outdated Show resolved Hide resolved

fees/estimator.go Outdated Show resolved Hide resolved

fees/estimator.go Outdated Show resolved Hide resolved

matheusd force-pushed the fee-estimator branch from 6349219 to e12ae0d Compare December 6, 2018 18:30

davecgh requested changes Dec 6, 2018

View reviewed changes

dcrjson/chainsvrcmds.go Outdated Show resolved Hide resolved

matheusd added 2 commits December 6, 2018 19:40

matheusd force-pushed the fee-estimator branch from efc2236 to 300b684 Compare December 6, 2018 21:51

davecgh changed the title ~~fees: add estimator~~ multi: Implement smart fee estimation. Dec 6, 2018

davecgh approved these changes Dec 6, 2018

View reviewed changes

davecgh merged commit 300b684 into decred:master Dec 6, 2018

matheusd deleted the fee-estimator branch November 25, 2020 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

multi: Implement smart fee estimation. #1434

multi: Implement smart fee estimation. #1434

matheusd commented Sep 3, 2018

matheusd commented Sep 3, 2018 •

edited

dnldd commented Sep 10, 2018

matheusd commented Sep 14, 2018

davecgh left a comment

davecgh Dec 5, 2018

matheusd Dec 6, 2018

matheusd commented Dec 6, 2018

multi: Implement smart fee estimation. #1434

multi: Implement smart fee estimation. #1434

Conversation

matheusd commented Sep 3, 2018

matheusd commented Sep 3, 2018 • edited

dnldd commented Sep 10, 2018

matheusd commented Sep 14, 2018

davecgh left a comment

Choose a reason for hiding this comment

davecgh Dec 5, 2018

Choose a reason for hiding this comment

matheusd Dec 6, 2018

Choose a reason for hiding this comment

matheusd commented Dec 6, 2018

matheusd commented Sep 3, 2018 •

edited