Limit amendment flapping (fixes #4350)

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

@@ -98,11 +98,17 @@ 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;
+
+    // minimum number of votes needed to begin activation countdown
+    int activationThreshold_ = 0;
+
+    // minimum number of votes needed to continue activate countdown
+    int deactivationThreshold_ = 0;
 
 public:
     AmendmentSet(
@@ -128,23 +134,25 @@ class AmendmentSet
             }
         }
 
-        threshold_ = !rules_.enabled(fixAmendmentMajorityCalc)
+        activationThreshold_ = !rules_.enabled(fixAmendmentMajorityCalc)
             ? std::max(
-                  1L,
-                  static_cast<long>(
+                  1,
+                  static_cast<int>(
                       (trustedValidations_ *
                        preFixAmendmentMajorityCalcThreshold.num) /
                       preFixAmendmentMajorityCalcThreshold.den))
             : std::max(
-                  1L,
-                  static_cast<long>(
+                  1,
+                  static_cast<int>(
                       (trustedValidations_ *
                        postFixAmendmentMajorityCalcThreshold.num) /
                       postFixAmendmentMajorityCalcThreshold.den));
+
+        deactivationThreshold_ = activationThreshold_;
     }
 
     bool
-    passes(uint256 const& amendment) const
+    passes(uint256 const& amendment, int threshold) const
     {
         auto const& it = votes_.find(amendment);
 
@@ -158,9 +166,9 @@ class AmendmentSet
         // to gain majority.
         if (!rules_.enabled(fixAmendmentMajorityCalc) ||
             trustedValidations_ == 1)
-            return it->second >= threshold_;
+            return it->second >= threshold;
 
-        return it->second > threshold_;
+        return it->second > threshold;
     }
 
     int
@@ -181,9 +189,15 @@ class AmendmentSet
     }
 
     int
-    threshold() const
+    activationThreshold() const
     {
-        return threshold_;
+        return activationThreshold_;
+    }
+
+    int
+    deactivationThreshold() const
+    {
+        return deactivationThreshold_;
     }
 };
 
