diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 398212fa556..8ca82ecae6d 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -77,9 +77,6 @@ namespace ripple { -// 204/256 about 80% -static int const MAJORITY_FRACTION(204); - //------------------------------------------------------------------------------ namespace detail { @@ -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), diff --git a/src/ripple/app/misc/AmendmentTable.h b/src/ripple/app/misc/AmendmentTable.h index 0ac55858074..bcd21f763b5 100644 --- a/src/ripple/app/misc/AmendmentTable.h +++ b/src/ripple/app/misc/AmendmentTable.h @@ -99,6 +99,7 @@ class AmendmentTable // inject pseudo-transactions virtual std::map doVoting( + Rules const& rules, NetClock::time_point closeTime, std::set const& enabledAmendments, majorityAmendments_t const& majorityAmendments, @@ -130,6 +131,7 @@ class AmendmentTable { // Ask implementation what to do auto actions = doVoting( + lastClosedLedger->rules(), lastClosedLedger->parentCloseTime(), getEnabledAmendments(*lastClosedLedger), getMajorityAmendments(*lastClosedLedger), @@ -164,7 +166,6 @@ class AmendmentTable std::unique_ptr make_AmendmentTable( std::chrono::seconds majorityTime, - int majorityFraction, Section const& supported, Section const& enabled, Section const& vetoed, diff --git a/src/ripple/app/misc/impl/AmendmentTable.cpp b/src/ripple/app/misc/impl/AmendmentTable.cpp index 4d499ea9172..2f29a2f5788 100644 --- a/src/ripple/app/misc/impl/AmendmentTable.cpp +++ b/src/ripple/app/misc/impl/AmendmentTable.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -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 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> 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 const& amendments) + threshold_ = !rules_.enabled(fixAmendmentMajorityCalc) + ? std::max( + 1L, + static_cast( + (trustedValidations_ * + preFixAmendmentMajorityCalcThreshold.num) / + preFixAmendmentMajorityCalcThreshold.den)) + : std::max( + 1L, + static_cast( + (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 @@ -126,6 +173,18 @@ struct AmendmentSet return it->second; } + + int + trustedValidations() const + { + return trustedValidations_; + } + + int + threshold() const + { + return threshold_; + } }; //------------------------------------------------------------------------------ @@ -138,7 +197,7 @@ struct AmendmentSet */ class AmendmentTableImpl final : public AmendmentTable { -protected: +private: mutable std::mutex mutex_; hash_map amendmentMap_; @@ -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 lastVote_; @@ -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, @@ -237,6 +291,7 @@ class AmendmentTableImpl final : public AmendmentTable std::map doVoting( + Rules const& rules, NetClock::time_point closeTime, std::set const& enabledAmendments, majorityAmendments_t const& majorityAmendments, @@ -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)) @@ -461,6 +512,7 @@ AmendmentTableImpl::getDesired() const std::map AmendmentTableImpl::doVoting( + Rules const& rules, NetClock::time_point closeTime, std::set const& enabledAmendments, majorityAmendments_t const& majorityAmendments, @@ -470,31 +522,11 @@ AmendmentTableImpl::doVoting( << ": " << enabledAmendments.size() << ", " << majorityAmendments.size() << ", " << valSet.size(); - auto vote = std::make_unique(); - - // process validations for ledger before flag ledger - for (auto const& val : valSet) - { - if (val->isTrusted()) - { - std::set ballot; + auto vote = std::make_unique(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 @@ -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); @@ -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; - } } } @@ -666,14 +694,13 @@ AmendmentTableImpl::getJson(uint256 const& amendmentID) const std::unique_ptr make_AmendmentTable( std::chrono::seconds majorityTime, - int majorityFraction, Section const& supported, Section const& enabled, Section const& vetoed, beast::Journal journal) { return std::make_unique( - majorityTime, majorityFraction, supported, enabled, vetoed, journal); + majorityTime, supported, enabled, vetoed, journal); } } // namespace ripple diff --git a/src/ripple/core/Config.h b/src/ripple/core/Config.h index 7943906fdae..7eec3bc0764 100644 --- a/src/ripple/core/Config.h +++ b/src/ripple/core/Config.h @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -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; diff --git a/src/ripple/core/ConfigSections.h b/src/ripple/core/ConfigSections.h index c9b61c2cb2b..3aae9774d10 100644 --- a/src/ripple/core/ConfigSections.h +++ b/src/ripple/core/ConfigSections.h @@ -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" diff --git a/src/ripple/core/impl/Config.cpp b/src/ripple/core/impl/Config.cpp index f12ba7dbcee..cd272fd885c 100644 --- a/src/ripple/core/impl/Config.cpp +++ b/src/ripple/core/impl/Config.cpp @@ -480,6 +480,37 @@ Config::loadFromString(std::string const& fileContents) if (getSingleSection(secConfig, SECTION_COMPRESSION, strTemp, j_)) COMPRESSION = beast::lexicalCastThrow(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( + "Invalid " SECTION_AMENDMENT_MAJORITY_TIME + ", must be: [0-9]+ [minutes|hours|days|weeks]"); + + std::uint32_t duration = + beast::lexicalCastThrow(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( + "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) { diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index e47778a03a4..ae05c8c1d11 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -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 features; boost::container::flat_map featureToIndex; @@ -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 diff --git a/src/ripple/protocol/SystemParameters.h b/src/ripple/protocol/SystemParameters.h index a74155a6a32..2a59de656d6 100644 --- a/src/ripple/protocol/SystemParameters.h +++ b/src/ripple/protocol/SystemParameters.h @@ -21,6 +21,7 @@ #define RIPPLE_PROTOCOL_SYSTEMPARAMETERS_H_INCLUDED #include +#include #include #include @@ -59,6 +60,18 @@ systemCurrencyCode() /** The XRP ledger network's earliest allowed sequence */ static std::uint32_t constexpr XRP_LEDGER_EARLIEST_SEQ{32570}; +/** The minimum amount of support an amendment should have. + + @note This value is used by legacy code and will become obsolete + once the fixAmendmentMajorityCalc amendment activates. +*/ +constexpr std::ratio<204, 256> preFixAmendmentMajorityCalcThreshold; + +constexpr std::ratio<80, 100> postFixAmendmentMajorityCalcThreshold; + +/** The minimum amount of time an amendment must hold a majority */ +constexpr std::chrono::seconds const defaultAmendmentMajorityTime = weeks{2}; + } // namespace ripple /** Default peer port (IANA registered) */ diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index b8cac9b8d68..9d88c9d211a 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -130,7 +130,8 @@ detail::supportedAmendments() "fixQualityUpperBound", "RequireFullyCanonicalSig", "fix1781", - "HardenedValidations"}; + "HardenedValidations", + "fixAmendmentMajorityCalc"}; return supported; } @@ -181,7 +182,8 @@ uint256 const fixQualityUpperBound = *getRegisteredFeature("fixQualityUpperBound"), featureRequireFullyCanonicalSig = *getRegisteredFeature("RequireFullyCanonicalSig"), fix1781 = *getRegisteredFeature("fix1781"), - featureHardenedValidations = *getRegisteredFeature("HardenedValidations"); + featureHardenedValidations = *getRegisteredFeature("HardenedValidations"), + fixAmendmentMajorityCalc = *getRegisteredFeature("fixAmendmentMajorityCalc"); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index ec9293281b0..3b4ec47d143 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -38,9 +38,6 @@ namespace ripple { class AmendmentTable_test final : public beast::unit_test::suite { private: - // 204/256 about 80% (we round down because the implementation rounds up) - static int const majorityFraction{204}; - static uint256 amendmentId(std::string in) { @@ -100,12 +97,7 @@ class AmendmentTable_test final : public beast::unit_test::suite Section const vetoed) { return make_AmendmentTable( - majorityTime, - majorityFraction, - supported, - enabled, - vetoed, - journal); + majorityTime, supported, enabled, vetoed, journal); } std::unique_ptr @@ -373,6 +365,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Execute a pretend consensus round for a flag ledger void doRound( + uint256 const& feat, AmendmentTable& table, weeks week, std::vector> const& validators, @@ -399,25 +392,25 @@ class AmendmentTable_test final : public beast::unit_test::suite validations.reserve(validators.size()); int i = 0; - for (auto const& val : validators) + for (auto const& [pub, sec] : validators) { ++i; std::vector field; - for (auto const& amendment : votes) + for (auto const& [hash, nVotes] : votes) { - if ((256 * i) < (validators.size() * amendment.second)) + if (feat == fixAmendmentMajorityCalc ? nVotes >= i : nVotes > i) { // We vote yes on this amendment - field.push_back(amendment.first); + field.push_back(hash); } } auto v = std::make_shared( ripple::NetClock::time_point{}, - val.first, - val.second, - calcNodeID(val.first), + pub, + sec, + calcNodeID(pub), [&field](STValidation& v) { if (!field.empty()) v.setFieldV256( @@ -430,14 +423,13 @@ class AmendmentTable_test final : public beast::unit_test::suite ourVotes = table.doValidation(enabled); - auto actions = - table.doVoting(roundTime, enabled, majority, validations); - for (auto const& action : actions) + auto actions = table.doVoting( + Rules({feat}), roundTime, enabled, majority, validations); + for (auto const& [hash, action] : actions) { // This code assumes other validators do as we do - auto const& hash = action.first; - switch (action.second) + switch (action) { case 0: // amendment goes from majority to enabled @@ -471,7 +463,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // No vote on unknown amendment void - testNoOnUnknown() + testNoOnUnknown(uint256 const& feat) { testcase("Vote NO on unknown"); @@ -487,15 +479,29 @@ class AmendmentTable_test final : public beast::unit_test::suite majorityAmendments_t majority; doRound( - *table, weeks{1}, validators, votes, ourVotes, enabled, majority); + feat, + *table, + weeks{1}, + validators, + votes, + ourVotes, + enabled, + majority); BEAST_EXPECT(ourVotes.empty()); BEAST_EXPECT(enabled.empty()); BEAST_EXPECT(majority.empty()); - votes.emplace_back(testAmendment, 256); + votes.emplace_back(testAmendment, validators.size()); doRound( - *table, weeks{2}, validators, votes, ourVotes, enabled, majority); + feat, + *table, + weeks{2}, + validators, + votes, + ourVotes, + enabled, + majority); BEAST_EXPECT(ourVotes.empty()); BEAST_EXPECT(enabled.empty()); @@ -504,14 +510,21 @@ class AmendmentTable_test final : public beast::unit_test::suite // Note that the simulation code assumes others behave as we do, // so the amendment won't get enabled doRound( - *table, weeks{5}, validators, votes, ourVotes, enabled, majority); + feat, + *table, + weeks{5}, + validators, + votes, + ourVotes, + enabled, + majority); BEAST_EXPECT(ourVotes.empty()); BEAST_EXPECT(enabled.empty()); } // No vote on vetoed amendment void - testNoOnVetoed() + testNoOnVetoed(uint256 const& feat) { testcase("Vote NO on vetoed"); @@ -528,29 +541,50 @@ class AmendmentTable_test final : public beast::unit_test::suite majorityAmendments_t majority; doRound( - *table, weeks{1}, validators, votes, ourVotes, enabled, majority); + feat, + *table, + weeks{1}, + validators, + votes, + ourVotes, + enabled, + majority); BEAST_EXPECT(ourVotes.empty()); BEAST_EXPECT(enabled.empty()); BEAST_EXPECT(majority.empty()); - votes.emplace_back(testAmendment, 256); + votes.emplace_back(testAmendment, validators.size()); doRound( - *table, weeks{2}, validators, votes, ourVotes, enabled, majority); + feat, + *table, + weeks{2}, + validators, + votes, + ourVotes, + enabled, + majority); BEAST_EXPECT(ourVotes.empty()); BEAST_EXPECT(enabled.empty()); majority[testAmendment] = weekTime(weeks{1}); doRound( - *table, weeks{5}, validators, votes, ourVotes, enabled, majority); + feat, + *table, + weeks{5}, + validators, + votes, + ourVotes, + enabled, + majority); BEAST_EXPECT(ourVotes.empty()); BEAST_EXPECT(enabled.empty()); } // Vote on and enable known, not-enabled amendment void - testVoteEnable() + testVoteEnable(uint256 const& feat) { testcase("voteEnable"); @@ -565,7 +599,14 @@ class AmendmentTable_test final : public beast::unit_test::suite // Week 1: We should vote for all known amendments not enabled doRound( - *table, weeks{1}, validators, votes, ourVotes, enabled, majority); + feat, + *table, + weeks{1}, + validators, + votes, + ourVotes, + enabled, + majority); BEAST_EXPECT(ourVotes.size() == supported_.size()); BEAST_EXPECT(enabled.empty()); for (auto const& i : supported_) @@ -573,11 +614,18 @@ class AmendmentTable_test final : public beast::unit_test::suite // Now, everyone votes for this feature for (auto const& i : supported_) - votes.emplace_back(amendmentId(i), 256); + votes.emplace_back(amendmentId(i), validators.size()); // Week 2: We should recognize a majority doRound( - *table, weeks{2}, validators, votes, ourVotes, enabled, majority); + feat, + *table, + weeks{2}, + validators, + votes, + ourVotes, + enabled, + majority); BEAST_EXPECT(ourVotes.size() == supported_.size()); BEAST_EXPECT(enabled.empty()); @@ -586,12 +634,26 @@ class AmendmentTable_test final : public beast::unit_test::suite // Week 5: We should enable the amendment doRound( - *table, weeks{5}, validators, votes, ourVotes, enabled, majority); + feat, + *table, + weeks{5}, + validators, + votes, + ourVotes, + enabled, + majority); BEAST_EXPECT(enabled.size() == supported_.size()); // Week 6: We should remove it from our votes and from having a majority doRound( - *table, weeks{6}, validators, votes, ourVotes, enabled, majority); + feat, + *table, + weeks{6}, + validators, + votes, + ourVotes, + enabled, + majority); BEAST_EXPECT(enabled.size() == supported_.size()); BEAST_EXPECT(ourVotes.empty()); for (auto const& i : supported_) @@ -600,7 +662,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Detect majority at 80%, enable later void - testDetectMajority() + testDetectMajority(uint256 const& feat) { testcase("detectMajority"); @@ -619,9 +681,10 @@ class AmendmentTable_test final : public beast::unit_test::suite std::vector ourVotes; if ((i > 0) && (i < 17)) - votes.emplace_back(testAmendment, i * 16); + votes.emplace_back(testAmendment, i); doRound( + feat, *table, weeks{i}, validators, @@ -630,7 +693,7 @@ class AmendmentTable_test final : public beast::unit_test::suite enabled, majority); - if (i < 13) + if (i < 13) // 13 => 13/16 = 0.8125 => > 80% { // We are voting yes, not enabled, no majority BEAST_EXPECT(!ourVotes.empty()); @@ -663,7 +726,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Detect loss of majority void - testLostMajority() + testLostMajority(uint256 const& feat) { testcase("lostMajority"); @@ -681,9 +744,10 @@ class AmendmentTable_test final : public beast::unit_test::suite std::vector> votes; std::vector ourVotes; - votes.emplace_back(testAmendment, 250); + votes.emplace_back(testAmendment, validators.size()); doRound( + feat, *table, weeks{1}, validators, @@ -696,15 +760,16 @@ class AmendmentTable_test final : public beast::unit_test::suite BEAST_EXPECT(!majority.empty()); } - for (int i = 1; i < 16; ++i) + for (int i = 1; i < 8; ++i) { std::vector> votes; std::vector ourVotes; // Gradually reduce support - votes.emplace_back(testAmendment, 256 - i * 8); + votes.emplace_back(testAmendment, validators.size() - i); doRound( + feat, *table, weeks{i + 1}, validators, @@ -713,8 +778,8 @@ class AmendmentTable_test final : public beast::unit_test::suite enabled, majority); - if (i < 8) - { + if (i < 4) // 16 - 3 = 13 => 13/16 = 0.8125 => > 80% + { // 16 - 4 = 12 => 12/16 = 0.75 => < 80% // We are voting yes, not enabled, majority BEAST_EXPECT(!ourVotes.empty()); BEAST_EXPECT(enabled.empty()); @@ -775,6 +840,16 @@ class AmendmentTable_test final : public beast::unit_test::suite BEAST_EXPECT(table->needValidatedLedger(257)); } + void + testFeature(uint256 const& feat) + { + testNoOnUnknown(feat); + testNoOnVetoed(feat); + testVoteEnable(feat); + testDetectMajority(feat); + testLostMajority(feat); + } + void run() override { @@ -782,12 +857,9 @@ class AmendmentTable_test final : public beast::unit_test::suite testGet(); testBadConfig(); testEnableVeto(); - testNoOnUnknown(); - testNoOnVetoed(); - testVoteEnable(); - testDetectMajority(); - testLostMajority(); testHasUnsupported(); + testFeature({}); + testFeature(fixAmendmentMajorityCalc); } }; diff --git a/src/test/core/Config_test.cpp b/src/test/core/Config_test.cpp index 0e829642af8..03282fd59de 100644 --- a/src/test/core/Config_test.cpp +++ b/src/test/core/Config_test.cpp @@ -1014,6 +1014,57 @@ r.ripple.com 51235 } } + void + testAmendment() + { + testcase("amendment"); + struct ConfigUnit + { + std::string unit; + std::uint32_t numSeconds; + std::uint32_t configVal; + bool shouldPass; + }; + + std::vector units = { + {"seconds", 1, 15 * 60, false}, + {"minutes", 60, 14, false}, + {"minutes", 60, 15, true}, + {"hours", 3600, 10, true}, + {"days", 86400, 10, true}, + {"weeks", 604800, 2, true}, + {"months", 2592000, 1, false}, + {"years", 31536000, 1, false}}; + + std::string space = ""; + for (auto& [unit, sec, val, shouldPass] : units) + { + Config c; + std::string toLoad(R"rippleConfig( +[amendment_majority_time] +)rippleConfig"); + toLoad += std::to_string(val) + space + unit; + space = space == "" ? " " : ""; + + try + { + c.loadFromString(toLoad); + if (shouldPass) + BEAST_EXPECT( + c.AMENDMENT_MAJORITY_TIME.count() == val * sec); + else + fail(); + } + catch (std::runtime_error&) + { + if (!shouldPass) + pass(); + else + fail(); + } + } + } + void run() override { @@ -1027,6 +1078,7 @@ r.ripple.com 51235 testWhitespace(); testComments(); testGetters(); + testAmendment(); } }; diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 731e3560dfc..5773f756667 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -218,10 +218,9 @@ class Feature_test : public beast::unit_test::suite BEAST_EXPECTS( feature.isMember(jss::validations), feature[jss::name].asString() + " validations"); - BEAST_EXPECTS( - feature.isMember(jss::vote), - feature[jss::name].asString() + " vote"); - BEAST_EXPECT(feature[jss::vote] == 256); + BEAST_EXPECT(feature[jss::count] == 1); + BEAST_EXPECT(feature[jss::threshold] == 1); + BEAST_EXPECT(feature[jss::validations] == 1); BEAST_EXPECT(feature[jss::majority] == 2740); } }