Skip to content

Commit

Permalink
[EXPERIMENT] Add AmendmentVote enum to RelationalDBInterface:
Browse files Browse the repository at this point in the history 
Also simplifies and un-generics a couple of lambdas in
AmendmentTableImpl::AmendmentTableImpl.
  • Loading branch information
@scottschurr
scottschurr committed Jul 23, 2021
1 parent 8afcfae commit bd282fd
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 38 deletions.
60 changes: 30 additions & 30 deletions src/ripple/app/misc/impl/AmendmentTable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ parseSection(Section const& section)
*/
struct AmendmentState
{
/** If an amendment is vetoed, a server will not support it */
bool vetoed = false;
/** If an amendment is down-voted, a server will not support it */
AmendmentVote vote = AmendmentVote::down;

/** Indicates that the amendment has been enabled.
This is a one-way switch: once an amendment is enabled
Expand Down Expand Up @@ -247,7 +247,7 @@ class AmendmentTableImpl final : public AmendmentTable
persistVote(
uint256 const& amendment,
std::string const& name,
bool vote_to_veto) const;
AmendmentVote vote) const;

public:
AmendmentTableImpl(
Expand Down Expand Up @@ -332,30 +332,29 @@ AmendmentTableImpl::AmendmentTableImpl(
return createFeatureVotes(*db);
}();

auto getName2 = [](auto const& amendment, auto const& name) {
auto getName = [](uint256 const& amendment,
std::string const& name) -> std::string {
if (name.empty())
return featureToName(amendment);
return name;
};
auto getName = [&getName2](auto const& amd) {
return getName2(amd.first, amd.second);
};
// Parse supported amendments
for (auto const& [name, amendment, defaultVote] : supported)
{
AmendmentState& s = add(amendment, lock);

s.name = getName2(amendment, name);
s.name = getName(amendment, name);
JLOG(j_.debug()) << "Amendment " << amendment << " (" << s.name
<< ") is supported.";

s.supported = true;
s.vetoed = !(defaultVote == DefaultVote::yes);
s.vote = defaultVote == DefaultVote::abstain ? AmendmentVote::down
: AmendmentVote::up;
}

hash_set<uint256> detect_conflict;
// Parse enabled amendments from config
for (auto const& a : parseSection(enabled))
for (std::pair<uint256, std::string> const& a : parseSection(enabled))
{
if (featureVotesExist)
{ // If the table existed, warn about duplicate config info
Expand All @@ -366,7 +365,7 @@ AmendmentTableImpl::AmendmentTableImpl(
else
{ // Otherwise transfer config data into the table
detect_conflict.insert(a.first);
persistVote(a.first, getName(a), false); // un-veto
persistVote(a.first, getName(a.first, a.second), AmendmentVote::up);
}
}

Expand All @@ -384,7 +383,8 @@ AmendmentTableImpl::AmendmentTableImpl(
{ // Otherwise transfer config data into the table
if (detect_conflict.count(a.first) == 0)
{
persistVote(a.first, getName(a), true); // veto
persistVote(
a.first, getName(a.first, a.second), AmendmentVote::down);
}
else
{
Expand All @@ -402,9 +402,9 @@ AmendmentTableImpl::AmendmentTableImpl(
*db,
[&](boost::optional<std::string> amendment_hash,
boost::optional<std::string> amendment_name,
boost::optional<int> vote_to_veto) {
boost::optional<AmendmentVote> vote) {
uint256 amend_hash;
if (!amendment_hash || !amendment_name || !vote_to_veto)
if (!amendment_hash || !amendment_name || !vote)
{
// These fields should never have nulls, but check
Throw<std::runtime_error>(
Expand All @@ -416,7 +416,7 @@ AmendmentTableImpl::AmendmentTableImpl(
"Invalid amendment ID '" + *amendment_hash +
" in wallet.db");
}
if (*vote_to_veto)
if (*vote == AmendmentVote::down)
{
// Unknown amendments are effectively vetoed already
if (auto s = get(amend_hash, lock))
Expand All @@ -425,18 +425,18 @@ AmendmentTableImpl::AmendmentTableImpl(
<< amend_hash << "} is vetoed.";
if (!amendment_name->empty())
s->name = *amendment_name;
s->vetoed = true;
s->vote = *vote;
}
}
else // un-veto
else // up-vote
{
auto s = add(amend_hash, lock);

JLOG(j_.debug()) << "Amendment {" << *amendment_name << ", "
<< amend_hash << "} is un-vetoed.";
if (!amendment_name->empty())
s.name = *amendment_name;
s.vetoed = false;
s.vote = *vote;
}
});
}
Expand Down Expand Up @@ -492,10 +492,10 @@ void
AmendmentTableImpl::persistVote(
uint256 const& amendment,
std::string const& name,
bool vote_to_veto) const
AmendmentVote vote) const
{
auto db = db_.checkoutDb();
voteAmendment(*db, amendment, name, vote_to_veto);
voteAmendment(*db, amendment, name, vote);
}

bool
Expand All @@ -504,10 +504,10 @@ AmendmentTableImpl::veto(uint256 const& amendment)
std::lock_guard lock(mutex_);
AmendmentState& s = add(amendment, lock);

if (s.vetoed)
if (s.vote == AmendmentVote::down)
return false;
s.vetoed = true;
persistVote(amendment, s.name, s.vetoed);
s.vote = AmendmentVote::down;
persistVote(amendment, s.name, s.vote);
return true;
}

Expand All @@ -517,10 +517,10 @@ AmendmentTableImpl::unVeto(uint256 const& amendment)
std::lock_guard lock(mutex_);
AmendmentState* const s = get(amendment, lock);

if (!s || !s->vetoed)
if (!s || s->vote == AmendmentVote::up)
return false;
s->vetoed = false;
persistVote(amendment, s->name, s->vetoed);
s->vote = AmendmentVote::up;
persistVote(amendment, s->name, s->vote);
return true;
}

Expand Down Expand Up @@ -587,7 +587,7 @@ AmendmentTableImpl::doValidation(std::set<uint256> const& enabled) const
amendments.reserve(amendmentMap_.size());
for (auto const& e : amendmentMap_)
{
if (e.second.supported && !e.second.vetoed &&
if (e.second.supported && e.second.vote == AmendmentVote::up &&
(enabled.count(e.first) == 0))
{
amendments.push_back(e.first);
Expand Down Expand Up @@ -652,7 +652,7 @@ AmendmentTableImpl::doVoting(
}
else if (
hasValMajority && (majorityTime == NetClock::time_point{}) &&
!entry.second.vetoed)
entry.second.vote == AmendmentVote::up)
{
// Ledger says no majority, validators say yes
JLOG(j_.debug()) << entry.first << ": amendment got majority";
Expand All @@ -667,7 +667,7 @@ AmendmentTableImpl::doVoting(
else if (
(majorityTime != NetClock::time_point{}) &&
((majorityTime + majorityTime_) <= closeTime) &&
!entry.second.vetoed)
entry.second.vote == AmendmentVote::up)
{
// Ledger says majority held
JLOG(j_.debug()) << entry.first << ": amendment majority held";
Expand Down Expand Up @@ -739,7 +739,7 @@ AmendmentTableImpl::injectJson(
v[jss::name] = fs.name;

v[jss::supported] = fs.supported;
v[jss::vetoed] = fs.vetoed;
v[jss::vetoed] = fs.vote == AmendmentVote::down;
v[jss::enabled] = fs.enabled;

if (!fs.enabled && lastVote_)
Expand Down
12 changes: 8 additions & 4 deletions src/ripple/app/rdb/RelationalDBInterface_global.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,33 +132,37 @@ deletePeerReservation(soci::session& session, PublicKey const& nodeId);
bool
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 };

/**
* @brief readAmendments Read all amendments from FeatureVotes table.
* @param session Session with walletDB database.
* @param callback Callback called for each amendment passing its hash, name
* and teh flag if it should be vetoed as callback parameters
* and the flag if it should be vetoed as callback parameters
*/
void
readAmendments(
soci::session& session,
std::function<void(
boost::optional<std::string> amendment_hash,
boost::optional<std::string> amendment_name,
boost::optional<int> vote_to_veto)> const& callback);
boost::optional<AmendmentVote> vote)> const& callback);

