Skip to content

Commit

Permalink
Declare consensus if peers enter into a stable state
Browse files Browse the repository at this point in the history 
- Stable state means that neither we, nor any of our peers has changed
  a vote on a disputed transaction in a while. This is undesirable if an
  80% consensus has not otherwise been reached. It will cause
  a validation to be sent, which will help get other (trusting)
  validators back on track using preferred ledger logic.
  • Loading branch information
@ximinez
ximinez committed Feb 12, 2025
1 parent a201bfe commit a6d3cea
Show file tree
Hide file tree
Showing 7 changed files with 271 additions and 64 deletions.
101 changes: 93 additions & 8 deletions src/test/consensus/Consensus_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,47 +82,96 @@ class Consensus_test : public beast::unit_test::suite
// Use default parameterss
ConsensusParms const p{};

///////////////
// Disputes still in doubt
//
// Not enough time has elapsed
BEAST_EXPECT(
ConsensusState::No ==
checkConsensus(10, 2, 2, 0, 3s, 2s, p, true, journal_));
checkConsensus(10, 2, 2, 0, 3s, 2s, false, p, true, journal_));

// If not enough peers have propsed, ensure
// more time for proposals
BEAST_EXPECT(
ConsensusState::No ==
checkConsensus(10, 2, 2, 0, 3s, 4s, p, true, journal_));
checkConsensus(10, 2, 2, 0, 3s, 4s, false, p, true, journal_));

// Enough time has elapsed and we all agree
BEAST_EXPECT(
ConsensusState::Yes ==
checkConsensus(10, 2, 2, 0, 3s, 10s, p, true, journal_));
checkConsensus(10, 2, 2, 0, 3s, 10s, false, p, true, journal_));

// Enough time has elapsed and we don't yet agree
BEAST_EXPECT(
ConsensusState::No ==
checkConsensus(10, 2, 1, 0, 3s, 10s, p, true, journal_));
checkConsensus(10, 2, 1, 0, 3s, 10s, false, p, true, journal_));

// Our peers have moved on
// Enough time has elapsed and we all agree
BEAST_EXPECT(
ConsensusState::MovedOn ==
checkConsensus(10, 2, 1, 8, 3s, 10s, p, true, journal_));
checkConsensus(10, 2, 1, 8, 3s, 10s, false, p, true, journal_));

// If no peers, don't agree until time has passed.
BEAST_EXPECT(
ConsensusState::No ==
checkConsensus(0, 0, 0, 0, 3s, 10s, p, true, journal_));
checkConsensus(0, 0, 0, 0, 3s, 10s, false, p, true, journal_));

// Agree if no peers and enough time has passed.
BEAST_EXPECT(
ConsensusState::Yes ==
checkConsensus(0, 0, 0, 0, 3s, 16s, p, true, journal_));
checkConsensus(0, 0, 0, 0, 3s, 16s, false, p, true, journal_));

// Expire if too much time has passed without agreement
BEAST_EXPECT(
ConsensusState::Expired ==
checkConsensus(10, 8, 1, 0, 1s, 19s, p, true, journal_));
checkConsensus(10, 8, 1, 0, 1s, 19s, false, p, true, journal_));
///////////////
// Stable state
//
// Not enough time has elapsed
BEAST_EXPECT(
ConsensusState::No ==
checkConsensus(10, 2, 2, 0, 3s, 2s, true, p, true, journal_));

// If not enough peers have propsed, ensure
// more time for proposals
BEAST_EXPECT(
ConsensusState::No ==
checkConsensus(10, 2, 2, 0, 3s, 4s, true, p, true, journal_));

// Enough time has elapsed and we all agree
BEAST_EXPECT(
ConsensusState::Yes ==
checkConsensus(10, 2, 2, 0, 3s, 10s, true, p, true, journal_));

// Enough time has elapsed and we don't yet agree, but there's nothing
// left to dispute
BEAST_EXPECT(
ConsensusState::Yes ==
checkConsensus(10, 2, 1, 0, 3s, 10s, true, p, true, journal_));

// Our peers have moved on
// Enough time has elapsed and we all agree, nothing left to dispute
BEAST_EXPECT(
ConsensusState::Yes ==
checkConsensus(10, 2, 1, 8, 3s, 10s, true, p, true, journal_));

// If no peers, don't agree until time has passed.
BEAST_EXPECT(
ConsensusState::No ==
checkConsensus(0, 0, 0, 0, 3s, 10s, true, p, true, journal_));

// Agree if no peers and enough time has passed.
BEAST_EXPECT(
ConsensusState::Yes ==
checkConsensus(0, 0, 0, 0, 3s, 16s, true, p, true, journal_));

