logging: remove duplicate categories from LogPrint output #25293

1. 
   ghost commented at 2:13 pm on June 7, 2022: none

   This is a follow up commit of #25203.

   logging: de-duplicate logging category output for the feeest messages (e.g. where I found duplicates), as these category prefixes are now printed automatically since #24464

2. 
   jonatack commented at 2:30 pm on June 7, 2022: member

   Thanks for looking. I left this message unchanged in #25286, as it seems useful to understanding the message itself rather than just being a category prefix. Though others may see it as a duplicate category, I don’t know.
3. DrahtBot added the label Refactoring on Jun 7, 2022
4. DrahtBot added the label TX fees and policy on Jun 7, 2022
5. in src/policy/fees.cpp:376 in 97a01ce2fd outdated

   372@@ -373,7 +373,7 @@ double TxConfirmStats::EstimateMedianVal(int confTarget, double sufficientTxVal,
   373         failed_within_target_perc = 100 * failBucket.withinTarget / (failBucket.totalConfirmed + failBucket.inMempool + failBucket.leftMempool);
   374     }
   375 
   376-    LogPrint(BCLog::ESTIMATEFEE, "FeeEst: %d > %.0f%% decay %.5f: feerate: %g from (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n",
   377+    LogPrint(BCLog::ESTIMATEFEE, " %d > %.0f%% decay %.5f: feerate: %g from (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n",
   

   ---

   ---

   

   jonatack commented at 2:47 pm on June 7, 2022:

   0    LogPrint(BCLog::ESTIMATEFEE, "%d > %.0f%% decay %.5f: feerate: %g from (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n",
   

   Note, the linter will fail because your commit message begins with “scripted-diff: " but doesn’t contain a scripted diff. The commit message would need to be something like this:

   0scripted-diff: remove duplicate categories from LogPrint ESTIMATEFEE output
   1
   2-BEGIN VERIFY SCRIPT-
   3s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; }
   4s 'BCLog::ESTIMATEFEE, "FeeEst: '   'BCLog::ESTIMATEFEE, "'
   5-END VERIFY SCRIPT-
   

   See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#scripted-diffs for more info.

   That said, this is the only occurrence, so if reviewers agree that this change should be done, you can just change the commit message (and PR title) from “scripted-diff:” to “logging:”.

   ---

   

   unknown commented at 2:51 pm on June 7, 2022:

   Thank you @jonatack, I will change the title

   ---

   

   unknown commented at 3:56 pm on June 7, 2022:

   I had changed the commit message too, now the linter should work. Thanks @jonatack.
6. unknown renamed this:
   scripted-diff: remove duplicate categories from LogPrint output
   logging: remove duplicate categories from LogPrint output
   on Jun 7, 2022
7. requested review from jonatack on Jun 7, 2022
8. 
   jonatack commented at 3:19 pm on June 7, 2022: member

   Yes, seems better now. Though, as I wrote above, I’m not sure this should be changed. Currently, the following would be logged, and without “FeeEst:” it may not be very clear.

   0[estimatefee] FeeEst: 5 > 60% decay 0.96200: feerate: 2326.67 from (2292.02 - 2925.26) 82.00% 16.3/(19.9 0 mem 0.0 out) Fail: (1979.93 - 2292.02) 56.56% 9.9/(17.3 0 mem 0.2 out)
   

   Here are the ESTIMATEFEE logging messages:

    0$ git grep BCLog::ESTIMATEFEE
    1src/policy/fees.cpp:376:    LogPrint(BCLog::ESTIMATEFEE, "FeeEst: %d > %.0f%% decay %.5f: feerate: %g from (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n",
    2src/policy/fees.cpp:457:    LogPrint(BCLog::ESTIMATEFEE, "Reading estimates: %u buckets counting confirms up to %u blocks\n",
    3src/policy/fees.cpp:476:        LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy error, blocks ago is negative for mempool tx\n");
    4src/policy/fees.cpp:484:            LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy error, mempool tx removed from >25 blocks,bucketIndex=%u already\n",
    5src/policy/fees.cpp:493:            LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy error, mempool tx removed from blockIndex=%u,bucketIndex=%u already\n",
    6src/policy/fees.cpp:566:        LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy error mempool tx %s already being tracked\n",
    7src/policy/fees.cpp:614:        LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy error Transaction had negative blocksToConfirm\n");
    8src/policy/fees.cpp:664:        LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy first recorded height %u\n", firstRecordedHeight);
    9src/policy/fees.cpp:668:    LogPrint(BCLog::ESTIMATEFEE, "Blockpolicy estimates updated by %u of %u block txs, since last block %u of %u tracked, mempool map size %u, max target %u from %s\n",
   10src/policy/fees.cpp:1014:    LogPrint(BCLog::ESTIMATEFEE, "Recorded %u unconfirmed txs from mempool in %gs\n", num_entries, (endclear - startclear)*0.000001);
   

9. logging: remove duplicate categories from LogPrint output a28a154bb4
10. unknown force-pushed on Jun 7, 2022
11. 
    laanwj commented at 4:11 pm on June 16, 2022: member

    Though, as I wrote above, I’m not sure this should be changed

    Also divided on this. The message is already hard to understand as-is, if it just begins with numbers doesn’t make it better.

12. 
    MarcoFalke commented at 4:14 pm on June 16, 2022: member

    Closing for now, as it appears questionable
13. MarcoFalke closed this on Jun 16, 2022

    ---

14. DrahtBot locked this on Jun 16, 2023

Contributors
ghost jonatack laanwj MarcoFalke

Review Requested
jonatack

Labels
Refactoring TX fees and policy