/**
* @brief voteAmendment Set veto value for particular amendment.
* @param session Session with walletDB database.
* @param amendment Hash of amendment.
* @param name Name of amendment.
* @param vote_to_veto Trus if this amendment should be vetoed.
* @param vote Whether to vote in favor of this amendment.
*/
void
voteAmendment(
soci::session& session,
uint256 const& amendment,
std::string const& name,
bool vote_to_veto);
AmendmentVote vote);

/* State DB */

Expand Down
14 changes: 10 additions & 4 deletions src/ripple/app/rdb/impl/RelationalDBInterface_global.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,14 @@ readAmendments(
std::function<void(
boost::optional<std::string> amendment_hash,
boost::optional<std::string> amendment_name,
boost::optional<int> vote_to_veto)> const& callback)
boost::optional<AmendmentVote> vote)> const& callback)
{
// lambda that converts the internally stored int to an AmendmentVote.
auto intToVote = [](boost::optional<int> const& dbVote)
-> boost::optional<AmendmentVote> {
return safe_cast<AmendmentVote>(dbVote.value_or(1));
};

soci::transaction tr(session);
std::string sql =
"SELECT AmendmentHash, AmendmentName, Veto FROM FeatureVotes";
Expand All @@ -275,7 +281,7 @@ readAmendments(
st.execute();
while (st.fetch())
{
callback(amendment_hash, amendment_name, vote_to_veto);
callback(amendment_hash, amendment_name, intToVote(vote_to_veto));
}
}

Expand All @@ -284,15 +290,15 @@ voteAmendment(
soci::session& session,
uint256 const& amendment,
std::string const& name,
bool vote_to_veto)
AmendmentVote vote)
{
soci::transaction tr(session);
std::string sql =
"INSERT INTO FeatureVotes (AmendmentHash, AmendmentName, Veto) VALUES "
"('";
sql += to_string(amendment);
sql += "', '" + name;
sql += "', '" + std::to_string(int{vote_to_veto}) + "');";
sql += "', '" + std::to_string(safe_cast<int>(vote)) + "');";
session << sql;
tr.commit();
}
Expand Down

0 comments on commit bd282fd

Please sign in to comment.