// We are done if there's nothing left to dispute, no matter how much
// time has passed
BEAST_EXPECT(
ConsensusState::Yes ==
checkConsensus(10, 8, 1, 0, 1s, 19s, true, p, true, journal_));
}

void
Expand Down Expand Up @@ -1057,6 +1106,41 @@ class Consensus_test : public beast::unit_test::suite
BEAST_EXPECT(sim.synchronized());
}

void
testDisputes()
{
using namespace csf;
using namespace std::chrono;

std::uint32_t numPeers = 35;

ConsensusParms const parms{};
Sim sim;

PeerGroup peers = sim.createGroup(numPeers);

SimDuration delay = round<milliseconds>(0.2 * parms.ledgerGRANULARITY);
peers.trustAndConnect(peers, delay);

// Initial round to set prior state
sim.run(1);
for (Peer* peer : peers)
{
// Every transaction will be seen by every node but two.
// To accomplish that, every peer will add the ids of every peer
// except itself, and the one following.
auto const myId = peer->id;
auto const nextId = (peer->id + PeerID(1)) % PeerID(numPeers);
for (Peer* to : sim.trustGraph.trustedPeers(peer))
{
if (to->id == myId || to->id == nextId)
continue;
peer->openTxs.insert(Tx{static_cast<std::uint32_t>(to->id)});
}
}
sim.run(1);
}

void
run() override
{
Expand All @@ -1073,6 +1157,7 @@ class Consensus_test : public beast::unit_test::suite
testHubNetwork();
testPreferredByBranch();
testPauseForLaggards();
testDisputes();
}
};

Expand Down
5 changes: 3 additions & 2 deletions src/xrpld/app/consensus/RCLValidations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,9 @@ handleNewValidation(
auto& validations = app.getValidations();

// masterKey is seated only if validator is trusted or listed
auto const outcome =
validations.add(calcNodeID(masterKey.value_or(signingKey)), val);
auto const nodeKey = masterKey.value_or(signingKey);
assert(nodeKey != app.getValidationPublicKey());
auto const outcome = validations.add(calcNodeID(nodeKey), val);

if (outcome == ValStatus::current)
{
Expand Down
15 changes: 10 additions & 5 deletions src/xrpld/consensus/Consensus.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ checkConsensusReached(
std::size_t total,
bool count_self,
std::size_t minConsensusPct,
bool reachedMax)
bool reachedMax,
bool stableState)
{
// If we are alone for too long, we have consensus.
// Delaying consensus like this avoids a circumstance where a peer
Expand All @@ -114,7 +115,7 @@ checkConsensusReached(

std::size_t currentPercentage = (agreeing * 100) / total;

return currentPercentage >= minConsensusPct;
return currentPercentage >= minConsensusPct || stableState;
}

ConsensusState
Expand All @@ -125,6 +126,7 @@ checkConsensus(
std::size_t currentFinished,
std::chrono::milliseconds previousAgreeTime,
std::chrono::milliseconds currentAgreeTime,
bool stableState,
ConsensusParms const& parms,
bool proposing,
beast::Journal j)
Expand Down Expand Up @@ -162,9 +164,11 @@ checkConsensus(
currentProposers,
proposing,
parms.minCONSENSUS_PCT,
currentAgreeTime > parms.ledgerMAX_CONSENSUS))
currentAgreeTime > parms.ledgerMAX_CONSENSUS,
stableState))
{
JLOG(j.debug()) << "normal consensus";
JLOG(j.debug()) << "normal consensus with " << (stableState ? "" : "un")
<< "stable state";
return ConsensusState::Yes;
}

