Skip to content

Commit

Permalink
Use base 10 for majority vote calculation:
Browse files Browse the repository at this point in the history
 *Add majority timer configuration
 *FIXES: XRPLF#3396
  • Loading branch information
gregtatcam authored and manojsdoshi committed Jun 24, 2020
1 parent 11271c2 commit ea0bcdd
Show file tree
Hide file tree
Showing 12 changed files with 318 additions and 118 deletions.
6 changes: 1 addition & 5 deletions src/ripple/app/main/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,6 @@

namespace ripple {

// 204/256 about 80%
static int const MAJORITY_FRACTION(204);

//------------------------------------------------------------------------------

namespace detail {
Expand Down Expand Up @@ -1533,8 +1530,7 @@ ApplicationImp::setup()
Section enabledAmendments = config_->section(SECTION_AMENDMENTS);

m_amendmentTable = make_AmendmentTable(
weeks{2},
MAJORITY_FRACTION,
config().AMENDMENT_MAJORITY_TIME,
supportedAmendments,
enabledAmendments,
config_->section(SECTION_VETO_AMENDMENTS),
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/app/misc/AmendmentTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class AmendmentTable
// inject pseudo-transactions
virtual std::map<uint256, std::uint32_t>
doVoting(
Rules const& rules,
NetClock::time_point closeTime,
std::set<uint256> const& enabledAmendments,
majorityAmendments_t const& majorityAmendments,
Expand Down Expand Up @@ -130,6 +131,7 @@ class AmendmentTable
{
// Ask implementation what to do
auto actions = doVoting(
lastClosedLedger->rules(),
lastClosedLedger->parentCloseTime(),
getEnabledAmendments(*lastClosedLedger),
getMajorityAmendments(*lastClosedLedger),
Expand Down Expand Up @@ -164,7 +166,6 @@ class AmendmentTable
std::unique_ptr<AmendmentTable>
make_AmendmentTable(
std::chrono::seconds majorityTime,
int majorityFraction,
Section const& supported,
Section const& enabled,
Section const& vetoed,
Expand Down
135 changes: 81 additions & 54 deletions src/ripple/app/misc/impl/AmendmentTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <ripple/app/misc/AmendmentTable.h>
#include <ripple/core/ConfigSections.h>
#include <ripple/core/DatabaseCon.h>
#include <ripple/protocol/Feature.h>
#include <ripple/protocol/STValidation.h>
#include <ripple/protocol/TxFlags.h>
#include <ripple/protocol/jss.h>
Expand Down Expand Up @@ -92,28 +93,74 @@ struct AmendmentState
};

/** The status of all amendments requested in a given window. */
struct AmendmentSet
class AmendmentSet
{
private:
// How many yes votes each amendment received
hash_map<uint256, int> votes_;

public:
Rules const& rules_;
// number of trusted validations
int mTrustedValidations = 0;

int trustedValidations_ = 0;
// number of votes needed
int mThreshold = 0;
int threshold_ = 0;

AmendmentSet() = default;
public:
AmendmentSet(
Rules const& rules,
std::vector<std::shared_ptr<STValidation>> const& valSet)
: rules_(rules)
{
// process validations for ledger before flag ledger
for (auto const& val : valSet)
{
if (val->isTrusted())
{
if (val->isFieldPresent(sfAmendments))
{
auto const choices = val->getFieldV256(sfAmendments);
std::for_each(
choices.begin(),
choices.end(),
[&](auto const& amendment) { ++votes_[amendment]; });
}

++trustedValidations_;
}
}

void
tally(std::set<uint256> const& amendments)
threshold_ = !rules_.enabled(fixAmendmentMajorityCalc)
? std::max(
1L,
static_cast<long>(
(trustedValidations_ *
preFixAmendmentMajorityCalcThreshold.num) /
preFixAmendmentMajorityCalcThreshold.den))
: std::max(
1L,
static_cast<long>(
(trustedValidations_ *
postFixAmendmentMajorityCalcThreshold.num) /
postFixAmendmentMajorityCalcThreshold.den));
}

bool
passes(uint256 const& amendment) const
{
++mTrustedValidations;
auto const& it = votes_.find(amendment);

for (auto const& amendment : amendments)
++votes_[amendment];
if (it == votes_.end())
return false;

// Before this fix, it was possible for an amendment to activate with a
// percentage slightly less than 80% because we compared for "greater
// than or equal to" instead of strictly "greater than".
// One validator is an exception, otherwise it is not possible
// to gain majority.
if (!rules_.enabled(fixAmendmentMajorityCalc) ||
trustedValidations_ == 1)
return it->second >= threshold_;

return it->second > threshold_;
}

int
Expand All @@ -126,6 +173,18 @@ struct AmendmentSet

return it->second;
}

int
trustedValidations() const
{
return trustedValidations_;
}

int
threshold() const
{
return threshold_;
}
};

//------------------------------------------------------------------------------
Expand All @@ -138,7 +197,7 @@ struct AmendmentSet
*/
class AmendmentTableImpl final : public AmendmentTable
{
protected:
private:
mutable std::mutex mutex_;

hash_map<uint256, AmendmentState> amendmentMap_;
Expand All @@ -147,10 +206,6 @@ class AmendmentTableImpl final : public AmendmentTable
// Time that an amendment must hold a majority for
std::chrono::seconds const majorityTime_;

// The amount of support that an amendment must receive
// 0 = 0% and 256 = 100%
int const majorityFraction_;

// The results of the last voting round - may be empty if
// we haven't participated in one yet.
std::unique_ptr<AmendmentSet> lastVote_;
Expand Down Expand Up @@ -187,7 +242,6 @@ class AmendmentTableImpl final : public AmendmentTable
public:
AmendmentTableImpl(
std::chrono::seconds majorityTime,
int majorityFraction,
Section const& supported,
Section const& enabled,
Section const& vetoed,
Expand Down Expand Up @@ -237,6 +291,7 @@ class AmendmentTableImpl final : public AmendmentTable

std::map<uint256, std::uint32_t>
doVoting(
Rules const& rules,
NetClock::time_point closeTime,
std::set<uint256> const& enabledAmendments,
majorityAmendments_t const& majorityAmendments,
Expand All @@ -247,19 +302,15 @@ class AmendmentTableImpl final : public AmendmentTable

AmendmentTableImpl::AmendmentTableImpl(
std::chrono::seconds majorityTime,
int majorityFraction,
Section const& supported,
Section const& enabled,
Section const& vetoed,
beast::Journal journal)
: lastUpdateSeq_(0)
, majorityTime_(majorityTime)
, majorityFraction_(majorityFraction)
, unsupportedEnabled_(false)
, j_(journal)
{
assert(majorityFraction_ != 0);

std::lock_guard sl(mutex_);

for (auto const& a : parseSection(supported))
Expand Down Expand Up @@ -461,6 +512,7 @@ AmendmentTableImpl::getDesired() const

std::map<uint256, std::uint32_t>
AmendmentTableImpl::doVoting(
Rules const& rules,
NetClock::time_point closeTime,
std::set<uint256> const& enabledAmendments,
majorityAmendments_t const& majorityAmendments,
Expand All @@ -470,31 +522,11 @@ AmendmentTableImpl::doVoting(
<< ": " << enabledAmendments.size() << ", "
<< majorityAmendments.size() << ", " << valSet.size();

auto vote = std::make_unique<AmendmentSet>();

// process validations for ledger before flag ledger
for (auto const& val : valSet)
{
if (val->isTrusted())
{
std::set<uint256> ballot;
auto vote = std::make_unique<AmendmentSet>(rules, valSet);

if (val->isFieldPresent(sfAmendments))
{
auto const choices = val->getFieldV256(sfAmendments);
ballot.insert(choices.begin(), choices.end());
}

vote->tally(ballot);
}
}

vote->mThreshold =
std::max(1, (vote->mTrustedValidations * majorityFraction_) / 256);

JLOG(j_.debug()) << "Received " << vote->mTrustedValidations
JLOG(j_.debug()) << "Received " << vote->trustedValidations()
<< " trusted validations, threshold is: "
<< vote->mThreshold;
<< vote->threshold();

// Map of amendments to the action to be taken for each one. The action is
// the value of the flags in the pseudo-transaction
Expand All @@ -507,8 +539,7 @@ AmendmentTableImpl::doVoting(
{
NetClock::time_point majorityTime = {};

bool const hasValMajority =
(vote->votes(entry.first) >= vote->mThreshold);
bool const hasValMajority = vote->passes(entry.first);

{
auto const it = majorityAmendments.find(entry.first);
Expand Down Expand Up @@ -614,18 +645,15 @@ AmendmentTableImpl::injectJson(

if (!fs.enabled && lastVote_)
{
auto const votesTotal = lastVote_->mTrustedValidations;
auto const votesNeeded = lastVote_->mThreshold;
auto const votesTotal = lastVote_->trustedValidations();
auto const votesNeeded = lastVote_->threshold();
auto const votesFor = lastVote_->votes(id);

v[jss::count] = votesFor;
v[jss::validations] = votesTotal;

if (votesNeeded)
{
v[jss::vote] = votesFor * 256 / votesNeeded;
v[jss::threshold] = votesNeeded;
}
}
}

Expand Down Expand Up @@ -666,14 +694,13 @@ AmendmentTableImpl::getJson(uint256 const& amendmentID) const
std::unique_ptr<AmendmentTable>
make_AmendmentTable(
std::chrono::seconds majorityTime,
int majorityFraction,
Section const& supported,
Section const& enabled,
Section const& vetoed,
beast::Journal journal)
{
return std::make_unique<AmendmentTableImpl>(
majorityTime, majorityFraction, supported, enabled, vetoed, journal);
majorityTime, supported, enabled, vetoed, journal);
}

} // namespace ripple
4 changes: 4 additions & 0 deletions src/ripple/core/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include <boost/lexical_cast.hpp>
#include <boost/optional.hpp>
#include <algorithm>
#include <chrono>
#include <cstdint>
#include <map>
#include <string>
Expand Down Expand Up @@ -172,6 +173,9 @@ class Config : public BasicConfig
// Compression
bool COMPRESSION = false;

// Amendment majority time
std::chrono::seconds AMENDMENT_MAJORITY_TIME = defaultAmendmentMajorityTime;

// Thread pool configuration
std::size_t WORKERS = 0;

Expand Down
1 change: 1 addition & 0 deletions src/ripple/core/ConfigSections.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ struct ConfigSection
#define SECTION_INSIGHT "insight"
#define SECTION_IPS "ips"
#define SECTION_IPS_FIXED "ips_fixed"
#define SECTION_AMENDMENT_MAJORITY_TIME "amendment_majority_time"
#define SECTION_NETWORK_QUORUM "network_quorum"
#define SECTION_NODE_SEED "node_seed"
#define SECTION_NODE_SIZE "node_size"
Expand Down
31 changes: 31 additions & 0 deletions src/ripple/core/impl/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,37 @@ Config::loadFromString(std::string const& fileContents)
if (getSingleSection(secConfig, SECTION_COMPRESSION, strTemp, j_))
COMPRESSION = beast::lexicalCastThrow<bool>(strTemp);

if (getSingleSection(
secConfig, SECTION_AMENDMENT_MAJORITY_TIME, strTemp, j_))
{
using namespace std::chrono;
boost::regex const re(
"^\\s*(\\d+)\\s*(minutes|hours|days|weeks)\\s*(\\s+.*)?$");
boost::smatch match;
if (!boost::regex_match(strTemp, match, re))
Throw<std::runtime_error>(
"Invalid " SECTION_AMENDMENT_MAJORITY_TIME
", must be: [0-9]+ [minutes|hours|days|weeks]");

std::uint32_t duration =
beast::lexicalCastThrow<std::uint32_t>(match[1].str());

if (boost::iequals(match[2], "minutes"))
AMENDMENT_MAJORITY_TIME = minutes(duration);
else if (boost::iequals(match[2], "hours"))
AMENDMENT_MAJORITY_TIME = hours(duration);
else if (boost::iequals(match[2], "days"))
AMENDMENT_MAJORITY_TIME = days(duration);
else if (boost::iequals(match[2], "weeks"))
AMENDMENT_MAJORITY_TIME = weeks(duration);

if (AMENDMENT_MAJORITY_TIME < minutes(15))
Throw<std::runtime_error>(
"Invalid " SECTION_AMENDMENT_MAJORITY_TIME
", the minimum amount of time an amendment must hold a "
"majority is 15 minutes");
}

// Do not load trusted validator configuration for standalone mode
if (!RUN_STANDALONE)
{
Expand Down
4 changes: 3 additions & 1 deletion src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ class FeatureCollections
"RequireFullyCanonicalSig",
"fix1781", // XRPEndpointSteps should be included in the circular
// payment check
"HardenedValidations"};
"HardenedValidations",
"fixAmendmentMajorityCalc"}; // Fix Amendment majority calculation

std::vector<uint256> features;
boost::container::flat_map<uint256, std::size_t> featureToIndex;
Expand Down Expand Up @@ -367,6 +368,7 @@ extern uint256 const fixQualityUpperBound;
extern uint256 const featureRequireFullyCanonicalSig;
extern uint256 const fix1781;
extern uint256 const featureHardenedValidations;
extern uint256 const fixAmendmentMajorityCalc;

} // namespace ripple

Expand Down
Loading

0 comments on commit ea0bcdd

Please sign in to comment.