Skip to content

Conversation

ismaelsadeeq
Copy link
Member

This PR is a simple refactoring that does four things:

  1. Renames test/policy_fee_tests.cpp to test/feerounder_tests.cpp.
  2. Renames policy/fees.{h,cpp} to policy/fees/block_policy_estimator.{h,cpp}.
  3. Renames policy/fees_args.cpp to policy/fees/block_policy_estimator_args.cpp.
  4. Modifies estimateSmartFee to return the block height at which the estimate was made by adding a best_height unsigned int value to the FeeCalculation struct.

Motivation

In preparation for adding a new fee estimator, the fees directory is created so we can organize code into block_policy_estimator and mempool because

a) It would be clunky to add more code directly under fees.
b) Having policy/fees.{h,cpp} and policy/mempool.{h,cpp} would also be undesirable.

Therefore, it makes sense to structure the it as policy/fees/block_policy_estimator, policy/fees/mempool, etc.
Hence test file were also updated accordingly.

The current block height is also returned because later in #30157 we log the height at which each estimate is made (at the debug log category of fee estimation :) ). This feature is particularly useful for empirical data analysis.

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 19, 2025

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33218.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #31664 (Fees: add Fee rate Forecaster Manager by ismaelsadeeq)
  • #30157 (Fee Estimation via Fee rate Forecasters by ismaelsadeeq)
  • #28690 (build: Introduce internal kernel library by TheCharlatan)

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.

@ismaelsadeeq ismaelsadeeq changed the title fees: refactor: Create policy/fees dir and rename fees.{h, cpp} to fees/block_policy_estimator{h, cpp} refactor: rename fees.{h, cpp} to fees/block_policy_estimator{h, cpp} Aug 19, 2025
- Also remame the test suite name to match the new name.
- Also move it to policy/fees and update the includes
- Also move them to policy/fees/ and update includes
- Note: the block_policy_estimator_args.h include in block_policy_estimator_args.cpp was done manually.
@ismaelsadeeq ismaelsadeeq force-pushed the 08-2025-block-policy-refactor branch from 6c917b8 to c70781d Compare September 1, 2025 19:15
@ismaelsadeeq ismaelsadeeq changed the title refactor: rename fees.{h, cpp} to fees/block_policy_estimator{h, cpp} refactor: rename fees.{h,cpp} to fees/block_policy_estimator{h,cpp} Sep 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants