diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index c959c01018a..30474f96fc6 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -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 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; } } @@ -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; } } diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index c749b733062..71b3cb0f193 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -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) { @@ -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; } }; @@ -1271,17 +1273,19 @@ bool NetworkOPsImp::checkLastClosedLedger ( hash_map ledgers; { - auto current = app_.getValidations ().currentTrustedDistribution ( - closedLedger, prevClosedLedger, - m_ledgerMaster.getValidLedgerIndex()); + hash_map 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; } } @@ -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) @@ -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; @@ -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; } } diff --git a/src/ripple/consensus/Validations.h b/src/ripple/consensus/Validations.h index 78bcd3761fd..fad007d403b 100644 --- a/src/ripple/consensus/Validations.h +++ b/src/ripple/consensus/Validations.h @@ -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 @@ -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 + hash_map currentTrustedDistribution( LedgerID const& currentLedger, LedgerID const& priorLedger, @@ -513,7 +507,7 @@ class Validations bool const valCurrentLedger = currentLedger != beast::zero; bool const valPriorLedger = priorLedger != beast::zero; - hash_map ret; + hash_map ret; current( stalePolicy_.now(), @@ -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()]++; } }); diff --git a/src/test/consensus/Validations_test.cpp b/src/test/consensus/Validations_test.cpp index 52878f752be..dfd6f1e6d07 100644 --- a/src/test/consensus/Validations_test.cpp +++ b/src/test/consensus/Validations_test.cpp @@ -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); } { @@ -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); } { @@ -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); } { @@ -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); } }