foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-01 09:35:52 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#29702: fees: Remove CLIENT_VERSION serialization

Browse source 
fa1c5cc9df fees: Log non-fatal errors as [warning], instead of info-level (MarcoFalke)
ddddbac9c1 fees: Pin required version to 149900 (MarcoFalke)
fa5126adcb fees: Pin "version that wrote" to 0 (MarcoFalke)

Pull request description:

  Coupling the fees serialization with CLIENT_VERSION is problematic, because:

  * `CLIENT_VERSION` may change, even though the serialization format does not change. This is harmless, but still confusing.
  * If a serialization format change was backported (unlikely), it may lead to incorrect results.
  * `CLIENT_VERSION` is changed at a different time during the release process than any serialization format change. This is harmless for releases of Bitcoin Core, but may be confusing when using the development branch.
  * It is harder to reason about a global `CLIENT_VERSION` when changing the format, than to reason about a versioning local to the module.

  Fix all issues by pinning the current version number in the module locally. In the future it can then be modified locally to the module, if needed.

ACKs for top commit:
  hodlinator:
    re-ACK fa1c5cc9df
  TheCharlatan:
    Re-ACK fa1c5cc9df

Tree-SHA512: 93870176ed50cc5a734576d66398a6036b31632228a9e05db1fa5452229e35ba4126f003e7db246aeb9891764ed47bde4470c674ec2bce7fd3ddd97e43944627
This commit is contained in:
merge-script 2024-10-24 10:09:36 +01:00
commit d94adc7270
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
1 changed files with 20 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
src/policy/fees.cpp
View file

@ -5,7 +5,6 @@
#include <policy/fees.h>
#include <clientversion.h>
#include <common/system.h>
#include <consensus/amount.h>
#include <kernel/mempool_entry.h>
@ -32,6 +31,10 @@
#include <stdexcept>
#include <utility>
// The current format written, and the version required to read. Must be
// increased to at least 289900+1 on the next breaking change.
constexpr int CURRENT_FEES_FILE_VERSION{149900};
static constexpr double INF_FEERATE = 1e99;
std::string StringForFeeEstimateHorizon(FeeEstimateHorizon horizon)
@ -168,7 +171,7 @@ public:
* Read saved state of estimation data from a file and replace all internal data structures and
* variables with this state.
*/
void Read(AutoFile& filein, int nFileVersion, size_t numBuckets);
void Read(AutoFile& filein, size_t numBuckets);
};
@ -414,7 +417,7 @@ void TxConfirmStats::Write(AutoFile& fileout) const
fileout << Using<VectorFormatter<VectorFormatter<EncodedDoubleFormatter>>>(failAvg);
}
void TxConfirmStats::Read(AutoFile& filein, int nFileVersion, size_t numBuckets)
void TxConfirmStats::Read(AutoFile& filein, size_t numBuckets)
{
// Read data file and do some very basic sanity checking
// buckets and bucketMap are not updated yet, so don't access them
@ -961,8 +964,8 @@ bool CBlockPolicyEstimator::Write(AutoFile& fileout) const
{
try {
LOCK(m_cs_fee_estimator);
fileout << 149900; // version required to read: 0.14.99 or later
fileout << CLIENT_VERSION; // version that wrote the file
fileout << CURRENT_FEES_FILE_VERSION;
fileout << int{0}; // Unused dummy field. Written files may contain any value in [0, 289900]
fileout << nBestSeenHeight;
if (BlockSpan() > HistoricalBlockSpan()/2) {
fileout << firstRecordedHeight << nBestSeenHeight;
@ -976,7 +979,7 @@ bool CBlockPolicyEstimator::Write(AutoFile& fileout) const
longStats->Write(fileout);
}
catch (const std::exception&) {
LogPrintf("CBlockPolicyEstimator::Write(): unable to write policy estimator data (non-fatal)\n");
LogWarning("Unable to write policy estimator data (non-fatal)");
return false;
}
return true;
@ -986,10 +989,10 @@ bool CBlockPolicyEstimator::Read(AutoFile& filein)
{
try {
LOCK(m_cs_fee_estimator);
int nVersionRequired, nVersionThatWrote;
filein >> nVersionRequired >> nVersionThatWrote;
if (nVersionRequired > CLIENT_VERSION) {
throw std::runtime_error(strprintf("up-version (%d) fee estimate file", nVersionRequired));
int nVersionRequired, dummy;
filein >> nVersionRequired >> dummy;
if (nVersionRequired > CURRENT_FEES_FILE_VERSION) {
throw std::runtime_error{strprintf("File version (%d) too high to be read.", nVersionRequired)};
}
// Read fee estimates file into temporary variables so existing data
@ -997,9 +1000,9 @@ bool CBlockPolicyEstimator::Read(AutoFile& filein)
unsigned int nFileBestSeenHeight;
filein >> nFileBestSeenHeight;
if (nVersionRequired < 149900) {
LogPrintf("%s: incompatible old fee estimation data (non-fatal). Version: %d\n", __func__, nVersionRequired);
} else { // New format introduced in 149900
if (nVersionRequired < CURRENT_FEES_FILE_VERSION) {
LogWarning("Incompatible old fee estimation data (non-fatal). Version: %d", nVersionRequired);
} else { // nVersionRequired == CURRENT_FEES_FILE_VERSION
unsigned int nFileHistoricalFirst, nFileHistoricalBest;
filein >> nFileHistoricalFirst >> nFileHistoricalBest;
if (nFileHistoricalFirst > nFileHistoricalBest || nFileHistoricalBest > nFileBestSeenHeight) {
@ -1015,9 +1018,9 @@ bool CBlockPolicyEstimator::Read(AutoFile& filein)
std::unique_ptr<TxConfirmStats> fileFeeStats(new TxConfirmStats(buckets, bucketMap, MED_BLOCK_PERIODS, MED_DECAY, MED_SCALE));
std::unique_ptr<TxConfirmStats> fileShortStats(new TxConfirmStats(buckets, bucketMap, SHORT_BLOCK_PERIODS, SHORT_DECAY, SHORT_SCALE));
std::unique_ptr<TxConfirmStats> fileLongStats(new TxConfirmStats(buckets, bucketMap, LONG_BLOCK_PERIODS, LONG_DECAY, LONG_SCALE));
fileFeeStats->Read(filein, nVersionThatWrote, numBuckets);
fileShortStats->Read(filein, nVersionThatWrote, numBuckets);
fileLongStats->Read(filein, nVersionThatWrote, numBuckets);
fileFeeStats->Read(filein, numBuckets);
fileShortStats->Read(filein, numBuckets);
fileLongStats->Read(filein, numBuckets);
// Fee estimates file parsed correctly
// Copy buckets from file and refresh our bucketmap
@ -1038,7 +1041,7 @@ bool CBlockPolicyEstimator::Read(AutoFile& filein)
}
}
catch (const std::exception& e) {
LogPrintf("CBlockPolicyEstimator::Read(): unable to read policy estimator data (non-fatal): %s\n",e.what());
LogWarning("Unable to read policy estimator data (non-fatal): %s", e.what());
return false;
}
return true;