diff --git a/src/ripple/app/misc/AmendmentTable.h b/src/ripple/app/misc/AmendmentTable.h index 22e226b664..0ac5585807 100644 --- a/src/ripple/app/misc/AmendmentTable.h +++ b/src/ripple/app/misc/AmendmentTable.h @@ -37,7 +37,7 @@ public: virtual ~AmendmentTable() = default; virtual uint256 - find(std::string const& name) = 0; + find(std::string const& name) const = 0; virtual bool veto(uint256 const& amendment) = 0; @@ -46,13 +46,11 @@ public: virtual bool enable(uint256 const& amendment) = 0; - virtual bool - disable(uint256 const& amendment) = 0; virtual bool - isEnabled(uint256 const& amendment) = 0; + isEnabled(uint256 const& amendment) const = 0; virtual bool - isSupported(uint256 const& amendment) = 0; + isSupported(uint256 const& amendment) const = 0; /** * @brief returns true if one or more amendments on the network @@ -61,16 +59,17 @@ public: * @return true if an unsupported feature is enabled on the network */ virtual bool - hasUnsupportedEnabled() = 0; + hasUnsupportedEnabled() const = 0; + virtual boost::optional - firstUnsupportedExpected() = 0; + firstUnsupportedExpected() const = 0; virtual Json::Value - getJson(int) = 0; + getJson() const = 0; /** Returns a Json::objectValue. */ virtual Json::Value - getJson(uint256 const&) = 0; + getJson(uint256 const& amendment) const = 0; /** Called when a new fully-validated ledger is accepted. */ void @@ -88,7 +87,7 @@ public: a new validated ledger. (If it could have changed things.) */ virtual bool - needValidatedLedger(LedgerIndex seq) = 0; + needValidatedLedger(LedgerIndex seq) const = 0; virtual void doValidatedLedger( @@ -108,14 +107,14 @@ public: // Called by the consensus code when we need to // add feature entries to a validation virtual std::vector - doValidation(std::set const& enabled) = 0; + doValidation(std::set const& enabled) const = 0; // The set of amendments to enable in the genesis ledger // This will return all known, non-vetoed amendments. // If we ever have two amendments that should not both be // enabled at the same time, we should ensure one is vetoed. virtual std::vector - getDesired() = 0; + getDesired() const = 0; // The function below adapts the API callers expect to the // internal amendment table API. This allows the amendment diff --git a/src/ripple/app/misc/README.md b/src/ripple/app/misc/README.md index d664e4e6a2..52e6e14934 100644 --- a/src/ripple/app/misc/README.md +++ b/src/ripple/app/misc/README.md @@ -62,32 +62,32 @@ of the administrator to set these values. # Amendment -An Amendment is a new or proposed change to a ledger rule. Ledger rules affect -transaction processing and consensus; peers must use the same set of rules for -consensus to succeed, otherwise different instances of rippled will get -different results. Amendments can be almost anything but they must be accepted -by a network majority through a consensus process before they are utilized. An -Amendment must receive at least an 80% approval rate from validating nodes for -a period of two weeks before being accepted. The following example outlines the -process of an Amendment from its conception to approval and usage. +An Amendment is a new or proposed change to a ledger rule. Ledger rules affect +transaction processing and consensus; peers must use the same set of rules for +consensus to succeed, otherwise different instances of rippled will get +different results. Amendments can be almost anything but they must be accepted +by a network majority through a consensus process before they are utilized. An +Amendment must receive at least an 80% approval rate from validating nodes for +a period of two weeks before being accepted. The following example outlines the +process of an Amendment from its conception to approval and usage. -* A community member makes proposes to change transaction processing in some - way. The proposal is discussed amongst the community and receives its support - creating a community or human consensus. +* A community member proposes to change transaction processing in some way. + The proposal is discussed amongst the community and receives its support + creating a community or human consensus. * Some members contribute their time and work to develop the Amendment. -* A pull request is created and the new code is folded into a rippled build +* A pull request is created and the new code is folded into a rippled build and made available for use. * The consensus process begins with the validating nodes. -* If the Amendment holds an 80% majority for a two week period, nodes will begin +* If the Amendment holds an 80% majority for a two week period, nodes will begin including the transaction to enable it in their initial sets. -Nodes may veto Amendments they consider undesirable by never announcing their -support for those Amendments. Just a few nodes vetoing an Amendment will normally -keep it from being accepted. Nodes could also vote yes on an Amendments even +Nodes may veto Amendments they consider undesirable by never announcing their +support for those Amendments. Just a few nodes vetoing an Amendment will normally +keep it from being accepted. Nodes could also vote yes on an Amendments even before it obtains a super-majority. This might make sense for a critical bug fix. Validators that support an amendment that is not yet enabled announce their @@ -101,7 +101,7 @@ the majority status from the ledger. If an amendment holds majority status for two weeks, validators will introduce a pseudo-transaction to enable the amendment. -All amednements are assumed to be critical and irreversible. Thus there +All amendments are assumed to be critical and irreversible. Thus there is no mechanism to disable or revoke an amendment, nor is there a way for a server to operate while an amendment it does not understand is enabled. diff --git a/src/ripple/app/misc/impl/AmendmentTable.cpp b/src/ripple/app/misc/impl/AmendmentTable.cpp index 099d656fe8..4d499ea917 100644 --- a/src/ripple/app/misc/impl/AmendmentTable.cpp +++ b/src/ripple/app/misc/impl/AmendmentTable.cpp @@ -139,7 +139,7 @@ public: class AmendmentTableImpl final : public AmendmentTable { protected: - std::mutex mutex_; + mutable std::mutex mutex_; hash_map amendmentMap_; std::uint32_t lastUpdateSeq_; @@ -157,6 +157,7 @@ protected: // True if an unsupported amendment is enabled bool unsupportedEnabled_; + // Unset if no unsupported amendments reach majority, // else set to the earliest time an unsupported amendment // will be enabled. @@ -164,16 +165,24 @@ protected: beast::Journal const j_; - // Finds or creates state + // Finds or creates state. Must be called with mutex_ locked. AmendmentState* - add(uint256 const& amendment); + add(uint256 const& amendment, std::lock_guard const& sl); - // Finds existing state + // Finds existing state. Must be called with mutex_ locked. AmendmentState* - get(uint256 const& amendment); + get(uint256 const& amendment, std::lock_guard const& sl); + AmendmentState const* + get(uint256 const& amendment, std::lock_guard const& sl) const; + + // Injects amendment json into v. Must be called with mutex_ locked. void - setJson(Json::Value& v, uint256 const& amendment, const AmendmentState&); + injectJson( + Json::Value& v, + uint256 const& amendment, + AmendmentState const& state, + std::lock_guard const& sl) const; public: AmendmentTableImpl( @@ -185,7 +194,7 @@ public: beast::Journal journal); uint256 - find(std::string const& name) override; + find(std::string const& name) const override; bool veto(uint256 const& amendment) override; @@ -194,26 +203,25 @@ public: bool enable(uint256 const& amendment) override; - bool - disable(uint256 const& amendment) override; bool - isEnabled(uint256 const& amendment) override; + isEnabled(uint256 const& amendment) const override; bool - isSupported(uint256 const& amendment) override; + isSupported(uint256 const& amendment) const override; bool - hasUnsupportedEnabled() override; + hasUnsupportedEnabled() const override; + boost::optional - firstUnsupportedExpected() override; + firstUnsupportedExpected() const override; Json::Value - getJson(int) override; + getJson() const override; Json::Value - getJson(uint256 const&) override; + getJson(uint256 const&) const override; bool - needValidatedLedger(LedgerIndex seq) override; + needValidatedLedger(LedgerIndex seq) const override; void doValidatedLedger( @@ -222,10 +230,10 @@ public: majorityAmendments_t const& majority) override; std::vector - doValidation(std::set const& enabledAmendments) override; + doValidation(std::set const& enabledAmendments) const override; std::vector - getDesired() override; + getDesired() const override; std::map doVoting( @@ -256,7 +264,7 @@ AmendmentTableImpl::AmendmentTableImpl( for (auto const& a : parseSection(supported)) { - if (auto s = add(a.first)) + if (auto s = add(a.first, sl)) { JLOG(j_.debug()) << "Amendment " << a.first << " is supported."; @@ -269,7 +277,7 @@ AmendmentTableImpl::AmendmentTableImpl( for (auto const& a : parseSection(enabled)) { - if (auto s = add(a.first)) + if (auto s = add(a.first, sl)) { JLOG(j_.debug()) << "Amendment " << a.first << " is enabled."; @@ -284,7 +292,7 @@ AmendmentTableImpl::AmendmentTableImpl( for (auto const& a : parseSection(vetoed)) { // Unknown amendments are effectively vetoed already - if (auto s = get(a.first)) + if (auto s = get(a.first, sl)) { JLOG(j_.info()) << "Amendment " << a.first << " is vetoed."; @@ -297,14 +305,28 @@ AmendmentTableImpl::AmendmentTableImpl( } AmendmentState* -AmendmentTableImpl::add(uint256 const& amendmentHash) +AmendmentTableImpl::add( + uint256 const& amendmentHash, + std::lock_guard const&) { // call with the mutex held return &amendmentMap_[amendmentHash]; } AmendmentState* -AmendmentTableImpl::get(uint256 const& amendmentHash) +AmendmentTableImpl::get( + uint256 const& amendmentHash, + std::lock_guard const& sl) +{ + // Forward to the const version of get. + return const_cast( + std::as_const(*this).get(amendmentHash, sl)); +} + +AmendmentState const* +AmendmentTableImpl::get( + uint256 const& amendmentHash, + std::lock_guard const&) const { // call with the mutex held auto ret = amendmentMap_.find(amendmentHash); @@ -316,7 +338,7 @@ AmendmentTableImpl::get(uint256 const& amendmentHash) } uint256 -AmendmentTableImpl::find(std::string const& name) +AmendmentTableImpl::find(std::string const& name) const { std::lock_guard sl(mutex_); @@ -333,7 +355,7 @@ bool AmendmentTableImpl::veto(uint256 const& amendment) { std::lock_guard sl(mutex_); - auto s = add(amendment); + auto s = add(amendment, sl); if (s->vetoed) return false; @@ -345,7 +367,7 @@ bool AmendmentTableImpl::unVeto(uint256 const& amendment) { std::lock_guard sl(mutex_); - auto s = get(amendment); + auto s = get(amendment, sl); if (!s || !s->vetoed) return false; @@ -357,7 +379,7 @@ bool AmendmentTableImpl::enable(uint256 const& amendment) { std::lock_guard sl(mutex_); - auto s = add(amendment); + auto s = add(amendment, sl); if (s->enabled) return false; @@ -375,58 +397,45 @@ AmendmentTableImpl::enable(uint256 const& amendment) } bool -AmendmentTableImpl::disable(uint256 const& amendment) +AmendmentTableImpl::isEnabled(uint256 const& amendment) const { std::lock_guard sl(mutex_); - auto s = get(amendment); - - if (!s || !s->enabled) - return false; - - s->enabled = false; - return true; -} - -bool -AmendmentTableImpl::isEnabled(uint256 const& amendment) -{ - std::lock_guard sl(mutex_); - auto s = get(amendment); + auto s = get(amendment, sl); return s && s->enabled; } bool -AmendmentTableImpl::isSupported(uint256 const& amendment) +AmendmentTableImpl::isSupported(uint256 const& amendment) const { std::lock_guard sl(mutex_); - auto s = get(amendment); + auto s = get(amendment, sl); return s && s->supported; } bool -AmendmentTableImpl::hasUnsupportedEnabled() +AmendmentTableImpl::hasUnsupportedEnabled() const { std::lock_guard sl(mutex_); return unsupportedEnabled_; } boost::optional -AmendmentTableImpl::firstUnsupportedExpected() +AmendmentTableImpl::firstUnsupportedExpected() const { std::lock_guard sl(mutex_); return firstUnsupportedExpected_; } std::vector -AmendmentTableImpl::doValidation(std::set const& enabled) +AmendmentTableImpl::doValidation(std::set const& enabled) const { // Get the list of amendments we support and do not // veto, but that are not already enabled std::vector amendments; - amendments.reserve(amendmentMap_.size()); { std::lock_guard sl(mutex_); + amendments.reserve(amendmentMap_.size()); for (auto const& e : amendmentMap_) { if (e.second.supported && !e.second.vetoed && @@ -444,7 +453,7 @@ AmendmentTableImpl::doValidation(std::set const& enabled) } std::vector -AmendmentTableImpl::getDesired() +AmendmentTableImpl::getDesired() const { // Get the list of amendments we support and do not veto return doValidation({}); @@ -491,63 +500,58 @@ AmendmentTableImpl::doVoting( // the value of the flags in the pseudo-transaction std::map actions; + std::lock_guard sl(mutex_); + + // process all amendments we know of + for (auto const& entry : amendmentMap_) { - std::lock_guard sl(mutex_); + NetClock::time_point majorityTime = {}; + + bool const hasValMajority = + (vote->votes(entry.first) >= vote->mThreshold); - // process all amendments we know of - for (auto const& entry : amendmentMap_) { - NetClock::time_point majorityTime = {}; - - bool const hasValMajority = - (vote->votes(entry.first) >= vote->mThreshold); - - { - auto const it = majorityAmendments.find(entry.first); - if (it != majorityAmendments.end()) - majorityTime = it->second; - } - - if (enabledAmendments.count(entry.first) != 0) - { - JLOG(j_.debug()) - << entry.first << ": amendment already enabled"; - } - else if ( - hasValMajority && (majorityTime == NetClock::time_point{}) && - !entry.second.vetoed) - { - // Ledger says no majority, validators say yes - JLOG(j_.debug()) << entry.first << ": amendment got majority"; - actions[entry.first] = tfGotMajority; - } - else if ( - !hasValMajority && (majorityTime != NetClock::time_point{})) - { - // Ledger says majority, validators say no - JLOG(j_.debug()) << entry.first << ": amendment lost majority"; - actions[entry.first] = tfLostMajority; - } - else if ( - (majorityTime != NetClock::time_point{}) && - ((majorityTime + majorityTime_) <= closeTime) && - !entry.second.vetoed) - { - // Ledger says majority held - JLOG(j_.debug()) << entry.first << ": amendment majority held"; - actions[entry.first] = 0; - } + auto const it = majorityAmendments.find(entry.first); + if (it != majorityAmendments.end()) + majorityTime = it->second; } - // Stash for reporting - lastVote_ = std::move(vote); + if (enabledAmendments.count(entry.first) != 0) + { + JLOG(j_.debug()) << entry.first << ": amendment already enabled"; + } + else if ( + hasValMajority && (majorityTime == NetClock::time_point{}) && + !entry.second.vetoed) + { + // Ledger says no majority, validators say yes + JLOG(j_.debug()) << entry.first << ": amendment got majority"; + actions[entry.first] = tfGotMajority; + } + else if (!hasValMajority && (majorityTime != NetClock::time_point{})) + { + // Ledger says majority, validators say no + JLOG(j_.debug()) << entry.first << ": amendment lost majority"; + actions[entry.first] = tfLostMajority; + } + else if ( + (majorityTime != NetClock::time_point{}) && + ((majorityTime + majorityTime_) <= closeTime) && + !entry.second.vetoed) + { + // Ledger says majority held + JLOG(j_.debug()) << entry.first << ": amendment majority held"; + actions[entry.first] = 0; + } } + // Stash for reporting + lastVote_ = std::move(vote); return actions; } bool -AmendmentTableImpl::needValidatedLedger(LedgerIndex ledgerSeq) +AmendmentTableImpl::needValidatedLedger(LedgerIndex ledgerSeq) const { std::lock_guard sl(mutex_); @@ -567,13 +571,17 @@ AmendmentTableImpl::doValidatedLedger( enable(e); std::lock_guard sl(mutex_); - // Since we have the whole list in `majority`, reset the time flag, even if - // it's currently set. If it's not set when the loop is done, then any + + // Remember the ledger sequence of this update. + lastUpdateSeq_ = ledgerSeq; + + // Since we have the whole list in `majority`, reset the time flag, even + // if it's currently set. If it's not set when the loop is done, then any // prior unknown amendments have lost majority. firstUnsupportedExpected_.reset(); for (auto const& [hash, time] : majority) { - auto s = add(hash); + auto s = add(hash, sl); if (s->enabled) continue; @@ -591,10 +599,11 @@ AmendmentTableImpl::doValidatedLedger( } void -AmendmentTableImpl::setJson( +AmendmentTableImpl::injectJson( Json::Value& v, const uint256& id, - const AmendmentState& fs) + const AmendmentState& fs, + std::lock_guard const&) const { if (!fs.name.empty()) v[jss::name] = fs.name; @@ -621,30 +630,34 @@ AmendmentTableImpl::setJson( } Json::Value -AmendmentTableImpl::getJson(int) +AmendmentTableImpl::getJson() const { Json::Value ret(Json::objectValue); { std::lock_guard sl(mutex_); for (auto const& e : amendmentMap_) { - setJson( - ret[to_string(e.first)] = Json::objectValue, e.first, e.second); + injectJson( + ret[to_string(e.first)] = Json::objectValue, + e.first, + e.second, + sl); } } return ret; } Json::Value -AmendmentTableImpl::getJson(uint256 const& amendmentID) +AmendmentTableImpl::getJson(uint256 const& amendmentID) const { Json::Value ret = Json::objectValue; Json::Value& jAmendment = (ret[to_string(amendmentID)] = Json::objectValue); { std::lock_guard sl(mutex_); - auto a = add(amendmentID); - setJson(jAmendment, amendmentID, *a); + auto a = get(amendmentID, sl); + if (a) + injectJson(jAmendment, amendmentID, *a, sl); } return ret; diff --git a/src/ripple/rpc/handlers/Feature1.cpp b/src/ripple/rpc/handlers/Feature1.cpp index caf41828a7..47a284f309 100644 --- a/src/ripple/rpc/handlers/Feature1.cpp +++ b/src/ripple/rpc/handlers/Feature1.cpp @@ -45,7 +45,7 @@ doFeature(RPC::JsonContext& context) if (!context.params.isMember(jss::feature)) { - auto features = table.getJson(0); + auto features = table.getJson(); for (auto const& [h, t] : majorities) { @@ -67,9 +67,9 @@ doFeature(RPC::JsonContext& context) if (context.params.isMember(jss::vetoed)) { if (context.params[jss::vetoed].asBool()) - context.app.getAmendmentTable().veto(feature); + table.veto(feature); else - context.app.getAmendmentTable().unVeto(feature); + table.unVeto(feature); } Json::Value jvReply = table.getJson(feature); diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index 01e08244c1..ec9293281b 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -29,6 +29,8 @@ #include #include #include +#include +#include #include namespace ripple { @@ -51,16 +53,6 @@ private: return result; } - static std::vector - createSet(int group, int count) - { - std::vector amendments; - for (int i = 0; i < count; i++) - amendments.push_back( - "Amendment" + std::to_string((1000000 * group) + i)); - return amendments; - } - static Section makeSection(std::vector const& amendments) { @@ -78,43 +70,52 @@ private: return section; } - std::vector const m_set1; - std::vector const m_set2; - std::vector const m_set3; - std::vector const m_set4; - std::vector const m_set5; + // 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 supported_{ + "a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", + "l", "m", "n", "o", "p", "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 unsupported_{"v", "w", "x"}; + std::vector const unsupportedMajority_{"y", "z"}; Section const emptySection; test::SuiteJournal journal; public: - AmendmentTable_test() - : m_set1(createSet(1, 12)) - , m_set2(createSet(2, 12)) - , m_set3(createSet(3, 12)) - , m_set4(createSet(4, 12)) - , m_set5(createSet(5, 12)) - , journal("AmendmentTable_test", *this) + AmendmentTable_test() : journal("AmendmentTable_test", *this) { } std::unique_ptr makeTable( - int w, + std::chrono::seconds majorityTime, Section const supported, Section const enabled, Section const vetoed) { return make_AmendmentTable( - weeks(w), majorityFraction, supported, enabled, vetoed, journal); + majorityTime, + majorityFraction, + supported, + enabled, + vetoed, + journal); } std::unique_ptr - makeTable(int w) + makeTable(std::chrono::seconds majorityTime) { return makeTable( - w, makeSection(m_set1), makeSection(m_set2), makeSection(m_set3)); + majorityTime, + makeSection(supported_), + makeSection(enabled_), + makeSection(vetoed_)); } void @@ -122,23 +123,22 @@ public: { testcase("Construction"); - auto table = makeTable(1); + auto table = makeTable(weeks(1)); - for (auto const& a : m_set1) + for (auto const& a : supported_) { BEAST_EXPECT(table->isSupported(amendmentId(a))); - BEAST_EXPECT(!table->isEnabled(amendmentId(a))); } - for (auto const& a : m_set2) + for (auto const& a : enabled_) { BEAST_EXPECT(table->isSupported(amendmentId(a))); BEAST_EXPECT(table->isEnabled(amendmentId(a))); } - for (auto const& a : m_set3) + for (auto const& a : vetoed_) { - BEAST_EXPECT(!table->isSupported(amendmentId(a))); + BEAST_EXPECT(table->isSupported(amendmentId(a))); BEAST_EXPECT(!table->isEnabled(amendmentId(a))); } } @@ -148,26 +148,43 @@ public: { testcase("Name to ID mapping"); - auto table = makeTable(1); + auto table = makeTable(weeks(1)); - for (auto const& a : m_set1) + for (auto const& a : supported_) BEAST_EXPECT(table->find(a) == amendmentId(a)); - for (auto const& a : m_set2) + for (auto const& a : enabled_) BEAST_EXPECT(table->find(a) == amendmentId(a)); - for (auto const& a : m_set3) + for (auto const& a : vetoed_) + BEAST_EXPECT(table->find(a) == amendmentId(a)); + for (auto const& a : unsupported_) BEAST_EXPECT(!table->find(a)); - for (auto const& a : m_set4) - BEAST_EXPECT(!table->find(a)); - for (auto const& a : m_set5) + for (auto const& a : unsupportedMajority_) BEAST_EXPECT(!table->find(a)); + + // Vetoing an unsupported amendment should add the amendment to table. + // Verify that unsupportedID is not in table. + uint256 const unsupportedID = amendmentId(unsupported_[0]); + { + Json::Value const unsupp = + table->getJson(unsupportedID)[to_string(unsupportedID)]; + BEAST_EXPECT(unsupp.size() == 0); + } + + // After vetoing unsupportedID verify that it is in table. + table->veto(unsupportedID); + { + Json::Value const unsupp = + table->getJson(unsupportedID)[to_string(unsupportedID)]; + BEAST_EXPECT(unsupp[jss::vetoed].asBool()); + } } void testBadConfig() { - auto const section = makeSection(m_set1); - auto const id = to_string(amendmentId(m_set2[0])); + auto const section = makeSection(supported_); + auto const id = to_string(amendmentId(enabled_[0])); testcase("Bad Config"); @@ -177,7 +194,7 @@ public: try { - if (makeTable(2, test, emptySection, emptySection)) + if (makeTable(weeks(2), test, emptySection, emptySection)) fail("Accepted only amendment ID"); } catch (...) @@ -192,7 +209,7 @@ public: try { - if (makeTable(2, test, emptySection, emptySection)) + if (makeTable(weeks(2), test, emptySection, emptySection)) fail("Accepted extra arguments"); } catch (...) @@ -210,7 +227,7 @@ public: try { - if (makeTable(2, test, emptySection, emptySection)) + if (makeTable(weeks(2), test, emptySection, emptySection)) fail("Accepted short amendment ID"); } catch (...) @@ -228,7 +245,7 @@ public: try { - if (makeTable(2, test, emptySection, emptySection)) + if (makeTable(weeks(2), test, emptySection, emptySection)) fail("Accepted long amendment ID"); } catch (...) @@ -247,7 +264,7 @@ public: try { - if (makeTable(2, test, emptySection, emptySection)) + if (makeTable(weeks(2), test, emptySection, emptySection)) fail("Accepted non-hex amendment ID"); } catch (...) @@ -257,77 +274,82 @@ public: } } - std::map - getState(AmendmentTable* table, std::set const& exclude) - { - std::map state; - - auto track = [&state, table](std::vector const& v) { - for (auto const& a : v) - { - auto const id = amendmentId(a); - state[id] = table->isEnabled(id); - } - }; - - track(m_set1); - track(m_set2); - track(m_set3); - track(m_set4); - track(m_set5); - - for (auto const& a : exclude) - state.erase(a); - - return state; - } - void - testEnableDisable() + testEnableVeto() { - testcase("enable & disable"); + testcase("enable and veto"); - auto const testAmendment = amendmentId("TestAmendment"); - auto table = makeTable(2); + std::unique_ptr table = makeTable(weeks(2)); - // Subset of amendments to enable - std::set enabled; - enabled.insert(testAmendment); - enabled.insert(amendmentId(m_set1[0])); - enabled.insert(amendmentId(m_set2[0])); - enabled.insert(amendmentId(m_set3[0])); - enabled.insert(amendmentId(m_set4[0])); - enabled.insert(amendmentId(m_set5[0])); + // Note which entries are pre-enabled. + std::set allEnabled; + for (std::string const& a : enabled_) + allEnabled.insert(amendmentId(a)); - // Get the state before, excluding the items we'll change: - auto const pre_state = getState(table.get(), enabled); + // Subset of amendments to late-enable + std::set lateEnabled; + lateEnabled.insert(amendmentId(supported_[0])); + lateEnabled.insert(amendmentId(enabled_[0])); + lateEnabled.insert(amendmentId(vetoed_[0])); - // Enable the subset and verify - for (auto const& a : enabled) + // Do the late enabling. + for (uint256 const& a : lateEnabled) table->enable(a); - for (auto const& a : enabled) - BEAST_EXPECT(table->isEnabled(a)); + // So far all enabled amendments are supported. + BEAST_EXPECT(!table->hasUnsupportedEnabled()); - // Disable the subset and verify - for (auto const& a : enabled) - table->disable(a); + // Verify all pre- and late-enables are enabled and nothing else. + allEnabled.insert(lateEnabled.begin(), lateEnabled.end()); + for (std::string const& a : supported_) + { + uint256 const supportedID = amendmentId(a); + BEAST_EXPECT( + table->isEnabled(supportedID) == + (allEnabled.find(supportedID) != allEnabled.end())); + } - for (auto const& a : enabled) - BEAST_EXPECT(!table->isEnabled(a)); + // All supported and unVetoed amendments should be returned as desired. + { + std::set vetoed; + for (std::string const& a : vetoed_) + vetoed.insert(amendmentId(a)); - // Get the state after, excluding the items we changed: - auto const post_state = getState(table.get(), enabled); + std::vector const desired = table->getDesired(); + for (uint256 const& a : desired) + BEAST_EXPECT(vetoed.count(a) == 0); - // Ensure the states are identical - auto ret = std::mismatch( - pre_state.begin(), - pre_state.end(), - post_state.begin(), - post_state.end()); + // Unveto an amendment that is already not vetoed. Shouldn't + // hurt anything, but the values returned by getDesired() + // shouldn't change. + table->unVeto(amendmentId(supported_[1])); + BEAST_EXPECT(desired == table->getDesired()); + } - BEAST_EXPECT(ret.first == pre_state.end()); - BEAST_EXPECT(ret.second == post_state.end()); + // UnVeto one of the vetoed amendments. It should now be desired. + { + uint256 const unvetoedID = amendmentId(vetoed_[0]); + table->unVeto(unvetoedID); + + std::vector const desired = table->getDesired(); + BEAST_EXPECT( + std::find(desired.begin(), desired.end(), unvetoedID) != + desired.end()); + } + + // Veto all supported amendments. Now desired should be empty. + for (std::string const& a : supported_) + { + table->veto(amendmentId(a)); + } + BEAST_EXPECT(table->getDesired().empty()); + + // Enable an unsupported amendment. + { + BEAST_EXPECT(!table->hasUnsupportedEnabled()); + table->enable(amendmentId(unsupported_[0])); + BEAST_EXPECT(table->hasUnsupportedEnabled()); + } } std::vector> @@ -456,7 +478,8 @@ public: auto const testAmendment = amendmentId("TestAmendment"); auto const validators = makeValidators(10); - auto table = makeTable(2, emptySection, emptySection, emptySection); + auto table = + makeTable(weeks(2), emptySection, emptySection, emptySection); std::vector> votes; std::vector ourVotes; @@ -495,7 +518,7 @@ public: auto const testAmendment = amendmentId("vetoedAmendment"); auto table = makeTable( - 2, emptySection, emptySection, makeSection(testAmendment)); + weeks(2), emptySection, emptySection, makeSection(testAmendment)); auto const validators = makeValidators(10); @@ -531,8 +554,8 @@ public: { testcase("voteEnable"); - auto table = - makeTable(2, makeSection(m_set1), emptySection, emptySection); + auto table = makeTable( + weeks(2), makeSection(supported_), emptySection, emptySection); auto const validators = makeValidators(10); std::vector> votes; @@ -543,35 +566,35 @@ public: // Week 1: We should vote for all known amendments not enabled doRound( *table, weeks{1}, validators, votes, ourVotes, enabled, majority); - BEAST_EXPECT(ourVotes.size() == m_set1.size()); + BEAST_EXPECT(ourVotes.size() == supported_.size()); BEAST_EXPECT(enabled.empty()); - for (auto const& i : m_set1) + for (auto const& i : supported_) BEAST_EXPECT(majority.find(amendmentId(i)) == majority.end()); // Now, everyone votes for this feature - for (auto const& i : m_set1) + for (auto const& i : supported_) votes.emplace_back(amendmentId(i), 256); // Week 2: We should recognize a majority doRound( *table, weeks{2}, validators, votes, ourVotes, enabled, majority); - BEAST_EXPECT(ourVotes.size() == m_set1.size()); + BEAST_EXPECT(ourVotes.size() == supported_.size()); BEAST_EXPECT(enabled.empty()); - for (auto const& i : m_set1) + for (auto const& i : supported_) BEAST_EXPECT(majority[amendmentId(i)] == weekTime(weeks{2})); // Week 5: We should enable the amendment doRound( *table, weeks{5}, validators, votes, ourVotes, enabled, majority); - BEAST_EXPECT(enabled.size() == m_set1.size()); + BEAST_EXPECT(enabled.size() == supported_.size()); // Week 6: We should remove it from our votes and from having a majority doRound( *table, weeks{6}, validators, votes, ourVotes, enabled, majority); - BEAST_EXPECT(enabled.size() == m_set1.size()); + BEAST_EXPECT(enabled.size() == supported_.size()); BEAST_EXPECT(ourVotes.empty()); - for (auto const& i : m_set1) + for (auto const& i : supported_) BEAST_EXPECT(majority.find(amendmentId(i)) == majority.end()); } @@ -583,7 +606,7 @@ public: auto const testAmendment = amendmentId("detectMajority"); auto table = makeTable( - 2, makeSection(testAmendment), emptySection, emptySection); + weeks(2), makeSection(testAmendment), emptySection, emptySection); auto const validators = makeValidators(16); @@ -648,7 +671,7 @@ public: auto const validators = makeValidators(16); auto table = makeTable( - 8, makeSection(testAmendment), emptySection, emptySection); + weeks(8), makeSection(testAmendment), emptySection, emptySection); std::set enabled; majorityAmendments_t majority; @@ -712,32 +735,44 @@ public: { testcase("hasUnsupportedEnabled"); - using namespace std::chrono_literals; + test::jtx::Env env(*this); // Used only for its Rules + env.close(); - int constexpr w = 1; + using namespace std::chrono_literals; + weeks constexpr w(1); auto table = makeTable(w); BEAST_EXPECT(!table->hasUnsupportedEnabled()); BEAST_EXPECT(!table->firstUnsupportedExpected()); + BEAST_EXPECT(table->needValidatedLedger(1)); std::set enabled; + std::for_each( + unsupported_.begin(), + unsupported_.end(), + [&enabled](auto const& s) { enabled.insert(amendmentId(s)); }); + majorityAmendments_t majority; - std::for_each(m_set4.begin(), m_set4.end(), [&enabled](auto const& s) { - enabled.insert(amendmentId(s)); - }); table->doValidatedLedger(1, enabled, majority); BEAST_EXPECT(table->hasUnsupportedEnabled()); BEAST_EXPECT(!table->firstUnsupportedExpected()); + NetClock::duration t{1000s}; std::for_each( - m_set5.begin(), m_set5.end(), [&majority, &t](auto const& s) { + unsupportedMajority_.begin(), + unsupportedMajority_.end(), + [&majority, &t](auto const& s) { majority[amendmentId(s)] = NetClock::time_point{--t}; }); + table->doValidatedLedger(1, enabled, majority); BEAST_EXPECT(table->hasUnsupportedEnabled()); BEAST_EXPECT( table->firstUnsupportedExpected() && - *table->firstUnsupportedExpected() == - NetClock::time_point{t} + weeks{w}); + *table->firstUnsupportedExpected() == NetClock::time_point{t} + w); + + // Make sure the table knows when it needs an update. + BEAST_EXPECT(!table->needValidatedLedger(256)); + BEAST_EXPECT(table->needValidatedLedger(257)); } void @@ -746,7 +781,7 @@ public: testConstruct(); testGet(); testBadConfig(); - testEnableDisable(); + testEnableVeto(); testNoOnUnknown(); testNoOnVetoed(); testVoteEnable();