search

dogecoin / dogecoin

very currency
MIT License
14.41k stars 2.82k forks source link

policy: only load fee_estimates.dat from 1.14.7 and later #3433

Closed patricklodder closed 3 months ago

patricklodder commented 3 months ago

This PR addresses the caveat of having to manually remove fee_estimates.dat from the datadir to fully benefit from the improved estimator parametrization (#3389) for most deployments.

The only deployments that will still need to manually remove the file are:

Note: using the client version number for file format versioning is suboptimal - this causes the above exceptions to automated detection. Upstream knows this too but chose to not change it when they did something similar to what we're doing now. I had addressed this in my planned follow-up to #3389 that makes parametrization user-configurable, but I feel that doing it now is too risky due to time constraints, so let's do a proper fix for this when we need it, i.e. when we fix configurability of these parameters in a next version, because that would require many more checks to decide whether to re-use historical estimates anyway.

To test:

  1. Run a 1.14.6 client on a separate datadir (regtest) and feed it some transactions and blocks
  2. Shut it down, check that it saved fee_estimates.dat
  3. Compile and run this PR on the same datadir
  4. During initialization, a non-fatal error message must appear in debug.log:
ERROR: CTxMemPool::ReadFeeEstimates(): cannot re-use fee estimate files from version 1140600, ignoring file (non-fatal)
  1. Feed it some transactions and blocks and shut it down, check that it saved a new fee_estimates.dat
  2. Run 1.14.6 again on the same datadir
  3. During initialization, a non-fatal error message must appear in debug.log:
CTxMemPool::ReadFeeEstimates(): up-version (1140700) fee estimate file
patricklodder commented 3 months ago

I'll rebase this after #3389 is merged.

victorsk2019 commented 3 months ago

Nice! I got into habit seeing red flags whenever m-word (manual) is encountered 🤣

patricklodder commented 3 months ago

I got into habit seeing red flags whenever m-word (manual) is encountered

It's worth fixing imho, yes. Just gotta fix it ugly for this round, but with minimal impact to operations.

patricklodder commented 3 months ago

rebased on top of fc128f3a4

chromatic commented 3 months ago

Code makes sense to me. I'll work on testing.