ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-21 03:26:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: mark 4 amendments as obsolete: (#4291)

Browse Source 
Add the ability to mark amendments as obsolete. There are some known
amendments that should not be voted for because they are broken (or
similar reasons).

This commit marks four amendments as obsolete:

1. `CryptoConditionsSuite`
2. `NonFungibleTokensV1`
3. `fixNFTokenDirV1`
4. `fixNFTokenNegOffer`

When an amendment is `Obsolete`, voting for the amendment is prevented.
A determined operator can still vote for the amendment by changing the
source, and doing so does not break any protocol rules.

The "feature" command now does not modify the vote for obsolete
amendments.

Before this change, there were two options for an amendment's
`DefaultVote` behavior: yes and no.

After this change, there are three options for an amendment's
`VoteBehavior`: DefaultYes, DefaultNo, and Obsolete.

To be clear, if an obsolete amendment were to (somehow) be activated by
consensus, the server still has the code to process transactions
according to that amendment, and would not be amendment blocked. It
would function the same as if it had been voting "no" on the amendment.

Resolves #4014.

Incorporates review feedback from @scottschurr.
This commit is contained in:
Ed Hennis
2023-03-23 22:28:53 -07:00
committed by GitHub
parent dffcdea12b
commit 7aad6e5127
7 changed files with 386 additions and 164 deletions
Download Patch File Download Diff File Expand all files Collapse all files

184
src/test/rpc/Feature_test.cpp
View File

@@ -31,25 +31,33 @@ class Feature_test : public beast::unit_test::suite
{
testcase("internals");
std::map<std::string, DefaultVote> const& supported =
std::map<std::string, VoteBehavior> 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<std::string const, DefaultVote> const& amendment :
std::size_t up = 0, down = 0, obsolete = 0;
for (std::pair<std::string const, VoteBehavior> 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<std::string, DefaultVote> const& votes =
std::map<std::string, VoteBehavior> 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<std::string, DefaultVote> const& votes =
std::map<std::string, VoteBehavior> 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<std::string, DefaultVote> const& votes =
std::map<std::string, VoteBehavior> 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", "MultiSignReserve", "maybe");
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", featureName, "maybe");
BEAST_EXPECT(jrr[jss::error] == "invalidParams");
BEAST_EXPECT(jrr[jss::error_message] == "Invalid parameters.");
}
@@ -376,6 +485,7 @@ public:
testSomeEnabled();
testWithMajorities();
testVeto();
testObsolete();
}
};