From 3462e089a1bae0da2cd43159345f0a3bce341563 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Thu, 6 Apr 2023 12:56:58 -0700 Subject: [PATCH 1/2] If present, set quorum based on command line. --- src/ripple/app/misc/impl/ValidatorList.cpp | 24 +++++++++------------- src/test/app/ValidatorList_test.cpp | 10 +++++---- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index 09f774f8af9..ae3619f90f4 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -1710,6 +1710,15 @@ ValidatorList::calculateQuorum( std::size_t effectiveUnlSize, std::size_t seenSize) { + // Use quorum if specified via command line. + if (minimumQuorum_ && *minimumQuorum_) + { + JLOG(j_.warn()) << "Using potentially unsafe quorum of " + << *minimumQuorum_ + << " as specified in the command line"; + return *minimumQuorum_; + } + // Do not use achievable quorum until lists from all configured // publishers are available for (auto const& list : publisherLists_) @@ -1752,21 +1761,8 @@ ValidatorList::calculateQuorum( // Note that the negative UNL protocol introduced the // AbsoluteMinimumQuorum which is 60% of the original UNL size. The // effective quorum should not be lower than it. - auto quorum = static_cast(std::max( + return static_cast(std::max( std::ceil(effectiveUnlSize * 0.8f), std::ceil(unlSize * 0.6f))); - - // 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 && seenSize < quorum) - { - quorum = *minimumQuorum_; - - JLOG(j_.warn()) << "Using unsafe quorum of " << quorum - << " as specified in the command line"; - } - - return quorum; } TrustChanges diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index fead5563f21..1d158c4905b 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/src/test/app/ValidatorList_test.cpp @@ -1318,7 +1318,7 @@ class ValidatorList_test : public beast::unit_test::suite BEAST_EXPECT(changes.added == expectedTrusted); BEAST_EXPECT(trustedKeys->quorum() == minQuorum); - // Use normal quorum when seen validators >= quorum + // Use configured quorum even when seen validators >= quorum activeValidators.emplace(toBeSeen); changes = trustedKeys->updateTrusted( activeValidators, @@ -1328,7 +1328,7 @@ class ValidatorList_test : public beast::unit_test::suite env.app().getHashRouter()); BEAST_EXPECT(changes.removed.empty()); BEAST_EXPECT(changes.added.empty()); - BEAST_EXPECT(trustedKeys->quorum() == std::ceil(n * 0.8f)); + BEAST_EXPECT(trustedKeys->quorum() == minQuorum); } { // Remove expired published list @@ -1828,7 +1828,9 @@ class ValidatorList_test : public beast::unit_test::suite env.app().getOPs(), env.app().overlay(), env.app().getHashRouter()); - if (trustedKeys->quorum() == std::ceil(cfgKeys.size() * 0.8f)) + if ((minimumQuorum.has_value() && + trustedKeys->quorum() == *minimumQuorum) || + (trustedKeys->quorum() == std::ceil(cfgKeys.size() * 0.8f))) return trustedKeys; } return nullptr; @@ -1980,7 +1982,7 @@ class ValidatorList_test : public beast::unit_test::suite env.app().getOPs(), env.app().overlay(), env.app().getHashRouter()); - BEAST_EXPECT(validators->quorum() == 48); + BEAST_EXPECT(validators->quorum() == 30); hash_set nUnl; it = unl.begin(); for (std::uint32_t i = 0; i < 20; ++i) From 4f7729db2b38818114f97bbc235452da8f869329 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Thu, 13 Apr 2023 11:59:54 -0700 Subject: [PATCH 2/2] Review fixes: simplify accessing optional. --- src/ripple/app/misc/impl/ValidatorList.cpp | 4 ++-- src/test/app/ValidatorList_test.cpp | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index ae3619f90f4..d17b85c4840 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -1711,11 +1711,11 @@ ValidatorList::calculateQuorum( std::size_t seenSize) { // Use quorum if specified via command line. - if (minimumQuorum_ && *minimumQuorum_) + if (minimumQuorum_ > 0) { JLOG(j_.warn()) << "Using potentially unsafe quorum of " << *minimumQuorum_ - << " as specified in the command line"; + << " as specified on the command line"; return *minimumQuorum_; } diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index 1d158c4905b..ff9b57f3ced 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/src/test/app/ValidatorList_test.cpp @@ -1828,9 +1828,8 @@ class ValidatorList_test : public beast::unit_test::suite env.app().getOPs(), env.app().overlay(), env.app().getHashRouter()); - if ((minimumQuorum.has_value() && - trustedKeys->quorum() == *minimumQuorum) || - (trustedKeys->quorum() == std::ceil(cfgKeys.size() * 0.8f))) + if (minimumQuorum == trustedKeys->quorum() || + trustedKeys->quorum() == std::ceil(cfgKeys.size() * 0.8f)) return trustedKeys; } return nullptr;