Skip to content

Commit

Permalink
Use ledger hash to break ties (RIPD-1468):
Browse files Browse the repository at this point in the history 
When two ledgers have the same number of validations, the code will now use the
ledger hash itself to break the tie rather than the highest node ID supporting
each validation.
  • Loading branch information
@bachase
bachase committed Jul 21, 2017
1 parent 4308b12 commit 1ef6605
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 56 deletions.
14 changes: 7 additions & 7 deletions src/ripple/app/consensus/RCLConsensus.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,21 +242,21 @@ RCLConsensus::Adaptor::getPrevLedger(

// Get validators that are on our ledger, or "close" to being on
// our ledger.
auto vals =
hash_map<uint256, std::uint32_t> ledgerCounts =
app_.getValidations().currentTrustedDistribution(
ledgerID, parentID, ledgerMaster_.getValidLedgerIndex());

uint256 netLgr = ledgerID;
int netLgrCount = 0;
for (auto& it : vals)
for (auto const & it : ledgerCounts)
{
// Switch to ledger supported by more peers
// Or stick with ours on a tie
if ((it.second.count > netLgrCount) ||
((it.second.count== netLgrCount) && (it.first == ledgerID)))
if ((it.second > netLgrCount) ||
((it.second== netLgrCount) && (it.first == ledgerID)))
{
netLgr = it.first;
netLgrCount = it.second.count;
netLgrCount = it.second;
}
}

Expand All @@ -267,8 +267,8 @@ RCLConsensus::Adaptor::getPrevLedger(

if (auto stream = j_.debug())
{
for (auto& it : vals)
stream << "V: " << it.first << ", " << it.second.count;
for (auto const & it : ledgerCounts)
stream << "V: " << it.first << ", " << it.second;
}
}

Expand Down
38 changes: 20 additions & 18 deletions src/ripple/app/misc/NetworkOPs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1220,7 +1220,9 @@ class ValidationCount
{
public:
int trustedValidations, nodesUsing;
NodeID highNodeUsing, highValidation;
// Ledger hashes to break ties
uint256 highUsingHash, highTrustedValHash;


ValidationCount () : trustedValidations (0), nodesUsing (0)
{
Expand All @@ -1242,10 +1244,10 @@ class ValidationCount
if (nodesUsing < v.nodesUsing)
return false;

return highNodeUsing > v.highNodeUsing;
return highUsingHash > v.highUsingHash;
}

return highValidation > v.highValidation;
return highTrustedValHash > v.highTrustedValHash;
}
};

