fees: skip pointless fee parameter calculation during IBD #22919

1. 
   rjnrohit commented at 12:05 pm on September 8, 2021: none

   Bitcoin nodes update their TxConfirmStats attributes (eg. confAvg, failAvg etc..) whenever they see a new block. However, during IBD, we know our best block height, so we can skip the parameter updates up to a certain height, when we know they are not going to affect fee estimates.

   We can see the max value of decay for parameters is .99931. If we do (max_decay)^6048 which is approximate 0.01, it means, that transactions in blocks older than this height, do not affect the current fee estimates significantly. (i.e ~ Best_Block_Height - 6048).

   By skipping these updates we can reduce the computation complexity of fee calculation during IBD significantly: (block_tree_size - 6048)/block_tree_size*100 which is ~ 99%.

   I got to know about the functions to optimize by analyzing the profiling data of bitcoind. It can be generated via ./configure CC=clang CXX=clang++ CXXFLAGS="-fprofile-instr-generate=bitcoind-%p.profraw

2. fanquake added the label TX fees and policy on Sep 8, 2021
3. 
   MarcoFalke commented at 12:27 pm on September 8, 2021: member

   By skipping these updates we can reduce the computation complexity of fee calculation during IBD significantly: (block_tree_size - 6048)/block_tree_size*100 which is ~ 99%.

   Can you explain this a bit better? During IBD the mempool should be empty, so the entries vector is empty as well.

4. 
   rjnrohit commented at 1:01 pm on September 8, 2021: none

   By skipping these updates we can reduce the computation complexity of fee calculation during IBD significantly: (block_tree_size - 6048)/block_tree_size*100 which is ~ 99%.

   Can you explain this a bit better? During IBD the mempool should be empty, so the entries vector is empty as well.

   Regardless of size of entries whenever a bitcoin node receives a new block then CBlockPolicyEstimator::processBlock function will be called. And so the UpdateMovingAverages function. The idea is not to call UpdateMovingAverages every time.

5. 
   MarcoFalke commented at 1:08 pm on September 8, 2021: member

   See also:

   • #11194
6. 
   DrahtBot commented at 1:09 pm on September 8, 2021: member

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #22508 (fuzz: replace every fuzzer-controlled while loop with a macro by apoelstra)
   • #10443 (Add fee_est tool for debugging fee estimation code by ryanofsky)

   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.

7. rjnrohit force-pushed on Sep 8, 2021
8. rjnrohit force-pushed on Sep 8, 2021
9. fees: skip pointless fee parameter calculation during IBD

   Bitcoin nodes update their TxConfirmStats attributes (eg. confAvg, failAvg etc..) whenever they see a new block.
   However, during IBD, we know our best block height, so we can skip the parameter updates up to a certain height,
   when we know they are not going to affect fee estimates.
   
   We can see the max value of decay for parameters is .99931. If we do (max_decay)^6048 which is approximate 0.01,
   it means, that transactions in blocks older than this height,
   do not affect the current fee estimates significantly. (i.e ~ Best_Block_Height - 6048).
   
   By skipping these updates we can reduce the computation complexity
   of fee calculation during IBD significantly:
   (block_tree_size - 6048)/block_tree_size*100 which is ~ 99%.

   39d5312415
10. rjnrohit force-pushed on Sep 8, 2021
11. DrahtBot added the label Needs rebase on Nov 15, 2021
12. 
    DrahtBot commented at 5:25 pm on November 15, 2021: member

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

    Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

13. 
    DrahtBot commented at 9:10 am on January 3, 2022: member

    • 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.
14. 
    fanquake commented at 10:56 am on May 6, 2022: member

    Closing for now.
15. fanquake closed this on May 6, 2022

    ---

16. DrahtBot locked this on May 6, 2023

Contributors
rjnrohit MarcoFalke DrahtBot fanquake

Labels
TX fees and policy Needs rebase