@@ -619,8 +633,9 @@ AmendmentTableImpl::doVoting(
     auto vote = std::make_unique<AmendmentSet>(rules, valSet);
 
     JLOG(j_.debug()) << "Received " << vote->trustedValidations()
-                     << " trusted validations, threshold is: "
-                     << vote->threshold();
+                     << " trusted validations. Thresholds are: "
+                     << vote->activationThreshold() << " and "
+                     << vote->deactivationThreshold();
 
     // Map of amendments to the action to be taken for each one. The action is
     // the value of the flags in the pseudo-transaction
@@ -633,7 +648,8 @@ AmendmentTableImpl::doVoting(
     {
         NetClock::time_point majorityTime = {};
 
-        bool const hasValMajority = vote->passes(entry.first);
+        bool const hasValMajority =
+            vote->passes(entry.first, vote->activationThreshold());
 
         {
             auto const it = majorityAmendments.find(entry.first);
@@ -653,7 +669,9 @@ AmendmentTableImpl::doVoting(
             JLOG(j_.debug()) << entry.first << ": amendment got majority";
             actions[entry.first] = tfGotMajority;
         }
-        else if (!hasValMajority && (majorityTime != NetClock::time_point{}))
+        else if (
+            !hasValMajority && (majorityTime != NetClock::time_point{}) &&
+            !vote->passes(entry.first, vote->deactivationThreshold()))
         {
             // Ledger says majority, validators say no
             JLOG(j_.debug()) << entry.first << ": amendment lost majority";
@@ -740,7 +758,7 @@ AmendmentTableImpl::injectJson(
     if (!fs.enabled && lastVote_)
     {
         auto const votesTotal = lastVote_->trustedValidations();
-        auto const votesNeeded = lastVote_->threshold();
+        auto const votesNeeded = lastVote_->activationThreshold();
         auto const votesFor = lastVote_->votes(id);
 
         v[jss::count] = votesFor;

src/ripple/app/tx/impl/Change.cpp

@@ -207,7 +207,7 @@ Change::activateTrustLinesToSelfFix()
 TER
 Change::applyAmendment()
 {
-    uint256 amendment(ctx_.tx.getFieldH256(sfAmendment));
+    uint256 const amendment(ctx_.tx.getFieldH256(sfAmendment));
 
     auto const k = keylet::amendments();
 
@@ -233,41 +233,67 @@ Change::applyAmendment()
     if (gotMajority && lostMajority)
         return temINVALID_FLAG;
 
-    STArray newMajorities(sfMajorities);
+    STArray majorities = amendmentObject->getFieldArray(sfMajorities);
 
-    bool found = false;
-    if (amendmentObject->isFieldPresent(sfMajorities))
+    auto it = std::find_if(
+        majorities.begin(), majorities.end(), [&amendment](STObject const& o) {
+            return o[sfAmendment] == amendment;
+        });
+
+    if (it == majorities.end() && lostMajority)
+        return tefALREADY;
+
+    if (it != majorities.end())
     {
-        const STArray& oldMajorities =
-            amendmentObject->getFieldArray(sfMajorities);
-        for (auto const& majority : oldMajorities)
+        if (gotMajority)
+            return tefALREADY;
+
+        majorities.erase(it);
+    }
+
+    if (view().rules().enabled(fixAmendmentFlapping))
+    {
+        STVector256 flappingAmendments =
+            amendmentObject->getFieldV256(sfFlappingAmendments);
+
+        auto it = std::find(
+            flappingAmendments.begin(), flappingAmendments.end(), amendment);
+
+        // When amendment loses a majority and is not already on the list we
+        // add it and give it second chance.
+        if (lostMajority && it == flappingAmendments.end())
         {
-            if (majority.getFieldH256(sfAmendment) == amendment)
-            {
-                if (gotMajority)
-                    return tefALREADY;
-                found = true;
-            }
+            flappingAmendments.push_back(amendment);
+            amendmentObject->setFieldV256(
+                sfFlappingAmendments, flappingAmendments);
+            view().update(amendmentObject);
+            return tesSUCCESS;
+        }
+
+        // Remove the amendment now because the tracking is
+        // not needed any more.
+        if (it != flappingAmendments.end())
+        {
+            flappingAmendments.erase(it);
+
+            if (flappingAmendments.empty())
+                amendmentObject->makeFieldAbsent(sfFlappingAmendments);
             else
-            {
-                // pass through
-                newMajorities.push_back(majority);
-            }
+                amendmentObject->setFieldV256(
+                    sfFlappingAmendments, flappingAmendments);
         }
     }
 
-    if (!found && lostMajority)
-        return tefALREADY;
-
     if (gotMajority)
     {
         // This amendment now has a majority
-        newMajorities.push_back(STObject(sfMajority));
-        auto& entry = newMajorities.back();
-        entry.emplace_back(STUInt256(sfAmendment, amendment));
-        entry.emplace_back(STUInt32(
+        STObject maj(sfMajority);
+        maj.emplace_back(STUInt256(sfAmendment, amendment));
+        maj.emplace_back(STUInt32(
             sfCloseTime, view().parentCloseTime().time_since_epoch().count()));
 
+        majorities.push_back(std::move(maj));
+
         if (!ctx_.app.getAmendmentTable().isSupported(amendment))
         {
             JLOG(j_.warn()) << "Unsupported amendment " << amendment
@@ -293,10 +319,10 @@ Change::applyAmendment()
         }
     }
 
-    if (newMajorities.empty())
+    if (majorities.empty())
         amendmentObject->makeFieldAbsent(sfMajorities);
     else
-        amendmentObject->setFieldArray(sfMajorities, newMajorities);
+        amendmentObject->setFieldArray(sfMajorities, majorities);
 
     view().update(amendmentObject);
 

src/ripple/protocol/Feature.h

@@ -74,7 +74,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 54;
+static constexpr std::size_t numFeatures = 55;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -341,6 +341,7 @@ extern uint256 const fixTrustLinesToSelf;
 extern uint256 const fixRemoveNFTokenAutoTrustLine;
 extern uint256 const featureImmediateOfferKilled;
 extern uint256 const featureDisallowIncoming;
+extern uint256 const fixAmendmentFlapping;
 
 }  // namespace ripple
 

src/ripple/protocol/SField.h

@@ -533,6 +533,7 @@ extern SF_VECTOR256 const sfIndexes;
 extern SF_VECTOR256 const sfHashes;
 extern SF_VECTOR256 const sfAmendments;
 extern SF_VECTOR256 const sfNFTokenOffers;
+extern SF_VECTOR256 const sfFlappingAmendments;
 
 // inner object
 // OBJECT/1 is reserved for end of object

src/ripple/protocol/impl/Feature.cpp

@@ -451,6 +451,7 @@ REGISTER_FIX    (fixTrustLinesToSelf,           Supported::yes, DefaultVote::no)
 REGISTER_FIX    (fixRemoveNFTokenAutoTrustLine, Supported::yes, DefaultVote::yes);
 REGISTER_FEATURE(ImmediateOfferKilled,          Supported::yes, DefaultVote::no);
 REGISTER_FEATURE(DisallowIncoming,              Supported::yes, DefaultVote::no);
+REGISTER_FIX    (fixAmendmentFlapping,          Supported::yes, DefaultVote::yes);
 
 // The following amendments have been active for at least two years. Their
 // pre-amendment code has been removed and the identifiers are deprecated.

src/ripple/protocol/impl/LedgerFormats.cpp

@@ -140,6 +140,7 @@ LedgerFormats::LedgerFormats()
         {
             {sfAmendments,           soeOPTIONAL},  // Enabled
             {sfMajorities,           soeOPTIONAL},
+            {sfFlappingAmendments,   soeOPTIONAL},
         },
         commonFields);
 

src/ripple/protocol/impl/SField.cpp

@@ -282,6 +282,7 @@ CONSTRUCT_TYPED_SFIELD(sfIndexes,               "Indexes",              VECTOR25
 CONSTRUCT_TYPED_SFIELD(sfHashes,                "Hashes",               VECTOR256,  2);
 CONSTRUCT_TYPED_SFIELD(sfAmendments,            "Amendments",           VECTOR256,  3);
 CONSTRUCT_TYPED_SFIELD(sfNFTokenOffers,         "NFTokenOffers",        VECTOR256,  4);
+CONSTRUCT_TYPED_SFIELD(sfFlappingAmendments,    "FlappingAmendments",   VECTOR256,  5);
 
 // path set
 CONSTRUCT_UNTYPED_SFIELD(sfPaths,               "Paths",                PATHSET,    1);