Use ledger hash to break ties (RIPD-1468)

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<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;
         }
     }
 
@@ -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;
         }
     }
 

src/ripple/app/consensus/RCLValidations.h

@@ -99,27 +99,6 @@ class RCLValidation
         return val_->isTrusted();
     }
 
-    /// Set the prior ledger hash this validation is following
-    void
-    setPreviousLedgerID(uint256 const& hash)
-    {
-        val_->setPreviousHash(hash);
-    }
-
-    /// Get the prior ledger hash this validation is following
-    uint256
-    getPreviousLedgerID() const
-    {
-        return val_->getPreviousHash();
-    }
-
-    /// Check whether the given hash matches this validation's prior hash
-    bool
-    isPreviousLedgerID(uint256 const& hash) const
-    {
-        return val_->isPreviousHash(hash);
-    }
-
     /// Get the load fee of the validation if it exists
     boost::optional<std::uint32_t>
     loadFee() const

src/ripple/app/misc/NetworkOPs.cpp

@@ -1262,39 +1262,6 @@ void NetworkOPsImp::setAmendmentBlocked ()
     setMode (omTRACKING);
 }
 
-class ValidationCount
-{
-public:
-    int trustedValidations, nodesUsing;
-    NodeID highNodeUsing, highValidation;
-
-    ValidationCount () : trustedValidations (0), nodesUsing (0)
-    {
-    }
-
-    bool operator> (const ValidationCount& v) const
-    {
-        if (trustedValidations > v.trustedValidations)
-            return true;
-
-        if (trustedValidations < v.trustedValidations)
-            return false;
-
-        if (trustedValidations == 0)
-        {
-            if (nodesUsing > v.nodesUsing)
-                return true;
-
-            if (nodesUsing < v.nodesUsing)
-                return false;
-
-            return highNodeUsing > v.highNodeUsing;
-        }
-
-        return highValidation > v.highValidation;
-    }
-};
-
 bool NetworkOPsImp::checkLastClosedLedger (
     const Overlay::PeerSequence& peerList, uint256& networkClosed)
 {
@@ -1315,55 +1282,58 @@ bool NetworkOPsImp::checkLastClosedLedger (
     JLOG(m_journal.trace()) << "OurClosed:  " << closedLedger;
     JLOG(m_journal.trace()) << "PrevClosed: " << prevClosedLedger;
 
-    hash_map<uint256, ValidationCount> ledgers;
+    struct ValidationCount
     {
-        auto current = app_.getValidations ().currentTrustedDistribution (
-            closedLedger, prevClosedLedger,
-            m_ledgerMaster.getValidLedgerIndex());
+        int trustedValidations, nodesUsing;
 
-        for (auto& it: current)
+        ValidationCount() : trustedValidations(0), nodesUsing(0)
+        {
+        }
+
+        auto
+        asTie() const
+        {
+            return std::tie(trustedValidations, nodesUsing);
+        }
+
+        bool
+        operator>(const ValidationCount& v) const
         {
-            auto& vc = ledgers[it.first];
-            vc.trustedValidations += it.second.count;
+            return asTie() > v.asTie();
+        }
 
-            if (it.second.highNode > vc.highValidation)
-                vc.highValidation = it.second.highNode;
+        bool
+        operator==(const ValidationCount& v) const
+        {
+            return asTie() == v.asTie();
         }
+    };
+
+    hash_map<uint256, ValidationCount> ledgers;
+    {
+        hash_map<uint256, std::uint32_t> current =
+            app_.getValidations().currentTrustedDistribution(
+                closedLedger,
+                prevClosedLedger,
+                m_ledgerMaster.getValidLedgerIndex());
+
+        for (auto& it: current)
+            ledgers[it.first].trustedValidations += it.second;
     }
 
     auto& ourVC = ledgers[closedLedger];
 
     if (mMode >= omTRACKING)
     {
         ++ourVC.nodesUsing;
-        auto const ourNodeID = calcNodeID(
-            app_.nodeIdentity().first);
-        if (ourNodeID > ourVC.highNodeUsing)
-            ourVC.highNodeUsing = ourNodeID;
     }
 
     for (auto& peer: peerList)
     {
         uint256 peerLedger = peer->getClosedLedgerHash ();
 
         if (peerLedger.isNonZero ())
-        {
-            try
-            {
-                auto& vc = ledgers[peerLedger];
-                auto const nodeId = calcNodeID(peer->getNodePublic ());
-                if (vc.nodesUsing == 0 || nodeId > vc.highNodeUsing)
-                {
-                    vc.highNodeUsing = nodeId;
-                }
-
-                ++vc.nodesUsing;
-            }
-            catch (std::exception const&)
-            {
-                // Peer is likely not connected anymore
-            }
-        }
+            ++ledgers[peerLedger].nodesUsing;
     }
 
     auto bestVC = ledgers[closedLedger];
@@ -1381,17 +1351,20 @@ 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.first;
         else
         {
             if (it.second.nodesUsing > 0)
             {
                 JLOG(m_journal.trace())
-                    << "  TieBreakNU: " << it.second.highNodeUsing;
+                    << "  TieBreakNU: " << it.first;
             }
         }
 
-        if (it.second > bestVC)
+        // Switch to a ledger with more support
+        // or the one with higher hash if they have the same support
+        if (it.second > bestVC ||
+            (it.second == bestVC && it.first > closedLedger))
         {
             bestVC = it.second;
             closedLedger = it.first;