Skip to content

Commit

Permalink
The fixAmendmentFlapping amendment:
Browse files Browse the repository at this point in the history
Amendment flapping usually occurs when an amendment is on
the verge of gaining majority, and a validator not in favor
of the amendment goes offline.

If fixAmendmentFlapping activates then two changes occur:

1. The number of validators in the UNL determines the required
threshold for an amendment to gain majority.

2. The AmendmentTable keeps a record of the most recent
Amendment vote received from each trusted validator in the
UNL.  If no validation arrives from a given validator, then
the AmendmentTable assumes that the previously received vote
has not changed.

Fixes XRPLF#4350
  • Loading branch information
scottschurr committed Feb 3, 2023
1 parent 0ce15e0 commit 09d94ca
Show file tree
Hide file tree
Showing 7 changed files with 471 additions and 54 deletions.
3 changes: 3 additions & 0 deletions src/ripple/app/main/Application.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1323,6 +1323,9 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline)
<< "Invalid entry in [" << SECTION_VALIDATOR_LIST_SITES << "]";
return false;
}

// Tell the AmendmentTable who the trusted validators are.
m_amendmentTable->trustChanged(validators_->getQuorumKeys().second);
}
//----------------------------------------------------------------------
//
Expand Down
4 changes: 4 additions & 0 deletions src/ripple/app/misc/AmendmentTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ class AmendmentTable
std::set<uint256> const& enabled,
majorityAmendments_t const& majority) = 0;

// Called when the set of trusted validators changes.
virtual void
trustChanged(hash_set<PublicKey> const& allTrusted) = 0;

// Called by the consensus code when we need to
// inject pseudo-transactions
virtual std::map<uint256, std::uint32_t>
Expand Down
5 changes: 5 additions & 0 deletions src/ripple/app/misc/NetworkOPs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1834,7 +1834,12 @@ NetworkOPsImp::beginConsensus(uint256 const& networkClosed)
app_.getHashRouter());

if (!changes.added.empty() || !changes.removed.empty())
{
app_.getValidations().trustChanged(changes.added, changes.removed);
// Update the AmendmentTable so it tracks the current validators.
app_.getAmendmentTable().trustChanged(
app_.validators().getQuorumKeys().second);
}

mConsensus.startRound(
app_.timeKeeper().closeTime(),
Expand Down
178 changes: 162 additions & 16 deletions src/ripple/app/misc/impl/AmendmentTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,113 @@ parseSection(Section const& section)
return names;
}

/** TrustedVotes records the most recent votes from trusted validators.
We keep a record in an effort to avoid "flapping" while amendment voting
is in process.
If a trusted validator loses synchronization near a flag ledger their
amendment votes may be lost during that round. If the validator is a
bit flaky, then this can cause an amendment to appear to repeatedly
gain and lose support.
TrustedVotes addresses the problem by holding on to the last vote seen
from every trusted validator. So if any given validator is off line near
a flag ledger we can assume that they did not change their vote.
*/
class TrustedVotes
{
private:
// Associates each trusted validator with the last votes we saw from them.
hash_map<PublicKey, std::vector<uint256>> recordedVotes_;

public:
TrustedVotes() = default;
TrustedVotes(TrustedVotes const& rhs) = delete;
TrustedVotes&
operator=(TrustedVotes const& rhs) = delete;

// Called when the list of trusted validators changes.
//
// Call with AmendmentTable::mutex_ locked.
void
trustChanged(
hash_set<PublicKey> const& allTrusted,
std::lock_guard<std::mutex> const& lock)
{
decltype(recordedVotes_) newRecordedVotes;
newRecordedVotes.reserve(allTrusted.size());

// Make sure every PublicKey in allTrusted is represented in
// recordedVotes_. Also make sure recordedVotes_ contains
// no additional PublicKeys.
for (auto& trusted : allTrusted)
{
if (recordedVotes_.contains(trusted))
{
// Preserve this validator's previously saved voting state.
newRecordedVotes.insert(recordedVotes_.extract(trusted));
}
else
{
// New validators have a starting position of no on everything.
// Add the entry with an empty vector.
newRecordedVotes[trusted];
}
}
// The votes of any no-longer-trusted validators will be destroyed
// when changedTrustedVotes goes out of scope.
recordedVotes_.swap(newRecordedVotes);
}

// Called when we receive the latest votes.
//
// Call with AmendmentTable::mutex_ locked.
void
recordVotes(
Rules const& rules,
std::vector<std::shared_ptr<STValidation>> const& valSet,
std::lock_guard<std::mutex> const& lock)
{
// Walk all validations and replace previous votes from trusted
// validators with these newest votes.
for (auto const& val : valSet)
{
// If this validation comes from one of our trusted validators...
if (auto const iter = recordedVotes_.find(val->getSignerPublic());
iter != recordedVotes_.end())
{
if (val->isFieldPresent(sfAmendments))
{
auto const& choices = val->getFieldV256(sfAmendments);
iter->second.assign(choices.begin(), choices.end());
}
else
{
// This validator does not support any amendments right now.
iter->second.clear();
}
}
}
}

// Return the information needed by AmendmentSet to determine votes.
//
// Call with AmendmentTable::mutex_ locked.
[[nodiscard]] std::pair<int, hash_map<uint256, int>>
getVotes(Rules const& rules, std::lock_guard<std::mutex> const& lock) const
{
hash_map<uint256, int> ret;
for (auto& validatorVotes : recordedVotes_)
{
for (uint256 const& amendment : validatorVotes.second)
{
ret[amendment] += 1;
}
}
return {recordedVotes_.size(), ret};
}
};

