Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the ability to mark amendments as obsolete #4291

Merged
merged 21 commits into from
Mar 24, 2023
Merged
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e0b261a
Add the ability to mark amendments as obsolete:
ximinez Aug 29, 2022
b1b6101
[FOLD] Review feedback from @scottschurr:
ximinez Sep 8, 2022
7dac139
[FOLD] Update VoteBehavior for new features from rebase:
ximinez Dec 15, 2022
8d2823b
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Jan 5, 2023
152b39c
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Jan 6, 2023
99089e7
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Jan 12, 2023
c9ce8bc
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Feb 3, 2023
686a308
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Feb 6, 2023
b41abd4
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Feb 8, 2023
72d1ab3
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Feb 9, 2023
a102cd0
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Feb 15, 2023
4c9191f
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Feb 17, 2023
f926185
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Feb 23, 2023
7b20b9e
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Feb 23, 2023
200e7b8
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Feb 28, 2023
b15618c
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Mar 3, 2023
2ad44cf
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Mar 14, 2023
52c991c
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Mar 14, 2023
4d120b4
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Mar 15, 2023
603ea5f
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Mar 20, 2023
530b993
Merge remote-tracking branch 'upstream/develop' into neverfeature
ximinez Mar 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 71 additions & 35 deletions src/test/app/AmendmentTable_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<AmendmentTable::FeatureInfo> in this class.
template <class T>
std::vector<T>
operator+(std::vector<T> left, std::vector<T> 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:
Expand Down Expand Up @@ -141,6 +126,44 @@ class AmendmentTable_test final : public beast::unit_test::suite
return makeFeatureInfo(amendments, VoteBehavior::Obsolete);
}

template <class Arg, class... Args>
static size_t
totalsize(std::vector<Arg> const& src, Args const&... args)
{
if constexpr (sizeof...(args) > 0)
return src.size() + totalsize(args...);
return src.size();
}

template <class Arg, class... Args>
static void
combine_arg(
std::vector<Arg>& dest,
std::vector<Arg> 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 <class Arg, class... Args>
static std::vector<Arg>
combine(
// Pass "left" by value. The values will need to be copied one way or
// another, so just reuse it.
std::vector<Arg> left,
std::vector<Arg> const& right,
Args const&... args)
{
left.reserve(totalsize(left, right, args...));

combine_arg(left, right, args...);

return left;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

// All useful amendments are supported amendments.
// Enabled amendments are typically a subset of supported amendments.
// Vetoed amendments should be supported but not enabled.
Expand All @@ -152,17 +175,17 @@ class AmendmentTable_test final : public beast::unit_test::suite
std::vector<std::string> const vetoed_{"a", "c", "e"};
std::vector<std::string> const obsolete_{"0", "1", "2"};
std::vector<std::string> const allSupported_{
yes_ + enabled_ + vetoed_ + obsolete_};
combine(yes_, enabled_, vetoed_, obsolete_)};
std::vector<std::string> const unsupported_{"v", "w", "x"};
std::vector<std::string> const unsupportedMajority_{"y", "z"};

ximinez marked this conversation as resolved.
Show resolved Hide resolved
Section const emptySection;
std::vector<AmendmentTable::FeatureInfo> const emptyYes;
Section const emptySection_;
std::vector<AmendmentTable::FeatureInfo> const emptyYes_;

test::SuiteJournal journal;
test::SuiteJournal journal_;

public:
AmendmentTable_test() : journal("AmendmentTable_test", *this)
AmendmentTable_test() : journal_("AmendmentTable_test", *this)
{
}

Expand All @@ -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<AmendmentTable>
Expand All @@ -192,11 +215,20 @@ class AmendmentTable_test final : public beast::unit_test::suite
std::unique_ptr<AmendmentTable>
makeTable(test::jtx::Env& env, std::chrono::seconds majorityTime)
{
static std::vector<AmendmentTable::FeatureInfo> 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_));
}
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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<std::pair<uint256, int>> votes;
std::vector<uint256> ourVotes;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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<std::pair<uint256, int>> votes;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -851,8 +887,8 @@ class AmendmentTable_test final : public beast::unit_test::suite
env,
weeks(8),
makeDefaultYes(testAmendment),
emptySection,
emptySection);
emptySection_,
emptySection_);

std::set<uint256> enabled;
majorityAmendments_t majority;
Expand Down