Improve amendment processing and activation logic:

* The amendment ballot counting code contained a minor technical
  flaw, caused by the use of integer arithmetic and rounding
  semantics, that could allow amendments to reach majority with
  slightly less than 80% support. This commit introduces an
  amendment which, if enabled, will ensure that activation
  requires at least 80% support.
* This commit also introduces a configuration option to adjust
  the amendment activation hysteresis. This option is useful on
  test networks, but should not be used on the main network as
  is a network-wide consensus parameter that should not be
  changed on a per-server basis; doing so can result in a
  hard-fork.

Fixes #3396

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},
+            config().AMENDMENT_MAJORITY_TIME,
-            MAJORITY_FRACTION,
             supportedAmendments,
             enabledAmendments,
             config_->section(SECTION_VETO_AMENDMENTS),

src/ripple/app/misc/AmendmentTable.h

@@ -99,6 +99,7 @@ public:
     // 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,
@@ -130,6 +131,7 @@ public:
     {
         // Ask implementation what to do
         auto actions = doVoting(
+            lastClosedLedger->rules(),
             lastClosedLedger->parentCloseTime(),
             getEnabledAmendments(*lastClosedLedger),
             getMajorityAmendments(*lastClosedLedger),
@@ -164,7 +166,6 @@ public:
 std::unique_ptr<AmendmentTable>
 make_AmendmentTable(
     std::chrono::seconds majorityTime,
-    int majorityFraction,
     Section const& supported,
     Section const& enabled,
     Section const& vetoed,

src/ripple/app/misc/impl/AmendmentTable.cpp

@@ -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>
@@ -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_;
+    Rules const& rules_;
+    // number of trusted validations
+    int trustedValidations_ = 0;
+    // number of votes needed
+    int threshold_ = 0;
 
 public:
-    // number of trusted validations
+    AmendmentSet(
-    int mTrustedValidations = 0;
+        Rules const& rules,
-
+        std::vector<std::shared_ptr<STValidation>> const& valSet)
-    // number of votes needed
+        : rules_(rules)
-    int mThreshold = 0;
-
-    AmendmentSet() = default;
-
-    void
-    tally(std::set<uint256> const& amendments)
     {
-        ++mTrustedValidations;
+        // 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]; });
+                }
 
-        for (auto const& amendment : amendments)
+                ++trustedValidations_;
-            ++votes_[amendment];
+            }
+        }
+
+        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
+    {
+        auto const& it = votes_.find(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 @@ public:
 
         return it->second;
     }
+
+    int
+    trustedValidations() const
+    {
+        return trustedValidations_;
+    }
+
+    int
+    threshold() const
+    {
+        return threshold_;
+    }
 };
 
 //------------------------------------------------------------------------------
@@ -138,7 +197,7 @@ public:
 */
 class AmendmentTableImpl final : public AmendmentTable
 {
-protected:
+private:
     mutable std::mutex mutex_;
 
     hash_map<uint256, AmendmentState> amendmentMap_;
@@ -147,10 +206,6 @@ protected:
     // 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_;
@@ -187,7 +242,6 @@ protected:
 public:
     AmendmentTableImpl(
         std::chrono::seconds majorityTime,
-        int majorityFraction,
         Section const& supported,
         Section const& enabled,
         Section const& vetoed,
@@ -237,6 +291,7 @@ public:
 
     std::map<uint256, std::uint32_t>
     doVoting(
+        Rules const& rules,
         NetClock::time_point closeTime,
         std::set<uint256> const& enabledAmendments,
         majorityAmendments_t const& majorityAmendments,
@@ -247,19 +302,15 @@ public:
 
 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<uint256, std::uint32_t>
 AmendmentTableImpl::doVoting(
+    Rules const& rules,
     NetClock::time_point closeTime,
     std::set<uint256> const& enabledAmendments,
     majorityAmendments_t const& majorityAmendments,
@@ -470,31 +522,11 @@ AmendmentTableImpl::doVoting(
                      << ": " << enabledAmendments.size() << ", "
                      << majorityAmendments.size() << ", " << valSet.size();
 
-    auto vote = std::make_unique<AmendmentSet>();
+    auto vote = std::make_unique<AmendmentSet>(rules, valSet);
 
-    // process validations for ledger before flag ledger
+    JLOG(j_.debug()) << "Received " << vote->trustedValidations()
-    for (auto const& val : valSet)
-    {
-        if (val->isTrusted())
-        {
-            std::set<uint256> ballot;
-
-            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
                      << " 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 =
+        bool const hasValMajority = vote->passes(entry.first);
-            (vote->votes(entry.first) >= vote->mThreshold);
 
         {
             auto const it = majorityAmendments.find(entry.first);
@@ -614,19 +645,16 @@ AmendmentTableImpl::injectJson(
 
     if (!fs.enabled && lastVote_)
     {
-        auto const votesTotal = lastVote_->mTrustedValidations;
+        auto const votesTotal = lastVote_->trustedValidations();
-        auto const votesNeeded = lastVote_->mThreshold;
+        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;
     }
-    }
 }
 
 Json::Value
@@ -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

src/ripple/core/Config.h

@@ -31,6 +31,7 @@
 #include <boost/lexical_cast.hpp>
 #include <boost/optional.hpp>
 #include <algorithm>
+#include <chrono>
 #include <cstdint>
 #include <map>
 #include <string>
@@ -172,6 +173,9 @@ public:
     // Compression
     bool COMPRESSION = false;
 
+    // Amendment majority time
+    std::chrono::seconds AMENDMENT_MAJORITY_TIME = defaultAmendmentMajorityTime;
+
     // Thread pool configuration
     std::size_t WORKERS = 0;
 

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"

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<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)
     {

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<uint256> features;
     boost::container::flat_map<uint256, std::size_t> 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
 

src/ripple/protocol/SystemParameters.h

@@ -21,6 +21,7 @@
 #define RIPPLE_PROTOCOL_SYSTEMPARAMETERS_H_INCLUDED
 
 #include <ripple/basics/XRPAmount.h>
+#include <ripple/basics/chrono.h>
 #include <cstdint>
 #include <string>
 
@@ -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) */

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.

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 @@ public:
         Section const vetoed)
     {
         return make_AmendmentTable(
-            majorityTime,
+            majorityTime, supported, enabled, vetoed, journal);
-            majorityFraction,
-            supported,
-            enabled,
-            vetoed,
-            journal);
     }
 
     std::unique_ptr<AmendmentTable>
@@ -373,6 +365,7 @@ public:
     // Execute a pretend consensus round for a flag ledger
     void
     doRound(
+        uint256 const& feat,
         AmendmentTable& table,
         weeks week,
         std::vector<std::pair<PublicKey, SecretKey>> const& validators,
@@ -399,25 +392,25 @@ public:
         validations.reserve(validators.size());
 
         int i = 0;
-        for (auto const& val : validators)
+        for (auto const& [pub, sec] : validators)
         {
             ++i;
             std::vector<uint256> 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<STValidation>(
                 ripple::NetClock::time_point{},
-                val.first,
+                pub,
-                val.second,
+                sec,
-                calcNodeID(val.first),
+                calcNodeID(pub),
                 [&field](STValidation& v) {
                     if (!field.empty())
                         v.setFieldV256(
@@ -430,14 +423,13 @@ public:
 
         ourVotes = table.doValidation(enabled);
 
-        auto actions =
+        auto actions = table.doVoting(
-            table.doVoting(roundTime, enabled, majority, validations);
+            Rules({feat}), roundTime, enabled, majority, validations);
-        for (auto const& action : actions)
+        for (auto const& [hash, action] : actions)
         {
             // This code assumes other validators do as we do
 
-            auto const& hash = action.first;
+            switch (action)
-            switch (action.second)
             {
                 case 0:
                     // amendment goes from majority to enabled
@@ -471,7 +463,7 @@ public:
 
     // No vote on unknown amendment
     void
-    testNoOnUnknown()
+    testNoOnUnknown(uint256 const& feat)
     {
         testcase("Vote NO on unknown");
 
@@ -487,15 +479,29 @@ public:
         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 @@ public:
         // 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 @@ public:
         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 @@ public:
 
         // 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 @@ public:
 
         // 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 @@ public:
 
         // 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 @@ public:
 
     // Detect majority at 80%, enable later
     void
-    testDetectMajority()
+    testDetectMajority(uint256 const& feat)
     {
         testcase("detectMajority");
 
@@ -619,9 +681,10 @@ public:
             std::vector<uint256> 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 @@ public:
                 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 @@ public:
 
     // Detect loss of majority
     void
-    testLostMajority()
+    testLostMajority(uint256 const& feat)
     {
         testcase("lostMajority");
 
@@ -681,9 +744,10 @@ public:
             std::vector<std::pair<uint256, int>> votes;
             std::vector<uint256> ourVotes;
 
-            votes.emplace_back(testAmendment, 250);
+            votes.emplace_back(testAmendment, validators.size());
 
             doRound(
+                feat,
                 *table,
                 weeks{1},
                 validators,
@@ -696,15 +760,16 @@ public:
             BEAST_EXPECT(!majority.empty());
         }
 
-        for (int i = 1; i < 16; ++i)
+        for (int i = 1; i < 8; ++i)
         {
             std::vector<std::pair<uint256, int>> votes;
             std::vector<uint256> 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 @@ public:
                 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 @@ public:
         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 @@ public:
         testGet();
         testBadConfig();
         testEnableVeto();
-        testNoOnUnknown();
-        testNoOnVetoed();
-        testVoteEnable();
-        testDetectMajority();
-        testLostMajority();
         testHasUnsupported();
+        testFeature({});
+        testFeature(fixAmendmentMajorityCalc);
     }
 };
 

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<ConfigUnit> 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();
     }
 };
 

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(
+            BEAST_EXPECT(feature[jss::count] == 1);
-                feature.isMember(jss::vote),
+            BEAST_EXPECT(feature[jss::threshold] == 1);
-                feature[jss::name].asString() + " vote");
+            BEAST_EXPECT(feature[jss::validations] == 1);
-            BEAST_EXPECT(feature[jss::vote] == 256);
             BEAST_EXPECT(feature[jss::majority] == 2740);
         }
     }