Skip to content

Commit

Permalink
Internal amendment down votes:
Browse files Browse the repository at this point in the history 
* Allows a version to have the code to support a given amendment, but
  not vote for it by default. This allows the amendment to be enabled in
  a future version without necessarily amendment blocking these older
  versions.
  • Loading branch information
@ximinez
ximinez committed Sep 10, 2020
1 parent a8d481c commit 4f8be72
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 63 deletions.
36 changes: 22 additions & 14 deletions src/ripple/app/main/Application.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1344,26 +1344,34 @@ ApplicationImp::setup()

// Configure the amendments the server supports
{
auto const& sa = detail::supportedAmendments();
std::vector<std::string> saHashes;
saHashes.reserve(sa.size());
for (auto const& name : sa)
{
auto const f = getRegisteredFeature(name);
BOOST_ASSERT(f);
if (f)
saHashes.push_back(to_string(*f) + " " + name);
}
Section supportedAmendments("Supported Amendments");
supportedAmendments.append(saHashes);
auto buildAmendmentList =
[](Section section, std::vector<std::string> const& amendments) {
std::vector<std::string> hashes;
hashes.reserve(amendments.size());
for (auto const& name : amendments)
{
auto const f = getRegisteredFeature(name);
// BOOST_ASSERT(f);
if (f)
hashes.push_back(to_string(*f) + " " + name);
}
section.append(hashes);
return section;
};
Section const supportedAmendments = buildAmendmentList(
Section("Supported Amendments"), detail::supportedAmendments());

Section const downVotedAmendments = buildAmendmentList(
config_->section(SECTION_VETO_AMENDMENTS),
detail::downVotedAmendments());

Section enabledAmendments = config_->section(SECTION_AMENDMENTS);
Section const enabledAmendments = config_->section(SECTION_AMENDMENTS);

m_amendmentTable = make_AmendmentTable(
config().AMENDMENT_MAJORITY_TIME,
supportedAmendments,
enabledAmendments,
config_->section(SECTION_VETO_AMENDMENTS),
downVotedAmendments,
logs_->journal("Amendments"));
}

Expand Down
65 changes: 37 additions & 28 deletions src/ripple/app/misc/impl/AmendmentTable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ class AmendmentTableImpl final : public AmendmentTable
beast::Journal const j_;

// Finds or creates state. Must be called with mutex_ locked.
AmendmentState*
AmendmentState&
add(uint256 const& amendment, std::lock_guard<std::mutex> const& sl);

