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

@@ -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);
     }
     //----------------------------------------------------------------------
     //

src/ripple/app/misc/AmendmentTable.h

@@ -111,6 +111,10 @@ public:
         std::set<uint256> const& enabled,
         majorityAmendments_t const& majority) = 0;
 
+    // Called when the set of trusted validators changes.
+    virtual void
+    trustChanged(hash_set<PublicKey> const& allTrusted) = 0;
+
     // Called by the consensus code when we need to
     // inject pseudo-transactions
     virtual std::map<uint256, std::uint32_t>

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(),

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<uint256> upVotes;
+        NetClock::time_point timeout = maxTimeout;
+    };
+    hash_map<PublicKey, UpvotesAndTimeout> 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<PublicKey> const& allTrusted,
+        std::lock_guard<std::mutex> 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<std::shared_ptr<STValidation>> const& valSet,
+        NetClock::time_point const closeTime,
+        std::lock_guard<std::mutex> 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<int, hash_map<uint256, int>>
+    getVotes(Rules const& rules, std::lock_guard<std::mutex> const& lock) const
+    {
+        hash_map<uint256, int> 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 @@ private:
     // number of votes needed
     int threshold_ = 0;
 
-public:
-    AmendmentSet(
-        Rules const& rules,
-        std::vector<std::shared_ptr<STValidation>> 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 @@ public:
                       postFixAmendmentMajorityCalcThreshold.den));
     }
 
+public:
+    AmendmentSet(
+        Rules const& rules,
+        TrustedVotes const& trustedVotes,
+        std::lock_guard<std::mutex> 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 @@ private:
     hash_map<uint256, AmendmentState> 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 @@ public:
         std::set<uint256> const& enabled,
         majorityAmendments_t const& majority) override;
 
+    void
+    trustChanged(hash_set<PublicKey> const& allTrusted) override;
+
     std::vector<uint256>
     doValidation(std::set<uint256> const& enabledAmendments) const override;
 
@@ -633,8 +783,14 @@ AmendmentTableImpl::doVoting(
                      << ": " << enabledAmendments.size() << ", "
                      << majorityAmendments.size() << ", " << valSet.size();
 
-    auto vote = std::make_unique<AmendmentSet>(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<AmendmentSet>(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<uint256, std::uint32_t> 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<PublicKey> const& allTrusted)
+{
+    std::lock_guard lock(mutex_);
+    previousTrustedVotes_.trustChanged(allTrusted, lock);
+}
+
 void
 AmendmentTableImpl::injectJson(
     Json::Value& v,

src/test/app/AmendmentTable_test.cpp

@@ -481,30 +481,37 @@ public:
         }
     }
 
+    // Make a list of trusted validators.
+    // Register the validators with AmendmentTable and return the list.
     std::vector<std::pair<PublicKey, SecretKey>>
-    makeValidators(int num)
+    makeValidators(int num, std::unique_ptr<AmendmentTable> const& table)
     {
         std::vector<std::pair<PublicKey, SecretKey>> ret;
         ret.reserve(num);
+        hash_set<PublicKey> 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<std::pair<PublicKey, SecretKey>> const& validators,
         std::vector<std::pair<uint256, int>> const& votes,
         std::vector<uint256>& ourVotes,
@@ -522,7 +529,7 @@ public:
         // 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<std::shared_ptr<STValidation>> validations;
@@ -536,7 +543,8 @@ public:
 
             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 @@ public:
 
         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 @@ public:
 
     // 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<std::pair<uint256, int>> votes;
         std::vector<uint256> ourVotes;
         std::set<uint256> enabled;
         majorityAmendments_t majority;
 
         doRound(
-            feat,
+            env.current()->rules(),
             *table,
             weeks{1},
             validators,
@@ -632,7 +641,7 @@ public:
         votes.emplace_back(testAmendment, validators.size());
 
         doRound(
-            feat,
+            env.current()->rules(),
             *table,
             weeks{2},
             validators,
@@ -643,12 +652,12 @@ public:
         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 @@ public:
 
     // 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 @@ public:
             emptySection_,
             makeSection(testAmendment));
 
-        auto const validators = makeValidators(10);
+        auto const validators = makeValidators(10, table);
 
         std::vector<std::pair<uint256, int>> votes;
         std::vector<uint256> ourVotes;
@@ -684,7 +693,7 @@ public:
         majorityAmendments_t majority;
 
         doRound(
-            feat,
+            env.current()->rules(),
             *table,
             weeks{1},
             validators,
@@ -699,7 +708,7 @@ public:
         votes.emplace_back(testAmendment, validators.size());
 
         doRound(
-            feat,
+            env.current()->rules(),
             *table,
             weeks{2},
             validators,
@@ -710,10 +719,10 @@ public:
         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 @@ public:
 
     // 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<std::pair<uint256, int>> votes;
         std::vector<uint256> ourVotes;
         std::set<uint256> enabled;
@@ -743,7 +753,7 @@ public:
 
         // Week 1: We should vote for all known amendments not enabled
         doRound(
-            feat,
+            env.current()->rules(),
             *table,
             weeks{1},
             validators,
@@ -762,7 +772,7 @@ public:
 
         // Week 2: We should recognize a majority
         doRound(
-            feat,
+            env.current()->rules(),
             *table,
             weeks{2},
             validators,
@@ -774,11 +784,11 @@ public:
         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 @@ public:
 
         // 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 @@ public:
 
     // 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 @@ public:
             emptySection_,
             emptySection_);
 
-        auto const validators = makeValidators(16);
+        auto const validators = makeValidators(16, table);
 
         std::set<uint256> enabled;
         majorityAmendments_t majority;
@@ -833,7 +843,7 @@ public:
                 votes.emplace_back(testAmendment, i);
 
             doRound(
-                feat,
+                env.current()->rules(),
                 *table,
                 weeks{i},
                 validators,
@@ -875,14 +885,13 @@ public:
 
     // 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 @@ public:
             emptySection_,
             emptySection_);
 
+        auto const validators = makeValidators(16, table);
+
         std::set<uint256> enabled;
         majorityAmendments_t majority;
 
@@ -901,7 +912,7 @@ public:
             votes.emplace_back(testAmendment, validators.size());
 
             doRound(
-                feat,
+                env.current()->rules(),
                 *table,
                 weeks{1},
                 validators,
@@ -923,7 +934,7 @@ public:
             votes.emplace_back(testAmendment, validators.size() - i);
 
             doRound(
-                feat,
+                env.current()->rules(),
                 *table,
                 weeks{i + 1},
                 validators,
@@ -949,6 +960,258 @@ public:
         }
     }
 
+    // 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<std::pair<PublicKey, SecretKey>> validators =
+            makeValidators(10, table);
+
+        std::set<uint256> enabled;
+        majorityAmendments_t majority;
+
+        {
+            // 10 validators with 2 voting against won't get majority.
+            std::vector<std::pair<uint256, int>> votes;
+            std::vector<uint256> 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<std::pair<PublicKey, SecretKey>> const& validators,
+               std::unique_ptr<AmendmentTable> const& table) {
+                // We need a hash_set to pass to trustChanged.
+                hash_set<PublicKey> 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<std::pair<uint256, int>> votes;
+            std::vector<uint256> 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<PublicKey, SecretKey> const savedValidator =
+                validators.front();
+            validators.erase(validators.begin());
+
+            std::vector<std::pair<uint256, int>> votes;
+            std::vector<uint256> 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<uint256> enabled;
+            majorityAmendments_t majority;
+
+            std::vector<std::pair<uint256, int>> votes;
+            std::vector<uint256> 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 @@ public:
     }
 
     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);
     }
 };