diff --git a/src/ripple/app/misc/ValidatorList.h b/src/ripple/app/misc/ValidatorList.h index 661e0e4ed1e..cb094b3dec4 100644 --- a/src/ripple/app/misc/ValidatorList.h +++ b/src/ripple/app/misc/ValidatorList.h @@ -308,10 +308,6 @@ class ValidatorList for_each_listed ( std::function func) const; - static std::size_t - calculateMinimumQuorum ( - std::size_t nListedKeys, bool unlistedLocal=false); - private: /** Check response for trusted valid published list @@ -342,6 +338,15 @@ class ValidatorList 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); }; //------------------------------------------------------------------------------ @@ -400,9 +405,8 @@ ValidatorList::onConsensusStart ( } } - // This minimum quorum of 2f + 1 guarantees safe overlap with the trusted - // sets of other nodes using the same set of published lists. - // Safety is guaranteed with up to 1/3 listed validators being malicious. + // 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); @@ -413,15 +417,13 @@ ValidatorList::onConsensusStart ( auto size = rankedKeys.size(); // Do not require 80% quorum for less than 10 trusted validators - if (size >= 10) + if (rankedKeys.size() >= 10) { // 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 - std::size_t const targetQuorum = size - size / 5; - if (targetQuorum > quorum) - quorum = targetQuorum; + quorum = std::max(quorum, size - size / 5); } else { diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index df8b07ba8cc..dae6be73b6e 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -412,19 +412,21 @@ std::size_t ValidatorList::calculateMinimumQuorum ( std::size_t nListedKeys, bool unlistedLocal) { - // Use 2f + 1 for large values of n, but have special cases for small numbers. - constexpr std::array quorum{{ 0, 1, 2, 2, 3, 3 }}; + if (nListedKeys == 0) + return 0; + // Only require 51% quorum for small number of validators to facilitate + // bootstrapping a network. + if (nListedKeys <= 5) + return nListedKeys/2 + 1; // 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 >= quorum.size()) + if (unlistedLocal) ++nListedKeys; - if (nListedKeys < quorum.size()) - return quorum[nListedKeys]; - + // Guarantee safety with up to 1/3 listed validators being malicious. return nListedKeys * 2/3 + 1; } diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index 428bc8fd6d0..dccf775ed26 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/src/test/app/ValidatorList_test.cpp @@ -124,30 +124,6 @@ class ValidatorList_test : public beast::unit_test::suite } } - void - testCalculateMinimumQuorum () - { - testcase ("Calculate Minimum Quorum"); - - for(std::size_t i = 1; i < 20; ++i) - { - auto const quorum = ValidatorList::calculateMinimumQuorum(i); - if (i <= 5) - { - BEAST_EXPECT(quorum >= (i/2 + 1)); - BEAST_EXPECT(ValidatorList::calculateMinimumQuorum( - i, true /* untrusted local validator */) == quorum); - } - else - { - BEAST_EXPECT(quorum == std::floor (i * 2/3) + 1); - BEAST_EXPECT(ValidatorList::calculateMinimumQuorum( - i, true /* untrusted local validator */) == - ValidatorList::calculateMinimumQuorum(i + 1)); - } - } - } - void testConfigLoad () { @@ -547,6 +523,16 @@ class ValidatorList_test : public beast::unit_test::suite 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 @@ -778,7 +764,8 @@ class ValidatorList_test : public beast::unit_test::suite emptyLocalKey, cfgKeys, cfgPublishers)); trustedKeys->onConsensusStart (activeValidators); BEAST_EXPECT(trustedKeys->quorum () == - ValidatorList::calculateMinimumQuorum(cfgKeys.size())); + (cfgKeys.size() <= 5) ? cfgKeys.size()/2 + 1 : + cfgKeys.size() * 2/3 + 1); for (auto const& key : activeValidators) BEAST_EXPECT(trustedKeys->trusted (key)); } @@ -807,14 +794,80 @@ class ValidatorList_test : public beast::unit_test::suite localKey, cfgKeys, cfgPublishers)); trustedKeys->onConsensusStart (activeValidators); - auto expectedQuorum = ValidatorList::calculateMinimumQuorum( - cfgKeys.size(), true /* unlisted local validator */); + BEAST_EXPECT(trustedKeys->quorum () == + (cfgKeys.size() <= 5) ? cfgKeys.size()/2 + 1 : + (cfgKeys.size() + 1) * 2/3 + 1); - BEAST_EXPECT(trustedKeys->quorum () == expectedQuorum); 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)); + + 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: @@ -822,7 +875,6 @@ class ValidatorList_test : public beast::unit_test::suite run() override { testGenesisQuorum (); - testCalculateMinimumQuorum (); testConfigLoad (); testApplyList (); testUpdate ();