// Finds existing state. Must be called with mutex_ locked.
Expand Down Expand Up @@ -315,25 +315,30 @@ AmendmentTableImpl::AmendmentTableImpl(

for (auto const& a : parseSection(supported))
{
if (auto s = add(a.first, sl))
{
JLOG(j_.debug()) << "Amendment " << a.first << " is supported.";
AmendmentState& s = add(a.first, sl);

if (!a.second.empty())
s->name = a.second;
JLOG(j_.debug()) << "Amendment " << a.first << " (" << a.second
<< ") is supported.";

s->supported = true;
}
if (!a.second.empty())
s.name = a.second;
else
s.name = featureToName(a.first);

s.supported = true;
}

for (auto const& a : parseSection(enabled))
{
if (auto s = add(a.first, sl))
if (AmendmentState* const s = get(a.first, sl))
{
JLOG(j_.debug()) << "Amendment " << a.first << " is enabled.";
JLOG(j_.debug()) << "Amendment " << a.first << " (" << a.second
<< ") is enabled.";

if (!a.second.empty())
s->name = a.second;
else
s->name = featureToName(a.first);

s->supported = true;
s->enabled = true;
Expand All @@ -343,25 +348,28 @@ AmendmentTableImpl::AmendmentTableImpl(
for (auto const& a : parseSection(vetoed))
{
// Unknown amendments are effectively vetoed already
if (auto s = get(a.first, sl))
if (AmendmentState* const s = get(a.first, sl))
{
JLOG(j_.info()) << "Amendment " << a.first << " is vetoed.";
JLOG(j_.debug()) << "Amendment " << a.first << " (" << a.second
<< ") is vetoed.";

if (!a.second.empty())
s->name = a.second;
else
s->name = featureToName(a.first);

s->vetoed = true;
}
}
}

AmendmentState*
AmendmentState&
AmendmentTableImpl::add(
uint256 const& amendmentHash,
std::lock_guard<std::mutex> const&)
{
// call with the mutex held
return &amendmentMap_[amendmentHash];
return amendmentMap_[amendmentHash];
}

AmendmentState*
Expand Down Expand Up @@ -406,19 +414,19 @@ bool
AmendmentTableImpl::veto(uint256 const& amendment)
{
std::lock_guard sl(mutex_);
auto s = add(amendment, sl);
AmendmentState& s = add(amendment, sl);

if (s->vetoed)
if (s.vetoed)
return false;
s->vetoed = true;
s.vetoed = true;
return true;
}

bool
AmendmentTableImpl::unVeto(uint256 const& amendment)
{
std::lock_guard sl(mutex_);
auto s = get(amendment, sl);
AmendmentState* const s = get(amendment, sl);

if (!s || !s->vetoed)
return false;
Expand All @@ -430,14 +438,14 @@ bool
AmendmentTableImpl::enable(uint256 const& amendment)
{
std::lock_guard sl(mutex_);
auto s = add(amendment, sl);
AmendmentState& s = add(amendment, sl);

if (s->enabled)
if (s.enabled)
return false;

s->enabled = true;
s.enabled = true;

if (!s->supported)
if (!s.supported)
{
JLOG(j_.error()) << "Unsupported amendment " << amendment
<< " activated.";
Expand All @@ -451,15 +459,15 @@ bool
AmendmentTableImpl::isEnabled(uint256 const& amendment) const
{
std::lock_guard sl(mutex_);
auto s = get(amendment, sl);
AmendmentState const* s = get(amendment, sl);
return s && s->enabled;
}

bool
AmendmentTableImpl::isSupported(uint256 const& amendment) const
{
std::lock_guard sl(mutex_);
auto s = get(amendment, sl);
AmendmentState const* s = get(amendment, sl);
return s && s->supported;
}

Expand Down Expand Up @@ -493,6 +501,7 @@ AmendmentTableImpl::doValidation(std::set<uint256> const& enabled) const
(enabled.count(e.first) == 0))
{
amendments.push_back(e.first);
JLOG(j_.info()) << "Voting for amendment " << e.second.name;
}
}
}
Expand Down Expand Up @@ -612,12 +621,12 @@ AmendmentTableImpl::doValidatedLedger(
firstUnsupportedExpected_.reset();
for (auto const& [hash, time] : majority)
{
auto s = add(hash, sl);
AmendmentState& s = add(hash, sl);

if (s->enabled)
if (s.enabled)
continue;

if (!s->supported)
if (!s.supported)
{
JLOG(j_.info()) << "Unsupported amendment " << hash
<< " reached majority at " << to_string(time);
Expand Down Expand Up @@ -683,7 +692,7 @@ AmendmentTableImpl::getJson(uint256 const& amendmentID) const

{
std::lock_guard sl(mutex_);
auto a = get(amendmentID, sl);
AmendmentState const* a = get(amendmentID, sl);
if (a)
injectJson(jAmendment, amendmentID, *a, sl);
}
Expand Down
27 changes: 23 additions & 4 deletions src/ripple/protocol/Feature.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,15 @@
* 2) add a uint256 declaration for the feature to the bottom of this file
* 3) add a uint256 definition for the feature to the corresponding source
* file (Feature.cpp)
* 4) if the feature is going to be supported in the near future, add its
* sha512half value and name (matching exactly the featureName here) to
* the supportedAmendments in Feature.cpp.
* 4) use the uint256 as the parameter to `view.rules.enabled()` to
* control flow into new code that this feature limits.
* 5) if the feature development is COMPLETE, and the feature is ready to be
* SUPPORTED, add its name (matching exactly the featureName here) to the
* supportedAmendments in Feature.cpp.
* 6) if the feature is not ready to be ENABLED, add its name (matching exactly
* the featureName here) to the downVotedAmendments in Feature.cpp.
* In general, any new amendments added to supportedAmendments should also be
* added to downVotedAmendments for at least one full release cycle.
*
*/

