Skip to content
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

Enhance the log for failures when reading or parsing the fee_estimates.dat file #3501

Open
wants to merge 7 commits into
base: 1.15.0-dev
Choose a base branch
from

Conversation

AjaxPop
Copy link
Contributor

@AjaxPop AjaxPop commented Mar 29, 2024

Use a try-catch block for error handling in the TxConfirmStats::Read function.

Use a try-catch block for error handling in the TxConfirmStats::Read function.
@patricklodder
Copy link
Member

What error didn't get caught?

@AjaxPop
Copy link
Contributor Author

AjaxPop commented Mar 29, 2024

Although the check on fileDecay ensures the integrity of the data by confirming that it is between 0 (no decay) and 1 (full decay), there remains a potential issue with data corruption or a software bug.

The try-catch block has the benefit of logging any potential errors.

@georgeartem
Copy link

georgeartem commented Mar 30, 2024 via email

@patricklodder
Copy link
Member

Okay. An overall try-catch already is implemented in the calling code:

LogPrintf("CTxMemPool::ReadFeeEstimates(): unable to read policy estimator data (non-fatal)\n");

If you're looking to enrich the logged message (I'd support that), it should be done in that catch block instead.

@AjaxPop AjaxPop changed the title Update fees.cpp [try-catch] try-catch Mar 31, 2024
@patricklodder patricklodder requested a review from a team April 1, 2024 19:51
@patricklodder patricklodder added this to the 1.15.0 milestone Apr 1, 2024
@patricklodder patricklodder added this to 🚀 needs review in Review & merge board Apr 1, 2024
@patricklodder patricklodder changed the title try-catch Enhance the log for failures when reading or parsing the fee_estimates.dat file Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Review & merge board
🚀 needs review
Development

Successfully merging this pull request may close these issues.

None yet

3 participants