From 8c155dd875ad593331de7d057880cef6a01a6a2a Mon Sep 17 00:00:00 2001 From: wilsonianb Date: Mon, 24 Apr 2017 14:33:27 -0700 Subject: [PATCH] Make minimum quorum Byzantine fault tolerant (RIPD-1461) --- src/ripple/app/misc/ValidatorList.h | 57 +++++----- src/ripple/app/misc/impl/ValidatorList.cpp | 30 ++++-- src/test/app/ValidatorList_test.cpp | 116 +++++++++++++++------ 3 files changed, 143 insertions(+), 60 deletions(-) diff --git a/src/ripple/app/misc/ValidatorList.h b/src/ripple/app/misc/ValidatorList.h index 4b6012f2eb..025d405bec 100644 --- a/src/ripple/app/misc/ValidatorList.h +++ b/src/ripple/app/misc/ValidatorList.h @@ -308,9 +308,6 @@ public: for_each_listed ( std::function func) const; - static std::size_t - calculateQuorum (std::size_t nTrustedKeys); - private: /** Check response for trusted valid published list @@ -341,6 +338,15 @@ private: bool removePublisherList (PublicKey const& publisherKey); + /** Return safe minimum quorum for listed validator set + + @param nListedKeys Number of list validator keys + + @param unListedLocal Whether the local node is an unlisted validator + */ + static std::size_t + calculateMinimumQuorum ( + std::size_t nListedKeys, bool unlistedLocal=false); }; //------------------------------------------------------------------------------ @@ -399,15 +405,10 @@ ValidatorList::onConsensusStart ( } } - // This quorum guarantees sufficient overlap with the trusted sets of other - // nodes using the same set of published lists. - std::size_t quorum = keyListings_.size()/2 + 1; - - // Increment the quorum to prevent two unlisted validators using the same - // even number of listed validators from forking. - if (localPubKey_.size() && ! localKeyListed && - rankedKeys.size () > 1 && keyListings_.size () % 2 != 0) - ++quorum; + // 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() << @@ -415,25 +416,31 @@ ValidatorList::onConsensusStart ( auto size = rankedKeys.size(); - // Use all eligible keys if there is less than 10 listed validators or - // only one trusted list - if (size < 10 || publisherLists_.size() == 1) + // Do not require 80% quorum for less than 10 trusted validators + if (rankedKeys.size() >= 10) { - // Try to raise the quorum toward or above 80% of the trusted set - std::size_t const targetQuorum = ValidatorList::calculateQuorum (size); - if (targetQuorum > quorum) - quorum = targetQuorum; - } - else - { - // reduce the trusted set size so that the quorum represents - // at least 80% - size = quorum * 1.25; + // Use all eligible keys if there is only one trusted list + if (publisherLists_.size() == 1) + { + // Try to raise the quorum to at least 80% of the trusted set + quorum = std::max(quorum, size - size / 5); + } + else + { + // Reduce the trusted set size so that the quorum represents + // at least 80% + size = quorum * 1.25; + } } if (minimumQuorum_ && (seenValidators.empty() || rankedKeys.size() < quorum)) + { quorum = *minimumQuorum_; + JLOG (j_.warn()) << + "Using unsafe quorum of " << quorum_ << + " as specified in the command line"; + } // Do not use achievable quorum until lists from all configured // publishers are available diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index 6818214875..4bfbb8846e 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -409,15 +409,33 @@ ValidatorList::for_each_listed ( } std::size_t -ValidatorList::calculateQuorum (std::size_t nTrustedKeys) +ValidatorList::calculateMinimumQuorum ( + std::size_t nListedKeys, bool unlistedLocal) { - // Use 80% for large values of n, but have special cases for small numbers. - constexpr std::array quorum{{ 0, 1, 2, 2, 3, 3, 4, 5, 6, 7 }}; + // Only require 51% quorum for small number of validators to facilitate + // bootstrapping a network. + if (nListedKeys <= 5) + return nListedKeys/2 + 1; - if (nTrustedKeys < quorum.size()) - return quorum[nTrustedKeys]; + // 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; - return nTrustedKeys - nTrustedKeys / 5; + // 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; } } // ripple diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index 8fa257c656..98a6dd3ef9 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/src/test/app/ValidatorList_test.cpp @@ -124,21 +124,6 @@ private: } } - void - testCalculateQuorum () - { - testcase ("Calculate Quorum"); - - for(std::size_t i = 1; i < 20; ++i) - { - auto const quorum = ValidatorList::calculateQuorum(i); - if (i < 10) - BEAST_EXPECT(quorum >= (i/2 + 1)); - else - BEAST_EXPECT(quorum == std::ceil (i * 0.8)); - } - } - void testConfigLoad () { @@ -522,7 +507,7 @@ private: // onConsensusStart should make all available configured // validators trusted trustedKeys->onConsensusStart (activeValidators); - BEAST_EXPECT(trustedKeys->quorum () == 12); + BEAST_EXPECT(trustedKeys->quorum () == 14); std::size_t i = 0; for (auto const& val : cfgKeys) { @@ -538,6 +523,16 @@ private: else fail (); } + + { + // Quorum should be 80% with all listed validators active + hash_set activeValidators; + for (auto const valKey : cfgKeys) + activeValidators.emplace (*parseBase58( + TokenType::TOKEN_NODE_PUBLIC, valKey)); + trustedKeys->onConsensusStart (activeValidators); + BEAST_EXPECT(trustedKeys->quorum () == cfgKeys.size() * 4/5); + } } { // update with manifests @@ -571,7 +566,7 @@ private: manifests.applyManifest(std::move (*m1)) == ManifestDisposition::accepted); trustedKeys->onConsensusStart (activeValidators); - BEAST_EXPECT(trustedKeys->quorum () == 13); + BEAST_EXPECT(trustedKeys->quorum () == 15); BEAST_EXPECT(trustedKeys->listed (masterPublic)); BEAST_EXPECT(trustedKeys->trusted (masterPublic)); BEAST_EXPECT(trustedKeys->listed (signingPublic1)); @@ -584,12 +579,11 @@ private: auto m2 = Manifest::make_Manifest (makeManifestString ( masterPublic, masterPrivate, signingPublic2, signingKeys2.second, 2)); - BEAST_EXPECT( manifests.applyManifest(std::move (*m2)) == ManifestDisposition::accepted); trustedKeys->onConsensusStart (activeValidators); - BEAST_EXPECT(trustedKeys->quorum () == 13); + BEAST_EXPECT(trustedKeys->quorum () == 15); BEAST_EXPECT(trustedKeys->listed (masterPublic)); BEAST_EXPECT(trustedKeys->trusted (masterPublic)); BEAST_EXPECT(trustedKeys->listed (signingPublic2)); @@ -613,7 +607,7 @@ private: BEAST_EXPECT(manifests.getSigningKey (masterPublic) == masterPublic); BEAST_EXPECT(manifests.revoked (masterPublic)); trustedKeys->onConsensusStart (activeValidators); - BEAST_EXPECT(trustedKeys->quorum () == 12); + BEAST_EXPECT(trustedKeys->quorum () == 15); BEAST_EXPECT(trustedKeys->listed (masterPublic)); BEAST_EXPECT(!trustedKeys->trusted (masterPublic)); BEAST_EXPECT(!trustedKeys->listed (signingPublicMax)); @@ -700,7 +694,7 @@ private: localKey, cfgKeys, cfgPublishers)); trustedKeys->onConsensusStart (activeValidators); - BEAST_EXPECT(trustedKeys->quorum () == 3); + BEAST_EXPECT(trustedKeys->quorum () == 2); // local validator key is always trusted BEAST_EXPECT(trustedKeys->trusted (localKey)); @@ -770,7 +764,8 @@ private: emptyLocalKey, cfgKeys, cfgPublishers)); trustedKeys->onConsensusStart (activeValidators); BEAST_EXPECT(trustedKeys->quorum () == - ValidatorList::calculateQuorum(cfgKeys.size())); + (cfgKeys.size() <= 5) ? cfgKeys.size()/2 + 1 : + cfgKeys.size() * 2/3 + 1); for (auto const& key : activeValidators) BEAST_EXPECT(trustedKeys->trusted (key)); } @@ -799,16 +794,80 @@ private: localKey, cfgKeys, cfgPublishers)); trustedKeys->onConsensusStart (activeValidators); - // When running as an unlisted validator, - // the quorum is incremented by 1 for 3 or 5 trusted validators. - auto expectedQuorum = ValidatorList::calculateQuorum(cfgKeys.size()); - if (cfgKeys.size() == 3 || cfgKeys.size() == 5) - ++expectedQuorum; - BEAST_EXPECT(trustedKeys->quorum () == expectedQuorum); + BEAST_EXPECT(trustedKeys->quorum () == + (cfgKeys.size() <= 5) ? cfgKeys.size()/2 + 1 : + (cfgKeys.size() + 1) * 2/3 + 1); + for (auto const& key : activeValidators) BEAST_EXPECT(trustedKeys->trusted (key)); } } + { + // Trusted set should be trimmed with multiple validator lists + ManifestCache manifests; + auto trustedKeys = std::make_unique ( + manifests, manifests, env.timeKeeper(), beast::Journal ()); + + hash_set activeValidators; + + std::vector valKeys; + valKeys.reserve(20); + + while (valKeys.size () != 20) + { + valKeys.push_back (randomNode()); + activeValidators.emplace (valKeys.back()); + } + + auto addPublishedList = [this, &env, &trustedKeys, &valKeys]() + { + auto const publisherSecret = randomSecretKey(); + auto const publisherPublic = + derivePublicKey(KeyType::ed25519, publisherSecret); + auto const pubSigningKeys = randomKeyPair(KeyType::secp256k1); + auto const manifest = beast::detail::base64_encode(makeManifestString ( + publisherPublic, publisherSecret, + pubSigningKeys.first, pubSigningKeys.second, 1)); + + std::vector cfgPublishers({ + strHex(publisherPublic)}); + PublicKey emptyLocalKey; + std::vector emptyCfgKeys; + + BEAST_EXPECT(trustedKeys->load ( + emptyLocalKey, emptyCfgKeys, cfgPublishers)); + + auto const version = 1; + auto const sequence = 1; + NetClock::time_point const expiration = + env.timeKeeper().now() + 3600s; + auto const blob = makeList ( + valKeys, sequence, expiration.time_since_epoch().count()); + auto const sig = signList (blob, pubSigningKeys); + + BEAST_EXPECT(ListDisposition::accepted == trustedKeys->applyList ( + manifest, blob, sig, version)); + }; + + // Apply multiple published lists + for (auto i = 0; i < 3; ++i) + addPublishedList(); + + trustedKeys->onConsensusStart (activeValidators); + + // Minimum quorum should be used + BEAST_EXPECT(trustedKeys->quorum () == (valKeys.size() * 2/3 + 1)); + + std::size_t nTrusted = 0; + for (auto const& key : activeValidators) + { + if (trustedKeys->trusted (key)) + ++nTrusted; + } + + // The number of trusted keys should be 125% of the minimum quorum + BEAST_EXPECT(nTrusted == trustedKeys->quorum () * 5 / 4); + } } public: @@ -816,7 +875,6 @@ public: run() override { testGenesisQuorum (); - testCalculateQuorum (); testConfigLoad (); testApplyList (); testUpdate ();