Remove dead feeest-file read code for old versions #11951

pull TheBlueMatt wants to merge 1 commits into bitcoin:master from TheBlueMatt:2017-12-dead-feeest-load changing 1 files +15 −24

TheBlueMatt commented at 4:20 PM on December 19, 2017: member

0.15.0 introduced a new feeest file format, and support for parsing old versions was never fully added. We now simply fail to read the old format, so remove the dead partial-implementation.

Follow up to #11273.

Remove dead feeest-file read code for old versions

0.15.0 introduced a new feeest file format, and support for parsing
old versions was never fully added. We now simply fail to read the
old format, so remove the dead partial-implementation.

62e7c04fb8

laanwj added the label Mempool on Dec 19, 2017
laanwj added the label TX fees and policy on Dec 19, 2017
Xekyo commented at 4:31 PM on December 19, 2017: member

utACK 62e7c04
jnewbery commented at 4:50 PM on December 19, 2017: member

Why not remove nFileVersion from the TxConfirmStats::Read() function signature entirely?
TheBlueMatt commented at 4:59 PM on December 19, 2017: member

I figured it may be useful to have in a future change, so might as well leave it instead of creating more diff to change the function signature.
jnewbery commented at 6:26 PM on December 19, 2017: member
I'm able to hit the new check in #11273 by manually editing the nVersionRequired bytes in the fee_estimate.dat file:
```
...
2017-12-19 18:20:23 Read: incompatible old fee estimation data (non-fatal). Version: 140000
```
which means that TxConfirmStats::Read() doesn't get hit at all.

It'd be good to add a check in CBlockPolicyEstimator::Read() that nVersionThatWrote <= nVersionRequired, although that doesn't necessarily have to be done in this PR. That would ensure that we couldn't possibly enter TxConfirmStats::Read() if nVersionThatWrote < 149900.

it may be useful to have in a future change, so might as well leave it

My default preference is the other way. If code is unused it should be removed at the earliest opportunity.

Tested ACK 62e7c04fb82c1b99ee27b61af17cda343c9f9912
meshcollider commented at 8:43 PM on December 19, 2017: contributor

utACK https://github.com/bitcoin/bitcoin/commit/62e7c04fb82c1b99ee27b61af17cda343c9f9912
laanwj commented at 9:41 AM on December 20, 2017: member

utACK 62e7c04
laanwj merged this on Dec 20, 2017
laanwj closed this on Dec 20, 2017
laanwj referenced this in commit 1fb34e0d1f on Dec 20, 2017
PastaPastaPasta referenced this in commit a22ab27e06 on Feb 13, 2020
PastaPastaPasta referenced this in commit 1a57b6ac34 on Feb 27, 2020
PastaPastaPasta referenced this in commit 71ca6a3452 on Feb 27, 2020
PastaPastaPasta referenced this in commit 4a7a7cdfa2 on Feb 27, 2020
ckti referenced this in commit ac22981e2f on Mar 28, 2021
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

TX fees and policy Mempool

Linked (view graph)

#11273 Ignore old format estimation file #15187 fees: Complete the removal of fee-estimation file read code for old versions