-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Fees: add Fee rate Forecaster Manager #31664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fees: add Fee rate Forecaster Manager #31664
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31664. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
ba0bede to
c6b9440
Compare
|
There is going to be a review club tomorrow on this PR |
|
I will update the PR to address recent comments, I also want to remove some redundancy and restructure the commits. |
c6b9440 to
9455f2f
Compare
|
Forced pushed c6b9440..9455f2f0927014f2cc2495d432be635b58cc8fe2 I've made the following changes:
|
9455f2f to
a665ff1
Compare
a665ff1 to
9ae1585
Compare
9ae1585 to
75e6bb5
Compare
|
ACK ba33047b6f3ee468eab250c8515e92b595e08322 reviewed the code and build and run the tests locally. |
ba33047 to
353cdbd
Compare
|
Rebased due to merge conflict. |
353cdbd to
aa4efc7
Compare
i - Forecaster abstract class is the base class of fee rate forecasters.
Derived classes must provide concrete implementation
of the virtual methods.
ii - ForecastResult struct represent the response returned by
a fee rate forecaster.
iii - ForecastType will be used to identify forecasters.
Each time a new forecaster is added, a corresponding
enum value should be added to ForecastType.
Co-authored-by: willcl-ark <will@256k1.dev>
- Its a module for managing and utilising multiple fee rate forecasters to provide fee rate forecast. - The ForecasterManager class allows for the registration of multiple fee rate forecasters. Co-authored-by: willcl-ark <will@256k1.dev>
- This changes `CBlockPolicyEstimator` to a shared pointer this gives us three advantages. - Registering to validation interface using shared pointer - Scheduling block policy estimator flushes using shared pointer - Registering block policy estimator to forecaster_man
- This method converts a ForecastType enum to its string representation.
- 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. - Also add a unit test for this function.
- The mempool forecaster uses the unconfirmed transactions in the mempool to generate a fee rate that a package should have for it to confirm as soon as possible. Co-authored-by: willcl-ark <will@256k1.dev>
- Provide new forecast 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. Co-authored-by: willcl-ark <will@256k1.dev>
aa4efc7 to
34cf9e1
Compare
- Polls all registered forecasters and selects the lowest fee rate.
34cf9e1 to
eb541fb
Compare
|
Closed in favour of #34075 |
The primary addition is a
ForecasterManagerthat coordinates multiple forecasting strategies to be able to provide better transaction fee rate predictions.Key Changes
Addition of Fee Rate Forecasting Utility Structures
Forecasterabstract class, serving as the base class for all fee rate forecasters.ForecastResult(the response from aForecaster) with all metadata.ForecastTypeenum, identify and distinguish forecasters.Introduce
FeeRateForecasterManagerclassFeeRateForecasterManagerclass, responsible for managing all fee rate forecasters, includingCBlockPolicyEstimator, which is now a forecaster.FeeRateForecasterManagerinstead ofCBlockPolicyEstimator.CBlockPolicyEstimatorinstance fromunique_ptrto ashared_pointer, allowing it to be registered in the validation interface without requiring explicit unregistration during shutdown (current master behaviour). Registering forCBlockPolicyEstimatorflushes also get ashared_pointer.CBlockPolicyEstimatorthroughFeeRateForecasterManagerfor compatibility withestimateSmartFee, friends, and node interfaces.CBlockPolicyEstimatorto inherit from theForecasterclass and implement the pure abstract classesIntroduce
MempoolForecasterclassMempoolForecaster, which forecast the fee rate required for a transaction to confirm as soon as possible. It generates a block template and returns the 75th and 50th percentile fee rates as low-priority and high-priority fee rate forecasts.The
FeeRateForecasterManagernow includes a methodForecastFeeRateFromForecasters. This method polls forecasts from all registered forecasters and returns the lowest value. This behavior aligns with discussions and research outlined in this post. But the primary aim of theFeeRateForecasterManageris to be able to perform sanity checks and potentially apply heuristics to determine the most appropriate fee rate forecast from polled forecasters, for now it just return the lowest.This PR also add unit tests for the classes.
In Draft because it depends on #33218