log: Clarify that failure to read/write fee_estimates.dat is non-fatal #20589

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2012-logFeeest changing 1 files +8 −4

MarcoFalke commented at 1:14 PM on December 7, 2020: member

two minor logging fixups
log: Clarify that failure to write fee_estimates.dat is non-fatal faefa5db5f

log: Clarify that failure to read fee_estimates.dat is non-fatal

An uppercase "ERROR" in the log might indicate a fatal error. Though,
all read-failures for fee_estimates.dat are non-fatal, so avoid the
"ERROR".

Before:
ERROR: CBlockPolicyEstimator::Read(): up-version (149900) fee estimate file

After:
CBlockPolicyEstimator::Read(): unable to read policy estimator data (non-fatal): up-version (149900) fee estimate file

fa0d8359b3

MarcoFalke added the label Utils/log/libs on Dec 7, 2020
in src/policy/fees.cpp:913 in fa0d8359b3
```
 908 | @@ -907,8 +909,9 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein)
 909 |          LOCK(m_cs_fee_estimator);
 910 |          int nVersionRequired, nVersionThatWrote;
 911 |          filein >> nVersionRequired >> nVersionThatWrote;
 912 | -        if (nVersionRequired > CLIENT_VERSION)
 913 | -            return error("CBlockPolicyEstimator::Read(): up-version (%d) fee estimate file", nVersionRequired);
 914 | +        if (nVersionRequired > CLIENT_VERSION) {
 915 | +            throw std::runtime_error(strprintf("up-version (%d) fee estimate file", nVersionRequired));
```
practicalswift commented at 1:52 PM on December 7, 2020:

It looks like this could change behaviour (other than logging), but perhaps the exception handling is such that only logging is affected?

MarcoFalke commented at 2:01 PM on December 7, 2020:

perhaps the exception handling is such that only logging is affected?

Yes, the exception handling will only log, but this is clear from reading the code. Maybe I am misunderstanding your question?

practicalswift commented at 3:53 PM on December 7, 2020:

Oh, when expanding the diff that is clearly so :) I should have expanded the diff before commenting.
practicalswift commented at 4:36 PM on December 7, 2020: contributor

ACK fa0d8359b351fd179a0a2f458671a4d7828c9a80: patch looks correct
laanwj commented at 10:59 AM on December 10, 2020: member

Code review ACK fa0d8359b351fd179a0a2f458671a4d7828c9a80
laanwj merged this on Dec 10, 2020
laanwj closed this on Dec 10, 2020
MarcoFalke deleted the branch on Dec 10, 2020
sidhujag referenced this in commit de7a978a14 on Dec 10, 2020
DrahtBot locked this on Feb 15, 2022

Contributors

Labels