Skip to content

Commit

Permalink
Enforce MaxMagnitudeUnit clamp in GetMagnitudeUnit()
Browse files Browse the repository at this point in the history
Also implement 256 bit integer arithmetic in ComputeMRCFee() as an
overflow fallback.
  • Loading branch information
jamescowens committed Jan 4, 2025
1 parent 3681308 commit f50edbc
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 30 deletions.
52 changes: 29 additions & 23 deletions src/gridcoin/accrual/snapshot.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -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;
}

Expand All @@ -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;
}

Expand Down
28 changes: 21 additions & 7 deletions src/gridcoin/mrc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<int64_t>::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<CAmount>::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;
}
Expand Down

0 comments on commit f50edbc

Please sign in to comment.