Adapt hard-coded version number #969
Conversation
Change the minimum required client version for the fee estimator file to 0.1.0. This fixes the bug which prevents loading of the file. Doing this as a minimal change for now. Could be implemented in a cleaner way in the future. Fixes #968 Signed-off-by: Cornelius Schumacher <cornelius@thirdhash.com>
| @@ -900,7 +900,7 @@ bool CBlockPolicyEstimator::Write(CAutoFile& fileout) const | |||
| { | |||
| try { | |||
| LOCK(cs_feeEstimator); | |||
| fileout << 149900; // version required to read: 0.14.99 or later | |||
| fileout << 10000; // version required to read: 0.1.0 or later | |||
There was a problem hiding this comment.
why do we even need to write the version number here? for which purpose is it?
There was a problem hiding this comment.
This is a legacy from Bitcoin Core, because they apparently changed the format with version 0.14.99, so they wrote the version to detect that. We, of course, only have one version now, so this could be done cleaner, but I'm not sure we want to clean this up now. There are other places in the code which could be cleaned up regarding the version handling as well.
There was a problem hiding this comment.
ok do we already have an issue for it, for cleaning the codebase from hardcoded version numbers or so?
There was a problem hiding this comment.
No, I don't think we have an issue yet. We removed most hard-coded version numbers by now, I believe. But there is logic left which does not make too much sense without them.
There was a problem hiding this comment.
There is an issue now: #980
| @@ -935,9 +935,9 @@ bool CBlockPolicyEstimator::Read(CAutoFile& filein) | |||
| unsigned int nFileBestSeenHeight; | |||
| filein >> nFileBestSeenHeight; | |||
|
|
|||
| if (nVersionRequired < 149900) { | |||
There was a problem hiding this comment.
Does this check make sense for us in the first place?
We only have one format, the one we'll start with.
|
I'll merge this to fix the bug now. I would suggest to revisit the versioning handling later from a bit more general point of view. I think it would make sense to do that after the 0.17 merge. I'll create an issue for later. |
Change the minimum required client version for the fee estimator file
to 0.1.0. This fixes the bug which prevents loading of the file.
Doing this as a minimal change for now. Could be implemented in a
cleaner way in the future.
Fixes #968