From e0b261a14bc8ab6b52923a8629ad2348044c5cdc Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Mon, 29 Aug 2022 15:36:54 -0700 Subject: [PATCH 1/3] Add the ability to mark amendments as obsolete: * Prevents the server from ever voting on the given amendment, regardless of operator configuration. * Also prevents the "feature" command from changing the amendment's vote. * Incidentally, resolves #4014. --- src/ripple/app/misc/AmendmentTable.h | 4 +- src/ripple/app/misc/impl/AmendmentTable.cpp | 41 ++++- src/ripple/app/rdb/Wallet.h | 2 +- src/ripple/protocol/Feature.h | 14 +- src/ripple/protocol/impl/Feature.cpp | 119 +++++++------ src/test/app/AmendmentTable_test.cpp | 102 +++++++---- src/test/rpc/Feature_test.cpp | 184 ++++++++++++++++---- 7 files changed, 326 insertions(+), 140 deletions(-) diff --git a/src/ripple/app/misc/AmendmentTable.h b/src/ripple/app/misc/AmendmentTable.h index 14558b7da51..0a5f6a011ad 100644 --- a/src/ripple/app/misc/AmendmentTable.h +++ b/src/ripple/app/misc/AmendmentTable.h @@ -40,14 +40,14 @@ class AmendmentTable struct FeatureInfo { FeatureInfo() = delete; - FeatureInfo(std::string const& n, uint256 const& f, DefaultVote v) + FeatureInfo(std::string const& n, uint256 const& f, VoteBehavior v) : name(n), feature(f), vote(v) { } std::string const name; uint256 const feature; - DefaultVote const vote; + VoteBehavior const vote; }; virtual ~AmendmentTable() = default; diff --git a/src/ripple/app/misc/impl/AmendmentTable.cpp b/src/ripple/app/misc/impl/AmendmentTable.cpp index 93113af800e..6f9ea86fa6c 100644 --- a/src/ripple/app/misc/impl/AmendmentTable.cpp +++ b/src/ripple/app/misc/impl/AmendmentTable.cpp @@ -333,19 +333,31 @@ AmendmentTableImpl::AmendmentTableImpl( }(); // Parse supported amendments - for (auto const& [name, amendment, defaultVote] : supported) + for (auto const& [name, amendment, votebehavior] : supported) { AmendmentState& s = add(amendment, lock); s.name = name; s.supported = true; - s.vote = defaultVote == DefaultVote::yes ? AmendmentVote::up - : AmendmentVote::down; + switch (votebehavior) + { + case VoteBehavior::DefaultYes: + s.vote = AmendmentVote::up; + break; + + case VoteBehavior::DefaultNo: + s.vote = AmendmentVote::down; + break; + + case VoteBehavior::Obsolete: + s.vote = AmendmentVote::obsolete; + break; + } JLOG(j_.debug()) << "Amendment " << amendment << " (" << s.name << ") is supported and will be " << (s.vote == AmendmentVote::up ? "up" : "down") - << " voted if not enabled on the ledger."; + << " voted by default if not enabled on the ledger."; } hash_set detect_conflict; @@ -420,7 +432,9 @@ AmendmentTableImpl::AmendmentTableImpl( << amend_hash << "} is downvoted."; if (!amendment_name->empty()) s->name = *amendment_name; - s->vote = *vote; + // An obsolete amendment's vote can never be changed + if (s->vote != AmendmentVote::obsolete) + s->vote = *vote; } } else // up-vote @@ -431,7 +445,9 @@ AmendmentTableImpl::AmendmentTableImpl( << amend_hash << "} is upvoted."; if (!amendment_name->empty()) s.name = *amendment_name; - s.vote = *vote; + // An obsolete amendment's vote can never be changed + if (s.vote != AmendmentVote::obsolete) + s.vote = *vote; } }); } @@ -489,6 +505,7 @@ AmendmentTableImpl::persistVote( std::string const& name, AmendmentVote vote) const { + assert(vote != AmendmentVote::obsolete); auto db = db_.checkoutDb(); voteAmendment(*db, amendment, name, vote); } @@ -499,7 +516,7 @@ AmendmentTableImpl::veto(uint256 const& amendment) std::lock_guard lock(mutex_); AmendmentState& s = add(amendment, lock); - if (s.vote == AmendmentVote::down) + if (s.vote != AmendmentVote::up) return false; s.vote = AmendmentVote::down; persistVote(amendment, s.name, s.vote); @@ -512,7 +529,7 @@ AmendmentTableImpl::unVeto(uint256 const& amendment) std::lock_guard lock(mutex_); AmendmentState* const s = get(amendment, lock); - if (!s || s->vote == AmendmentVote::up) + if (!s || s->vote != AmendmentVote::down) return false; s->vote = AmendmentVote::up; persistVote(amendment, s->name, s->vote); @@ -734,7 +751,13 @@ AmendmentTableImpl::injectJson( v[jss::name] = fs.name; v[jss::supported] = fs.supported; - v[jss::vetoed] = fs.vote == AmendmentVote::down; + if (!fs.enabled) + { + if (fs.vote == AmendmentVote::obsolete) + v[jss::vetoed] = "Obsolete"; + else + v[jss::vetoed] = fs.vote == AmendmentVote::down; + } v[jss::enabled] = fs.enabled; if (!fs.enabled && lastVote_) diff --git a/src/ripple/app/rdb/Wallet.h b/src/ripple/app/rdb/Wallet.h index 2769e459acc..e9846714ece 100644 --- a/src/ripple/app/rdb/Wallet.h +++ b/src/ripple/app/rdb/Wallet.h @@ -144,7 +144,7 @@ createFeatureVotes(soci::session& session); // For historical reasons the up-vote and down-vote integer representations // are unintuitive. -enum class AmendmentVote : int { up = 0, down = 1 }; +enum class AmendmentVote : int { obsolete = -1, up = 0, down = 1 }; /** * @brief readAmendments Reads all amendments from the FeatureVotes table. diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index d4e65a31af8..57c0cf3e3a1 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -36,17 +36,17 @@ * for the feature at the bottom * 2) Add a uint256 definition for the feature to the corresponding source * file (Feature.cpp). Use `registerFeature` to create the feature with - * the feature's name, `Supported::no`, and `DefaultVote::no`. This + * the feature's name, `Supported::no`, and `VoteBehavior::DefaultNo`. This * should be the only place the feature's name appears in code as a string. * 3) Use the uint256 as the parameter to `view.rules.enabled()` to * control flow into new code that this feature limits. * 4) If the feature development is COMPLETE, and the feature is ready to be * SUPPORTED, change the `registerFeature` parameter to Supported::yes. * 5) When the feature is ready to be ENABLED, change the `registerFeature` - * parameter to `DefaultVote::yes`. + * parameter to `VoteBehavior::DefaultYes`. * In general, any newly supported amendments (`Supported::yes`) should have - * a `DefaultVote::no` for at least one full release cycle. High priority - * bug fixes can be an exception to this rule of thumb. + * a `VoteBehavior::DefaultNo` for at least one full release cycle. High + * priority bug fixes can be an exception to this rule of thumb. * * When a feature has been enabled for several years, the conditional code * may be removed, and the feature "retired". To retire a feature: @@ -55,7 +55,7 @@ * section at the end of the file. * 3) CHANGE the name of the variable to start with "retired". * 4) CHANGE the parameters of the `registerFeature` call to `Supported::yes` - * and `DefaultVote::no`. + * and `VoteBehavior::DefaultNo`. * The feature must remain registered and supported indefinitely because it * still exists in the ledger, but there is no need to vote for it because * there's nothing to vote for. If it is removed completely from the code, any @@ -66,7 +66,7 @@ namespace ripple { -enum class DefaultVote : bool { no = false, yes }; +enum class VoteBehavior : int { Obsolete = -1, DefaultNo = 0, DefaultYes }; namespace detail { @@ -79,7 +79,7 @@ static constexpr std::size_t numFeatures = 54; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated ledger */ -std::map const& +std::map const& supportedAmendments(); /** Amendments that this server won't vote for by default. diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 5903603f975..a295fdf59c1 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -67,9 +67,10 @@ enum class Supported : bool { no = false, yes }; // updated. // // Generally, amendments which introduce new features should be set as -// "DefaultVote::no" whereas in rare cases, amendments that fix critical -// bugs should be set as "DefaultVote::yes", if off-chain consensus is -// reached amongst reviewers, validator operators, and other participants. +// "VoteBehavior::DefaultNo" whereas in rare cases, amendments that fix +// critical bugs should be set as "VoteBehavior::DefaultYes", if off-chain +// consensus is reached amongst reviewers, validator operators, and other +// participants. class FeatureCollections { @@ -115,7 +116,7 @@ class FeatureCollections // name, index, and uint256 feature identifier boost::multi_index::multi_index_container features; - std::map supported; + std::map supported; std::size_t upVotes = 0; std::size_t downVotes = 0; mutable std::atomic readOnly = false; @@ -163,7 +164,7 @@ class FeatureCollections registerFeature( std::string const& name, Supported support, - DefaultVote vote); + VoteBehavior vote); /** Tell FeatureCollections when registration is complete. */ bool @@ -181,7 +182,7 @@ class FeatureCollections /** Amendments that this server supports. Whether they are enabled depends on the Rules defined in the validated ledger */ - std::map const& + std::map const& supportedAmendments() const { return supported; @@ -230,11 +231,11 @@ uint256 FeatureCollections::registerFeature( std::string const& name, Supported support, - DefaultVote vote) + VoteBehavior vote) { check(!readOnly, "Attempting to register a feature after startup."); check( - support == Supported::yes || vote == DefaultVote::no, + support == Supported::yes || vote == VoteBehavior::DefaultNo, "Invalid feature parameters. Must be supported to be up-voted."); Feature const* i = getByName(name); if (!i) @@ -254,7 +255,7 @@ FeatureCollections::registerFeature( { supported.emplace(name, vote); - if (vote == DefaultVote::yes) + if (vote == VoteBehavior::DefaultYes) ++upVotes; else ++downVotes; @@ -315,7 +316,7 @@ static FeatureCollections featureCollections; /** Amendments that this server supports. Whether they are enabled depends on the Rules defined in the validated ledger */ -std::map const& +std::map const& detail::supportedAmendments() { return featureCollections.supportedAmendments(); @@ -344,7 +345,7 @@ getRegisteredFeature(std::string const& name) } uint256 -registerFeature(std::string const& name, Supported support, DefaultVote vote) +registerFeature(std::string const& name, Supported support, VoteBehavior vote) { return featureCollections.registerFeature(name, support, vote); } @@ -354,7 +355,7 @@ registerFeature(std::string const& name, Supported support, DefaultVote vote) uint256 retireFeature(std::string const& name) { - return registerFeature(name, Supported::yes, DefaultVote::no); + return registerFeature(name, Supported::yes, VoteBehavior::Obsolete); } /** Tell FeatureCollections when registration is complete. */ @@ -390,9 +391,9 @@ Takes the name of a feature, whether it's supported, and the default vote. Will register the feature, and create a variable whose name is "feature" plus the feature name. */ -#define REGISTER_FEATURE(fName, supported, defaultvote) \ - uint256 const feature##fName = \ - registerFeature(#fName, supported, defaultvote) +#define REGISTER_FEATURE(fName, supported, votebehavior) \ + uint256 const feature##fName = \ + registerFeature(#fName, supported, votebehavior) #pragma push_macro("REGISTER_FIX") #undef REGISTER_FIX @@ -402,56 +403,68 @@ Takes the name of a feature, whether it's supported, and the default vote. Will register the feature, and create a variable whose name is the unmodified feature name. */ -#define REGISTER_FIX(fName, supported, defaultvote) \ - uint256 const fName = registerFeature(#fName, supported, defaultvote) +#define REGISTER_FIX(fName, supported, votebehavior) \ + uint256 const fName = registerFeature(#fName, supported, votebehavior) // clang-format off // All known amendments must be registered either here or below with the // "retired" amendments -REGISTER_FEATURE(OwnerPaysFee, Supported::no, DefaultVote::no); -REGISTER_FEATURE(Flow, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(FlowCross, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(CryptoConditionsSuite, Supported::yes, DefaultVote::no); -REGISTER_FIX (fix1513, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(DepositAuth, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(Checks, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fix1571, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fix1543, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fix1623, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(DepositPreauth, Supported::yes, DefaultVote::yes); +REGISTER_FEATURE(OwnerPaysFee, Supported::no, VoteBehavior::DefaultNo); +REGISTER_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(FlowCross, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fix1513, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(DepositAuth, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(Checks, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fix1571, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fix1543, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fix1623, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(DepositPreauth, Supported::yes, VoteBehavior::DefaultYes); // Use liquidity from strands that consume max offers, but mark as dry -REGISTER_FIX (fix1515, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fix1578, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(MultiSignReserve, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fixTakerDryOfferRemoval, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fixMasterKeyAsRegularKey, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fixCheckThreading, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fixPayChanRecipientOwnerDir, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(DeletableAccounts, Supported::yes, DefaultVote::yes); +REGISTER_FIX (fix1515, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fix1578, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(MultiSignReserve, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fixTakerDryOfferRemoval, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fixMasterKeyAsRegularKey, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fixCheckThreading, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fixPayChanRecipientOwnerDir, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(DeletableAccounts, Supported::yes, VoteBehavior::DefaultYes); // fixQualityUpperBound should be activated before FlowCross -REGISTER_FIX (fixQualityUpperBound, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(RequireFullyCanonicalSig, Supported::yes, DefaultVote::yes); +REGISTER_FIX (fixQualityUpperBound, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes); // fix1781: XRPEndpointSteps should be included in the circular payment check -REGISTER_FIX (fix1781, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(HardenedValidations, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fixAmendmentMajorityCalc, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(NegativeUNL, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(TicketBatch, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(FlowSortStrands, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fixSTAmountCanonicalize, Supported::yes, DefaultVote::yes); -REGISTER_FIX (fixRmSmallIncreasedQOffers, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(CheckCashMakesTrustLine, Supported::yes, DefaultVote::no); -REGISTER_FEATURE(NonFungibleTokensV1, Supported::yes, DefaultVote::no); -REGISTER_FEATURE(ExpandedSignerList, Supported::yes, DefaultVote::no); -REGISTER_FIX (fixNFTokenDirV1, Supported::yes, DefaultVote::no); -REGISTER_FIX (fixNFTokenNegOffer, Supported::yes, DefaultVote::no); -REGISTER_FEATURE(NonFungibleTokensV1_1, Supported::yes, DefaultVote::no); -REGISTER_FIX (fixTrustLinesToSelf, Supported::yes, DefaultVote::no); +REGISTER_FIX (fix1781, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(HardenedValidations, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fixAmendmentMajorityCalc, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(NegativeUNL, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(TicketBatch, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fixSTAmountCanonicalize, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FIX (fixRmSmallIncreasedQOffers, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(CheckCashMakesTrustLine, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FEATURE(ExpandedSignerList, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FEATURE(NonFungibleTokensV1_1, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FIX (fixTrustLinesToSelf, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixRemoveNFTokenAutoTrustLine, Supported::yes, DefaultVote::yes); REGISTER_FEATURE(ImmediateOfferKilled, Supported::yes, DefaultVote::no); REGISTER_FEATURE(DisallowIncoming, Supported::yes, DefaultVote::no); +// The following amendments are obsolete, but must remain supported +// because they could potentially get enabled. +// +// Obsolete features are (usually) not in the ledger, and may have code +// controlled by the feature. They need to be supported because at some +// time in the past, the feature was supported and votable, but never +// passed. So the feature needs to be supported in case it is ever +// enabled (added to the ledger). +// +// If a feature remains obsolete for long enough that no clients are able +// to vote for it, the feature can be removed (entirely?) from the code. +REGISTER_FEATURE(CryptoConditionsSuite, Supported::yes, VoteBehavior::Obsolete); +REGISTER_FEATURE(NonFungibleTokensV1, Supported::yes, VoteBehavior::Obsolete); +REGISTER_FIX (fixNFTokenDirV1, Supported::yes, VoteBehavior::Obsolete); +REGISTER_FIX (fixNFTokenNegOffer, Supported::yes, VoteBehavior::Obsolete); + // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. // All known amendments and amendments that may appear in a validated diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index 99922e863a4..c7f9932133d 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -35,6 +35,21 @@ namespace ripple { +// Even though this is written as a template, and could potentially be a +// global helper, it's only intended to be used for +// std::vector in this class. +template +std::vector +operator+(std::vector left, std::vector right) +{ + left.reserve(left.size() + right.size()); + for (auto& item : right) + { + left.emplace_back(std::move(item)); + } + return left; +} + class AmendmentTable_test final : public beast::unit_test::suite { private: @@ -87,35 +102,57 @@ class AmendmentTable_test final : public beast::unit_test::suite } static std::vector - makeDefaultYes(std::vector const& amendments) + makeFeatureInfo( + std::vector const& amendments, + VoteBehavior voteBehavior) { std::vector result; result.reserve(amendments.size()); for (auto const& a : amendments) { - result.emplace_back(a, amendmentId(a), DefaultVote::yes); + result.emplace_back(a, amendmentId(a), voteBehavior); } return result; } + static std::vector + makeDefaultYes(std::vector const& amendments) + { + return makeFeatureInfo(amendments, VoteBehavior::DefaultYes); + } + static std::vector makeDefaultYes(uint256 const amendment) { std::vector result{ - {to_string(amendment), amendment, DefaultVote::yes}}; + {to_string(amendment), amendment, VoteBehavior::DefaultYes}}; return result; } + static std::vector + makeDefaultNo(std::vector const& amendments) + { + return makeFeatureInfo(amendments, VoteBehavior::DefaultNo); + } + + static std::vector + makeObsolete(std::vector const& amendments) + { + return makeFeatureInfo(amendments, VoteBehavior::Obsolete); + } + // All useful amendments are supported amendments. // Enabled amendments are typically a subset of supported amendments. // Vetoed amendments should be supported but not enabled. // Unsupported amendments may be added to the AmendmentTable. - std::vector const supportedYes_{ - "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", - "l", "m", "n", "o", "p", "q", "r", "s", "t", "u"}; + std::vector const + yes_{"g", "i", "k", "m", "o", "q", "r", "s", "t", "u"}; std::vector const enabled_{"b", "d", "f", "h", "j", "l", "n", "p"}; std::vector const vetoed_{"a", "c", "e"}; + std::vector const obsolete_{"0", "1", "2"}; + std::vector const allSupported_{ + yes_ + enabled_ + vetoed_ + obsolete_}; std::vector const unsupported_{"v", "w", "x"}; std::vector const unsupportedMajority_{"y", "z"}; @@ -158,7 +195,8 @@ class AmendmentTable_test final : public beast::unit_test::suite return makeTable( env.app(), majorityTime, - makeDefaultYes(supportedYes_), + makeDefaultYes(yes_) + makeDefaultNo(enabled_) + + makeDefaultYes(vetoed_) + makeObsolete(obsolete_), makeSection(enabled_), makeSection(vetoed_)); } @@ -170,17 +208,22 @@ class AmendmentTable_test final : public beast::unit_test::suite test::jtx::Env env{*this, makeConfig()}; auto table = makeTable(env, weeks(1)); - for (auto const& a : supportedYes_) - { + for (auto const& a : allSupported_) + BEAST_EXPECT(table->isSupported(amendmentId(a))); + + for (auto const& a : yes_) BEAST_EXPECT(table->isSupported(amendmentId(a))); - } for (auto const& a : enabled_) + BEAST_EXPECT(table->isSupported(amendmentId(a))); + + for (auto const& a : vetoed_) { BEAST_EXPECT(table->isSupported(amendmentId(a))); + BEAST_EXPECT(!table->isEnabled(amendmentId(a))); } - for (auto const& a : vetoed_) + for (auto const& a : obsolete_) { BEAST_EXPECT(table->isSupported(amendmentId(a))); BEAST_EXPECT(!table->isEnabled(amendmentId(a))); @@ -195,13 +238,14 @@ class AmendmentTable_test final : public beast::unit_test::suite test::jtx::Env env{*this, makeConfig()}; auto table = makeTable(env, weeks(1)); - for (auto const& a : supportedYes_) + for (auto const& a : yes_) BEAST_EXPECT(table->find(a) == amendmentId(a)); for (auto const& a : enabled_) BEAST_EXPECT(table->find(a) == amendmentId(a)); - for (auto const& a : vetoed_) BEAST_EXPECT(table->find(a) == amendmentId(a)); + for (auto const& a : obsolete_) + BEAST_EXPECT(table->find(a) == amendmentId(a)); for (auto const& a : unsupported_) BEAST_EXPECT(!table->find(a)); for (auto const& a : unsupportedMajority_) @@ -228,7 +272,7 @@ class AmendmentTable_test final : public beast::unit_test::suite void testBadConfig() { - auto const yesVotes = makeDefaultYes(supportedYes_); + auto const yesVotes = makeDefaultYes(yes_); auto const section = makeSection(vetoed_); auto const id = to_string(amendmentId(enabled_[0])); @@ -339,7 +383,7 @@ class AmendmentTable_test final : public beast::unit_test::suite test::jtx::Env env{*this, makeConfig()}; std::unique_ptr table = makeTable(env, weeks(2)); - // Note which entries are enabled + // Note which entries are enabled (convert the amendment names to IDs) std::set allEnabled; for (auto const& a : enabled_) allEnabled.insert(amendmentId(a)); @@ -351,7 +395,7 @@ class AmendmentTable_test final : public beast::unit_test::suite BEAST_EXPECT(!table->hasUnsupportedEnabled()); // Verify all enables are enabled and nothing else. - for (std::string const& a : supportedYes_) + for (std::string const& a : yes_) { uint256 const supportedID = amendmentId(a); bool const enabled = table->isEnabled(supportedID); @@ -375,7 +419,7 @@ class AmendmentTable_test final : public beast::unit_test::suite // Unveto an amendment that is already not vetoed. Shouldn't // hurt anything, but the values returned by getDesired() // shouldn't change. - BEAST_EXPECT(!table->unVeto(amendmentId(supportedYes_[1]))); + BEAST_EXPECT(!table->unVeto(amendmentId(yes_[1]))); BEAST_EXPECT(desired == table->getDesired()); } @@ -391,7 +435,7 @@ class AmendmentTable_test final : public beast::unit_test::suite } // Veto all supported amendments. Now desired should be empty. - for (std::string const& a : supportedYes_) + for (std::string const& a : allSupported_) { table->veto(amendmentId(a)); } @@ -653,11 +697,7 @@ class AmendmentTable_test final : public beast::unit_test::suite test::jtx::Env env{*this}; auto table = makeTable( - env, - weeks(2), - makeDefaultYes(supportedYes_), - emptySection, - emptySection); + env, weeks(2), makeDefaultYes(yes_), emptySection, emptySection); auto const validators = makeValidators(10); std::vector> votes; @@ -675,13 +715,13 @@ class AmendmentTable_test final : public beast::unit_test::suite ourVotes, enabled, majority); - BEAST_EXPECT(ourVotes.size() == supportedYes_.size()); + BEAST_EXPECT(ourVotes.size() == yes_.size()); BEAST_EXPECT(enabled.empty()); - for (auto const& i : supportedYes_) + for (auto const& i : yes_) BEAST_EXPECT(majority.find(amendmentId(i)) == majority.end()); // Now, everyone votes for this feature - for (auto const& i : supportedYes_) + for (auto const& i : yes_) votes.emplace_back(amendmentId(i), validators.size()); // Week 2: We should recognize a majority @@ -694,10 +734,10 @@ class AmendmentTable_test final : public beast::unit_test::suite ourVotes, enabled, majority); - BEAST_EXPECT(ourVotes.size() == supportedYes_.size()); + BEAST_EXPECT(ourVotes.size() == yes_.size()); BEAST_EXPECT(enabled.empty()); - for (auto const& i : supportedYes_) + for (auto const& i : yes_) BEAST_EXPECT(majority[amendmentId(i)] == weekTime(weeks{2})); // Week 5: We should enable the amendment @@ -710,7 +750,7 @@ class AmendmentTable_test final : public beast::unit_test::suite ourVotes, enabled, majority); - BEAST_EXPECT(enabled.size() == supportedYes_.size()); + BEAST_EXPECT(enabled.size() == yes_.size()); // Week 6: We should remove it from our votes and from having a majority doRound( @@ -722,9 +762,9 @@ class AmendmentTable_test final : public beast::unit_test::suite ourVotes, enabled, majority); - BEAST_EXPECT(enabled.size() == supportedYes_.size()); + BEAST_EXPECT(enabled.size() == yes_.size()); BEAST_EXPECT(ourVotes.empty()); - for (auto const& i : supportedYes_) + for (auto const& i : yes_) BEAST_EXPECT(majority.find(amendmentId(i)) == majority.end()); } diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 1ba4d865d44..dcd95c8a968 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -31,25 +31,33 @@ class Feature_test : public beast::unit_test::suite { testcase("internals"); - std::map const& supported = + std::map const& supported = ripple::detail::supportedAmendments(); BEAST_EXPECT( supported.size() == ripple::detail::numDownVotedAmendments() + ripple::detail::numUpVotedAmendments()); - std::size_t up = 0, down = 0; - for (std::pair const& amendment : + std::size_t up = 0, down = 0, obsolete = 0; + for (std::pair const& amendment : supported) { - if (amendment.second == DefaultVote::no) - ++down; - else + switch (amendment.second) { - if (BEAST_EXPECT(amendment.second == DefaultVote::yes)) + case VoteBehavior::DefaultYes: ++up; + break; + case VoteBehavior::DefaultNo: + ++down; + break; + case VoteBehavior::Obsolete: + ++obsolete; + break; + default: + fail("Unknown VoteBehavior", __FILE__, __LINE__); } } - BEAST_EXPECT(down == ripple::detail::numDownVotedAmendments()); + BEAST_EXPECT( + down + obsolete == ripple::detail::numDownVotedAmendments()); BEAST_EXPECT(up == ripple::detail::numUpVotedAmendments()); } @@ -105,7 +113,7 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env{*this}; - std::map const& votes = + std::map const& votes = ripple::detail::supportedAmendments(); auto jrr = env.rpc("feature")[jss::result]; @@ -118,15 +126,26 @@ class Feature_test : public beast::unit_test::suite // default config - so all should be disabled, and // supported. Some may be vetoed. bool expectVeto = - !(votes.at(feature[jss::name].asString()) == DefaultVote::yes); + (votes.at(feature[jss::name].asString()) == + VoteBehavior::DefaultNo); + bool expectObsolete = + (votes.at(feature[jss::name].asString()) == + VoteBehavior::Obsolete); BEAST_EXPECTS( - !feature[jss::enabled].asBool(), + feature.isMember(jss::enabled) && + !feature[jss::enabled].asBool(), feature[jss::name].asString() + " enabled"); BEAST_EXPECTS( - feature[jss::vetoed].asBool() == expectVeto, + feature.isMember(jss::vetoed) && + feature[jss::vetoed].isBool() == !expectObsolete && + (!feature[jss::vetoed].isBool() || + feature[jss::vetoed].asBool() == expectVeto) && + (feature[jss::vetoed].isBool() || + feature[jss::vetoed].asString() == "Obsolete"), feature[jss::name].asString() + " vetoed"); BEAST_EXPECTS( - feature[jss::supported].asBool(), + feature.isMember(jss::supported) && + feature[jss::supported].asBool(), feature[jss::name].asString() + " supported"); } } @@ -150,7 +169,9 @@ class Feature_test : public beast::unit_test::suite BEAST_EXPECTS(feature[jss::name] == "MultiSignReserve", "name"); BEAST_EXPECTS(!feature[jss::enabled].asBool(), "enabled"); - BEAST_EXPECTS(!feature[jss::vetoed].asBool(), "vetoed"); + BEAST_EXPECTS( + feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(), + "vetoed"); BEAST_EXPECTS(feature[jss::supported].asBool(), "supported"); // feature names are case-sensitive - expect error here @@ -200,7 +221,7 @@ class Feature_test : public beast::unit_test::suite Env env{ *this, FeatureBitset(featureDepositAuth, featureDepositPreauth)}; - std::map const& votes = + std::map const& votes = ripple::detail::supportedAmendments(); auto jrr = env.rpc("feature")[jss::result]; @@ -218,15 +239,31 @@ class Feature_test : public beast::unit_test::suite bool expectSupported = env.app().getAmendmentTable().isSupported(id); bool expectVeto = - !(votes.at((*it)[jss::name].asString()) == DefaultVote::yes); + (votes.at((*it)[jss::name].asString()) == + VoteBehavior::DefaultNo); + bool expectObsolete = + (votes.at((*it)[jss::name].asString()) == + VoteBehavior::Obsolete); BEAST_EXPECTS( - (*it)[jss::enabled].asBool() == expectEnabled, + (*it).isMember(jss::enabled) && + (*it)[jss::enabled].asBool() == expectEnabled, (*it)[jss::name].asString() + " enabled"); + if (expectEnabled) + BEAST_EXPECTS( + !(*it).isMember(jss::vetoed), + (*it)[jss::name].asString() + " vetoed"); + else + BEAST_EXPECTS( + (*it).isMember(jss::vetoed) && + (*it)[jss::vetoed].isBool() == !expectObsolete && + (!(*it)[jss::vetoed].isBool() || + (*it)[jss::vetoed].asBool() == expectVeto) && + ((*it)[jss::vetoed].isBool() || + (*it)[jss::vetoed].asString() == "Obsolete"), + (*it)[jss::name].asString() + " vetoed"); BEAST_EXPECTS( - (*it)[jss::vetoed].asBool() == expectVeto, - (*it)[jss::name].asString() + " vetoed"); - BEAST_EXPECTS( - (*it)[jss::supported].asBool() == expectSupported, + (*it).isMember(jss::supported) && + (*it)[jss::supported].asBool() == expectSupported, (*it)[jss::name].asString() + " supported"); } } @@ -282,7 +319,7 @@ class Feature_test : public beast::unit_test::suite // There should be at least 5 amendments. Don't do exact comparison // to avoid maintenance as more amendments are added in the future. BEAST_EXPECT(majorities.size() >= 5); - std::map const& votes = + std::map const& votes = ripple::detail::supportedAmendments(); jrr = env.rpc("feature")[jss::result]; @@ -293,13 +330,22 @@ class Feature_test : public beast::unit_test::suite if (!BEAST_EXPECT(feature.isMember(jss::name))) return; bool expectVeto = - !(votes.at(feature[jss::name].asString()) == DefaultVote::yes); + (votes.at(feature[jss::name].asString()) == + VoteBehavior::DefaultNo); + bool expectObsolete = + (votes.at(feature[jss::name].asString()) == + VoteBehavior::Obsolete); BEAST_EXPECTS( - expectVeto ^ feature.isMember(jss::majority), + (expectVeto || expectObsolete) ^ + feature.isMember(jss::majority), feature[jss::name].asString() + " majority"); BEAST_EXPECTS( feature.isMember(jss::vetoed) && - feature[jss::vetoed].asBool() == expectVeto, + feature[jss::vetoed].isBool() == !expectObsolete && + (!feature[jss::vetoed].isBool() || + feature[jss::vetoed].asBool() == expectVeto) && + (feature[jss::vetoed].isBool() || + feature[jss::vetoed].asString() == "Obsolete"), feature[jss::name].asString() + " vetoed"); BEAST_EXPECTS( feature.isMember(jss::count), @@ -310,11 +356,13 @@ class Feature_test : public beast::unit_test::suite BEAST_EXPECTS( feature.isMember(jss::validations), feature[jss::name].asString() + " validations"); - BEAST_EXPECT(feature[jss::count] == (expectVeto ? 0 : 1)); + BEAST_EXPECT( + feature[jss::count] == + ((expectVeto || expectObsolete) ? 0 : 1)); BEAST_EXPECT(feature[jss::threshold] == 1); BEAST_EXPECT(feature[jss::validations] == 1); BEAST_EXPECTS( - expectVeto || feature[jss::majority] == 2540, + expectVeto || expectObsolete || feature[jss::majority] == 2540, "Majority: " + feature[jss::majority].asString()); } } @@ -326,39 +374,100 @@ class Feature_test : public beast::unit_test::suite using namespace test::jtx; Env env{*this, FeatureBitset(featureMultiSignReserve)}; + constexpr const char* featureName = "MultiSignReserve"; - auto jrr = env.rpc("feature", "MultiSignReserve")[jss::result]; + auto jrr = env.rpc("feature", featureName)[jss::result]; if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) return; jrr.removeMember(jss::status); if (!BEAST_EXPECT(jrr.size() == 1)) return; auto feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "MultiSignReserve", "name"); - BEAST_EXPECTS(!feature[jss::vetoed].asBool(), "vetoed"); + BEAST_EXPECTS(feature[jss::name] == featureName, "name"); + BEAST_EXPECTS( + feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(), + "vetoed"); - jrr = env.rpc("feature", "MultiSignReserve", "reject")[jss::result]; + jrr = env.rpc("feature", featureName, "reject")[jss::result]; if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) return; jrr.removeMember(jss::status); if (!BEAST_EXPECT(jrr.size() == 1)) return; feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "MultiSignReserve", "name"); - BEAST_EXPECTS(feature[jss::vetoed].asBool(), "vetoed"); + BEAST_EXPECTS(feature[jss::name] == featureName, "name"); + BEAST_EXPECTS( + feature[jss::vetoed].isBool() && feature[jss::vetoed].asBool(), + "vetoed"); - jrr = env.rpc("feature", "MultiSignReserve", "accept")[jss::result]; + jrr = env.rpc("feature", featureName, "accept")[jss::result]; if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) return; jrr.removeMember(jss::status); if (!BEAST_EXPECT(jrr.size() == 1)) return; feature = *(jrr.begin()); - BEAST_EXPECTS(feature[jss::name] == "MultiSignReserve", "name"); - BEAST_EXPECTS(!feature[jss::vetoed].asBool(), "vetoed"); + BEAST_EXPECTS(feature[jss::name] == featureName, "name"); + BEAST_EXPECTS( + feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(), + "vetoed"); + + // anything other than accept or reject is an error + jrr = env.rpc("feature", featureName, "maybe"); + BEAST_EXPECT(jrr[jss::error] == "invalidParams"); + BEAST_EXPECT(jrr[jss::error_message] == "Invalid parameters."); + } + + void + testObsolete() + { + testcase("Obsolete"); + + using namespace test::jtx; + Env env{*this}; + constexpr const char* featureName = "NonFungibleTokensV1"; + + auto jrr = env.rpc("feature", featureName)[jss::result]; + if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) + return; + jrr.removeMember(jss::status); + if (!BEAST_EXPECT(jrr.size() == 1)) + return; + auto feature = *(jrr.begin()); + BEAST_EXPECTS(feature[jss::name] == featureName, "name"); + BEAST_EXPECTS( + feature[jss::vetoed].isString() && + feature[jss::vetoed].asString() == "Obsolete", + "vetoed"); + + jrr = env.rpc("feature", featureName, "reject")[jss::result]; + if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) + return; + jrr.removeMember(jss::status); + if (!BEAST_EXPECT(jrr.size() == 1)) + return; + feature = *(jrr.begin()); + BEAST_EXPECTS(feature[jss::name] == featureName, "name"); + BEAST_EXPECTS( + feature[jss::vetoed].isString() && + feature[jss::vetoed].asString() == "Obsolete", + "vetoed"); + + jrr = env.rpc("feature", featureName, "accept")[jss::result]; + if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status")) + return; + jrr.removeMember(jss::status); + if (!BEAST_EXPECT(jrr.size() == 1)) + return; + feature = *(jrr.begin()); + BEAST_EXPECTS(feature[jss::name] == featureName, "name"); + BEAST_EXPECTS( + feature[jss::vetoed].isString() && + feature[jss::vetoed].asString() == "Obsolete", + "vetoed"); // anything other than accept or reject is an error - jrr = env.rpc("feature", "MultiSignReserve", "maybe"); + jrr = env.rpc("feature", featureName, "maybe"); BEAST_EXPECT(jrr[jss::error] == "invalidParams"); BEAST_EXPECT(jrr[jss::error_message] == "Invalid parameters."); } @@ -376,6 +485,7 @@ class Feature_test : public beast::unit_test::suite testSomeEnabled(); testWithMajorities(); testVeto(); + testObsolete(); } }; From b1b6101484525cd5cb9123f4f5e80fb29d129f61 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 8 Sep 2022 16:43:57 -0700 Subject: [PATCH 2/3] [FOLD] Review feedback from @scottschurr: * Use an explicit function to combine arrays instead of an operator+ * Rename a few variables for consistency --- src/test/app/AmendmentTable_test.cpp | 106 ++++++++++++++++++--------- 1 file changed, 71 insertions(+), 35 deletions(-) diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index c7f9932133d..4284190a18a 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -35,21 +35,6 @@ namespace ripple { -// Even though this is written as a template, and could potentially be a -// global helper, it's only intended to be used for -// std::vector in this class. -template -std::vector -operator+(std::vector left, std::vector right) -{ - left.reserve(left.size() + right.size()); - for (auto& item : right) - { - left.emplace_back(std::move(item)); - } - return left; -} - class AmendmentTable_test final : public beast::unit_test::suite { private: @@ -141,6 +126,44 @@ class AmendmentTable_test final : public beast::unit_test::suite return makeFeatureInfo(amendments, VoteBehavior::Obsolete); } + template + static size_t + totalsize(std::vector const& src, Args const&... args) + { + if constexpr (sizeof...(args) > 0) + return src.size() + totalsize(args...); + return src.size(); + } + + template + static void + combine_arg( + std::vector& dest, + std::vector const& src, + Args const&... args) + { + assert(dest.capacity() >= dest.size() + src.size()); + std::copy(src.begin(), src.end(), std::back_inserter(dest)); + if constexpr (sizeof...(args) > 0) + combine_arg(dest, args...); + } + + template + static std::vector + combine( + // Pass "left" by value. The values will need to be copied one way or + // another, so just reuse it. + std::vector left, + std::vector const& right, + Args const&... args) + { + left.reserve(totalsize(left, right, args...)); + + combine_arg(left, right, args...); + + return left; + } + // All useful amendments are supported amendments. // Enabled amendments are typically a subset of supported amendments. // Vetoed amendments should be supported but not enabled. @@ -152,17 +175,17 @@ class AmendmentTable_test final : public beast::unit_test::suite std::vector const vetoed_{"a", "c", "e"}; std::vector const obsolete_{"0", "1", "2"}; std::vector const allSupported_{ - yes_ + enabled_ + vetoed_ + obsolete_}; + combine(yes_, enabled_, vetoed_, obsolete_)}; std::vector const unsupported_{"v", "w", "x"}; std::vector const unsupportedMajority_{"y", "z"}; - Section const emptySection; - std::vector const emptyYes; + Section const emptySection_; + std::vector const emptyYes_; - test::SuiteJournal journal; + test::SuiteJournal journal_; public: - AmendmentTable_test() : journal("AmendmentTable_test", *this) + AmendmentTable_test() : journal_("AmendmentTable_test", *this) { } @@ -175,7 +198,7 @@ class AmendmentTable_test final : public beast::unit_test::suite Section const& vetoed) { return make_AmendmentTable( - app, majorityTime, supported, enabled, vetoed, journal); + app, majorityTime, supported, enabled, vetoed, journal_); } std::unique_ptr @@ -192,11 +215,20 @@ class AmendmentTable_test final : public beast::unit_test::suite std::unique_ptr makeTable(test::jtx::Env& env, std::chrono::seconds majorityTime) { + static std::vector const supported = + combine( + makeDefaultYes(yes_), + // Use non-intuitive default votes for "enabled_" and "vetoed_" + // so that when the tests later explicitly enable or veto them, + // we can be certain that they are not simply going by their + // default vote setting. + makeDefaultNo(enabled_), + makeDefaultYes(vetoed_), + makeObsolete(obsolete_)); return makeTable( env.app(), majorityTime, - makeDefaultYes(yes_) + makeDefaultNo(enabled_) + - makeDefaultYes(vetoed_) + makeObsolete(obsolete_), + supported, makeSection(enabled_), makeSection(vetoed_)); } @@ -285,7 +317,7 @@ class AmendmentTable_test final : public beast::unit_test::suite try { test::jtx::Env env{*this, makeConfig()}; - if (makeTable(env, weeks(2), yesVotes, test, emptySection)) + if (makeTable(env, weeks(2), yesVotes, test, emptySection_)) fail("Accepted only amendment ID"); } catch (std::exception const& e) @@ -302,7 +334,7 @@ class AmendmentTable_test final : public beast::unit_test::suite try { test::jtx::Env env{*this, makeConfig()}; - if (makeTable(env, weeks(2), yesVotes, test, emptySection)) + if (makeTable(env, weeks(2), yesVotes, test, emptySection_)) fail("Accepted extra arguments"); } catch (std::exception const& e) @@ -323,7 +355,7 @@ class AmendmentTable_test final : public beast::unit_test::suite try { test::jtx::Env env{*this, makeConfig()}; - if (makeTable(env, weeks(2), yesVotes, test, emptySection)) + if (makeTable(env, weeks(2), yesVotes, test, emptySection_)) fail("Accepted short amendment ID"); } catch (std::exception const& e) @@ -343,7 +375,7 @@ class AmendmentTable_test final : public beast::unit_test::suite try { test::jtx::Env env{*this, makeConfig()}; - if (makeTable(env, weeks(2), yesVotes, test, emptySection)) + if (makeTable(env, weeks(2), yesVotes, test, emptySection_)) fail("Accepted long amendment ID"); } catch (std::exception const& e) @@ -364,7 +396,7 @@ class AmendmentTable_test final : public beast::unit_test::suite try { test::jtx::Env env{*this, makeConfig()}; - if (makeTable(env, weeks(2), yesVotes, test, emptySection)) + if (makeTable(env, weeks(2), yesVotes, test, emptySection_)) fail("Accepted non-hex amendment ID"); } catch (std::exception const& e) @@ -577,7 +609,7 @@ class AmendmentTable_test final : public beast::unit_test::suite test::jtx::Env env{*this}; auto table = - makeTable(env, weeks(2), emptyYes, emptySection, emptySection); + makeTable(env, weeks(2), emptyYes_, emptySection_, emptySection_); std::vector> votes; std::vector ourVotes; @@ -638,7 +670,11 @@ class AmendmentTable_test final : public beast::unit_test::suite test::jtx::Env env{*this}; auto table = makeTable( - env, weeks(2), emptyYes, emptySection, makeSection(testAmendment)); + env, + weeks(2), + emptyYes_, + emptySection_, + makeSection(testAmendment)); auto const validators = makeValidators(10); @@ -697,7 +733,7 @@ class AmendmentTable_test final : public beast::unit_test::suite test::jtx::Env env{*this}; auto table = makeTable( - env, weeks(2), makeDefaultYes(yes_), emptySection, emptySection); + env, weeks(2), makeDefaultYes(yes_), emptySection_, emptySection_); auto const validators = makeValidators(10); std::vector> votes; @@ -780,8 +816,8 @@ class AmendmentTable_test final : public beast::unit_test::suite env, weeks(2), makeDefaultYes(testAmendment), - emptySection, - emptySection); + emptySection_, + emptySection_); auto const validators = makeValidators(16); @@ -851,8 +887,8 @@ class AmendmentTable_test final : public beast::unit_test::suite env, weeks(8), makeDefaultYes(testAmendment), - emptySection, - emptySection); + emptySection_, + emptySection_); std::set enabled; majorityAmendments_t majority; From 7dac139c7ac4b50e1173cddf73382a8172a093de Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Thu, 15 Dec 2022 16:19:29 -0500 Subject: [PATCH 3/3] [FOLD] Update VoteBehavior for new features from rebase: * Thanks @scottschurr --- src/ripple/protocol/impl/Feature.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index a295fdf59c1..93b96eaac5c 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -445,9 +445,9 @@ REGISTER_FEATURE(CheckCashMakesTrustLine, Supported::yes, VoteBehavior::De REGISTER_FEATURE(ExpandedSignerList, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FEATURE(NonFungibleTokensV1_1, Supported::yes, VoteBehavior::DefaultNo); REGISTER_FIX (fixTrustLinesToSelf, Supported::yes, VoteBehavior::DefaultNo); -REGISTER_FIX (fixRemoveNFTokenAutoTrustLine, Supported::yes, DefaultVote::yes); -REGISTER_FEATURE(ImmediateOfferKilled, Supported::yes, DefaultVote::no); -REGISTER_FEATURE(DisallowIncoming, Supported::yes, DefaultVote::no); +REGISTER_FIX (fixRemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes); +REGISTER_FEATURE(ImmediateOfferKilled, Supported::yes, VoteBehavior::DefaultNo); +REGISTER_FEATURE(DisallowIncoming, Supported::yes, VoteBehavior::DefaultNo); // The following amendments are obsolete, but must remain supported // because they could potentially get enabled.