Expand All @@ -175,7 +179,8 @@ checkConsensus(
currentProposers,
false,
parms.minCONSENSUS_PCT,
currentAgreeTime > parms.ledgerMAX_CONSENSUS))
currentAgreeTime > parms.ledgerMAX_CONSENSUS,
false))
{
JLOG(j.warn()) << "We see no consensus, but 80% of nodes have moved on";
return ConsensusState::MovedOn;
Expand Down
51 changes: 36 additions & 15 deletions src/xrpld/consensus/Consensus.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ shouldCloseLedger(
last ledger
@param currentAgreeTime how long, in milliseconds, we've been trying to
agree
@param stableState the network appears to be in a stable state, where
neither we nor our peers have changed their vote on any disputes in a
while. This is undesirable, and will cause us to end consensus
without 80% agreement.
@param parms Consensus constant parameters
@param proposing whether we should count ourselves
@param j journal for logging
Expand All @@ -91,6 +95,7 @@ checkConsensus(
std::size_t currentFinished,
std::chrono::milliseconds previousAgreeTime,
std::chrono::milliseconds currentAgreeTime,
bool stableState,
ConsensusParms const& parms,
bool proposing,
beast::Journal j);
Expand Down Expand Up @@ -559,6 +564,9 @@ class Consensus

NetClock::duration closeResolution_ = ledgerDefaultTimeResolution;

ConsensusParms::AvalancheState closeTimeAvalancheState_ =
ConsensusParms::init;

// Time it took for the last consensus round to converge
std::chrono::milliseconds prevRoundTime_;

Expand Down Expand Up @@ -676,6 +684,7 @@ Consensus<Adaptor>::startRoundInternal(
previousLedger_ = prevLedger;
result_.reset();
convergePercent_ = 0;
closeTimeAvalancheState_ = ConsensusParms::init;
haveCloseTimeConsensus_ = false;
openTime_.reset(clock_.now());
currPeerPositions_.clear();
Expand Down Expand Up @@ -832,6 +841,8 @@ Consensus<Adaptor>::timerEntry(NetClock::time_point const& now)
}
else if (phase_ == ConsensusPhase::establish)
{
if (result_)
++result_->peerUnchangedCounter;
phaseEstablish();
}
}
Expand Down Expand Up @@ -1443,16 +1454,10 @@ Consensus<Adaptor>::updateOurPositions()
}
else
{
int neededWeight;

if (convergePercent_ < parms.avMID_CONSENSUS_TIME)
neededWeight = parms.avINIT_CONSENSUS_PCT;
else if (convergePercent_ < parms.avLATE_CONSENSUS_TIME)
neededWeight = parms.avMID_CONSENSUS_PCT;
else if (convergePercent_ < parms.avSTUCK_CONSENSUS_TIME)
neededWeight = parms.avLATE_CONSENSUS_PCT;
else
neededWeight = parms.avSTUCK_CONSENSUS_PCT;
auto const [neededWeight, newState] = getNeededWeight(
parms, closeTimeAvalancheState_, convergePercent_, {});
if (newState)
closeTimeAvalancheState_ = *newState;

int participants = currPeerPositions_.size();
if (mode_.get() == ConsensusMode::proposing)
Expand Down Expand Up @@ -1566,7 +1571,8 @@ Consensus<Adaptor>::haveConsensus()
}
else
{
JLOG(j_.debug()) << nodeId << " has " << peerProp.position();
JLOG(j_.debug()) << "Proposal disagreement: Peer " << nodeId
<< " has " << peerProp.position();
++disagree;
}
}
Expand All @@ -1576,6 +1582,18 @@ Consensus<Adaptor>::haveConsensus()
JLOG(j_.debug()) << "Checking for TX consensus: agree=" << agree
<< ", disagree=" << disagree;

ConsensusParms const& parms = adaptor_.parms();
// stable state is NOT desirable if we don't have 80% agreement
bool stableState = true;
for (auto const& [txid, dt] : result_->disputes)
{
if (!dt.stableState(parms, result_->peerUnchangedCounter))
{
stableState = false;
break;
}
}

// Determine if we actually have consensus or not
result_->state = checkConsensus(
prevProposers_,
Expand All @@ -1584,7 +1602,8 @@ Consensus<Adaptor>::haveConsensus()
currentFinished,
prevRoundTime_,
result_->roundTime.read(),
adaptor_.parms(),
stableState,
parms,
mode_.get() == ConsensusMode::proposing,
j_);

Expand Down Expand Up @@ -1676,8 +1695,9 @@ Consensus<Adaptor>::createDisputes(TxSet_t const& o)
{
Proposal_t const& peerProp = peerPos.proposal();
auto const cit = acquired_.find(peerProp.position());
if (cit != acquired_.end())
dtx.setVote(nodeId, cit->second.exists(txID));
if (cit != acquired_.end() &&
dtx.setVote(nodeId, cit->second.exists(txID)))
result_->peerUnchangedCounter = 0;
}
adaptor_.share(dtx.tx());

Expand All @@ -1701,7 +1721,8 @@ Consensus<Adaptor>::updateDisputes(NodeID_t const& node, TxSet_t const& other)
for (auto& it : result_->disputes)
{
auto& d = it.second;
d.setVote(node, other.exists(d.tx().id()));
if (d.setVote(node, other.exists(d.tx().id())))
result_->peerUnchangedCounter = 0;
}
}

Expand Down
Loading

0 comments on commit a6d3cea

Please sign in to comment.