From 2bb8de030fc887787f311e428e2e91466e41a240 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Thu, 28 Sep 2023 11:47:48 -0700 Subject: [PATCH] fix: stabilize voting threshold for amendment majority mechanism (#4410) Amendment "flapping" (an amendment repeatedly gaining and losing majority) usually occurs when an amendment is on the verge of gaining majority, and a validator not in favor of the amendment goes offline or loses sync. This fix makes two changes: 1. The number of validators in the UNL determines the threshold required for an amendment to gain majority. 2. The AmendmentTable keeps a record of the most recent Amendment vote received from each trusted validator (and, with `trustChanged`, stays up-to-date when the set of trusted validators changes). If no validation arrives from a given validator, then the AmendmentTable assumes that the previously-received vote has not changed. In other words, when missing an `STValidation` from a remote validator, each server now uses the last vote seen. There is a 24 hour timeout for recorded validator votes. These changes do not require an amendment because they do not impact transaction processing, but only the threshold at which each individual validator decides to propose an EnableAmendment pseudo-transaction. Fix #4350 --- src/ripple/app/main/Application.cpp | 3 + src/ripple/app/misc/AmendmentTable.h | 4 + src/ripple/app/misc/NetworkOPs.cpp | 5 + src/ripple/app/misc/impl/AmendmentTable.cpp | 213 ++++++++++-- src/test/app/AmendmentTable_test.cpp | 356 +++++++++++++++++--- 5 files changed, 511 insertions(+), 70 deletions(-) diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 8fcbb8a971c..08ba296b271 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -1333,6 +1333,9 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline) << "Invalid entry in [" << SECTION_VALIDATOR_LIST_SITES << "]"; return false; } + + // Tell the AmendmentTable who the trusted validators are. + m_amendmentTable->trustChanged(validators_->getQuorumKeys().second); } //---------------------------------------------------------------------- // diff --git a/src/ripple/app/misc/AmendmentTable.h b/src/ripple/app/misc/AmendmentTable.h index 10396d8591f..d7f7c8b26bb 100644 --- a/src/ripple/app/misc/AmendmentTable.h +++ b/src/ripple/app/misc/AmendmentTable.h @@ -111,6 +111,10 @@ class AmendmentTable std::set const& enabled, majorityAmendments_t const& majority) = 0; + // Called when the set of trusted validators changes. + virtual void + trustChanged(hash_set const& allTrusted) = 0; + // Called by the consensus code when we need to // inject pseudo-transactions virtual std::map diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index cd7fe3861ec..541f4f16fe0 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -1850,7 +1850,12 @@ NetworkOPsImp::beginConsensus(uint256 const& networkClosed) app_.getHashRouter()); if (!changes.added.empty() || !changes.removed.empty()) + { app_.getValidations().trustChanged(changes.added, changes.removed); + // Update the AmendmentTable so it tracks the current validators. + app_.getAmendmentTable().trustChanged( + app_.validators().getQuorumKeys().second); + } mConsensus.startRound( app_.timeKeeper().closeTime(), diff --git a/src/ripple/app/misc/impl/AmendmentTable.cpp b/src/ripple/app/misc/impl/AmendmentTable.cpp index 6f9ea86fa6c..8f4f0321992 100644 --- a/src/ripple/app/misc/impl/AmendmentTable.cpp +++ b/src/ripple/app/misc/impl/AmendmentTable.cpp @@ -67,6 +67,155 @@ parseSection(Section const& section) return names; } +/** TrustedVotes records the most recent votes from trusted validators. + We keep a record in an effort to avoid "flapping" while amendment voting + is in process. + + If a trusted validator loses synchronization near a flag ledger their + amendment votes may be lost during that round. If the validator is a + bit flaky, then this can cause an amendment to appear to repeatedly + gain and lose support. + + TrustedVotes addresses the problem by holding on to the last vote seen + from every trusted validator. So if any given validator is off line near + a flag ledger we can assume that they did not change their vote. + + If we haven't seen any STValidations from a validator for several hours we + lose confidence that the validator hasn't changed their position. So + there's a timeout. We remove upVotes if they haven't been updated in + several hours. +*/ +class TrustedVotes +{ +private: + static constexpr NetClock::time_point maxTimeout = + NetClock::time_point::max(); + + // Associates each trusted validator with the last votes we saw from them + // and an expiration for that record. + struct UpvotesAndTimeout + { + std::vector upVotes; + NetClock::time_point timeout = maxTimeout; + }; + hash_map recordedVotes_; + +public: + TrustedVotes() = default; + TrustedVotes(TrustedVotes const& rhs) = delete; + TrustedVotes& + operator=(TrustedVotes const& rhs) = delete; + + // Called when the list of trusted validators changes. + // + // Call with AmendmentTable::mutex_ locked. + void + trustChanged( + hash_set const& allTrusted, + std::lock_guard const& lock) + { + decltype(recordedVotes_) newRecordedVotes; + newRecordedVotes.reserve(allTrusted.size()); + + // Make sure every PublicKey in allTrusted is represented in + // recordedVotes_. Also make sure recordedVotes_ contains + // no additional PublicKeys. + for (auto& trusted : allTrusted) + { + if (recordedVotes_.contains(trusted)) + { + // Preserve this validator's previously saved voting state. + newRecordedVotes.insert(recordedVotes_.extract(trusted)); + } + else + { + // New validators have a starting position of no on everything. + // Add the entry with an empty vector and maxTimeout. + newRecordedVotes[trusted]; + } + } + // The votes of any no-longer-trusted validators will be destroyed + // when changedTrustedVotes goes out of scope. + recordedVotes_.swap(newRecordedVotes); + } + + // Called when we receive the latest votes. + // + // Call with AmendmentTable::mutex_ locked. + void + recordVotes( + Rules const& rules, + std::vector> const& valSet, + NetClock::time_point const closeTime, + std::lock_guard const& lock) + { + // When we get an STValidation we save the upVotes it contains, but + // we also set an expiration for those upVotes. The following constant + // controls the timeout. + // + // There really is no "best" timeout to choose for when we finally + // lose confidence that we know how a validator is voting. But part + // of the point of recording validator votes is to avoid flapping of + // amendment votes. A 24h timeout says that we will change the local + // record of a validator's vote to "no" 24h after the last vote seen + // from that validator. So flapping due to that validator being off + // line will happen less frequently than every 24 hours. + using namespace std::chrono_literals; + static constexpr NetClock::duration expiresAfter = 24h; + + // Walk all validations and replace previous votes from trusted + // validators with these newest votes. + for (auto const& val : valSet) + { + // If this validation comes from one of our trusted validators... + if (auto const iter = recordedVotes_.find(val->getSignerPublic()); + iter != recordedVotes_.end()) + { + iter->second.timeout = closeTime + expiresAfter; + if (val->isFieldPresent(sfAmendments)) + { + auto const& choices = val->getFieldV256(sfAmendments); + iter->second.upVotes.assign(choices.begin(), choices.end()); + } + else + { + // This validator does not upVote any amendments right now. + iter->second.upVotes.clear(); + } + } + } + + // Now remove any expired records from recordedVotes_. + std::for_each( + recordedVotes_.begin(), + recordedVotes_.end(), + [&closeTime](decltype(recordedVotes_)::value_type& votes) { + if (closeTime > votes.second.timeout) + { + votes.second.timeout = maxTimeout; + votes.second.upVotes.clear(); + } + }); + } + + // Return the information needed by AmendmentSet to determine votes. + // + // Call with AmendmentTable::mutex_ locked. + [[nodiscard]] std::pair> + getVotes(Rules const& rules, std::lock_guard const& lock) const + { + hash_map ret; + for (auto& validatorVotes : recordedVotes_) + { + for (uint256 const& amendment : validatorVotes.second.upVotes) + { + ret[amendment] += 1; + } + } + return {recordedVotes_.size(), ret}; + } +}; + /** Current state of an amendment. Tells if a amendment is supported, enabled or vetoed. A vetoed amendment means the node will never announce its support. @@ -104,30 +253,9 @@ class AmendmentSet // number of votes needed int threshold_ = 0; -public: - AmendmentSet( - Rules const& rules, - std::vector> const& valSet) - : rules_(rules) + void + computeThreshold(int trustedValidations, Rules const& 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_; - } - } - threshold_ = !rules_.enabled(fixAmendmentMajorityCalc) ? std::max( 1L, @@ -143,6 +271,22 @@ class AmendmentSet postFixAmendmentMajorityCalcThreshold.den)); } +public: + AmendmentSet( + Rules const& rules, + TrustedVotes const& trustedVotes, + std::lock_guard const& lock) + : rules_(rules) + { + // process validations for ledger before flag ledger. + auto [trustedCount, newVotes] = trustedVotes.getVotes(rules, lock); + + trustedValidations_ = trustedCount; + votes_.swap(newVotes); + + computeThreshold(trustedValidations_, rules); + } + bool passes(uint256 const& amendment) const { @@ -203,6 +347,9 @@ class AmendmentTableImpl final : public AmendmentTable hash_map amendmentMap_; std::uint32_t lastUpdateSeq_; + // Record of the last votes seen from trusted validators. + TrustedVotes previousTrustedVotes_; + // Time that an amendment must hold a majority for std::chrono::seconds const majorityTime_; @@ -294,6 +441,9 @@ class AmendmentTableImpl final : public AmendmentTable std::set const& enabled, majorityAmendments_t const& majority) override; + void + trustChanged(hash_set const& allTrusted) override; + std::vector doValidation(std::set const& enabledAmendments) const override; @@ -633,8 +783,14 @@ AmendmentTableImpl::doVoting( << ": " << enabledAmendments.size() << ", " << majorityAmendments.size() << ", " << valSet.size(); - auto vote = std::make_unique(rules, valSet); + std::lock_guard lock(mutex_); + + // Keep a record of the votes we received. + previousTrustedVotes_.recordVotes(rules, valSet, closeTime, lock); + // Tally the most recent votes. + auto vote = + std::make_unique(rules, previousTrustedVotes_, lock); JLOG(j_.debug()) << "Received " << vote->trustedValidations() << " trusted validations, threshold is: " << vote->threshold(); @@ -643,8 +799,6 @@ AmendmentTableImpl::doVoting( // the value of the flags in the pseudo-transaction std::map actions; - std::lock_guard lock(mutex_); - // process all amendments we know of for (auto const& entry : amendmentMap_) { @@ -740,6 +894,13 @@ AmendmentTableImpl::doValidatedLedger( firstUnsupportedExpected_ = *firstUnsupportedExpected_ + majorityTime_; } +void +AmendmentTableImpl::trustChanged(hash_set const& allTrusted) +{ + std::lock_guard lock(mutex_); + previousTrustedVotes_.trustChanged(allTrusted, lock); +} + void AmendmentTableImpl::injectJson( Json::Value& v, diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index 4284190a18a..64e6a038653 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -481,30 +481,37 @@ class AmendmentTable_test final : public beast::unit_test::suite } } + // Make a list of trusted validators. + // Register the validators with AmendmentTable and return the list. std::vector> - makeValidators(int num) + makeValidators(int num, std::unique_ptr const& table) { std::vector> ret; ret.reserve(num); + hash_set trustedValidators; + trustedValidators.reserve(num); for (int i = 0; i < num; ++i) { - ret.emplace_back(randomKeyPair(KeyType::secp256k1)); + auto const& back = + ret.emplace_back(randomKeyPair(KeyType::secp256k1)); + trustedValidators.insert(back.first); } + table->trustChanged(trustedValidators); return ret; } static NetClock::time_point - weekTime(weeks w) + hourTime(std::chrono::hours h) { - return NetClock::time_point{w}; + return NetClock::time_point{h}; } // Execute a pretend consensus round for a flag ledger void doRound( - uint256 const& feat, + Rules const& rules, AmendmentTable& table, - weeks week, + std::chrono::hours hour, std::vector> const& validators, std::vector> const& votes, std::vector& ourVotes, @@ -522,7 +529,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // enabled: In/out enabled amendments // majority: In/our majority amendments (and when they got a majority) - auto const roundTime = weekTime(week); + auto const roundTime = hourTime(hour); // Build validations std::vector> validations; @@ -536,7 +543,8 @@ class AmendmentTable_test final : public beast::unit_test::suite for (auto const& [hash, nVotes] : votes) { - if (feat == fixAmendmentMajorityCalc ? nVotes >= i : nVotes > i) + if (rules.enabled(fixAmendmentMajorityCalc) ? nVotes >= i + : nVotes > i) { // We vote yes on this amendment field.push_back(hash); @@ -560,8 +568,8 @@ class AmendmentTable_test final : public beast::unit_test::suite ourVotes = table.doValidation(enabled); - auto actions = table.doVoting( - Rules({feat}), roundTime, enabled, majority, validations); + auto actions = + table.doVoting(rules, roundTime, enabled, majority, validations); for (auto const& [hash, action] : actions) { // This code assumes other validators do as we do @@ -600,24 +608,25 @@ class AmendmentTable_test final : public beast::unit_test::suite // No vote on unknown amendment void - testNoOnUnknown(uint256 const& feat) + testNoOnUnknown(FeatureBitset const& feat) { testcase("Vote NO on unknown"); auto const testAmendment = amendmentId("TestAmendment"); - auto const validators = makeValidators(10); - test::jtx::Env env{*this}; + test::jtx::Env env{*this, feat}; auto table = makeTable(env, weeks(2), emptyYes_, emptySection_, emptySection_); + auto const validators = makeValidators(10, table); + std::vector> votes; std::vector ourVotes; std::set enabled; majorityAmendments_t majority; doRound( - feat, + env.current()->rules(), *table, weeks{1}, validators, @@ -632,7 +641,7 @@ class AmendmentTable_test final : public beast::unit_test::suite votes.emplace_back(testAmendment, validators.size()); doRound( - feat, + env.current()->rules(), *table, weeks{2}, validators, @@ -643,12 +652,12 @@ class AmendmentTable_test final : public beast::unit_test::suite BEAST_EXPECT(ourVotes.empty()); BEAST_EXPECT(enabled.empty()); - majority[testAmendment] = weekTime(weeks{1}); + majority[testAmendment] = hourTime(weeks{1}); // Note that the simulation code assumes others behave as we do, // so the amendment won't get enabled doRound( - feat, + env.current()->rules(), *table, weeks{5}, validators, @@ -662,13 +671,13 @@ class AmendmentTable_test final : public beast::unit_test::suite // No vote on vetoed amendment void - testNoOnVetoed(uint256 const& feat) + testNoOnVetoed(FeatureBitset const& feat) { testcase("Vote NO on vetoed"); auto const testAmendment = amendmentId("vetoedAmendment"); - test::jtx::Env env{*this}; + test::jtx::Env env{*this, feat}; auto table = makeTable( env, weeks(2), @@ -676,7 +685,7 @@ class AmendmentTable_test final : public beast::unit_test::suite emptySection_, makeSection(testAmendment)); - auto const validators = makeValidators(10); + auto const validators = makeValidators(10, table); std::vector> votes; std::vector ourVotes; @@ -684,7 +693,7 @@ class AmendmentTable_test final : public beast::unit_test::suite majorityAmendments_t majority; doRound( - feat, + env.current()->rules(), *table, weeks{1}, validators, @@ -699,7 +708,7 @@ class AmendmentTable_test final : public beast::unit_test::suite votes.emplace_back(testAmendment, validators.size()); doRound( - feat, + env.current()->rules(), *table, weeks{2}, validators, @@ -710,10 +719,10 @@ class AmendmentTable_test final : public beast::unit_test::suite BEAST_EXPECT(ourVotes.empty()); BEAST_EXPECT(enabled.empty()); - majority[testAmendment] = weekTime(weeks{1}); + majority[testAmendment] = hourTime(weeks{1}); doRound( - feat, + env.current()->rules(), *table, weeks{5}, validators, @@ -727,15 +736,16 @@ class AmendmentTable_test final : public beast::unit_test::suite // Vote on and enable known, not-enabled amendment void - testVoteEnable(uint256 const& feat) + testVoteEnable(FeatureBitset const& feat) { testcase("voteEnable"); - test::jtx::Env env{*this}; + test::jtx::Env env{*this, feat}; auto table = makeTable( env, weeks(2), makeDefaultYes(yes_), emptySection_, emptySection_); - auto const validators = makeValidators(10); + auto const validators = makeValidators(10, table); + std::vector> votes; std::vector ourVotes; std::set enabled; @@ -743,7 +753,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Week 1: We should vote for all known amendments not enabled doRound( - feat, + env.current()->rules(), *table, weeks{1}, validators, @@ -762,7 +772,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Week 2: We should recognize a majority doRound( - feat, + env.current()->rules(), *table, weeks{2}, validators, @@ -774,11 +784,11 @@ class AmendmentTable_test final : public beast::unit_test::suite BEAST_EXPECT(enabled.empty()); for (auto const& i : yes_) - BEAST_EXPECT(majority[amendmentId(i)] == weekTime(weeks{2})); + BEAST_EXPECT(majority[amendmentId(i)] == hourTime(weeks{2})); // Week 5: We should enable the amendment doRound( - feat, + env.current()->rules(), *table, weeks{5}, validators, @@ -790,7 +800,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Week 6: We should remove it from our votes and from having a majority doRound( - feat, + env.current()->rules(), *table, weeks{6}, validators, @@ -806,12 +816,12 @@ class AmendmentTable_test final : public beast::unit_test::suite // Detect majority at 80%, enable later void - testDetectMajority(uint256 const& feat) + testDetectMajority(FeatureBitset const& feat) { testcase("detectMajority"); auto const testAmendment = amendmentId("detectMajority"); - test::jtx::Env env{*this}; + test::jtx::Env env{*this, feat}; auto table = makeTable( env, weeks(2), @@ -819,7 +829,7 @@ class AmendmentTable_test final : public beast::unit_test::suite emptySection_, emptySection_); - auto const validators = makeValidators(16); + auto const validators = makeValidators(16, table); std::set enabled; majorityAmendments_t majority; @@ -833,7 +843,7 @@ class AmendmentTable_test final : public beast::unit_test::suite votes.emplace_back(testAmendment, i); doRound( - feat, + env.current()->rules(), *table, weeks{i}, validators, @@ -875,14 +885,13 @@ class AmendmentTable_test final : public beast::unit_test::suite // Detect loss of majority void - testLostMajority(uint256 const& feat) + testLostMajority(FeatureBitset const& feat) { testcase("lostMajority"); auto const testAmendment = amendmentId("lostMajority"); - auto const validators = makeValidators(16); - test::jtx::Env env{*this}; + test::jtx::Env env{*this, feat}; auto table = makeTable( env, weeks(8), @@ -890,6 +899,8 @@ class AmendmentTable_test final : public beast::unit_test::suite emptySection_, emptySection_); + auto const validators = makeValidators(16, table); + std::set enabled; majorityAmendments_t majority; @@ -901,7 +912,7 @@ class AmendmentTable_test final : public beast::unit_test::suite votes.emplace_back(testAmendment, validators.size()); doRound( - feat, + env.current()->rules(), *table, weeks{1}, validators, @@ -923,7 +934,7 @@ class AmendmentTable_test final : public beast::unit_test::suite votes.emplace_back(testAmendment, validators.size() - i); doRound( - feat, + env.current()->rules(), *table, weeks{i + 1}, validators, @@ -949,6 +960,258 @@ class AmendmentTable_test final : public beast::unit_test::suite } } + // Exercise the UNL changing while voting is in progress. + void + testChangedUNL(FeatureBitset const& feat) + { + // This test doesn't work without fixAmendmentMajorityCalc enabled. + if (!feat[fixAmendmentMajorityCalc]) + return; + + testcase("changedUNL"); + + auto const testAmendment = amendmentId("changedUNL"); + test::jtx::Env env{*this, feat}; + auto table = makeTable( + env, + weeks(8), + makeDefaultYes(testAmendment), + emptySection_, + emptySection_); + + std::vector> validators = + makeValidators(10, table); + + std::set enabled; + majorityAmendments_t majority; + + { + // 10 validators with 2 voting against won't get majority. + std::vector> votes; + std::vector ourVotes; + + votes.emplace_back(testAmendment, validators.size() - 2); + + doRound( + env.current()->rules(), + *table, + weeks{1}, + validators, + votes, + ourVotes, + enabled, + majority); + + BEAST_EXPECT(enabled.empty()); + BEAST_EXPECT(majority.empty()); + } + + // Add one new validator to the UNL. + validators.emplace_back(randomKeyPair(KeyType::secp256k1)); + + // A lambda that updates the AmendmentTable with the latest + // trusted validators. + auto callTrustChanged = + [](std::vector> const& validators, + std::unique_ptr const& table) { + // We need a hash_set to pass to trustChanged. + hash_set trustedValidators; + trustedValidators.reserve(validators.size()); + std::for_each( + validators.begin(), + validators.end(), + [&trustedValidators](auto const& val) { + trustedValidators.insert(val.first); + }); + + // Tell the AmendmentTable that the UNL changed. + table->trustChanged(trustedValidators); + }; + + // Tell the table that there's been a change in trusted validators. + callTrustChanged(validators, table); + + { + // 11 validators with 2 voting against gains majority. + std::vector> votes; + std::vector ourVotes; + + votes.emplace_back(testAmendment, validators.size() - 2); + + doRound( + env.current()->rules(), + *table, + weeks{2}, + validators, + votes, + ourVotes, + enabled, + majority); + + BEAST_EXPECT(enabled.empty()); + BEAST_EXPECT(!majority.empty()); + } + { + // One of the validators goes flaky and doesn't send validations + // (without the UNL changing) so the amendment loses majority. + std::pair const savedValidator = + validators.front(); + validators.erase(validators.begin()); + + std::vector> votes; + std::vector ourVotes; + + votes.emplace_back(testAmendment, validators.size() - 2); + + doRound( + env.current()->rules(), + *table, + weeks{3}, + validators, + votes, + ourVotes, + enabled, + majority); + + BEAST_EXPECT(enabled.empty()); + BEAST_EXPECT(majority.empty()); + + // Simulate the validator re-syncing to the network by adding it + // back to the validators vector + validators.insert(validators.begin(), savedValidator); + + votes.front().second = validators.size() - 2; + + doRound( + env.current()->rules(), + *table, + weeks{4}, + validators, + votes, + ourVotes, + enabled, + majority); + + BEAST_EXPECT(enabled.empty()); + BEAST_EXPECT(!majority.empty()); + + // Finally, remove one validator from the UNL and see that majority + // is lost. + validators.erase(validators.begin()); + + // Tell the table that there's been a change in trusted validators. + callTrustChanged(validators, table); + + votes.front().second = validators.size() - 2; + + doRound( + env.current()->rules(), + *table, + weeks{5}, + validators, + votes, + ourVotes, + enabled, + majority); + + BEAST_EXPECT(enabled.empty()); + BEAST_EXPECT(majority.empty()); + } + } + + // Exercise a validator losing connectivity and then regaining it after + // extended delays. Depending on how long that delay is an amendment + // either will or will not go live. + void + testValidatorFlapping(FeatureBitset const& feat) + { + // This test doesn't work without fixAmendmentMajorityCalc enabled. + if (!feat[fixAmendmentMajorityCalc]) + return; + + testcase("validatorFlapping"); + + // We run a test where a validator flaps on and off every 23 hours + // and another one one where it flaps on and off every 25 hours. + // + // Since the local validator vote record expires after 24 hours, + // with 23 hour flapping the amendment will go live. But with 25 + // hour flapping the amendment will not go live. + for (int flapRateHours : {23, 25}) + { + test::jtx::Env env{*this, feat}; + auto const testAmendment = amendmentId("validatorFlapping"); + auto table = makeTable( + env, + weeks(1), + makeDefaultYes(testAmendment), + emptySection_, + emptySection_); + + // Make two lists of validators, one with a missing validator, to + // make it easy to simulate validator flapping. + auto const allValidators = makeValidators(11, table); + decltype(allValidators) const mostValidators( + allValidators.begin() + 1, allValidators.end()); + BEAST_EXPECT(allValidators.size() == mostValidators.size() + 1); + + std::set enabled; + majorityAmendments_t majority; + + std::vector> votes; + std::vector ourVotes; + + votes.emplace_back(testAmendment, allValidators.size() - 2); + + int delay = flapRateHours; + // Loop for 1 week plus a day. + for (int hour = 1; hour < (24 * 8); ++hour) + { + decltype(allValidators) const& thisHoursValidators = + (delay < flapRateHours) ? mostValidators : allValidators; + delay = delay == flapRateHours ? 0 : delay + 1; + + votes.front().second = thisHoursValidators.size() - 2; + + using namespace std::chrono; + doRound( + env.current()->rules(), + *table, + hours(hour), + thisHoursValidators, + votes, + ourVotes, + enabled, + majority); + + if (hour <= (24 * 7) || flapRateHours > 24) + { + // The amendment should not be enabled under any + // circumstance until one week has elapsed. + BEAST_EXPECT(enabled.empty()); + + // If flapping is less than 24 hours, there should be + // no flapping. Otherwise we should only have majority + // if allValidators vote -- which means there are no + // missing validators. + bool const expectMajority = (delay <= 24) + ? true + : &thisHoursValidators == &allValidators; + BEAST_EXPECT(majority.empty() != expectMajority); + } + else + { + // We're... + // o Past one week, and + // o AmendmentFlapping was less than 24 hours. + // The amendment should be enabled. + BEAST_EXPECT(!enabled.empty()); + BEAST_EXPECT(majority.empty()); + } + } + } + } + void testHasUnsupported() { @@ -993,25 +1256,30 @@ class AmendmentTable_test final : public beast::unit_test::suite } void - testFeature(uint256 const& feat) + testFeature(FeatureBitset const& feat) { testNoOnUnknown(feat); testNoOnVetoed(feat); testVoteEnable(feat); testDetectMajority(feat); testLostMajority(feat); + testChangedUNL(feat); + testValidatorFlapping(feat); } void run() override { + FeatureBitset const all{test::jtx::supported_amendments()}; + FeatureBitset const fixMajorityCalc{fixAmendmentMajorityCalc}; + testConstruct(); testGet(); testBadConfig(); testEnableVeto(); testHasUnsupported(); - testFeature({}); - testFeature(fixAmendmentMajorityCalc); + testFeature(all - fixMajorityCalc); + testFeature(all); } };