Expand Down Expand Up @@ -136,12 +142,22 @@ class FeatureCollections

uint256 const&
bitsetIndexToFeature(size_t i) const;

std::string
featureToName(uint256 const& f) const;
};

/** Amendments that this server supports, but doesn't enable by default */
/** Amendments that this server supports and will vote for by default.
Whether they are enabled depends on the Rules defined in the validated
ledger */
std::vector<std::string> const&
supportedAmendments();

/** Amendments that this server won't vote for by default. Overrides the default
vote behavior of `supportedAmendments()` */
std::vector<std::string> const&
downVotedAmendments();

} // namespace detail

boost::optional<uint256>
Expand All @@ -153,6 +169,9 @@ featureToBitsetIndex(uint256 const& f);
uint256
bitsetIndexToFeature(size_t i);

std::string
featureToName(uint256 const& f);

class FeatureBitset
: private std::bitset<detail::FeatureCollections::numFeatures()>
{
Expand Down
60 changes: 58 additions & 2 deletions src/ripple/protocol/impl/Feature.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,32 @@ detail::FeatureCollections::bitsetIndexToFeature(size_t i) const
return features[i];
}

std::string
detail::FeatureCollections::featureToName(uint256 const& f) const
{
auto const i = featureToIndex.find(f);
assert(featureToIndex.size() == numFeatures());
return i == featureToIndex.end() ? to_string(f) : featureNames[i->second];
}

static detail::FeatureCollections const featureCollections;

/** Amendments that this server supports, but doesn't enable by default */
/** Amendments that this server supports and will vote for by default.
Whether they are enabled depends on the Rules defined in the validated
ledger */
std::vector<std::string> const&
detail::supportedAmendments()
{
// Commented out amendments will be supported in a future release (and
// uncommented at that time).
// uncommented at that time). Including an amendment here indicates
// that development of the feature is complete. Any future behavior
// changes will require another amendment.
//
// In general, any new non-fix amendments added to this list should also be
// added to downVotedAmendments for at least one full release cycle to
// prevent rippled from automatically voting to enable that amendment by
// default for some time. Fix amendments should be carefully considered
// based on the risk and severity of the thing they are fixing.
//
// There are also unconditionally supported amendments in the list.
// Those are amendments that were enabled some time ago and the
Expand Down Expand Up @@ -137,6 +155,38 @@ detail::supportedAmendments()
return supported;
}

/** Amendments that this server won't vote for by default. Overrides the default
vote behavior of `supportedAmendments()` */
std::vector<std::string> const&
detail::downVotedAmendments()
{
// Amendment names included in this list should also be present and
// uncommented in supportedAmendments above. This allows a particular
// version of rippled to be able to understand the amendment if it gets
// enabled, but not vote for the amendment by default. Additionally, some
// tests will fail if an entry in this list is not in supportedAmendments.
//
// Note that this list can be overridden by the "feature" admin RPC command.
//
// For example, if amendment FOO is first complete and released in 2.1, but
// down voted (included in this list) until 2.3 when it is removed from this
// list, then when it is finally enabled by receiving votes from the
// majority of UNL validators some time after 2.3 is released, versions 2.1
// and later will know how to handle the new behavior and will NOT be
// amendment blocked.
//
// Suggested format for entries in the string vector:
// "AmendmentName", // Added in 2.1, planned to enable in 2.3
//
// To enable automatically voting for the amendment, simply remove it from
// this list. There should never be a need to comment entries out aside from
// testing.
static std::vector<std::string> const vetoed{
"CryptoConditionsSuite", // Added in 0.60.0, incomplete, do not enable
};
return vetoed;
}

//------------------------------------------------------------------------------

boost::optional<uint256>
Expand All @@ -157,6 +207,12 @@ bitsetIndexToFeature(size_t i)
return featureCollections.bitsetIndexToFeature(i);
}

std::string
featureToName(uint256 const& f)
{
return featureCollections.featureToName(f);
}

// clang-format off

uint256 const
Expand Down
Loading

0 comments on commit 4f8be72

Please sign in to comment.