/** Current state of an amendment.
Tells if a amendment is supported, enabled or vetoed. A vetoed amendment
means the node will never announce its support.
Expand Down Expand Up @@ -104,6 +211,24 @@ class AmendmentSet
// number of votes needed
int threshold_ = 0;

void
computeThreshold(int trustedValidations, Rules const& rules)
{
threshold_ = !rules_.enabled(fixAmendmentMajorityCalc)
? std::max(
1L,
static_cast<long>(
(trustedValidations_ *
preFixAmendmentMajorityCalcThreshold.num) /
preFixAmendmentMajorityCalcThreshold.den))
: std::max(
1L,
static_cast<long>(
(trustedValidations_ *
postFixAmendmentMajorityCalcThreshold.num) /
postFixAmendmentMajorityCalcThreshold.den));
}

public:
AmendmentSet(
Rules const& rules,
Expand All @@ -127,20 +252,22 @@ class AmendmentSet
++trustedValidations_;
}
}
computeThreshold(trustedValidations_, rules);
}

threshold_ = !rules_.enabled(fixAmendmentMajorityCalc)
? std::max(
1L,
static_cast<long>(
(trustedValidations_ *
preFixAmendmentMajorityCalcThreshold.num) /
preFixAmendmentMajorityCalcThreshold.den))
: std::max(
1L,
static_cast<long>(
(trustedValidations_ *
postFixAmendmentMajorityCalcThreshold.num) /
postFixAmendmentMajorityCalcThreshold.den));
AmendmentSet(
Rules const& rules,
TrustedVotes const& trustedVotes,
std::lock_guard<std::mutex> const& lock)
: rules_(rules)
{
// process validations for ledger before flag ledger.
auto [trustedCount, newVotes] = trustedVotes.getVotes(rules, lock);

trustedValidations_ = trustedCount;
votes_.swap(newVotes);

computeThreshold(trustedValidations_, rules);
}

bool
Expand Down Expand Up @@ -203,6 +330,9 @@ class AmendmentTableImpl final : public AmendmentTable
hash_map<uint256, AmendmentState> amendmentMap_;
std::uint32_t lastUpdateSeq_;

// Record of the last votes seen from trusted validators.
TrustedVotes previousTrustedVotes_;

// Time that an amendment must hold a majority for
std::chrono::seconds const majorityTime_;

Expand Down Expand Up @@ -294,6 +424,9 @@ class AmendmentTableImpl final : public AmendmentTable
std::set<uint256> const& enabled,
majorityAmendments_t const& majority) override;

void
trustChanged(hash_set<PublicKey> const& allTrusted) override;

std::vector<uint256>
doValidation(std::set<uint256> const& enabledAmendments) const override;

Expand Down Expand Up @@ -616,8 +749,16 @@ AmendmentTableImpl::doVoting(
<< ": " << enabledAmendments.size() << ", "
<< majorityAmendments.size() << ", " << valSet.size();

auto vote = std::make_unique<AmendmentSet>(rules, valSet);
std::lock_guard lock(mutex_);

// We need to call recordVotes() before fixAmendmentFlapping is enabled
// so we have current state when it does enable.
previousTrustedVotes_.recordVotes(rules, valSet, lock);

// The way we compute votes changes based on fixAmendmentFlapping.
auto vote = rules.enabled(fixAmendmentFlapping)
? std::make_unique<AmendmentSet>(rules, previousTrustedVotes_, lock)
: std::make_unique<AmendmentSet>(rules, valSet);
JLOG(j_.debug()) << "Received " << vote->trustedValidations()
<< " trusted validations, threshold is: "
<< vote->threshold();
Expand All @@ -626,8 +767,6 @@ AmendmentTableImpl::doVoting(
// the value of the flags in the pseudo-transaction
std::map<uint256, std::uint32_t> actions;

std::lock_guard lock(mutex_);

// process all amendments we know of
for (auto const& entry : amendmentMap_)
{
Expand Down Expand Up @@ -723,6 +862,13 @@ AmendmentTableImpl::doValidatedLedger(
firstUnsupportedExpected_ = *firstUnsupportedExpected_ + majorityTime_;
}

void
AmendmentTableImpl::trustChanged(hash_set<PublicKey> const& allTrusted)
{
std::lock_guard lock(mutex_);
previousTrustedVotes_.trustChanged(allTrusted, lock);
}

void
AmendmentTableImpl::injectJson(
Json::Value& v,
Expand Down
3 changes: 2 additions & 1 deletion src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ namespace detail {
// Feature.cpp. Because it's only used to reserve storage, and determine how
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
// the actual number of amendments. A LogicError on startup will verify this.
static constexpr std::size_t numFeatures = 54;
static constexpr std::size_t numFeatures = 55;

/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated
Expand Down Expand Up @@ -341,6 +341,7 @@ extern uint256 const fixTrustLinesToSelf;
extern uint256 const fixRemoveNFTokenAutoTrustLine;
extern uint256 const featureImmediateOfferKilled;
extern uint256 const featureDisallowIncoming;
extern uint256 const fixAmendmentFlapping;

} // namespace ripple

Expand Down
1 change: 1 addition & 0 deletions src/ripple/protocol/impl/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ REGISTER_FIX (fixTrustLinesToSelf, Supported::yes, DefaultVote::no)
REGISTER_FIX (fixRemoveNFTokenAutoTrustLine, Supported::yes, DefaultVote::yes);
REGISTER_FEATURE(ImmediateOfferKilled, Supported::yes, DefaultVote::no);
REGISTER_FEATURE(DisallowIncoming, Supported::yes, DefaultVote::no);
REGISTER_FIX (fixAmendmentFlapping, Supported::yes, DefaultVote::no);

// The following amendments have been active for at least two years. Their
// pre-amendment code has been removed and the identifiers are deprecated.
Expand Down
Loading

0 comments on commit 09d94ca

Please sign in to comment.