Expand All @@ -1271,17 +1273,19 @@ bool NetworkOPsImp::checkLastClosedLedger (

hash_map<uint256, ValidationCount> ledgers;
{
auto current = app_.getValidations ().currentTrustedDistribution (
closedLedger, prevClosedLedger,
m_ledgerMaster.getValidLedgerIndex());
hash_map<uint256, std::uint32_t> current =
app_.getValidations().currentTrustedDistribution(
closedLedger,
prevClosedLedger,
m_ledgerMaster.getValidLedgerIndex());

for (auto& it: current)
{
auto& vc = ledgers[it.first];
vc.trustedValidations += it.second.count;
vc.trustedValidations += it.second;

if (it.second.highNode > vc.highValidation)
vc.highValidation = it.second.highNode;
if (it.first > vc.highTrustedValHash)
vc.highTrustedValHash = it.first;
}
}

Expand All @@ -1290,10 +1294,9 @@ bool NetworkOPsImp::checkLastClosedLedger (
if (mMode >= omTRACKING)
{
++ourVC.nodesUsing;
auto const ourNodeID = calcNodeID(
app_.nodeIdentity().first);
if (ourNodeID > ourVC.highNodeUsing)
ourVC.highNodeUsing = ourNodeID;

if (closedLedger > ourVC.highUsingHash)
ourVC.highUsingHash = closedLedger;
}

for (auto& peer: peerList)
Expand All @@ -1305,10 +1308,9 @@ bool NetworkOPsImp::checkLastClosedLedger (
try
{
auto& vc = ledgers[peerLedger];
auto const nodeId = calcNodeID(peer->getNodePublic ());
if (vc.nodesUsing == 0 || nodeId > vc.highNodeUsing)
if (vc.nodesUsing == 0 || peerLedger > vc.highUsingHash)
{
vc.highNodeUsing = nodeId;
vc.highUsingHash = peerLedger;
}

++vc.nodesUsing;
Expand All @@ -1335,13 +1337,13 @@ bool NetworkOPsImp::checkLastClosedLedger (
// Temporary logging to make sure tiebreaking isn't broken
if (it.second.trustedValidations > 0)
JLOG(m_journal.trace())
<< " TieBreakTV: " << it.second.highValidation;
<< " TieBreakTV: " << it.second.highTrustedValHash;
else
{
if (it.second.nodesUsing > 0)
{
JLOG(m_journal.trace())
<< " TieBreakNU: " << it.second.highNodeUsing;
<< " TieBreakNU: " << it.second.highUsingHash;
}
}

Expand Down
25 changes: 8 additions & 17 deletions src/ripple/consensus/Validations.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,14 +487,6 @@ class Validations
beast::expire(byLedger_, parms_.validationSET_EXPIRES);
}

struct ValidationCounts
{
//! The number of trusted validations
std::size_t count;
//! The highest trusted node ID
NodeID highNode;
};

/** Distribution of current trusted validations

Calculates the distribution of current validations but allows
Expand All @@ -503,8 +495,10 @@ class Validations
@param currentLedger The identifier of the ledger we believe is current
@param priorLedger The identifier of our previous current ledger
@param cutoffBefore Ignore ledgers with sequence number before this

@return Map representing the distribution of ledgerID by count
*/
hash_map<LedgerID, ValidationCounts>
hash_map<LedgerID, std::uint32_t>
currentTrustedDistribution(
LedgerID const& currentLedger,
LedgerID const& priorLedger,
Expand All @@ -513,7 +507,7 @@ class Validations
bool const valCurrentLedger = currentLedger != beast::zero;
bool const valPriorLedger = priorLedger != beast::zero;

hash_map<LedgerID, ValidationCounts> ret;
hash_map<LedgerID, std::uint32_t> ret;

current(
stalePolicy_.now(),
Expand Down Expand Up @@ -543,13 +537,10 @@ class Validations
<< " not " << v.ledgerID();
}

ValidationCounts& p =
countPreferred ? ret[currentLedger] : ret[v.ledgerID()];
++(p.count);

NodeID const ni = v.nodeID();
if (ni > p.highNode)
p.highNode = ni;
if (countPreferred)
ret[currentLedger]++;
else
ret[v.ledgerID()]++;
}
});

Expand Down
21 changes: 7 additions & 14 deletions src/test/consensus/Validations_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -761,8 +761,7 @@ class Validations_test : public beast::unit_test::suite
Seq{0}); // No cutoff

BEAST_EXPECT(res.size() == 1);
BEAST_EXPECT(res[ID{2}].count == 3);
BEAST_EXPECT(res[ID{2}].highNode == mama.nodeID());
BEAST_EXPECT(res[ID{2}] == 3);
}

{
Expand All @@ -773,10 +772,8 @@ class Validations_test : public beast::unit_test::suite
Seq{0}); // No cutoff

BEAST_EXPECT(res.size() == 2);
BEAST_EXPECT(res[ID{2}].count == 2);
BEAST_EXPECT(res[ID{2}].highNode == mama.nodeID());
BEAST_EXPECT(res[ID{1}].count == 1);
BEAST_EXPECT(res[ID{1}].highNode == papa.nodeID());
BEAST_EXPECT(res[ID{2}] == 2);
BEAST_EXPECT(res[ID{1}] == 1);
}

{
Expand All @@ -787,12 +784,9 @@ class Validations_test : public beast::unit_test::suite
Seq{0}); // No cutoff

BEAST_EXPECT(res.size() == 3);
BEAST_EXPECT(res[ID{1}].count == 1);
BEAST_EXPECT(res[ID{1}].highNode == papa.nodeID());
BEAST_EXPECT(res[ID{2}].count == 1);
BEAST_EXPECT(res[ID{2}].highNode == baby.nodeID());
BEAST_EXPECT(res[ID{3}].count == 1);
BEAST_EXPECT(res[ID{3}].highNode == mama.nodeID());
BEAST_EXPECT(res[ID{1}] == 1);
BEAST_EXPECT(res[ID{2}] == 1);
BEAST_EXPECT(res[ID{3}] == 1);
}

{
Expand All @@ -802,8 +796,7 @@ class Validations_test : public beast::unit_test::suite
ID{1}, // prior ledger
Seq{2}); // Only sequence 2 or later
BEAST_EXPECT(res.size() == 1);
BEAST_EXPECT(res[ID{2}].count == 2);
BEAST_EXPECT(res[ID{2}].highNode == mama.nodeID());
BEAST_EXPECT(res[ID{2}] == 2);
}
}

Expand Down

0 comments on commit 1ef6605

Please sign in to comment.