ismaelsadeeq
commented at 11:44 am on May 23, 2024:
member
This PR aims to improve Bitcoin Core Fee Estimation by:
Reducing Overestimation: Address and mitigate the overestimation issues present in the current BlockPolicyEstimator. This issue has been documented and acknowledged by various sources:
Mempool Awareness: Enable the fee estimator to be aware of the mempool state, allowing it to react faster and more accurately to rapidly changing fee market conditions, when targeting very short timeframes.
Empowering Node Users: Allow node users to be self-sovereign by using their node’s estimates, reducing reliance on third-party fee estimations.
Simplifying Strategy Integration: Simplify the process of adding new fee estimation strategies in the future.
The detailed design document for this PR can be found here. Please note that the design is subject to change as we refine the approach.
We are currently seeking conceptual acknowledgment for this PR.
This is a collaborative effort with @willcl-ark and incorporates insights from other contributors.
DrahtBot
commented at 11:44 am on May 23, 2024:
contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
#31318 (Drop script_pub_key arg from createNewBlock by Sjors)
#31283 (Add waitNext() to BlockTemplate interface by Sjors)
#31260 (scripted-diff: Type-safe settings retrieval by ryanofsky)
#30079 (Fee Estimation: Ignore all transactions that are CPFP’d by ismaelsadeeq)
#28676 ([WIP] Cluster mempool implementation by sdaftuar)
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.
DrahtBot added the label
CI failed
on May 23, 2024
DrahtBot
commented at 3:18 pm on May 23, 2024:
contributor
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.
Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
Leave a comment here, if you need help tracking down a confusing failure.
luke-jr
commented at 5:20 pm on May 23, 2024:
member
Make the fee estimator aware of the state of the mempool, allowing it to respond to changing conditions immediately.
The state of the node’s mempool may not accurately reflect the state of others’ mempools, and not even its own mempool when the block is found in the future. It isn’t a good single source of information. Perhaps it is a good idea to use it as a secondary source, but probably it should only ever adjust fee estimations upward, not down.
willcl-ark
commented at 8:41 am on May 24, 2024:
member
The state of the node’s mempool may not accurately reflect the state of others’ mempools, and not even its own mempool when the block is found in the future. It isn’t a good single source of information.
Correct. The rationale behind this set of changes can be summed up briefly as follows:
Add a new standalone modular fee estimation manager to which (many) Forcasters can be trivially added or removed (vs modifying BlockPolicyEstimator).
Provide two forcaster implementations which do not exist today ( 1. mempool-based and 2. previous-6-blocks-seen-in-mempool) to demonstrate functionality.
In the present, there is a strong tendency for users of Bitcoin Core to consult external fee estimation services when they want timely (next 1 or 2 blocks) confirmation of transactions, whilst also not overpaying. Examples of these include mempool.space, whatthefee.io, Samourai’s nextblock.is (now down), johoe, blockchair etc., with more popping up every month.
We also analysed/estimated Bitcoin Core users not using in-built estimation in a post here.
In our opinion having users feel the need to use external fee estimation services that they could equally have served to them by their own node, feels sub-optimal.
In addition to this, when users do use the current Bitcoin Core fee estimator, there are often times when they end up overpaying, see delving bitcoin post Mempool based fee estimation and various issues over the years e.g. #30009 . This is avoidable.
Work from a student of @renepickhardtlink demonstrated something we also measured independently – that bitcoin core’s current estimates are often overpaying significantly following fee spikes. This effect can be directly mitigated by using a mempool-based estimation.
Perhaps it is a good idea to use it as a secondary source, but probably it should only ever adjust fee estimations upward, not down.
In this changeset we take the approach of using the lowest result from all “confident” Forcasters. The rationale is that we expect users wanting fast confirmation to have RBF enabled, allowing them to bump fees if we still undershoot.
Comments from @hardinglink explained that it may be possible for miners to artificially increase a strictly-mempool-based fee-rate. By taking the lower (confident) result from nForcasters, we attempt to protect against this attack (and others like it), at the potential cost of having to RBF.
We do plan to add additional sanity checks to the mempool-based Forcaster as described in #27995, but these are not yet implemented. In any case, even without these additional checks we have been seeing much-improved short time-scale estimations from Bitcoin Core.
ismaelsadeeq force-pushed
on May 29, 2024
ismaelsadeeq force-pushed
on May 29, 2024
DrahtBot removed the label
CI failed
on May 29, 2024
DrahtBot added the label
Needs rebase
on Jun 11, 2024
ismaelsadeeq force-pushed
on Jul 3, 2024
DrahtBot removed the label
Needs rebase
on Jul 3, 2024
ismaelsadeeq force-pushed
on Jul 3, 2024
DrahtBot added the label
CI failed
on Jul 3, 2024
DrahtBot
commented at 5:02 pm on July 3, 2024:
contributor
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.
Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
Leave a comment here, if you need help tracking down a confusing failure.
Make sure to run all tests locally, according to the documentation.
The failure may happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
DrahtBot added the label
CI failed
on Jul 24, 2024
willcl-ark added the label
Needs Conceptual Review
on Jul 24, 2024
DrahtBot removed the label
Needs rebase
on Jul 24, 2024
ismaelsadeeq force-pushed
on Jul 24, 2024
DrahtBot removed the label
CI failed
on Jul 24, 2024
ismaelsadeeq force-pushed
on Aug 13, 2024
DrahtBot added the label
CI failed
on Aug 13, 2024
ismaelsadeeq force-pushed
on Aug 13, 2024
DrahtBot removed the label
CI failed
on Aug 13, 2024
hebasto added the label
Needs CMake port
on Aug 16, 2024
DrahtBot added the label
Needs rebase
on Aug 21, 2024
glozow added the label
TX fees and policy
on Aug 21, 2024
maflcko removed the label
Needs CMake port
on Aug 29, 2024
ismaelsadeeq force-pushed
on Sep 3, 2024
ismaelsadeeq
commented at 9:43 am on September 3, 2024:
member
Rebased and added newly introduced files to cmakelist file instead makefile
DrahtBot removed the label
Needs rebase
on Sep 3, 2024
DrahtBot added the label
Needs rebase
on Sep 20, 2024
ismaelsadeeq force-pushed
on Oct 14, 2024
DrahtBot removed the label
Needs rebase
on Oct 14, 2024
DrahtBot added the label
CI failed
on Oct 14, 2024
DrahtBot
commented at 5:03 pm on October 14, 2024:
contributor
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
murchandamus
commented at 5:57 pm on October 21, 2024:
contributor
Concept ACK! Thank you for working on this.
vasild
commented at 4:28 am on November 6, 2024:
contributor
Concept ACK
Allow node users to be self-sovereign by using their node’s estimates, reducing reliance on third-party fee estimations.
having users feel the need to use external fee estimation services that they could equally have served to them by their own node, feels sub-optimal
Indeed, I do that :face_with_head_bandage:
ismaelsadeeq force-pushed
on Nov 11, 2024
ismaelsadeeq force-pushed
on Nov 11, 2024
ismaelsadeeq force-pushed
on Nov 11, 2024
ismaelsadeeq force-pushed
on Nov 11, 2024
DrahtBot removed the label
CI failed
on Nov 11, 2024
ismaelsadeeq force-pushed
on Nov 14, 2024
ismaelsadeeq force-pushed
on Nov 14, 2024
DrahtBot added the label
CI failed
on Nov 14, 2024
DrahtBot
commented at 8:52 pm on November 14, 2024:
contributor
Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
ismaelsadeeq force-pushed
on Nov 14, 2024
ismaelsadeeq
commented at 9:52 pm on November 14, 2024:
member
I have updated this PR based on feedback from #30391 and in-person conversations:
I am now using the newly introduced FeeFrac datatype, which does not lose precision like CFeeRate.
Updated the CalculatePercentile function to return a monotonically decreasing estimate for high and low priority. The previous approach took the fee rate of the package at the exact percentile, which could be high due to the ancestors of the percentile package included previously in a sibling package.
Relevant commit: 2ac86a8fd7fe108078511ef2f6ad77e315591a2b.
I’ve also added a test for this.
Added a functional test for the estimatefee RPC behavior (commit ac8f2caf050e73b038a8e03a51dfe69e1ea7aa19).
Removed the tracing commit after merging #26593, as it changes the tracing structure, which I haven’t reviewed yet. The tracing commit was relevant for collecting node’s estimation data for analysis, it can be added later on.
This PR has now received a conceptual review from a few contributors, so we can proceed to code and approach review.
DrahtBot removed the label
CI failed
on Nov 14, 2024
DrahtBot added the label
Needs rebase
on Nov 20, 2024
ismaelsadeeq force-pushed
on Nov 25, 2024
miner: `CreateNewBlock` will now track fee and virtual size of selected packages
- The commit also tests this new behaviour.
Co-authored-by: willcl-ark <will@256k1.dev>
8541e46254
===== End of miner commits =====4603729b54
fees: add forecast utility structure7ab86b2f3c
fees: add Forecaster abstract class
- This commit implements `Forecaster` abstract class
as the base class of fee rate forecasters.
- Derived classes must provide concrete implementation
of the virtual methods.
Co-authored-by: willcl-ark <will@256k1.dev>
6e8ce6809d
fees: add `ForecastType` enum
- ForecastType will be used to identify forecasters.
- Each time a new forecaster is added, a corresponding
enum value should be added to ForecastType.
- This allows users to identify which fee estimation strategy
was used to make a fee rate estimate.
8ac8e94056
fees: add `FeeEstimator` class
- Its a module for managing and utilising multiple
fee rate forecasters to provide fee estimates.
- The FeeEstimator class allows for the registration of
multiple fee rate forecasters.
Co-authored-by: willcl-ark <will@256k1.dev>
9d52a2d0ed
fees: add `CBlockPolicyEstimator` to `FeeEstimator` class
- This commit made CBlockPolicyEstimator member of FeeEstimator class
the FeeEstimator will own a pointer to CBlockPolicyEstimator
- These does not change any behaviour of CBlockPolicyEstimator.
Co-authored-by: willcl-ark <will@256k1.dev>
5ad4c5f1e6
fees: add `forecastTypeToString` method
- This method converts a ForecastType enum to its
string representation.
560c02af87
fees: Add CalculatePercentiles function
- The CalculatePercentiles function, given
a vector of feerates in the order they were added
to the block, will return the 25th, 50th, 75th,
and 95th percentile feerates.
- This function maintains monotonicity by taking the
of package with feerates.
- Also add a unit test for this function.
10e9c8a265
fees: add `MemPoolForecaster` class
- The mempool based fee rate forecaster generate a predicted fee rate estimate
for a given confirmation target using the mempool unconfirmed transactions.
Co-authored-by: willcl-ark <will@256k1.dev>
1a4ace3645
test: add `MemPoolPolicyEstimator` unit testb5b8146a9b
fees: cache `MemPoolPolicyEstimator` forecasts
- Provide new estimates only when the time delta from previous
forecast is older than 30 seconds.
- This caching helps avoid the high cost of frequently generating block templates,
preventing users from inadvertently calling `estimateFee` repeatedly.
Co-authored-by: willcl-ark <will@256k1.dev>
9e9e0702c7
fees: return current block height in `estimateSmartFee`5180964b70
fees: add `GetPolicyEstimatorEstimate` method
- This commit Added a new method to the fee estimator module.
- This method, GetPolicyEstimatorEstimate, returns the fee rate
estimate for a given confirmation target.
- It provides both conservative and economical mode estimates from
CBlockPolicyEstimator, outputting them as low and high fee rate estimates.
2596200fe9
fees: add `GetFeeEstimateFromForecasters` method
- Fallback to Block policy estimator estimates whenever mempool forecaster
estimates are higher than block policy estimator.
bb617c9dda
===== End of FeeEstimator commits =====df49b176be
rpc: create an rpc `estimatefee`
- Given a confirmation target, we use fee estimator module that call all
available fee estimator forcasters and return the lowest fee rate that if
a transaction use will likely confirm in a given confirmation target.
Co-authored-by: willcl-ark <will@256k1.dev>
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2024-12-03 15:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me
