Limit amendment flapping (fixes XRPLF#4350)

This commit implements a new amendment that follows one of the
recommendations in XRPLF#4350 to reduce the flapping of
amendments when there is validator outage and support drops below
80% required majority.

A new FlappingAmendment vector is used to track amendments that
lose majority and add delay of one flag ledger.

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,69 @@ 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 +321,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);