From 75e1dbfabcdfe317a233031cd7cd28631d94dc48 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sat, 20 Jan 2024 22:50:11 -0500 Subject: [PATCH] Modify sidestake code to use new Allocation class instead of double Remove error banding in mandatory sidestake validation. --- src/chainparams.cpp | 4 +- src/consensus/params.h | 4 +- src/gridcoin/sidestake.cpp | 146 ++++++++++++++++++++------------- src/gridcoin/sidestake.h | 92 +++++++++++++++------ src/miner.cpp | 22 ++--- src/qt/sidestaketablemodel.cpp | 130 +++++++++++++---------------- src/rpc/mining.cpp | 2 +- src/rpc/rawtransaction.cpp | 2 +- src/validation.cpp | 18 ++-- 9 files changed, 241 insertions(+), 179 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index f69ff806ce..fa9e268052 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -75,7 +75,7 @@ class CMainParams : public CChainParams { // Zero day interval is 14 days on mainnet consensus.MRCZeroPaymentInterval = 14 * 24 * 60 * 60; // The maximum ratio of rewards that can be allocated to all of the mandatory sidestakes. - consensus.MaxMandatorySideStakeTotalAlloc = 0.25; + consensus.MaxMandatorySideStakeTotalAlloc = Fraction(1, 4); // The "standard" contract replay lookback for those contract types // that do not have a registry db. consensus.StandardContractReplayLookback = 180 * 24 * 60 * 60; @@ -190,7 +190,7 @@ class CTestNetParams : public CChainParams { // Zero day interval is 10 minutes on testnet. The very short interval facilitates testing. consensus.MRCZeroPaymentInterval = 10 * 60; // The maximum ratio of rewards that can be allocated to all of the mandatory sidestakes. - consensus.MaxMandatorySideStakeTotalAlloc = 0.25; + consensus.MaxMandatorySideStakeTotalAlloc = Fraction(1, 4); // The "standard" contract replay lookback for those contract types // that do not have a registry db. consensus.StandardContractReplayLookback = 180 * 24 * 60 * 60; diff --git a/src/consensus/params.h b/src/consensus/params.h index 10f5ab230f..b4e5e1a82e 100644 --- a/src/consensus/params.h +++ b/src/consensus/params.h @@ -48,9 +48,9 @@ struct Params { */ int64_t MRCZeroPaymentInterval; /** - * @brief The maximum allocation (as a floating point) that can be used by all of the mandatory sidestakes + * @brief The maximum allocation (as a Fraction) that can be used by all of the mandatory sidestakes */ - double MaxMandatorySideStakeTotalAlloc; + Fraction MaxMandatorySideStakeTotalAlloc; int64_t StandardContractReplayLookback; diff --git a/src/gridcoin/sidestake.cpp b/src/gridcoin/sidestake.cpp index a9c285b02f..ff53267003 100644 --- a/src/gridcoin/sidestake.cpp +++ b/src/gridcoin/sidestake.cpp @@ -33,18 +33,30 @@ SideStakeRegistry& GRC::GetSideStakeRegistry() return g_sidestake_entries; } -/* // ----------------------------------------------------------------------------- -// Class: CBitcoinAddressForStorage +// Class: Allocation // ----------------------------------------------------------------------------- -CBitcoinAddressForStorage::CBitcoinAddressForStorage() - : CBitcoinAddress() +Allocation::Allocation() + : Fraction() {} -CBitcoinAddressForStorage::CBitcoinAddressForStorage(CBitcoinAddress address) - : CBitcoinAddress(address) +Allocation::Allocation(const double& allocation) + : Fraction(static_cast(std::round(allocation * (double) 10000.0)), static_cast(10000), true) {} -*/ + +Allocation::Allocation(const Fraction& f) + : Fraction(f) +{} + +CAmount Allocation::ToCAmount() const +{ + return GetNumerator() / GetDenominator(); +} + +double Allocation::ToPercent() const +{ + return ToDouble() * 100.0; +} // ----------------------------------------------------------------------------- // Class: LocalSideStake @@ -56,7 +68,7 @@ LocalSideStake::LocalSideStake() , m_status(LocalSideStakeStatus::UNKNOWN) {} -LocalSideStake::LocalSideStake(CTxDestination destination, double allocation, std::string description) +LocalSideStake::LocalSideStake(CTxDestination destination, Allocation allocation, std::string description) : m_destination(destination) , m_allocation(allocation) , m_description(description) @@ -64,7 +76,7 @@ LocalSideStake::LocalSideStake(CTxDestination destination, double allocation, st {} LocalSideStake::LocalSideStake(CTxDestination destination, - double allocation, + Allocation allocation, std::string description, LocalSideStakeStatus status) : m_destination(destination) @@ -75,7 +87,7 @@ LocalSideStake::LocalSideStake(CTxDestination destination, bool LocalSideStake::WellFormed() const { - return CBitcoinAddress(m_destination).IsValid() && m_allocation >= 0.0 && m_allocation <= 1.0; + return CBitcoinAddress(m_destination).IsValid() && m_allocation >= 0 && m_allocation <= 1; } std::string LocalSideStake::StatusToString() const @@ -141,7 +153,7 @@ MandatorySideStake::MandatorySideStake() , m_status(MandatorySideStakeStatus::UNKNOWN) {} -MandatorySideStake::MandatorySideStake(CTxDestination destination, double allocation, std::string description) +MandatorySideStake::MandatorySideStake(CTxDestination destination, Allocation allocation, std::string description) : m_destination(destination) , m_allocation(allocation) , m_description(description) @@ -152,7 +164,7 @@ MandatorySideStake::MandatorySideStake(CTxDestination destination, double alloca {} MandatorySideStake::MandatorySideStake(CTxDestination destination, - double allocation, + Allocation allocation, std::string description, int64_t timestamp, uint256 hash, @@ -168,7 +180,7 @@ MandatorySideStake::MandatorySideStake(CTxDestination destination, bool MandatorySideStake::WellFormed() const { - return CBitcoinAddress(m_destination).IsValid() && m_allocation >= 0.0 && m_allocation <= 1.0; + return CBitcoinAddress(m_destination).IsValid() && m_allocation >= 0 && m_allocation <= 1; } CTxDestination MandatorySideStake::Key() const @@ -262,51 +274,67 @@ bool SideStake::IsMandatory() const CTxDestination SideStake::GetDestination() const { - if (IsMandatory()) { + if (m_type == Type::MANDATORY && m_mandatory_sidestake_ptr != nullptr) { return m_mandatory_sidestake_ptr->m_destination; - } else { + } else if (m_type == Type::LOCAL && m_local_sidestake_ptr != nullptr) { return m_local_sidestake_ptr->m_destination; } + + return CNoDestination(); } -double SideStake::GetAllocation() const +Allocation SideStake::GetAllocation() const { - if (IsMandatory()) { + if (m_type == Type::MANDATORY && m_mandatory_sidestake_ptr != nullptr) { return m_mandatory_sidestake_ptr->m_allocation; - } else { + } else if (m_type == Type::LOCAL && m_local_sidestake_ptr != nullptr) { return m_local_sidestake_ptr->m_allocation; } + + return Allocation(Fraction()); } std::string SideStake::GetDescription() const { - if (IsMandatory()) { + if (m_type == Type::MANDATORY && m_mandatory_sidestake_ptr != nullptr) { return m_mandatory_sidestake_ptr->m_description; - } else { + } else if (m_type == Type::LOCAL && m_local_sidestake_ptr != nullptr) { return m_local_sidestake_ptr->m_description; } + + return std::string {}; } SideStake::Status SideStake::GetStatus() const { - Status status; + // For trivial initializer case + if (m_mandatory_sidestake_ptr == nullptr && m_local_sidestake_ptr == nullptr) { + return {}; + } - if (IsMandatory()) { - status = m_mandatory_sidestake_ptr->m_status; - } else { - status = m_local_sidestake_ptr->m_status; + if (m_type == Type::MANDATORY && m_mandatory_sidestake_ptr != nullptr) { + return m_mandatory_sidestake_ptr->m_status; + } else if (m_type == Type::LOCAL && m_local_sidestake_ptr != nullptr) { + return m_local_sidestake_ptr->m_status; } - return status; + return {}; } std::string SideStake::StatusToString() const { - if (IsMandatory()) { + // For trivial initializer case + if (m_mandatory_sidestake_ptr == nullptr && m_local_sidestake_ptr == nullptr) { + return {}; + } + + if (m_type == Type::MANDATORY && m_mandatory_sidestake_ptr != nullptr) { return m_mandatory_sidestake_ptr->StatusToString(); - } else { + } else if (m_type == Type::LOCAL && m_local_sidestake_ptr != nullptr){ return m_local_sidestake_ptr->StatusToString(); } + + return std::string {}; } // ----------------------------------------------------------------------------- @@ -323,7 +351,7 @@ SideStakePayload::SideStakePayload(uint32_t version) SideStakePayload::SideStakePayload(const uint32_t version, CTxDestination destination, - double allocation, + Allocation allocation, std::string description, MandatorySideStake::MandatorySideStakeStatus status) : IContractPayload() @@ -368,7 +396,7 @@ const std::vector SideStakeRegistry::ActiveSideStakeEntries(const const bool& include_zero_alloc) const { std::vector sidestakes; - double allocation_sum = 0.0; + Allocation allocation_sum; // Note that LoadLocalSideStakesFromConfig is called upon a receipt of the core signal RwSettingsUpdated, which // occurs immediately after the settings r-w file is updated. @@ -382,7 +410,7 @@ const std::vector SideStakeRegistry::ActiveSideStakeEntries(const { if (entry.second->m_status == MandatorySideStake::MandatorySideStakeStatus::MANDATORY && allocation_sum + entry.second->m_allocation <= Params().GetConsensus().MaxMandatorySideStakeTotalAlloc) { - if ((include_zero_alloc && entry.second->m_allocation == 0.0) || entry.second->m_allocation > 0.0) { + if ((include_zero_alloc && entry.second->m_allocation == 0) || entry.second->m_allocation > 0) { sidestakes.push_back(std::make_shared(entry.second)); allocation_sum += entry.second->m_allocation; } @@ -400,8 +428,8 @@ const std::vector SideStakeRegistry::ActiveSideStakeEntries(const for (const auto& entry : m_local_sidestake_entries) { if (entry.second->m_status == LocalSideStake::LocalSideStakeStatus::ACTIVE - && allocation_sum + entry.second->m_allocation <= 1.0) { - if ((include_zero_alloc && entry.second->m_allocation == 0.0) || entry.second->m_allocation > 0.0) { + && allocation_sum + entry.second->m_allocation <= 1) { + if ((include_zero_alloc && entry.second->m_allocation == 0) || entry.second->m_allocation > 0) { sidestakes.push_back(std::make_shared(entry.second)); allocation_sum += entry.second->m_allocation; } @@ -516,7 +544,7 @@ void SideStakeRegistry::AddDelete(const ContractContext& ctx) ctx->m_version, payload.m_version, CBitcoinAddress(payload.m_entry.m_destination).ToString(), - payload.m_entry.m_allocation, + payload.m_entry.m_allocation.ToPercent(), payload.m_entry.m_timestamp, payload.m_entry.m_hash.ToString(), payload.m_entry.m_previous_hash.ToString(), @@ -532,7 +560,7 @@ void SideStakeRegistry::AddDelete(const ContractContext& ctx) "of the wallet to ensure multiple contracts in the same block get stored/replayed.", __func__, CBitcoinAddress(historical.m_destination).ToString(), - historical.m_allocation, + historical.m_allocation.ToPercent(), historical.m_hash.GetHex()); } @@ -556,7 +584,7 @@ void SideStakeRegistry::NonContractAdd(const LocalSideStake& sidestake, const bo { LOCK(cs_lock); - // Using this form of insert because we want the latest record with the same key to override any previous one. + // Using this form of insert because we want the latest record with the same key to override any previous one. m_local_sidestake_entries[sidestake.m_destination] = std::make_shared(sidestake); if (save_to_file) { @@ -661,7 +689,7 @@ bool SideStakeRegistry::Validate(const Contract& contract, const CTransaction& t return false; } - double allocation = payload->m_entry.m_allocation; + Allocation allocation = payload->m_entry.m_allocation; // Contracts that would result in a total active mandatory sidestake allocation greater than the maximum allowed by consensus // protocol must be rejected. Note that this is not a perfect validation, because there could be more than one sidestake @@ -747,7 +775,7 @@ void SideStakeRegistry::LoadLocalSideStakesFromConfig() std::vector vLocalSideStakes; std::vector> raw_vSideStakeAlloc; - double dSumAllocation = 0.0; + Allocation sum_allocation; // Parse destinations and allocations. We don't need to worry about any that are rejected other than a warning // message, because any unallocated rewards will go back into the coinstake output(s). @@ -808,17 +836,15 @@ void SideStakeRegistry::LoadLocalSideStakesFromConfig() } // First, add the allocation already taken by mandatory sidestakes, because they must be allocated first. - dSumAllocation += GetMandatoryAllocationsTotal(); + sum_allocation += GetMandatoryAllocationsTotal(); LOCK(cs_lock); for (const auto& entry : raw_vSideStakeAlloc) { - std::string sAddress; - - double dAllocation = 0.0; - - sAddress = std::get<0>(entry); + std::string sAddress = std::get<0>(entry); + std::string sAllocation = std::get<1>(entry); + std::string sDescription = std::get<2>(entry); CBitcoinAddress address(sAddress); if (!address.IsValid()) @@ -827,15 +853,21 @@ void SideStakeRegistry::LoadLocalSideStakesFromConfig() continue; } - if (!ParseDouble(std::get<1>(entry), &dAllocation)) + double read_allocation = 0.0; + if (!ParseDouble(sAllocation, &read_allocation)) { - LogPrintf("WARN: %s: Invalid allocation %s provided. Skipping allocation.", __func__, std::get<1>(entry)); + LogPrintf("WARN: %s: Invalid allocation %s provided. Skipping allocation.", __func__, sAllocation); continue; } - dAllocation /= 100.0; + LogPrintf("INFO: %s: allocation = %f", __func__, read_allocation); + + //int64_t numerator = read_allocation * 100.0; + //Allocation allocation(Fraction(numerator, 10000, true)); + + Allocation allocation(read_allocation / 100.0); - if (dAllocation < 0) + if (allocation < 0) { LogPrintf("WARN: %s: Negative allocation provided. Skipping allocation.", __func__); continue; @@ -846,16 +878,16 @@ void SideStakeRegistry::LoadLocalSideStakesFromConfig() // 1. Early alertment in the debug log, rather than when the first kernel is found, and 2. When the UI is // hooked up, the SideStakeAlloc vector will be filled in by other than reading the config file and will // skip the above code. - dSumAllocation += dAllocation; - if (dSumAllocation > 1.0) + sum_allocation += allocation; + if (sum_allocation > 1) { LogPrintf("WARN: %s: allocation percentage over 100 percent, ending sidestake allocations.", __func__); break; } LocalSideStake sidestake(address.Get(), - dAllocation, - std::get<2>(entry), + allocation, + sDescription, LocalSideStake::LocalSideStakeStatus::ACTIVE); // This will add or update (replace) a non-contract entry in the registry for the local sidestake. @@ -866,7 +898,7 @@ void SideStakeRegistry::LoadLocalSideStakesFromConfig() vLocalSideStakes.push_back(sidestake); LogPrint(BCLog::LogFlags::MINER, "INFO: %s: SideStakeAlloc Address %s, Allocation %f", - __func__, sAddress, dAllocation); + __func__, sAddress, allocation.ToPercent()); } for (auto& entry : m_local_sidestake_entries) @@ -885,7 +917,7 @@ void SideStakeRegistry::LoadLocalSideStakesFromConfig() // If we get here and dSumAllocation is zero then the enablesidestaking flag was set, but no VALID distribution // was provided in the config file, so warn in the debug log. - if (!dSumAllocation) + if (!sum_allocation) LogPrintf("WARN: %s: enablesidestaking was set in config but nothing has been allocated for" " distribution!", __func__); } @@ -911,7 +943,7 @@ bool SideStakeRegistry::SaveLocalSideStakesToConfig() } addresses += separator + CBitcoinAddress(iter.second->m_destination).ToString(); - allocations += separator + ToString(iter.second->m_allocation * 100.0); + allocations += separator + ToString(iter.second->m_allocation.ToPercent()); descriptions += separator + iter.second->m_description; ++i; @@ -928,10 +960,10 @@ bool SideStakeRegistry::SaveLocalSideStakesToConfig() return status; } -double SideStakeRegistry::GetMandatoryAllocationsTotal() const +Allocation SideStakeRegistry::GetMandatoryAllocationsTotal() const { std::vector sidestakes = ActiveSideStakeEntries(SideStake::FilterFlag::MANDATORY, false); - double allocation_total = 0.0; + Allocation allocation_total; for (const auto& entry : sidestakes) { allocation_total += entry->GetAllocation(); diff --git a/src/gridcoin/sidestake.h b/src/gridcoin/sidestake.h index f5ba1ba474..d27c91e19d 100644 --- a/src/gridcoin/sidestake.h +++ b/src/gridcoin/sidestake.h @@ -15,6 +15,50 @@ namespace GRC { +//! +//! \brief The Allocation class extends the Fraction class to provide functionality useful for sidestake allocations. +//! +class Allocation : public Fraction +{ +public: + //! + //! \brief Default constructor. Creates a zero allocation fraction. + //! + Allocation(); + + //! + //! \brief Allocation constructor from a double input. This multiplies the double by 1000, rounds, casts to int64_t, + //! and then constructs Fraction(x, 1000, true), which essentially creates a fraction representative of the double + //! to the third decimal place. + //! + //! \param double allocation + //! + Allocation(const double& allocation); + + //! + //! \brief Initialize an allocation from a Fraction. This is primarily used for casting. Note that no attempt to + //! limit the denominator size or simplify the fraction is made. + //! + //! \param Fraction f + //! + Allocation(const Fraction& f); + + //! + //! \brief Allocations extend the Fraction class and can also represent the result of the allocation constructed fraction + //! and the result of the muliplication of that fraction times the reward, which is in CAmount (i.e. int64_t). + //! + //! \return CAmount of the Fraction representation of the actual allocation. + //! + CAmount ToCAmount() const; + + //! + //! \brief Returns a double equivalent of the allocation fraction multiplied times 100. + //! + //! \return double percent representation of the allocation fraction. + //! + double ToPercent() const; +}; + //! //! \brief The LocalSideStake class. This class formalizes the local sidestake, which is a directive to apportion //! a percentage of the total stake value to a designated destination. This destination must be valid, but @@ -40,13 +84,13 @@ class LocalSideStake //! using Status = EnumByte; - CTxDestination m_destination; //!< The destination of the sidestake. + CTxDestination m_destination; //!< The destination of the sidestake. - double m_allocation; //!< The allocation is a double precision floating point between 0.0 and 1.0 inclusive + Allocation m_allocation; //!< The allocation is a Fraction in the form x / 1000 where x is between 0 and 1000 inclusive. - std::string m_description; //!< The description of the sidestake (optional) + std::string m_description; //!< The description of the sidestake (optional) - Status m_status; //!< The status of the sidestake. It is of type int instead of enum for serialization. + Status m_status; //!< The status of the sidestake. It is of type int instead of enum for serialization. //! @@ -62,7 +106,7 @@ class LocalSideStake //! \param allocation //! \param description (optional) //! - LocalSideStake(CTxDestination destination, double allocation, std::string description); + LocalSideStake(CTxDestination destination, Allocation allocation, std::string description); //! //! \brief Initialize a sidestake instance with the provided parameters. @@ -72,7 +116,7 @@ class LocalSideStake //! \param description (optional) //! \param status //! - LocalSideStake(CTxDestination destination, double allocation, std::string description, LocalSideStakeStatus status); + LocalSideStake(CTxDestination destination, Allocation allocation, std::string description, LocalSideStakeStatus status); //! //! \brief Determine whether a sidestake contains each of the required elements. @@ -156,19 +200,19 @@ class MandatorySideStake //! using Status = EnumByte; - CTxDestination m_destination; //!< The destination of the sidestake. + CTxDestination m_destination; //!< The destination of the sidestake. - double m_allocation; //!< The allocation is a double precision floating point between 0.0 and 1.0 inclusive + Allocation m_allocation; //!< The allocation is a Fraction in the form x / 1000 where x is between 0 and 1000 inclusive. - std::string m_description; //!< The description of the sidestake (optional) + std::string m_description; //!< The description of the sidestake (optional) - int64_t m_timestamp; //!< Time of the sidestake contract transaction. + int64_t m_timestamp; //!< Time of the sidestake contract transaction. - uint256 m_hash; //!< The hash of the transaction that contains a mandatory sidestake. + uint256 m_hash; //!< The hash of the transaction that contains a mandatory sidestake. - uint256 m_previous_hash; //!< The m_hash of the previous mandatory sidestake allocation with the same destination. + uint256 m_previous_hash; //!< The m_hash of the previous mandatory sidestake allocation with the same destination. - Status m_status; //!< The status of the sidestake. It is of type int instead of enum for serialization. + Status m_status; //!< The status of the sidestake. It is of type EnumByte instead of enum for serialization. //! //! \brief Initialize an empty, invalid sidestake instance. @@ -183,7 +227,7 @@ class MandatorySideStake //! \param allocation //! \param description (optional) //! - MandatorySideStake(CTxDestination destination, double allocation, std::string description); + MandatorySideStake(CTxDestination destination, Allocation allocation, std::string description); //! //! \brief Initialize a sidestake instance with the provided parameters. @@ -193,7 +237,7 @@ class MandatorySideStake //! \param description (optional) //! \param status //! - MandatorySideStake(CTxDestination destination, double allocation, std::string description, MandatorySideStakeStatus status); + MandatorySideStake(CTxDestination destination, Allocation allocation, std::string description, MandatorySideStakeStatus status); //! //! \brief Initialize a sidestake instance with the provided parameters. This form is normally used to construct a @@ -206,7 +250,7 @@ class MandatorySideStake //! \param hash //! \param status //! - MandatorySideStake(CTxDestination destination, double allocation, std::string description, int64_t timestamp, + MandatorySideStake(CTxDestination destination, Allocation allocation, std::string description, int64_t timestamp, uint256 hash, MandatorySideStakeStatus status); //! @@ -329,9 +373,9 @@ class SideStake CTxDestination GetDestination() const; //! //! \brief Gets the allocation of the sidestake - //! \return A double between 0.0 and 1.0 inclusive representing the allocation fraction of the sidestake + //! \return A Fraction representing the allocation fraction of the sidestake. //! - double GetAllocation() const; + Allocation GetAllocation() const; //! //! \brief Gets the description of the sidestake //! \return The description string of the sidestake @@ -412,7 +456,7 @@ class SideStakePayload : public IContractPayload //! \param description. Description string for the sidstake entry //! \param status. Status of the sidestake entry //! - SideStakePayload(const uint32_t version, CTxDestination destination, double allocation, + SideStakePayload(const uint32_t version, CTxDestination destination, Allocation allocation, std::string description, MandatorySideStake::MandatorySideStakeStatus status); //! @@ -468,7 +512,7 @@ class SideStakePayload : public IContractPayload __func__, valid, CBitcoinAddress(m_entry.m_destination).ToString(), - m_entry.m_allocation, + m_entry.m_allocation.ToPercent(), m_entry.StatusToString() ); @@ -491,7 +535,7 @@ class SideStakePayload : public IContractPayload //! std::string LegacyValueString() const override { - return ToString(m_entry.m_allocation); + return ToString(m_entry.m_allocation.ToDouble()); } //! @@ -771,10 +815,10 @@ class SideStakeRegistry : public IContractHandler bool SaveLocalSideStakesToConfig(); //! - //! \brief Provides the total allocation for all active mandatory sidestakes as a floating point fraction. - //! \return total active mandatory sidestake allocation as a double. + //! \brief Provides the total allocation for all active mandatory sidestakes as a Fraction. + //! \return total active mandatory sidestake allocation as a Fraction. //! - double GetMandatoryAllocationsTotal() const; + Allocation GetMandatoryAllocationsTotal() const; void SubscribeToCoreSignals(); void UnsubscribeFromCoreSignals(); diff --git a/src/miner.cpp b/src/miner.cpp index 257a7a2931..1c45764078 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -205,7 +205,7 @@ bool CreateMRCRewards(CBlock &blocknew, std::mapget()->GetDestination()); - double allocation = iterSideStake->get()->GetAllocation(); + GRC::Allocation allocation = iterSideStake->get()->GetAllocation(); if (!address.IsValid()) { @@ -967,16 +967,16 @@ void SplitCoinStakeOutput(CBlock &blocknew, int64_t &nReward, bool &fEnableStake // maximum, which means that the maximum number of mandatory outputs MUST be present and valid. // // Note that nOutputsUsed is NOT incremented if the output is suppressed by this check. - if (nReward * allocation < CENT) + if (allocation * nReward < CENT) { LogPrintf("WARN: SplitCoinStakeOutput: distribution %f too small to address %s.", - CoinToDouble(nReward * allocation), + CoinToDouble(static_cast(allocation * nReward).ToCAmount()), address.ToString() ); continue; } - if (dSumAllocation + allocation > 1.0) + if (SumAllocation + allocation > 1) { LogPrintf("WARN: SplitCoinStakeOutput: allocation percentage over 100 percent, " "ending sidestake allocations."); @@ -999,11 +999,11 @@ void SplitCoinStakeOutput(CBlock &blocknew, int64_t &nReward, bool &fEnableStake int64_t nSideStake = 0; // For allocations ending less than 100% assign using sidestake allocation. - if (dSumAllocation + allocation < 1.0) - nSideStake = nReward * allocation; + if (SumAllocation + allocation < 1) + nSideStake = static_cast(allocation * nReward).ToCAmount(); // We need to handle the final sidestake differently in the case it brings the total allocation up to 100%, // because testing showed in corner cases the output return to the staking address could be off by one Halford. - else if (dSumAllocation + allocation == 1.0) + else if (SumAllocation + allocation == 1) // Simply assign the special case final nSideStake the remaining output value minus input value to ensure // a match on the output flowing down. nSideStake = nRemainingStakeOutputValue - nInputValue; @@ -1012,10 +1012,10 @@ void SplitCoinStakeOutput(CBlock &blocknew, int64_t &nReward, bool &fEnableStake LogPrintf("SplitCoinStakeOutput: create sidestake UTXO %i value %f to address %s", nOutputsUsed, - CoinToDouble(nReward * allocation), + CoinToDouble(static_cast(allocation * nReward).ToCAmount()), address.ToString() ); - dSumAllocation += allocation; + SumAllocation += allocation; nRemainingStakeOutputValue -= nSideStake; nOutputsUsed++; } diff --git a/src/qt/sidestaketablemodel.cpp b/src/qt/sidestaketablemodel.cpp index 884a6f6f29..1b7af78559 100644 --- a/src/qt/sidestaketablemodel.cpp +++ b/src/qt/sidestaketablemodel.cpp @@ -151,18 +151,29 @@ QVariant SideStakeTableModel::data(const QModelIndex &index, int role) const GRC::SideStake* rec = static_cast(index.internalPointer()); const auto column = static_cast(index.column()); - if (role == Qt::DisplayRole || role == Qt::EditRole) { + if (role == Qt::DisplayRole) { switch (column) { case Address: return QString::fromStdString(CBitcoinAddress(rec->GetDestination()).ToString()); case Allocation: - return rec->GetAllocation() * 100.0; + return QString().setNum(rec->GetAllocation().ToPercent(), 'f', 2) + QString("\%"); case Description: return QString::fromStdString(rec->GetDescription()); case Status: return QString::fromStdString(rec->StatusToString()); } // no default case, so the compiler can warn about missing cases assert(false); + } else if (role == Qt::EditRole) { + switch (column) { + case Address: + return QString::fromStdString(CBitcoinAddress(rec->GetDestination()).ToString()); + case Allocation: + return QString().setNum(rec->GetAllocation().ToPercent(), 'f', 2); + case Description: + return QString::fromStdString(rec->GetDescription()); + case Status: + return QString::fromStdString(rec->StatusToString()); + } // no default case, so the compiler can warn about missing cases } else if (role == Qt::TextAlignmentRole) { switch (column) { case Address: @@ -201,58 +212,35 @@ bool SideStakeTableModel::setData(const QModelIndex &index, const QVariant &valu { case Address: { - CBitcoinAddress address; - address.SetString(value.toString().toStdString()); - - if (CBitcoinAddress(rec->GetDestination()) == address) { - m_edit_status = NO_CHANGES; - return false; - } else if (!address.IsValid()) { - m_edit_status = INVALID_ADDRESS; - return false; - } - - std::vector sidestakes = registry.Try(address.Get(), GRC::SideStake::FilterFlag::LOCAL); - - if (!sidestakes.empty()) { - m_edit_status = DUPLICATE_ADDRESS; - return false; - } - - // There is no valid state change left for address. If you are editing the item, the address field is - // not editable, so will be NO_CHANGES. For a non-matching address, it will be covered by the dialog - // in New mode. - break; + // The address of a sidestake entry is not editable. + return false; } case Allocation: { - double prior_total_allocation = 0.0; + GRC::Allocation prior_total_allocation; // Save the original local sidestake (also in the core). GRC::SideStake orig_sidestake = *rec; - CTxDestination orig_destination = rec->GetDestination(); - double orig_allocation = rec->GetAllocation(); - std::string orig_description = rec->GetDescription(); - GRC::SideStake::Status orig_status = rec->GetStatus(); + if (orig_sidestake.GetAllocation().ToPercent() == value.toDouble()) { + m_edit_status = NO_CHANGES; + return false; + } for (const auto& entry : registry.ActiveSideStakeEntries(GRC::SideStake::FilterFlag::ALL, true)) { CTxDestination destination = entry->GetDestination(); - double allocation = entry->GetAllocation(); + GRC::Allocation allocation = entry->GetAllocation(); - if (destination == orig_destination) { + if (destination == orig_sidestake.GetDestination()) { continue; } - prior_total_allocation += allocation * 100.0; + prior_total_allocation += allocation; } - if (orig_allocation * 100.0 == value.toDouble()) { - m_edit_status = NO_CHANGES; - return false; - } + GRC::Allocation modified_allocation(value.toDouble() / 100.0); - if (value.toDouble() < 0.0 || prior_total_allocation + value.toDouble() > 100.0) { + if (modified_allocation < 0 || prior_total_allocation + modified_allocation > 1) { m_edit_status = INVALID_ALLOCATION; LogPrint(BCLog::LogFlags::VERBOSE, "INFO: %s: m_edit_status = %i", @@ -262,14 +250,12 @@ bool SideStakeTableModel::setData(const QModelIndex &index, const QVariant &valu return false; } - // Delete the original sidestake - registry.NonContractDelete(orig_destination, false); - - // Add back the sidestake with the modified allocation - registry.NonContractAdd(GRC::LocalSideStake(orig_destination, - value.toDouble() / 100.0, - orig_description, - std::get(orig_status).Value()), true); + // Overwrite the existing sidestake entry with the modified allocation + registry.NonContractAdd(GRC::LocalSideStake(orig_sidestake.GetDestination(), + modified_allocation, + orig_sidestake.GetDescription(), + std::get(orig_sidestake.GetStatus()).Value()), + true); break; } @@ -291,14 +277,12 @@ bool SideStakeTableModel::setData(const QModelIndex &index, const QVariant &valu // Save the original local sidestake (also in the core). GRC::SideStake orig_sidestake = *rec; - // Delete the original sidestake - registry.NonContractDelete(orig_sidestake.GetDestination(), false); - - // Add back the sidestake with the modified allocation + // Overwrite the existing sidestake entry with the modified description registry.NonContractAdd(GRC::LocalSideStake(orig_sidestake.GetDestination(), orig_sidestake.GetAllocation(), san_value, - std::get(orig_sidestake.GetStatus()).Value()), true); + std::get(orig_sidestake.GetStatus()).Value()), + true); break; } @@ -334,12 +318,7 @@ Qt::ItemFlags SideStakeTableModel::flags(const QModelIndex &index) const Qt::ItemFlags retval = Qt::ItemIsSelectable | Qt::ItemIsEnabled; - GRC::SideStake::Status status = rec->GetStatus(); - GRC::LocalSideStake::Status* local_status_ptr = std::get_if(&status); - - if (!rec->IsMandatory() && local_status_ptr - && *local_status_ptr == GRC::LocalSideStake::LocalSideStakeStatus::ACTIVE - && (index.column() == Allocation || index.column() == Description)) { + if (!rec->IsMandatory() && (index.column() == Allocation || index.column() == Description)) { retval |= Qt::ItemIsEditable; } @@ -363,8 +342,6 @@ QString SideStakeTableModel::addRow(const QString &address, const QString &alloc CBitcoinAddress sidestake_address; sidestake_address.SetString(address.toStdString()); - double sidestake_allocation = 0.0; - m_edit_status = OK; if (!sidestake_address.IsValid()) { @@ -376,27 +353,36 @@ QString SideStakeTableModel::addRow(const QString &address, const QString &alloc // UI model. std::vector core_local_sidestake = registry.Try(sidestake_address.Get(), GRC::SideStake::FilterFlag::LOCAL); - double prior_total_allocation = 0.0; - - // Get total allocation of all active/mandatory sidestake entries - for (const auto& entry : registry.ActiveSideStakeEntries(GRC::SideStake::FilterFlag::ALL, true)) { - prior_total_allocation += entry->GetAllocation() * 100.0; - } - if (!core_local_sidestake.empty()) { m_edit_status = DUPLICATE_ADDRESS; return QString(); } - // The new allocation must be parseable as a double, must be greater than or equal to 0, and - // must result in a total allocation of less than 100. - if (!ParseDouble(allocation.toStdString(), &sidestake_allocation) - || sidestake_allocation < 0.0 || prior_total_allocation + sidestake_allocation > 100.0) { - m_edit_status = INVALID_ALLOCATION; - return QString(); + GRC::Allocation prior_total_allocation; + GRC::Allocation sidestake_allocation; + + // Get total allocation of all active/mandatory sidestake entries + for (const auto& entry : registry.ActiveSideStakeEntries(GRC::SideStake::FilterFlag::ALL, true)) { + prior_total_allocation += entry->GetAllocation(); } - sidestake_allocation /= 100.0; + // The new allocation must be parseable as a double, must be greater than or equal to 0, and + // must result in a total allocation of less than 100%. + double read_allocation = 0.0; + + if (!ParseDouble(allocation.toStdString(), &read_allocation)) { + if (read_allocation < 0.0) { + m_edit_status = INVALID_ALLOCATION; + return QString(); + } + + sidestake_allocation += GRC::Allocation(read_allocation / 100.0); + + if (prior_total_allocation + sidestake_allocation > 1) { + m_edit_status = INVALID_ALLOCATION; + return QString(); + } + } std::string sidestake_description = description.toStdString(); std::string sanitized_description = SanitizeString(sidestake_description, SAFE_CHARS_CSV); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 2d923c69bd..69eb2eaae9 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -118,7 +118,7 @@ UniValue getstakinginfo(const UniValue& params, bool fHelp) for (const auto& alloc : vSideStakeAlloc) { sidestakingalloc.pushKV("address", CBitcoinAddress(alloc->GetDestination()).ToString()); - sidestakingalloc.pushKV("allocation_pct", alloc->GetAllocation() * 100); + sidestakingalloc.pushKV("allocation_pct", alloc->GetAllocation().ToPercent()); sidestakingalloc.pushKV("status", alloc->StatusToString()); vsidestakingalloc.push_back(sidestakingalloc); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 16bd776f8d..d3688e459b 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -253,7 +253,7 @@ UniValue SideStakePayloadToJson (const GRC::ContractPayload& payload) UniValue out(UniValue::VOBJ); out.pushKV("address", CBitcoinAddress(sidestake.m_entry.m_destination).ToString()); - out.pushKV("allocation", sidestake.m_entry.m_allocation); + out.pushKV("allocation", sidestake.m_entry.m_allocation.ToPercent()); out.pushKV("description", sidestake.m_entry.m_description); out.pushKV("status", sidestake.m_entry.StatusToString()); diff --git a/src/validation.cpp b/src/validation.cpp index 26b4bd44e4..75abc76caa 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -657,7 +657,7 @@ unsigned int GetMRCOutputLimit(const int& block_version, bool include_foundation // in the returned limit) AND the foundation sidestake allocation is greater than zero, then reduce the reported // output limit by 1. If the foundation sidestake allocation is zero, then there will be no foundation sidestake // output, so the output_limit should be as above. If the output limit was already zero then it remains zero. - if (!include_foundation_sidestake && FoundationSideStakeAllocation().isNonZero() && output_limit) { + if (!include_foundation_sidestake && FoundationSideStakeAllocation().IsNonZero() && output_limit) { --output_limit; } @@ -805,7 +805,7 @@ class ClaimValidator // sidestake even though there will not be a corresponding mrc rewards output. (Zero value outputs are // suppressed because that is wasteful. bool foundation_mrc_sidestake_present = (m_claim.m_mrc_tx_map.size() - && FoundationSideStakeAllocation().isNonZero()) ? true : false; + && FoundationSideStakeAllocation().IsNonZero()) ? true : false; // If there is no mrc, then this is coinstake.vout.size() - 0 - 0, which is one beyond the last coinstake // element. @@ -857,7 +857,7 @@ class ClaimValidator // to an address that staked the coinstake (i.e. local to the staker's wallet), in favor of simply returning // the funds back to the staker on the coinstake return, is also removed from the vector here. for (auto iter = mandatory_sidestakes.begin(); iter != mandatory_sidestakes.end();) { - if (total_owed_to_staker * iter->get()->GetAllocation() < CENT + if (iter->get()->GetAllocation() * total_owed_to_staker < CENT || iter->get()->GetDestination() == coinstake_destination) { iter = mandatory_sidestakes.erase(iter); } else { @@ -875,18 +875,18 @@ class ClaimValidator return error("%s: FAILED: coinstake output has invalid destination."); } - double computed_output_alloc = (double) coinstake.vout[i].nValue / (double) total_owed_to_staker; + GRC::Allocation computed_output_alloc(Fraction(coinstake.vout[i].nValue, total_owed_to_staker, true)); std::vector mandatory_sidestake = GRC::GetSideStakeRegistry().TryActive(output_destination, GRC::SideStake::FilterFlag::MANDATORY);; - // The output is deemed to match if the destination matches AND - // the output amount expressed as a double fraction of the awards owed to staker is within 1% - // of the required mandatory allocation. We allow some leeway here because the sidestake allocations - // are double precision floating point fractions. + // The output is deemed to match if the destination matches AND the computed allocation matches or exceeds + // what is required by the mandatory sidestake. Note that the test uses the GRC::Allocation class, which + // extends the Fraction class, and provides comparison operators. This is now a precise calculation as it + // is integer arithmetic. if (!mandatory_sidestake.empty() - && abs(computed_output_alloc - mandatory_sidestake[0]->GetAllocation()) < 0.01) { + && computed_output_alloc >= mandatory_sidestake[0]->GetAllocation()) { ++validated_mandatory_sidestakes; }