diff --git a/src/ripple/app/misc/impl/AmendmentTable.cpp b/src/ripple/app/misc/impl/AmendmentTable.cpp index 93113af800e..30b836493bc 100644 --- a/src/ripple/app/misc/impl/AmendmentTable.cpp +++ b/src/ripple/app/misc/impl/AmendmentTable.cpp @@ -98,11 +98,17 @@ class AmendmentSet private: // How many yes votes each amendment received hash_map 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( + 1, + static_cast( (trustedValidations_ * preFixAmendmentMajorityCalcThreshold.num) / preFixAmendmentMajorityCalcThreshold.den)) : std::max( - 1L, - static_cast( + 1, + static_cast( (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(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; diff --git a/src/ripple/app/tx/impl/Change.cpp b/src/ripple/app/tx/impl/Change.cpp index 93ed1a04f92..eac63309cb8 100644 --- a/src/ripple/app/tx/impl/Change.cpp +++ b/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); diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index d4e65a31af8..5e3f86fb8eb 100644 --- a/src/ripple/protocol/Feature.h +++ b/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 diff --git a/src/ripple/protocol/SField.h b/src/ripple/protocol/SField.h index 253d956408f..f49cbfc2fc6 100644 --- a/src/ripple/protocol/SField.h +++ b/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 diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 5903603f975..534c3001431 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/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. diff --git a/src/ripple/protocol/impl/LedgerFormats.cpp b/src/ripple/protocol/impl/LedgerFormats.cpp index 7d5cf9d21aa..91f2730570a 100644 --- a/src/ripple/protocol/impl/LedgerFormats.cpp +++ b/src/ripple/protocol/impl/LedgerFormats.cpp @@ -140,6 +140,7 @@ LedgerFormats::LedgerFormats() { {sfAmendments, soeOPTIONAL}, // Enabled {sfMajorities, soeOPTIONAL}, + {sfFlappingAmendments, soeOPTIONAL}, }, commonFields); diff --git a/src/ripple/protocol/impl/SField.cpp b/src/ripple/protocol/impl/SField.cpp index 73098319b28..776c2178bca 100644 --- a/src/ripple/protocol/impl/SField.cpp +++ b/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); diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index 99922e863a4..1af4112696d 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -426,7 +426,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Execute a pretend consensus round for a flag ledger void doRound( - uint256 const& feat, + std::unordered_set, beast::uhash<>> const features, AmendmentTable& table, weeks week, std::vector> const& validators, @@ -460,7 +460,8 @@ class AmendmentTable_test final : public beast::unit_test::suite for (auto const& [hash, nVotes] : votes) { - if (feat == fixAmendmentMajorityCalc ? nVotes >= i : nVotes > i) + if (features.contains(fixAmendmentMajorityCalc) ? nVotes >= i + : nVotes > i) { // We vote yes on this amendment field.push_back(hash); @@ -485,7 +486,7 @@ class AmendmentTable_test final : public beast::unit_test::suite ourVotes = table.doValidation(enabled); auto actions = table.doVoting( - Rules({feat}), roundTime, enabled, majority, validations); + Rules{features}, roundTime, enabled, majority, validations); for (auto const& [hash, action] : actions) { // This code assumes other validators do as we do @@ -524,7 +525,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // No vote on unknown amendment void - testNoOnUnknown(uint256 const& feat) + testNoOnUnknown(std::unordered_set> const& features) { testcase("Vote NO on unknown"); @@ -541,7 +542,7 @@ class AmendmentTable_test final : public beast::unit_test::suite majorityAmendments_t majority; doRound( - feat, + features, *table, weeks{1}, validators, @@ -556,7 +557,7 @@ class AmendmentTable_test final : public beast::unit_test::suite votes.emplace_back(testAmendment, validators.size()); doRound( - feat, + features, *table, weeks{2}, validators, @@ -572,7 +573,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Note that the simulation code assumes others behave as we do, // so the amendment won't get enabled doRound( - feat, + features, *table, weeks{5}, validators, @@ -586,7 +587,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // No vote on vetoed amendment void - testNoOnVetoed(uint256 const& feat) + testNoOnVetoed(std::unordered_set> const& features) { testcase("Vote NO on vetoed"); @@ -604,7 +605,7 @@ class AmendmentTable_test final : public beast::unit_test::suite majorityAmendments_t majority; doRound( - feat, + features, *table, weeks{1}, validators, @@ -619,7 +620,7 @@ class AmendmentTable_test final : public beast::unit_test::suite votes.emplace_back(testAmendment, validators.size()); doRound( - feat, + features, *table, weeks{2}, validators, @@ -633,7 +634,7 @@ class AmendmentTable_test final : public beast::unit_test::suite majority[testAmendment] = weekTime(weeks{1}); doRound( - feat, + features, *table, weeks{5}, validators, @@ -647,7 +648,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Vote on and enable known, not-enabled amendment void - testVoteEnable(uint256 const& feat) + testVoteEnable(std::unordered_set> const& features) { testcase("voteEnable"); @@ -667,7 +668,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Week 1: We should vote for all known amendments not enabled doRound( - feat, + features, *table, weeks{1}, validators, @@ -686,7 +687,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Week 2: We should recognize a majority doRound( - feat, + features, *table, weeks{2}, validators, @@ -702,7 +703,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Week 5: We should enable the amendment doRound( - feat, + features, *table, weeks{5}, validators, @@ -714,7 +715,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, + features, *table, weeks{6}, validators, @@ -730,7 +731,8 @@ class AmendmentTable_test final : public beast::unit_test::suite // Detect majority at 80%, enable later void - testDetectMajority(uint256 const& feat) + testDetectMajority( + std::unordered_set> const& features) { testcase("detectMajority"); @@ -757,7 +759,7 @@ class AmendmentTable_test final : public beast::unit_test::suite votes.emplace_back(testAmendment, i); doRound( - feat, + features, *table, weeks{i}, validators, @@ -799,7 +801,8 @@ class AmendmentTable_test final : public beast::unit_test::suite // Detect loss of majority void - testLostMajority(uint256 const& feat) + testLostMajority( + std::unordered_set> const& features) { testcase("lostMajority"); @@ -807,6 +810,7 @@ class AmendmentTable_test final : public beast::unit_test::suite auto const validators = makeValidators(16); test::jtx::Env env{*this}; + auto table = makeTable( env, weeks(8), @@ -825,7 +829,7 @@ class AmendmentTable_test final : public beast::unit_test::suite votes.emplace_back(testAmendment, validators.size()); doRound( - feat, + features, *table, weeks{1}, validators, @@ -847,7 +851,7 @@ class AmendmentTable_test final : public beast::unit_test::suite votes.emplace_back(testAmendment, validators.size() - i); doRound( - feat, + features, *table, weeks{i + 1}, validators, @@ -856,8 +860,11 @@ class AmendmentTable_test final : public beast::unit_test::suite enabled, majority); - if (i < 4) // 16 - 3 = 13 => 13/16 = 0.8125 => > 80% - { // 16 - 4 = 12 => 12/16 = 0.75 => < 80% + // For 80% cut off is 4: (16-4)/16 < 0.80 < (16-3)/16 + int lim = 4; + + if (i < lim) + { // We are voting yes, not enabled, majority BEAST_EXPECT(!ourVotes.empty()); BEAST_EXPECT(enabled.empty()); @@ -917,13 +924,13 @@ class AmendmentTable_test final : public beast::unit_test::suite } void - testFeature(uint256 const& feat) + testFeature(std::unordered_set> const& features) { - testNoOnUnknown(feat); - testNoOnVetoed(feat); - testVoteEnable(feat); - testDetectMajority(feat); - testLostMajority(feat); + testNoOnUnknown(features); + testNoOnVetoed(features); + testVoteEnable(features); + testDetectMajority(features); + testLostMajority(features); } void @@ -935,7 +942,8 @@ class AmendmentTable_test final : public beast::unit_test::suite testEnableVeto(); testHasUnsupported(); testFeature({}); - testFeature(fixAmendmentMajorityCalc); + testFeature({fixAmendmentMajorityCalc}); + testFeature({fixAmendmentMajorityCalc, fixAmendmentFlapping}); } };