From d90a0647d66ee97563e965f50569002db1bf91d6 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Mon, 7 Aug 2017 03:00:22 -0700 Subject: [PATCH] Change UNL and quorum rules: * Use fixed size UNL if the total listed validators are below threshold. * Set quorum to provide Byzantine fault tolerance until a threshold of total validators is exceeded, at which time quorum is 80%. * Ensure that a quorum of 0 cannot be configured. --- src/ripple/app/main/Main.cpp | 18 +++++++---- src/ripple/app/misc/ValidatorList.h | 37 +++++++++++++++------- src/ripple/app/misc/impl/ValidatorList.cpp | 2 +- src/ripple/core/Config.h | 2 +- src/test/app/ValidatorList_test.cpp | 27 +++++++++------- 5 files changed, 54 insertions(+), 32 deletions(-) diff --git a/src/ripple/app/main/Main.cpp b/src/ripple/app/main/Main.cpp index 787fb1ba8..78b177991 100644 --- a/src/ripple/app/main/Main.cpp +++ b/src/ripple/app/main/Main.cpp @@ -49,6 +49,7 @@ #include #include #include +#include #if defined(BEAST_LINUX) || defined(BEAST_MAC) || defined(BEAST_BSD) @@ -379,12 +380,11 @@ int run (int argc, char** argv) vm["rpc_port"].as()); if (*config->rpc_port == 0) - Throw (""); + throw std::domain_error("0"); } - catch(std::exception const&) + catch(std::exception const& e) { - std::cerr << "Invalid rpc_port = " << - vm["rpc_port"].as() << std::endl; + std::cerr << "Invalid rpc_port = " << e.what() << "\n"; return -1; } } @@ -394,11 +394,15 @@ int run (int argc, char** argv) try { config->VALIDATION_QUORUM = vm["quorum"].as (); + if (config->VALIDATION_QUORUM == std::size_t{}) + { + throw std::domain_error("0"); + } } - catch(std::exception const&) + catch(std::exception const& e) { - std::cerr << "Invalid quorum = " << - vm["quorum"].as () << std::endl; + std::cerr << "Invalid value specified for --quorum (" + << e.what() << ")\n"; return -1; } } diff --git a/src/ripple/app/misc/ValidatorList.h b/src/ripple/app/misc/ValidatorList.h index 025d405be..0f1a20685 100644 --- a/src/ripple/app/misc/ValidatorList.h +++ b/src/ripple/app/misc/ValidatorList.h @@ -125,6 +125,15 @@ class ValidatorList PublicKey localPubKey_; + // The minimum number of listed validators required to allow removing + // non-communicative validators from the trusted set. In other words, if the + // number of listed validators is less, then use all of them in the + // trusted set. + std::size_t const MINIMUM_RESIZEABLE_UNL {25}; + // The maximum size of a trusted set for which greater than Byzantine fault + // tolerance isn't needed. + std::size_t const BYZANTINE_THRESHOLD {32}; + public: ValidatorList ( ManifestCache& validatorManifests, @@ -395,10 +404,13 @@ ValidatorList::onConsensusStart ( std::pair( std::numeric_limits::max(), localPubKey_)); } - // If no validations are being received, use all validators. - // Otherwise, do not use validators whose validations aren't being received - else if (seenValidators.empty() || - seenValidators.find (val->first) != seenValidators.end ()) + // If the total number of validators is too small, or + // no validations are being received, use all validators. + // Otherwise, do not use validators whose validations aren't + // being received. + else if (keyListings_.size() < MINIMUM_RESIZEABLE_UNL || + seenValidators.empty() || + seenValidators.find (val->first) != seenValidators.end ()) { rankedKeys.insert ( std::pair(val->second, val->first)); @@ -416,11 +428,12 @@ ValidatorList::onConsensusStart ( auto size = rankedKeys.size(); - // Do not require 80% quorum for less than 10 trusted validators - if (rankedKeys.size() >= 10) + // Require 80% quorum if there are lots of validators. + if (rankedKeys.size() > BYZANTINE_THRESHOLD) { // Use all eligible keys if there is only one trusted list - if (publisherLists_.size() == 1) + if (publisherLists_.size() == 1 || + keyListings_.size() < MINIMUM_RESIZEABLE_UNL) { // Try to raise the quorum to at least 80% of the trusted set quorum = std::max(quorum, size - size / 5); @@ -433,13 +446,13 @@ ValidatorList::onConsensusStart ( } } - if (minimumQuorum_ && (seenValidators.empty() || - rankedKeys.size() < quorum)) + if (minimumQuorum_ && seenValidators.size() < quorum) { quorum = *minimumQuorum_; - JLOG (j_.warn()) << - "Using unsafe quorum of " << quorum_ << - " as specified in the command line"; + JLOG (j_.warn()) + << "Using unsafe quorum of " + << quorum_ + << " as specified in the command line"; } // Do not use achievable quorum until lists from all configured diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index 4bfbb8846..7a71c6bfb 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -36,7 +36,7 @@ ValidatorList::ValidatorList ( , publisherManifests_ (publisherManifests) , timeKeeper_ (timeKeeper) , j_ (j) - , quorum_ (minimumQuorum ? *minimumQuorum : 1) // Genesis ledger quorum + , quorum_ (minimumQuorum.value_or(1)) // Genesis ledger quorum , minimumQuorum_ (minimumQuorum) { } diff --git a/src/ripple/core/Config.h b/src/ripple/core/Config.h index 6ef9c7537..b2d123eab 100644 --- a/src/ripple/core/Config.h +++ b/src/ripple/core/Config.h @@ -150,7 +150,7 @@ public: int PATH_SEARCH_MAX = 10; // Validation - boost::optional VALIDATION_QUORUM; // Minimum validations to consider ledger authoritative + boost::optional VALIDATION_QUORUM; // validations to consider ledger authoritative std::uint64_t FEE_DEFAULT = 10; std::uint64_t FEE_ACCOUNT_RESERVE = 200*SYSTEM_CURRENCY_PARTS; diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index 98a6dd3ef..01fa2d116 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/src/test/app/ValidatorList_test.cpp @@ -488,16 +488,19 @@ private: std::vector cfgPublishers; hash_set activeValidators; + // BFT: n >= 3f+1 + std::size_t const n = 40; + std::size_t const f = 13; { std::vector cfgKeys; - cfgKeys.reserve(20); + cfgKeys.reserve(n); - while (cfgKeys.size () != 20) + while (cfgKeys.size () != n) { auto const valKey = randomNode(); cfgKeys.push_back (toBase58( TokenType::TOKEN_NODE_PUBLIC, valKey)); - if (cfgKeys.size () <= 15) + if (cfgKeys.size () <= n - 5) activeValidators.emplace (valKey); } @@ -507,7 +510,8 @@ private: // onConsensusStart should make all available configured // validators trusted trustedKeys->onConsensusStart (activeValidators); - BEAST_EXPECT(trustedKeys->quorum () == 14); + // Add 1 to n because I'm not on a published list. + BEAST_EXPECT(trustedKeys->quorum () == n + 1 - f); std::size_t i = 0; for (auto const& val : cfgKeys) { @@ -566,7 +570,7 @@ private: manifests.applyManifest(std::move (*m1)) == ManifestDisposition::accepted); trustedKeys->onConsensusStart (activeValidators); - BEAST_EXPECT(trustedKeys->quorum () == 15); + BEAST_EXPECT(trustedKeys->quorum () == n + 2 - f); BEAST_EXPECT(trustedKeys->listed (masterPublic)); BEAST_EXPECT(trustedKeys->trusted (masterPublic)); BEAST_EXPECT(trustedKeys->listed (signingPublic1)); @@ -583,7 +587,7 @@ private: manifests.applyManifest(std::move (*m2)) == ManifestDisposition::accepted); trustedKeys->onConsensusStart (activeValidators); - BEAST_EXPECT(trustedKeys->quorum () == 15); + BEAST_EXPECT(trustedKeys->quorum () == n + 2 - f); BEAST_EXPECT(trustedKeys->listed (masterPublic)); BEAST_EXPECT(trustedKeys->trusted (masterPublic)); BEAST_EXPECT(trustedKeys->listed (signingPublic2)); @@ -607,7 +611,7 @@ private: BEAST_EXPECT(manifests.getSigningKey (masterPublic) == masterPublic); BEAST_EXPECT(manifests.revoked (masterPublic)); trustedKeys->onConsensusStart (activeValidators); - BEAST_EXPECT(trustedKeys->quorum () == 15); + BEAST_EXPECT(trustedKeys->quorum () == n + 1 - f); BEAST_EXPECT(trustedKeys->listed (masterPublic)); BEAST_EXPECT(!trustedKeys->trusted (masterPublic)); BEAST_EXPECT(!trustedKeys->listed (signingPublicMax)); @@ -658,7 +662,7 @@ private: } { // Should use custom minimum quorum - std::size_t const minQuorum = 0; + std::size_t const minQuorum = 1; ManifestCache manifests; auto trustedKeys = std::make_unique ( manifests, manifests, env.timeKeeper(), beast::Journal (), minQuorum); @@ -811,9 +815,9 @@ private: hash_set activeValidators; std::vector valKeys; - valKeys.reserve(20); + valKeys.reserve(n); - while (valKeys.size () != 20) + while (valKeys.size () != n) { valKeys.push_back (randomNode()); activeValidators.emplace (valKeys.back()); @@ -866,7 +870,8 @@ private: } // The number of trusted keys should be 125% of the minimum quorum - BEAST_EXPECT(nTrusted == trustedKeys->quorum () * 5 / 4); + BEAST_EXPECT(nTrusted == + static_cast(trustedKeys->quorum () * 5 / 4)); } }