diff --git a/cfg/validators-example.txt b/cfg/validators-example.txt index 28b82f177e..456ae1edc7 100644 --- a/cfg/validators-example.txt +++ b/cfg/validators-example.txt @@ -47,6 +47,12 @@ # The default validator list publishers that the rippled instance # trusts. +# +# WARNING: Changing these values can cause your rippled instance to see a +# validated ledger that contradicts other rippled instances' +# validated ledgers (aka a ledger fork) if your validator list(s) +# do not sufficiently overlap with the list(s) used by others. +# See: https://arxiv.org/pdf/1802.07242.pdf [validator_list_sites] https://vl.ripple.com diff --git a/src/ripple/app/misc/ValidatorList.h b/src/ripple/app/misc/ValidatorList.h index 6bfba350c1..6bbf54f4de 100644 --- a/src/ripple/app/misc/ValidatorList.h +++ b/src/ripple/app/misc/ValidatorList.h @@ -148,17 +148,6 @@ class ValidatorList // Currently supported version of publisher list format static constexpr std::uint32_t requiredListVersion = 1; - // 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, @@ -393,15 +382,15 @@ private: bool removePublisherList (PublicKey const& publisherKey); - /** Return safe minimum quorum for listed validator set + /** Return quorum for trusted validator set - @param nListedKeys Number of list validator keys + @param trusted Number of trusted validator keys - @param unListedLocal Whether the local node is an unlisted validator - */ - static std::size_t - calculateMinimumQuorum ( - std::size_t nListedKeys, bool unlistedLocal=false); + @param seen Number of trusted validators that have signed + recently received validations */ + std::size_t + calculateQuorum ( + std::size_t trusted, std::size_t seen); }; } // ripple diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index 7329632fb3..9d86f1e36e 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -436,8 +436,7 @@ ValidatorList::removePublisherList (PublicKey const& publisherKey) return false; JLOG (j_.debug()) << - "Removing validator list for revoked publisher " << - toBase58(TokenType::NodePublic, publisherKey); + "Removing validator list for publisher " << strHex(publisherKey); for (auto const& val : iList->second.list) { @@ -569,33 +568,62 @@ ValidatorList::for_each_listed ( } std::size_t -ValidatorList::calculateMinimumQuorum ( - std::size_t nListedKeys, bool unlistedLocal) +ValidatorList::calculateQuorum ( + std::size_t trusted, std::size_t seen) { - // Only require 51% quorum for small number of validators to facilitate - // bootstrapping a network. - if (nListedKeys <= 6) - return nListedKeys/2 + 1; + // Do not use achievable quorum until lists from all configured + // publishers are available + for (auto const& list : publisherLists_) + { + if (! list.second.available) + return std::numeric_limits::max(); + } - // The number of listed validators is increased to preserve the safety - // guarantee for two unlisted validators using the same set of listed - // validators. - if (unlistedLocal) - ++nListedKeys; + // Use an 80% quorum to balance fork safety, liveness, and required UNL + // overlap. + // + // Theorem 8 of the Analysis of the XRP Ledger Consensus Protocol + // (https://arxiv.org/abs/1802.07242) says: + // XRP LCP guarantees fork safety if Oi,j > nj/2 + ni − qi + ti,j for + // every pair of nodes Pi, Pj. + // + // ni: size of Pi's UNL + // nj: size of Pj's UNL + // Oi,j: number of validators in both UNLs + // qi: validation quorum for Pi's UNL + // ti, tj: maximum number of allowed Byzantine faults in Pi and Pj's UNLs + // ti,j: min{ti, tj, Oi,j} + // + // Assume ni < nj, meaning and ti,j = ti + // + // For qi = .8*ni, we make ti <= .2*ni + // (We could make ti lower and tolerate less UNL overlap. However in order + // to prioritize safety over liveness, we need ti >= ni - qi) + // + // An 80% quorum allows two UNLs to safely have < .2*ni unique validators + // between them: + // + // pi = ni - Oi,j + // pj = nj - Oi,j + // + // Oi,j > nj/2 + ni − qi + ti,j + // ni - pi > (ni - pi + pj)/2 + ni − .8*ni + .2*ni + // pi + pj < .2*ni + auto quorum = static_cast(std::ceil(trusted * 0.8f)); - // Guarantee safety with up to 1/3 listed validators being malicious. - // This prioritizes safety (Byzantine fault tolerance) over liveness. - // It takes at least as many malicious nodes to split/fork the network as - // to stall the network. - // At 67%, the overlap of two quorums is 34% - // 67 + 67 - 100 = 34 - // So under certain conditions, 34% of validators could vote for two - // different ledgers and split the network. - // Similarly 34% could prevent quorum from being met (by not voting) and - // stall the network. - // If/when the quorum is subsequently raised to/towards 80%, it becomes - // harder to split the network (more safe) and easier to stall it (less live). - return nListedKeys * 2/3 + 1; + // Use lower quorum specified via command line if the normal quorum appears + // unreachable based on the number of recently received validations. + if (minimumQuorum_ && *minimumQuorum_ < quorum && seen < quorum) + { + quorum = *minimumQuorum_; + + JLOG (j_.warn()) + << "Using unsafe quorum of " + << quorum + << " as specified in the command line"; + } + + return quorum; } TrustChanges @@ -603,120 +631,43 @@ ValidatorList::updateTrusted(hash_set const& seenValidators) { boost::unique_lock lock{mutex_}; - // Check that lists from all configured publishers are available - bool allListsAvailable = true; - + // Remove any expired published lists for (auto const& list : publisherLists_) { - // Remove any expired published lists - if (TimeKeeper::time_point{} < list.second.expiration && + if (list.second.available && list.second.expiration <= timeKeeper_.now()) removePublisherList(list.first); - - if (! list.second.available) - allListsAvailable = false; } - std::multimap rankedKeys; - bool localKeyListed = false; + TrustChanges trustChanges; - // "Iterate" the listed keys in random order so that the rank of multiple - // keys with the same number of listings is not deterministic - std::vector indexes (keyListings_.size()); - std::iota (indexes.begin(), indexes.end(), 0); - std::shuffle (indexes.begin(), indexes.end(), crypto_prng()); - - for (auto const& index : indexes) + auto it = trustedKeys_.cbegin(); + while (it != trustedKeys_.cend()) { - auto const& val = std::next (keyListings_.begin(), index); - - if (validatorManifests_.revoked (val->first)) - continue; - - if (val->first == localPubKey_) + if (! keyListings_.count(*it) || + validatorManifests_.revoked(*it)) { - localKeyListed = val->second > 1; - rankedKeys.insert ( - std::pair( - std::numeric_limits::max(), localPubKey_)); - } - // 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(calcNodeID(val->first)) != seenValidators.end()) - { - rankedKeys.insert ( - std::pair(val->second, val->first)); - } - } - - // This minimum quorum guarantees safe overlap with the trusted sets of - // other nodes using the same set of published lists. - std::size_t quorum = calculateMinimumQuorum (keyListings_.size(), - localPubKey_.size() && !localKeyListed); - - JLOG (j_.debug()) << - rankedKeys.size() << " of " << keyListings_.size() << - " listed validators eligible for inclusion in the trusted set"; - - auto size = rankedKeys.size(); - - // 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 || - 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); + trustChanges.removed.insert(calcNodeID(*it)); + it = trustedKeys_.erase(it); } else { - // Reduce the trusted set size so that the quorum represents - // at least 80% - size = quorum * 1.25; + ++it; } } - if (minimumQuorum_ && seenValidators.size() < quorum) + for (auto const& val : keyListings_) { - quorum = *minimumQuorum_; - JLOG (j_.warn()) - << "Using unsafe quorum of " - << quorum_ - << " as specified in the command line"; + if (! validatorManifests_.revoked(val.first) && + trustedKeys_.emplace(val.first).second) + trustChanges.added.insert(calcNodeID(val.first)); } - // Do not use achievable quorum until lists from all configured - // publishers are available - else if (! allListsAvailable) - quorum = std::numeric_limits::max(); - - TrustChanges trustChanges; - { - hash_set newTrustedKeys; - for (auto const& val : boost::adaptors::reverse(rankedKeys)) - { - if (size <= newTrustedKeys.size()) - break; - newTrustedKeys.insert(val.second); - - if (trustedKeys_.erase(val.second) == 0) - trustChanges.added.insert(calcNodeID(val.second)); - } - - for (auto const& k : trustedKeys_) - trustChanges.removed.insert(calcNodeID(k)); - trustedKeys_ = std::move(newTrustedKeys); - } - - quorum_ = quorum; + JLOG (j_.debug()) << + trustedKeys_.size() << " of " << keyListings_.size() << + " listed validators eligible for inclusion in the trusted set"; + quorum_ = calculateQuorum (trustedKeys_.size(), seenValidators.size()); JLOG(j_.debug()) << "Using quorum of " << quorum_ << " for new set of " << trustedKeys_.size() << " trusted validators (" diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index 6dd126e587..e9e66645dc 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/src/test/app/ValidatorList_test.cpp @@ -531,14 +531,12 @@ private: std::vector cfgPublishers; hash_set activeValidators; - hash_set secondAddedValidators; - // BFT: n >= 3f+1 std::size_t const n = 40; - std::size_t const f = 13; { std::vector cfgKeys; cfgKeys.reserve(n); + hash_set unseenValidators; while (cfgKeys.size () != n) { @@ -547,52 +545,43 @@ private: TokenType::NodePublic, valKey)); if (cfgKeys.size () <= n - 5) activeValidators.emplace (calcNodeID(valKey)); + else + unseenValidators.emplace (calcNodeID(valKey)); } BEAST_EXPECT(trustedKeys->load ( emptyLocalKey, cfgKeys, cfgPublishers)); - // updateTrusted should make all available configured - // validators trusted + // updateTrusted should make all configured validators trusted + // even if they are not active/seen TrustChanges changes = trustedKeys->updateTrusted(activeValidators); + + for (auto const& val : unseenValidators) + activeValidators.emplace (val); + BEAST_EXPECT(changes.added == activeValidators); BEAST_EXPECT(changes.removed.empty()); - // Add 1 to n because I'm not on a published list. - BEAST_EXPECT(trustedKeys->quorum () == n + 1 - f); - std::size_t i = 0; + BEAST_EXPECT(trustedKeys->quorum () == + std::ceil(cfgKeys.size() * 0.8f)); for (auto const& val : cfgKeys) { if (auto const valKey = parseBase58( TokenType::NodePublic, val)) { BEAST_EXPECT(trustedKeys->listed (*valKey)); - if (i++ < activeValidators.size ()) - BEAST_EXPECT(trustedKeys->trusted (*valKey)); - else - BEAST_EXPECT(!trustedKeys->trusted (*valKey)); + BEAST_EXPECT(trustedKeys->trusted (*valKey)); } else fail (); } - { - // Quorum should be 80% with all listed validators active - hash_set activeValidatorsNew{activeValidators}; - for (auto const valKey : cfgKeys) - { - auto const ins = activeValidatorsNew.emplace( - calcNodeID(*parseBase58( - TokenType::NodePublic, valKey))); - if(ins.second) - secondAddedValidators.insert(*ins.first); - } - TrustChanges changes = - trustedKeys->updateTrusted(activeValidatorsNew); - BEAST_EXPECT(changes.added == secondAddedValidators); - BEAST_EXPECT(changes.removed.empty()); - BEAST_EXPECT(trustedKeys->quorum () == cfgKeys.size() * 4/5); - } + changes = + trustedKeys->updateTrusted(activeValidators); + BEAST_EXPECT(changes.added.empty()); + BEAST_EXPECT(changes.removed.empty()); + BEAST_EXPECT(trustedKeys->quorum () == + std::ceil(cfgKeys.size() * 0.8f)); } { // update with manifests @@ -614,7 +603,8 @@ private: TrustChanges changes = trustedKeys->updateTrusted(activeValidators); BEAST_EXPECT(changes.added == asNodeIDs({masterPublic})); - BEAST_EXPECT(changes.removed == secondAddedValidators); + BEAST_EXPECT(changes.removed.empty()); + BEAST_EXPECT(trustedKeys->quorum () == std::ceil((n + 1) * 0.8f)); BEAST_EXPECT(trustedKeys->listed (masterPublic)); BEAST_EXPECT(trustedKeys->trusted (masterPublic)); BEAST_EXPECT(!trustedKeys->listed (signingPublic1)); @@ -628,10 +618,6 @@ private: BEAST_EXPECT( manifests.applyManifest(std::move (*m1)) == ManifestDisposition::accepted); - changes = trustedKeys->updateTrusted(activeValidators); - BEAST_EXPECT(changes.removed.empty()); - BEAST_EXPECT(changes.added.empty()); - BEAST_EXPECT(trustedKeys->quorum () == n + 2 - f); BEAST_EXPECT(trustedKeys->listed (masterPublic)); BEAST_EXPECT(trustedKeys->trusted (masterPublic)); BEAST_EXPECT(trustedKeys->listed (signingPublic1)); @@ -647,10 +633,6 @@ private: BEAST_EXPECT( manifests.applyManifest(std::move (*m2)) == ManifestDisposition::accepted); - changes = trustedKeys->updateTrusted (activeValidators); - BEAST_EXPECT(changes.removed.empty()); - BEAST_EXPECT(changes.added.empty()); - BEAST_EXPECT(trustedKeys->quorum () == n + 2 - f); BEAST_EXPECT(trustedKeys->listed (masterPublic)); BEAST_EXPECT(trustedKeys->trusted (masterPublic)); BEAST_EXPECT(trustedKeys->listed (signingPublic2)); @@ -673,10 +655,15 @@ private: ManifestDisposition::accepted); BEAST_EXPECT(manifests.getSigningKey (masterPublic) == masterPublic); BEAST_EXPECT(manifests.revoked (masterPublic)); + + // Revoked key remains trusted until list is updated + BEAST_EXPECT(trustedKeys->listed (masterPublic)); + BEAST_EXPECT(trustedKeys->trusted (masterPublic)); + changes = trustedKeys->updateTrusted (activeValidators); BEAST_EXPECT(changes.removed == asNodeIDs({masterPublic})); BEAST_EXPECT(changes.added.empty()); - BEAST_EXPECT(trustedKeys->quorum () == n + 1 - f); + BEAST_EXPECT(trustedKeys->quorum () == std::ceil(n * 0.8f)); BEAST_EXPECT(trustedKeys->listed (masterPublic)); BEAST_EXPECT(!trustedKeys->trusted (masterPublic)); BEAST_EXPECT(!trustedKeys->listed (signingPublicMax)); @@ -708,29 +695,6 @@ private: BEAST_EXPECT(trustedKeys->quorum () == std::numeric_limits::max()); } - { - // Trust all listed validators if none are active - auto trustedKeys = std::make_unique ( - manifests, manifests, env.timeKeeper(), beast::Journal ()); - - std::vector keys ({ randomNode (), randomNode () }); - hash_set activeValidators; - std::vector cfgKeys ({ - toBase58 (TokenType::NodePublic, keys[0]), - toBase58 (TokenType::NodePublic, keys[1])}); - - BEAST_EXPECT(trustedKeys->load ( - emptyLocalKey, cfgKeys, cfgPublishers)); - - TrustChanges changes = - trustedKeys->updateTrusted(activeValidators); - BEAST_EXPECT(changes.removed.empty()); - BEAST_EXPECT(changes.added == asNodeIDs({keys[0], keys[1]})); - - BEAST_EXPECT(trustedKeys->quorum () == 2); - for (auto const& key : keys) - BEAST_EXPECT(trustedKeys->trusted (key)); - } { // Should use custom minimum quorum std::size_t const minQuorum = 1; @@ -738,10 +702,24 @@ private: auto trustedKeys = std::make_unique ( manifests, manifests, env.timeKeeper(), beast::Journal (), minQuorum); - auto const node = randomNode (); - std::vector cfgKeys ({ - toBase58 (TokenType::NodePublic, node)}); + std::size_t n = 10; + std::vector cfgKeys; + cfgKeys.reserve(n); + hash_set expectedTrusted; hash_set activeValidators; + NodeID toBeSeen; + + while (cfgKeys.size () < n) + { + auto const valKey = randomNode(); + cfgKeys.push_back (toBase58( + TokenType::NodePublic, valKey)); + expectedTrusted.emplace (calcNodeID(valKey)); + if (cfgKeys.size () < std::ceil(n*0.8f)) + activeValidators.emplace (calcNodeID(valKey)); + else if (cfgKeys.size () < std::ceil(n*0.8f)) + toBeSeen = calcNodeID(valKey); + } BEAST_EXPECT(trustedKeys->load ( emptyLocalKey, cfgKeys, cfgPublishers)); @@ -749,39 +727,15 @@ private: TrustChanges changes = trustedKeys->updateTrusted(activeValidators); BEAST_EXPECT(changes.removed.empty()); - BEAST_EXPECT(changes.added == asNodeIDs({node})); + BEAST_EXPECT(changes.added == expectedTrusted); BEAST_EXPECT(trustedKeys->quorum () == minQuorum); - activeValidators.emplace (calcNodeID(node)); + // Use normal quorum when seen validators >= quorum + activeValidators.emplace (toBeSeen); changes = trustedKeys->updateTrusted(activeValidators); BEAST_EXPECT(changes.removed.empty()); BEAST_EXPECT(changes.added.empty()); - BEAST_EXPECT(trustedKeys->quorum () == 1); - } - { - // Increase quorum when running as an unlisted validator - auto trustedKeys = std::make_unique ( - manifests, manifests, env.timeKeeper(), beast::Journal ()); - - std::vector keys ({ randomNode (), randomNode () }); - hash_set activeValidators (asNodeIDs({ keys[0] })); - std::vector cfgKeys ({ - toBase58 (TokenType::NodePublic, keys[0]), - toBase58 (TokenType::NodePublic, keys[1])}); - - auto const localKey = randomNode (); - BEAST_EXPECT(trustedKeys->load ( - localKey, cfgKeys, cfgPublishers)); - - TrustChanges changes = - trustedKeys->updateTrusted(activeValidators); - BEAST_EXPECT(changes.removed.empty()); - BEAST_EXPECT( - changes.added == asNodeIDs({keys[0], keys[1], localKey})); - BEAST_EXPECT(trustedKeys->quorum () == 2); - - // local validator key is always trusted - BEAST_EXPECT(trustedKeys->trusted (localKey)); + BEAST_EXPECT(trustedKeys->quorum () == std::ceil(n * 0.8f)); } { // Remove expired published list @@ -894,8 +848,7 @@ private: BEAST_EXPECT(changes.removed.empty()); BEAST_EXPECT(changes.added == asNodeIDs({valKey})); BEAST_EXPECT(trustedKeys->quorum () == - ((cfgKeys.size() <= 6) ? cfgKeys.size()/2 + 1 : - cfgKeys.size() * 2/3 + 1)); + std::ceil(cfgKeys.size() * 0.8f)); for (auto const& key : activeKeys) BEAST_EXPECT(trustedKeys->trusted (key)); } @@ -933,15 +886,14 @@ private: changes.added == asNodeIDs({localKey, valKey})); BEAST_EXPECT(trustedKeys->quorum () == - ((cfgKeys.size() <= 6) ? cfgKeys.size()/2 + 1 : - (cfgKeys.size() + 1) * 2/3 + 1)); + std::ceil(cfgKeys.size() * 0.8f)); for (auto const& key : activeKeys) BEAST_EXPECT(trustedKeys->trusted (key)); } } { - // Trusted set should be trimmed with multiple validator lists + // Trusted set should include all validators from multiple lists ManifestCache manifests; auto trustedKeys = std::make_unique ( manifests, manifests, env.timeKeeper(), beast::Journal ()); @@ -994,25 +946,17 @@ private: TrustChanges changes = trustedKeys->updateTrusted(activeValidators); - // Minimum quorum should be used - BEAST_EXPECT(trustedKeys->quorum () == (valKeys.size() * 2/3 + 1)); + BEAST_EXPECT(trustedKeys->quorum () == + std::ceil(valKeys.size() * 0.8f)); hash_set added; - std::size_t nTrusted = 0; for (auto const& val : valKeys) { - if (trustedKeys->trusted (val.masterPublic)) - { - added.insert(calcNodeID(val.masterPublic)); - ++nTrusted; - } + BEAST_EXPECT(trustedKeys->trusted (val.masterPublic)); + added.insert(calcNodeID(val.masterPublic)); } BEAST_EXPECT(changes.added == added); BEAST_EXPECT(changes.removed.empty()); - - // The number of trusted keys should be 125% of the minimum quorum - BEAST_EXPECT(nTrusted == - static_cast(trustedKeys->quorum () * 5 / 4)); } }