From f50edbcbb5e01da87b32ee6b3244e3a475da6af6 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sat, 28 Dec 2024 17:10:39 -0500 Subject: [PATCH] Enforce MaxMagnitudeUnit clamp in GetMagnitudeUnit() Also implement 256 bit integer arithmetic in ComputeMRCFee() as an overflow fallback. --- src/gridcoin/accrual/snapshot.h | 52 ++++++++++++++++++--------------- src/gridcoin/mrc.cpp | 28 +++++++++++++----- 2 files changed, 50 insertions(+), 30 deletions(-) diff --git a/src/gridcoin/accrual/snapshot.h b/src/gridcoin/accrual/snapshot.h index 3b27f5e13f..78a43cb9c8 100644 --- a/src/gridcoin/accrual/snapshot.h +++ b/src/gridcoin/accrual/snapshot.h @@ -32,20 +32,6 @@ namespace { using namespace GRC; using LogFlags = BCLog::LogFlags; -/* -//! -//! \brief Numerator of the static magnitude unit coefficient for snapshot -//! accrual (block version 11 and greater). -//! -constexpr int64_t MAG_UNIT_NUMERATOR = 1; - -//! -//! \brief Denominator of the static magnitude unit coefficient for snapshot -//! accrual (block version 11 and greater). -//! -constexpr int64_t MAG_UNIT_DENOMINATOR = 4; -*/ - //! //! \brief Calculates the current accrual for a CPID by adding the snapshot of //! accrued research rewards of the CPID's research account to rewards accrued @@ -77,13 +63,14 @@ class SnapshotCalculator //! Allocation GetMagnitudeUnit() const { - Allocation magnitude_unit = Params().GetConsensus().DefaultMagnitudeUnit; - - // Before V13 magnitude unit is 1/4. + // Fern+ and before V13 magnitude unit is fixed at 1/4. if (!IsV13Enabled(m_superblock.m_height)) { return Allocation(1, 4); } + Allocation magnitude_unit = Params().GetConsensus().DefaultMagnitudeUnit; + Allocation max_magnitude_unit = Params().GetConsensus().MaxMagnitudeUnit; + // Find the current protocol entry value for Magnitude Weight Factor, if it exists. ProtocolEntryOption protocol_entry = GetProtocolRegistry().TryLastBeforeTimestamp("magnitudeunit", m_superblock.m_timestamp); @@ -93,6 +80,15 @@ class SnapshotCalculator magnitude_unit = Fraction().FromString(protocol_entry->m_value); } + // Clamp to MaxMagnitudeUnit if necessary + if (magnitude_unit > max_magnitude_unit) { + magnitude_unit = max_magnitude_unit; + LogPrintf("WARN: %s: Magnitude Unit specified by protocol is greater than %s. Clamping to %s.", + __func__, + max_magnitude_unit.ToString(), + max_magnitude_unit.ToString()); + } + return magnitude_unit; } @@ -107,22 +103,32 @@ class SnapshotCalculator { CBlockIndex* sb_index = BlockFinder::FindLatestSuperblock(index); - Allocation magnitude_unit = Params().GetConsensus().DefaultMagnitudeUnit; - - // Before V13 magnitude unit is 1/4. + // Before V13 magnitude unit is 1/4. if (!IsV13Enabled(sb_index->nHeight)) { return Allocation(1, 4); } - // Find the current protocol entry value for Magnitude Weight Factor, if it exists. + Allocation magnitude_unit = Params().GetConsensus().DefaultMagnitudeUnit; + Allocation max_magnitude_unit = Params().GetConsensus().MaxMagnitudeUnit; + + // Find the current protocol entry value for Magnitude Weight Factor, if it exists. ProtocolEntryOption protocol_entry = GetProtocolRegistry().TryLastBeforeTimestamp("magnitudeunit", sb_index->GetBlockTime()); - // If their is an entry prior or equal in timestamp to the superblock and it is active then set the magnitude unit - // to that value. If the last entry is not active (i.e. deleted), then leave at the default. + // If their is an entry prior or equal in timestamp to the superblock and it is active then set the magnitude unit + // to that value. If the last entry is not active (i.e. deleted), then leave at the default. if (protocol_entry != nullptr && protocol_entry->m_status == ProtocolEntryStatus::ACTIVE) { magnitude_unit = Fraction().FromString(protocol_entry->m_value); } + // Clamp to MaxMagnitudeUnit if necessary + if (magnitude_unit > max_magnitude_unit) { + magnitude_unit = max_magnitude_unit; + LogPrintf("WARN: %s: Magnitude Unit specified by protocol is greater than %s. Clamping to %s.", + __func__, + max_magnitude_unit.ToString(), + max_magnitude_unit.ToString()); + } + return magnitude_unit; } diff --git a/src/gridcoin/mrc.cpp b/src/gridcoin/mrc.cpp index ef8fcc7205..2250483a00 100644 --- a/src/gridcoin/mrc.cpp +++ b/src/gridcoin/mrc.cpp @@ -204,13 +204,27 @@ CAmount MRC::ComputeMRCFee() const // // If the MRC is exactly at the end of the zero_payout_interval, the fees are effectively // fee_fraction * m_research_subsidy. - // - // Overflow analysis. The accrual limit in the snapshot computer is 16384. For a zero_payout_interval of 14 days, - // m_research_subsidy * zero_payout_interval = 19818086400. std::numeric_limits::max() / 19818086400 - // = 465401747207, which means the numerator of the fee_fraction would have to be that number or higher to cause an - // overflow of the below. This is not likely to happen for any reasonable choice of the fee_fraction. - fee = m_research_subsidy * zero_payout_interval * fee_fraction.GetNumerator() - / mrc_payment_interval / fee_fraction.GetDenominator(); + + // To accommodate V13+ blocks which can have much larger magnitude units (up to 5), we have changed the fee calculation + // to switch to 256 bit integer calculations if necessary similar to what is in AccrualDelta(). The rationale for the overflow + // test is as follows. The m_research_subsidy converted back to GRC (which divides by 100,000,000 and therefore rounds down) + 1 + // to get the equivalent of rounding up, then multipled by the zero_payout_interval and the fee_fraction numerator is checked + // against the max numeric limit of CAmount divided by 100,000,000. If it is greater than this, then 256 bit integer arithmetic + // is used else the original calculation is used. + CAmount overflow_test = (m_research_subsidy / COIN + 1) * zero_payout_interval * fee_fraction.GetNumerator(); + + if (overflow_test > std::numeric_limits::max() / COIN) { + arith_uint256 fee_bn = m_research_subsidy; + fee_bn *= zero_payout_interval; + fee_bn *= fee_fraction.GetNumerator(); + fee_bn /= mrc_payment_interval; + fee_bn /= fee_fraction.GetDenominator(); + + fee = fee_bn.GetLow64(); + } else { + fee = m_research_subsidy * zero_payout_interval * fee_fraction.GetNumerator() + / mrc_payment_interval / fee_fraction.GetDenominator(); + } return fee; }