Allow fee estimation to work with lower fees #13990

ajtowns · 2018-08-16T08:16:00Z

Changes the minimum bucket for fee estimation from 1000 satoshi per kilobyte to 4 satoshi per kilobyte, and upgrades any saved fee estimation data.

Empact · 2018-09-01T19:58:41Z

src/policy/fees.cpp

+    : buckets(defaultBuckets), bucketMap(defaultBucketMap)
+{
+    decay = txstatsSource.decay;
+    scale = txstatsSource.scale;


nit: these could go in the initializer list

Empact · 2018-09-01T19:59:22Z

src/policy/fees.cpp

+    avg.resize(buckets.size());
+
+    unsigned int src = 0;
+    for (unsigned int dst = 0; dst < buckets.size(); dst++) {


nit: ++dst here and elsewhere

Empact · 2018-09-01T20:00:40Z

src/policy/fees.cpp

+        }
+
+        src++;
+        if (src >= txstatsSource.buckets.size()) break;


nit: can move these conditions into the for, so that it manages both src and dst.

ajtowns · 2018-09-03T08:47:49Z

The really high fee rates were due to not handling the buckets/bucketMap objects correctly, believe that's fixed now.

ajtowns · 2018-09-07T04:39:01Z

Dropped the WIP tag, this should be good to review/nitpick now.

I couldn't come up with a reasonable way to add functional tests for this change; if someone has a good idea, suggestions/patches welcome :) Though... I guess just generating an estimates file directly in python, pointing bitcoind at it, and seeing what estimates bitcoind comes up with after upgrading it without ever actually putting txes in the blockchain could work okay...

ajtowns · 2019-01-03T15:33:16Z

Reworked based on a suggestion from @Sjors; could still use some functional tests to check that loading a particular set of fee rates produces an expected set of fee estimates.

DrahtBot · 2019-01-05T17:38:53Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept NACK	tryphe, ghost
Concept ACK	Empact

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#26376 (test: Use type-safe NodeSeconds for TestMemPoolEntryHelper 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.

src/policy/fees.h

stevenroose · 2020-03-17T16:19:35Z

Is this work abandoned?

Empact · 2020-03-18T17:27:59Z

Concept ACK

ajtowns · 2020-03-19T23:50:38Z

Had stalled this on writing some tests. Having now written a test, it's found a bug, so now trying to fix that... :)

ajtowns · 2020-03-23T04:35:27Z

Rebased, test case added as first commit, should be reviewable.

Note that this only makes estimations work for fee rates below 1sat/byte if you ever actually accept fee rates that low into your mempool. To make your mempool accept/relay transactions that cheap requires setting -minrelaytxfee or changing DEFAULT_MIN_RELAY_TX_FEE; and to have your wallet use fees that low means setting-mintxfee or changing DEFAULT_TRANSACTION_MINFEE and potentially also WALLET_INCREMENTAL_RELAY_FEE. Doing those things now for mainnet would probably just be a spam vector, rather than particularly useful IMHO.

maflcko · 2020-03-23T12:31:52Z

Doing those things now for mainnet would probably just be a spam vector

Also open a huge surface for privacy leaks

murchandamus · 2022-02-07T20:59:24Z

Seems reasonable, definitely agree that making everything else ready before changing the DEFAULT_MIN_RELAY_TX_FEE is the way to go. But if we're looking at a full block at minRelayTxFee bringing more than 1% of the fees, that's about a decade away, so I agree that it doesn't seem very urgent.

glozow · 2022-10-12T17:55:38Z

@ajtowns are you still working on this?

Makes feature_fee_estimates load a fixed fee_estimates.dat and checks that the estimates given afterwards are also fixed.

ajtowns · 2022-10-17T06:19:24Z

Rebased, but is failing fuzz testing for reasons that aren't immediately obvious, so converted to draft

DrahtBot · 2022-11-22T11:18:46Z

🐙 This pull request conflicts with the target branch and needs rebase.

DrahtBot · 2023-02-20T01:22:20Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

DrahtBot · 2023-05-21T01:18:59Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

stevenroose · 2023-06-03T20:53:28Z

Lately definitely looks like it is less relevant. Nothing says we can't go back to the situation from a year or two ago where txs with 1 sat/vB would easily confirm in a few hours.

DrahtBot · 2023-09-01T02:00:55Z

There hasn't been much activity lately and the patch still needs rebase. What is the status here?

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

achow101 · 2023-09-23T16:37:47Z

Closing as up for grabs due to lack of activity.

ajtowns mentioned this pull request Aug 16, 2018

Lower default relay fees #13922

Closed

fanquake added the TX fees and policy label Aug 16, 2018

ken2812221 mentioned this pull request Aug 24, 2018

Can the default "minrelayfee" be decreased from 1 sat/B to 0.1 sat/B? The default value is too high at this moment. #14012

Closed

Empact reviewed Sep 1, 2018

View reviewed changes

ajtowns force-pushed the 201808-fee-buckets branch 2 times, most recently from d3956db to c1ca0ea Compare September 3, 2018 08:47

ajtowns force-pushed the 201808-fee-buckets branch 2 times, most recently from 031d860 to 7f1e3a4 Compare September 7, 2018 04:25

ajtowns changed the title ~~WIP: allow fee estimation to work with lower fees~~ Allow fee estimation to work with lower fees Sep 7, 2018

ajtowns force-pushed the 201808-fee-buckets branch from 7f1e3a4 to f28cf3c Compare January 3, 2019 15:15

practicalswift reviewed Jan 7, 2019

View reviewed changes

src/policy/fees.h Outdated Show resolved Hide resolved

This was referenced Feb 11, 2020

Switch to weight units for all feerates computation #17566

Closed

Add fee_est tool for debugging fee estimation code #10443

Closed

DrahtBot closed this Mar 9, 2020

DrahtBot reopened this Mar 9, 2020

ajtowns force-pushed the 201808-fee-buckets branch from f28cf3c to 41b4c64 Compare March 20, 2020 15:41

DrahtBot mentioned this pull request Mar 20, 2020

Test some transaction creation with non-empty fee estimator #18245

Closed

ajtowns force-pushed the 201808-fee-buckets branch 2 times, most recently from 9dccf61 to c68f796 Compare March 23, 2020 02:04

ajtowns force-pushed the 201808-fee-buckets branch from 8c691b3 to b7b1867 Compare January 12, 2022 10:52

DrahtBot removed the Needs rebase label Jan 12, 2022

ajtowns force-pushed the 201808-fee-buckets branch from b7b1867 to 46e2b1e Compare April 22, 2022 12:52

This was referenced Jun 7, 2022

[kernel 3a/n] Decouple CTxMemPool from ArgsManager #25290

Merged

Add AutoFile without ser-type and ser-version and use it where possible #25285

Merged

DrahtBot mentioned this pull request Jun 21, 2022

Add a -mempoolfullrbf node setting #25353

Merged

DrahtBot added the Needs rebase label Jun 29, 2022

ajtowns added 6 commits October 17, 2022 15:59

[tests] Check estimations when loaded from file

2ebb1a4

Makes feature_fee_estimates load a fixed fee_estimates.dat and checks that the estimates given afterwards are also fixed.

Add comment to CBlockPolicyEstimator::Read

8050f44

[refactor] Add CBlockPolicyEstimator::InitBucketMap helper

a20e5d3

Add CBlockPolicyEstimator::CheckConsistent helper

19c84e6

Allow extending fee estimation buckets

48218dd

Make fee buckets go much lower

d7f903c

ajtowns force-pushed the 201808-fee-buckets branch from 46e2b1e to d7f903c Compare October 17, 2022 06:16

ajtowns marked this pull request as draft October 17, 2022 06:18

DrahtBot removed the Needs rebase label Oct 17, 2022

DrahtBot mentioned this pull request Oct 24, 2022

test: Use type-safe NodeSeconds for TestMemPoolEntryHelper #26376

Merged

DrahtBot added the Needs rebase label Nov 22, 2022

achow101 added the Up for grabs label Sep 23, 2023

achow101 closed this Sep 23, 2023

bitcoin locked and limited conversation to collaborators Sep 22, 2024

Allow fee estimation to work with lower fees #13990

Allow fee estimation to work with lower fees #13990

Conversation

ajtowns commented Aug 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Empact Sep 1, 2018

Choose a reason for hiding this comment

Uh oh!

Empact Sep 1, 2018

Choose a reason for hiding this comment

Uh oh!

Empact Sep 1, 2018

Choose a reason for hiding this comment

Uh oh!

ajtowns commented Sep 3, 2018

Uh oh!

ajtowns commented Sep 7, 2018

Uh oh!

ajtowns commented Jan 3, 2019

Uh oh!

DrahtBot commented Jan 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

Uh oh!

stevenroose commented Mar 17, 2020

Uh oh!

Empact commented Mar 18, 2020

Uh oh!

ajtowns commented Mar 19, 2020

Uh oh!

ajtowns commented Mar 23, 2020

Uh oh!

maflcko commented Mar 23, 2020

Uh oh!

murchandamus commented Feb 7, 2022

Uh oh!

glozow commented Oct 12, 2022

Uh oh!

ajtowns commented Oct 17, 2022

Uh oh!

DrahtBot commented Nov 22, 2022

Uh oh!

DrahtBot commented Feb 20, 2023

Uh oh!

DrahtBot commented May 21, 2023

Uh oh!

stevenroose commented Jun 3, 2023

Uh oh!

DrahtBot commented Sep 1, 2023

Uh oh!

achow101 commented Sep 23, 2023

Uh oh!

Uh oh!

ajtowns commented Aug 16, 2018 •

edited

Loading

DrahtBot commented Jan 5, 2019 •

edited

Loading