Skip to content

Commit

Permalink
Disable quorum also when threshold > 1
Browse files Browse the repository at this point in the history
  • Loading branch information
Bronek committed Sep 3, 2024
1 parent fc8e62d commit 22f73da
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 12 deletions.
7 changes: 3 additions & 4 deletions src/test/app/ValidatorList_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2763,6 +2763,7 @@ class ValidatorList_test : public beast::unit_test::suite
};

// All test cases use 5 publishers.
constexpr auto quorumDisabled = std::numeric_limits<std::size_t>::max();
{
// List threshold = 5 (same as number of trusted publishers)
ManifestCache pubManifests;
Expand Down Expand Up @@ -2811,7 +2812,7 @@ class ValidatorList_test : public beast::unit_test::suite
env.app().getOPs(),
env.app().overlay(),
env.app().getHashRouter());
BEAST_EXPECT(trustedKeys->quorum() == 1);
BEAST_EXPECT(trustedKeys->quorum() == quorumDisabled);
BEAST_EXPECT(trustedKeys->getTrustedMasterKeys().size() == 1);

hash_set<NodeID> removed;
Expand Down Expand Up @@ -2879,7 +2880,7 @@ class ValidatorList_test : public beast::unit_test::suite
env.app().getOPs(),
env.app().overlay(),
env.app().getHashRouter());
BEAST_EXPECT(trustedKeys->quorum() == 1);
BEAST_EXPECT(trustedKeys->quorum() == quorumDisabled);
BEAST_EXPECT(trustedKeys->getTrustedMasterKeys().size() == 1);

hash_set<NodeID> removed;
Expand All @@ -2896,8 +2897,6 @@ class ValidatorList_test : public beast::unit_test::suite
BEAST_EXPECT(changes.added.empty());
BEAST_EXPECT(changes.removed == removed);
}

constexpr auto quorumDisabled = std::numeric_limits<std::size_t>::max();
{
// List threshold = 3 (default), 2 publishers are revoked
ManifestCache pubManifests;
Expand Down
40 changes: 32 additions & 8 deletions src/xrpld/app/misc/detail/ValidatorList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1780,16 +1780,40 @@ ValidatorList::calculateQuorum(
return *minimumQuorum_;
}

// Do not use achievable quorum until lists from a sufficient number of
// configured publishers are available
std::size_t unavailable = 0;
for (auto const& list : publisherLists_)
if (!publisherLists_.empty())
{
if (list.second.status != PublisherStatus::available)
unavailable += 1;
// Do not use achievable quorum until lists from a sufficient number of
// configured publishers are available
std::size_t unavailable = 0;
for (auto const& list : publisherLists_)
{
if (list.second.status != PublisherStatus::available)
unavailable += 1;
}
// There are two, subtly different, sides to list threshold
//
// 1. The minimum required intersection between lists listThreshold_
// for a validator to be included in trustedMasterKeys_.
// If this many (or more) publishers are unavailable, we are likely
// to NOT include a validator which otherwise would have been used.
// 2. The minimum number of lists which, when UNAVAILABLE, will prevent
// us from hitting the above threshold on ANY validator. This is
// calculated as:
// N - M + 1
// where
// N: number of lists i.e. publisherLists_.size()
// M: minimum required intersection i.e. listThreshold_
//
// Ideally we want both to be equal, but it is only possible when we
// have an odd number of publisher lists; also the user might have
// configured a non-standard listThreshold_.
auto const errorThreshold = std::min(
listThreshold_, //
publisherLists_.size() - listThreshold_ + 1);
assert(errorThreshold > 0);
if (unavailable >= errorThreshold)
return std::numeric_limits<std::size_t>::max();
}
if (unavailable >= listThreshold_)
return std::numeric_limits<std::size_t>::max();

// Use an 80% quorum to balance fork safety, liveness, and required UNL
// overlap.
Expand Down

0 comments on commit 22f73da

Please sign in to comment.