diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 5b4531af49..c03c84b601 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -1360,6 +1360,7 @@ ApplicationImp::setup() Section enabledAmendments = config_->section(SECTION_AMENDMENTS); m_amendmentTable = make_AmendmentTable( + *this, config().AMENDMENT_MAJORITY_TIME, supportedAmendments, enabledAmendments, diff --git a/src/ripple/app/misc/AmendmentTable.h b/src/ripple/app/misc/AmendmentTable.h index e0884fecbb..c811b6003a 100644 --- a/src/ripple/app/misc/AmendmentTable.h +++ b/src/ripple/app/misc/AmendmentTable.h @@ -164,6 +164,7 @@ public: std::unique_ptr make_AmendmentTable( + Application& app, std::chrono::seconds majorityTime, Section const& supported, Section const& enabled, diff --git a/src/ripple/app/misc/impl/AmendmentTable.cpp b/src/ripple/app/misc/impl/AmendmentTable.cpp index 68ad92b204..ffb259729a 100644 --- a/src/ripple/app/misc/impl/AmendmentTable.cpp +++ b/src/ripple/app/misc/impl/AmendmentTable.cpp @@ -220,6 +220,9 @@ private: beast::Journal const j_; + // Database which persists veto/unveto vote + DatabaseCon& db_; + // Finds or creates state. Must be called with mutex_ locked. AmendmentState* add(uint256 const& amendment, std::lock_guard const& sl); @@ -239,8 +242,15 @@ private: AmendmentState const& state, std::lock_guard const& sl) const; + void + persistVote( + uint256 const& amendment, + std::string const& name, + bool vote_to_veto) const; + public: AmendmentTableImpl( + Application& app, std::chrono::seconds majorityTime, Section const& supported, Section const& enabled, @@ -301,6 +311,7 @@ public: //------------------------------------------------------------------------------ AmendmentTableImpl::AmendmentTableImpl( + Application& app, std::chrono::seconds majorityTime, Section const& supported, Section const& enabled, @@ -310,9 +321,34 @@ AmendmentTableImpl::AmendmentTableImpl( , majorityTime_(majorityTime) , unsupportedEnabled_(false) , j_(journal) + , db_(app.getWalletDB()) { std::lock_guard sl(mutex_); + // Find out if the FeatureVotes table exists in WalletDB + bool const featureVotesExist = [this]() { + auto db = db_.checkoutDb(); + soci::transaction tr(*db); + std::string sql = + "SELECT count(*) FROM sqlite_master " + "WHERE type='table' AND name='FeatureVotes'"; + boost::optional featureVotesCount; + *db << sql, soci::into(featureVotesCount); + bool exists = static_cast(*featureVotesCount); + + // Create FeatureVotes table in WalletDB if it doesn't exist + if (!exists) + { + *db << "CREATE TABLE FeatureVotes ( " + "AmendmentHash CHARACTER(64) NOT NULL, " + "AmendmentName TEXT, " + "Veto INTEGER NOT NULL );"; + tr.commit(); + } + return exists; + }(); + + // Parse supported amendments for (auto const& a : parseSection(supported)) { if (auto s = add(a.first, sl)) @@ -326,31 +362,93 @@ AmendmentTableImpl::AmendmentTableImpl( } } + hash_set detect_conflict; + // Parse enabled amendments from config for (auto const& a : parseSection(enabled)) { - if (auto s = add(a.first, sl)) - { - JLOG(j_.debug()) << "Amendment " << a.first << " is enabled."; - - if (!a.second.empty()) - s->name = a.second; - - s->supported = true; - s->enabled = true; + if (featureVotesExist) + { // If the table existed, warn about duplicate config info + JLOG(j_.warn()) << "[amendments] section in config file ignored" + " in favor of data in db/wallet.db."; + break; + } + else + { // Otherwise transfer config data into the table + detect_conflict.insert(a.first); + persistVote(a.first, a.second, false); // un-veto } } + // Parse vetoed amendments from config for (auto const& a : parseSection(vetoed)) { - // Unknown amendments are effectively vetoed already - if (auto s = get(a.first, sl)) + if (featureVotesExist) + { // If the table existed, warn about duplicate config info + JLOG(j_.warn()) + << "[veto_amendments] section in config file ignored" + " in favor of data in db/wallet.db."; + break; + } + else + { // Otherwise transfer config data into the table + if (detect_conflict.count(a.first) == 0) + { + persistVote(a.first, a.second, true); // veto + } + else + { + JLOG(j_.warn()) + << "[veto_amendments] section in config has amendment " + << '(' << a.first << ", " << a.second + << ") both [veto_amendments] and [amendments]."; + } + } + } + + // Read amendment votes from wallet.db + auto db = db_.checkoutDb(); + soci::transaction tr(*db); + std::string sql = + "SELECT AmendmentHash, AmendmentName, Veto FROM FeatureVotes"; + boost::optional amendment_hash; + boost::optional amendment_name; + boost::optional vote_to_veto; + soci::statement st = + (db->prepare << sql, + soci::into(amendment_hash), + soci::into(amendment_name), + soci::into(vote_to_veto)); + st.execute(); + while (st.fetch()) + { + uint256 amend_hash; + if (!amend_hash.parseHex(*amendment_hash)) { - JLOG(j_.info()) << "Amendment " << a.first << " is vetoed."; - - if (!a.second.empty()) - s->name = a.second; - - s->vetoed = true; + Throw( + "Invalid amendment ID '" + *amendment_hash + " in wallet.db"); + } + if (*vote_to_veto) + { + // Unknown amendments are effectively vetoed already + if (auto s = get(amend_hash, sl)) + { + JLOG(j_.info()) << "Amendment {" << *amendment_name << ", " + << amend_hash << "} is vetoed."; + if (!amendment_name->empty()) + s->name = *amendment_name; + s->vetoed = true; + } + } + else // un-veto + { + if (auto s = add(amend_hash, sl)) + { + JLOG(j_.debug()) << "Amendment {" << *amendment_name << ", " + << amend_hash << "} is un-vetoed."; + if (!amendment_name->empty()) + s->name = *amendment_name; + s->vetoed = false; + } } } } @@ -402,6 +500,24 @@ AmendmentTableImpl::find(std::string const& name) const return {}; } +void +AmendmentTableImpl::persistVote( + uint256 const& amendment, + std::string const& name, + bool vote_to_veto) const +{ + auto db = db_.checkoutDb(); + soci::transaction tr(*db); + std::string sql = + "INSERT INTO FeatureVotes (AmendmentHash, AmendmentName, Veto) VALUES " + "('"; + sql += to_string(amendment); + sql += "', '" + name; + sql += "', '" + std::to_string(int{vote_to_veto}) + "');"; + *db << sql; + tr.commit(); +} + bool AmendmentTableImpl::veto(uint256 const& amendment) { @@ -411,6 +527,7 @@ AmendmentTableImpl::veto(uint256 const& amendment) if (s->vetoed) return false; s->vetoed = true; + persistVote(amendment, s->name, s->vetoed); return true; } @@ -423,6 +540,7 @@ AmendmentTableImpl::unVeto(uint256 const& amendment) if (!s || !s->vetoed) return false; s->vetoed = false; + persistVote(amendment, s->name, s->vetoed); return true; } @@ -693,6 +811,7 @@ AmendmentTableImpl::getJson(uint256 const& amendmentID) const std::unique_ptr make_AmendmentTable( + Application& app, std::chrono::seconds majorityTime, Section const& supported, Section const& enabled, @@ -700,7 +819,7 @@ make_AmendmentTable( beast::Journal journal) { return std::make_unique( - majorityTime, supported, enabled, vetoed, journal); + app, majorityTime, supported, enabled, vetoed, journal); } } // namespace ripple diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index 3b4ec47d14..1f626c0a30 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -51,14 +51,22 @@ private: } static Section - makeSection(std::vector const& amendments) + makeSection( + std::string const& name, + std::vector const& amendments) { - Section section("Test"); + Section section(name); for (auto const& a : amendments) section.append(to_string(amendmentId(a)) + " " + a); return section; } + static Section + makeSection(std::vector const& amendments) + { + return makeSection("Test", amendments); + } + static Section makeSection(uint256 const& amendment) { @@ -67,6 +75,17 @@ private: return section; } + std::unique_ptr + makeConfig() + { + auto cfg = test::jtx::envconfig(); + cfg->section(SECTION_AMENDMENTS) = + makeSection(SECTION_AMENDMENTS, enabled_); + cfg->section(SECTION_VETO_AMENDMENTS) = + makeSection(SECTION_VETO_AMENDMENTS, vetoed_); + return cfg; + } + // All useful amendments are supported amendments. // Enabled amendments are typically a subset of supported amendments. // Vetoed amendments should be supported but not enabled. @@ -91,19 +110,32 @@ public: std::unique_ptr makeTable( + Application& app, std::chrono::seconds majorityTime, - Section const supported, - Section const enabled, - Section const vetoed) + Section const& supported, + Section const& enabled, + Section const& vetoed) { return make_AmendmentTable( - majorityTime, supported, enabled, vetoed, journal); + app, majorityTime, supported, enabled, vetoed, journal); } std::unique_ptr - makeTable(std::chrono::seconds majorityTime) + makeTable( + test::jtx::Env& env, + std::chrono::seconds majorityTime, + Section const& supported, + Section const& enabled, + Section const& vetoed) + { + return makeTable(env.app(), majorityTime, supported, enabled, vetoed); + } + + std::unique_ptr + makeTable(test::jtx::Env& env, std::chrono::seconds majorityTime) { return makeTable( + env.app(), majorityTime, makeSection(supported_), makeSection(enabled_), @@ -114,8 +146,8 @@ public: testConstruct() { testcase("Construction"); - - auto table = makeTable(weeks(1)); + test::jtx::Env env{*this, makeConfig()}; + auto table = makeTable(env, weeks(1)); for (auto const& a : supported_) { @@ -125,7 +157,6 @@ public: for (auto const& a : enabled_) { BEAST_EXPECT(table->isSupported(amendmentId(a))); - BEAST_EXPECT(table->isEnabled(amendmentId(a))); } for (auto const& a : vetoed_) @@ -140,7 +171,8 @@ public: { testcase("Name to ID mapping"); - auto table = makeTable(weeks(1)); + test::jtx::Env env{*this, makeConfig()}; + auto table = makeTable(env, weeks(1)); for (auto const& a : supported_) BEAST_EXPECT(table->find(a) == amendmentId(a)); @@ -186,7 +218,8 @@ public: try { - if (makeTable(weeks(2), test, emptySection, emptySection)) + test::jtx::Env env{*this, makeConfig()}; + if (makeTable(env, weeks(2), test, emptySection, emptySection)) fail("Accepted only amendment ID"); } catch (...) @@ -201,7 +234,8 @@ public: try { - if (makeTable(weeks(2), test, emptySection, emptySection)) + test::jtx::Env env{*this, makeConfig()}; + if (makeTable(env, weeks(2), test, emptySection, emptySection)) fail("Accepted extra arguments"); } catch (...) @@ -219,7 +253,8 @@ public: try { - if (makeTable(weeks(2), test, emptySection, emptySection)) + test::jtx::Env env{*this, makeConfig()}; + if (makeTable(env, weeks(2), test, emptySection, emptySection)) fail("Accepted short amendment ID"); } catch (...) @@ -237,7 +272,8 @@ public: try { - if (makeTable(weeks(2), test, emptySection, emptySection)) + test::jtx::Env env{*this, makeConfig()}; + if (makeTable(env, weeks(2), test, emptySection, emptySection)) fail("Accepted long amendment ID"); } catch (...) @@ -256,7 +292,8 @@ public: try { - if (makeTable(weeks(2), test, emptySection, emptySection)) + test::jtx::Env env{*this, makeConfig()}; + if (makeTable(env, weeks(2), test, emptySection, emptySection)) fail("Accepted non-hex amendment ID"); } catch (...) @@ -271,28 +308,24 @@ public: { testcase("enable and veto"); - std::unique_ptr table = makeTable(weeks(2)); + test::jtx::Env env{*this, makeConfig()}; + std::unique_ptr table = makeTable(env, weeks(2)); - // Note which entries are pre-enabled. + // Note which entries are enabled. std::set allEnabled; - for (std::string const& a : enabled_) - allEnabled.insert(amendmentId(a)); - // Subset of amendments to late-enable - std::set lateEnabled; - lateEnabled.insert(amendmentId(supported_[0])); - lateEnabled.insert(amendmentId(enabled_[0])); - lateEnabled.insert(amendmentId(vetoed_[0])); + // Subset of amendments to enable + allEnabled.insert(amendmentId(supported_[0])); + allEnabled.insert(amendmentId(enabled_[0])); + allEnabled.insert(amendmentId(vetoed_[0])); - // Do the late enabling. - for (uint256 const& a : lateEnabled) + for (uint256 const& a : allEnabled) table->enable(a); // So far all enabled amendments are supported. BEAST_EXPECT(!table->hasUnsupportedEnabled()); - // Verify all pre- and late-enables are enabled and nothing else. - allEnabled.insert(lateEnabled.begin(), lateEnabled.end()); + // Verify all enables are enabled and nothing else. for (std::string const& a : supported_) { uint256 const supportedID = amendmentId(a); @@ -470,8 +503,9 @@ public: auto const testAmendment = amendmentId("TestAmendment"); auto const validators = makeValidators(10); + test::jtx::Env env{*this}; auto table = - makeTable(weeks(2), emptySection, emptySection, emptySection); + makeTable(env, weeks(2), emptySection, emptySection, emptySection); std::vector> votes; std::vector ourVotes; @@ -530,8 +564,13 @@ public: auto const testAmendment = amendmentId("vetoedAmendment"); + test::jtx::Env env{*this}; auto table = makeTable( - weeks(2), emptySection, emptySection, makeSection(testAmendment)); + env, + weeks(2), + emptySection, + emptySection, + makeSection(testAmendment)); auto const validators = makeValidators(10); @@ -588,8 +627,9 @@ public: { testcase("voteEnable"); + test::jtx::Env env{*this}; auto table = makeTable( - weeks(2), makeSection(supported_), emptySection, emptySection); + env, weeks(2), makeSection(supported_), emptySection, emptySection); auto const validators = makeValidators(10); std::vector> votes; @@ -667,8 +707,13 @@ public: testcase("detectMajority"); auto const testAmendment = amendmentId("detectMajority"); + test::jtx::Env env{*this}; auto table = makeTable( - weeks(2), makeSection(testAmendment), emptySection, emptySection); + env, + weeks(2), + makeSection(testAmendment), + emptySection, + emptySection); auto const validators = makeValidators(16); @@ -733,8 +778,13 @@ public: auto const testAmendment = amendmentId("lostMajority"); auto const validators = makeValidators(16); + test::jtx::Env env{*this}; auto table = makeTable( - weeks(8), makeSection(testAmendment), emptySection, emptySection); + env, + weeks(8), + makeSection(testAmendment), + emptySection, + emptySection); std::set enabled; majorityAmendments_t majority; @@ -800,12 +850,10 @@ public: { testcase("hasUnsupportedEnabled"); - test::jtx::Env env(*this); // Used only for its Rules - env.close(); - using namespace std::chrono_literals; weeks constexpr w(1); - auto table = makeTable(w); + test::jtx::Env env{*this, makeConfig()}; + auto table = makeTable(env, w); BEAST_EXPECT(!table->hasUnsupportedEnabled()); BEAST_EXPECT(!table->firstUnsupportedExpected()); BEAST_EXPECT(table->needValidatedLedger(1));