Properly use ledger hash to break ties

src/ripple/app/consensus/RCLConsensus.cpp

@@ -381,7 +381,7 @@ RCLConsensus::Adaptor::onAccept(
     app_.getJobQueue().addJob(
         jtACCEPT,
         "acceptLedger",
-        [&, cj = std::move(consensusJson) ](auto&) mutable {
+        [=, cj = std::move(consensusJson) ](auto&) mutable {
             // Note that no lock is held or acquired during this job.
             // This is because generic Consensus guarantees that once a ledger
             // is accepted, the consensus results and capture by reference state
@@ -876,6 +876,16 @@ RCLConsensus::Adaptor::validate(RCLCxLedger const& ledger, bool proposing)
     app_.overlay().send(val);
 }
 
+void
+RCLConsensus::Adaptor::onModeChange(
+    ConsensusMode before,
+    ConsensusMode after)
+{
+    JLOG(j_.info()) << "Consensus mode change before=" << to_string(before)
+                    << ", after=" << to_string(after);
+    mode_ = after;
+}
+
 Json::Value
 RCLConsensus::getJson(bool full) const
 {
@@ -950,14 +960,18 @@ RCLConsensus::Adaptor::preStartRound(RCLCxLedger const & prevLgr)
                   prevLgr.seq() >= app_.getMaxDisallowedLedger() &&
                   !app_.getOPs().isAmendmentBlocked();
 
+    const bool synced = app_.getOPs().getOperatingMode() == NetworkOPs::omFULL;
+
     if (validating_)
     {
-        JLOG(j_.info()) << "Entering consensus process, validating";
+        JLOG(j_.info()) << "Entering consensus process, validating, synced="
+                        << (synced ? "yes" : "no");
     }
     else
     {
         // Otherwise we just want to monitor the validation process.
-        JLOG(j_.info()) << "Entering consensus process, watching";
+        JLOG(j_.info()) << "Entering consensus process, watching, synced="
+                        << (synced ? "yes" : "no");
     }
 
     // Notify inbound ledgers that we are starting a new round
@@ -967,8 +981,7 @@ RCLConsensus::Adaptor::preStartRound(RCLCxLedger const & prevLgr)
     parms_.useRoundedCloseTime = prevLgr.ledger_->rules().enabled(fix1528);
 
     // propose only if we're in sync with the network (and validating)
-    return validating_ &&
-        (app_.getOPs().getOperatingMode() == NetworkOPs::omFULL);
+    return validating_ && synced;
 }
 
 void

src/ripple/app/consensus/RCLConsensus.h

@@ -239,11 +239,15 @@ class RCLConsensus
             RCLCxLedger const& ledger,
             ConsensusMode mode);
 
+        /** Notified of change in consensus mode
+
+            @param before The prior consensus mode
+            @param after The new consensus mode
+        */
         void
-        onModeChange(ConsensusMode before, ConsensusMode after)
-        {
-            mode_ = after;
-        }
+        onModeChange(
+            ConsensusMode before,
+            ConsensusMode after);
 
         /** Close the open ledger and return initial consensus position.
 

src/ripple/app/misc/NetworkOPs.cpp

@@ -1275,29 +1275,8 @@ bool NetworkOPsImp::checkLastClosedLedger (
 
     struct ValidationCount
     {
-        int trustedValidations, nodesUsing;
-
-        ValidationCount() : trustedValidations(0), nodesUsing(0)
-        {
-        }
-
-        auto
-        asTie() const
-        {
-            return std::tie(trustedValidations, nodesUsing);
-        }
-
-        bool
-        operator>(const ValidationCount& v) const
-        {
-            return asTie() > v.asTie();
-        }
-
-        bool
-        operator==(const ValidationCount& v) const
-        {
-            return asTie() == v.asTie();
-        }
+        std::uint32_t trustedValidations = 0;
+        std::uint32_t nodesUsing = 0;
     };
 
     hash_map<uint256, ValidationCount> ledgers;
@@ -1327,38 +1306,47 @@ bool NetworkOPsImp::checkLastClosedLedger (
             ++ledgers[peerLedger].nodesUsing;
     }
 
-    auto bestVC = ledgers[closedLedger];
 
     // 3) Is there a network ledger we'd like to switch to? If so, do we have
     // it?
     bool switchLedgers = false;
+    ValidationCount bestCounts = ledgers[closedLedger];
 
     for (auto const& it: ledgers)
     {
-        JLOG(m_journal.debug()) << "L: " << it.first
-                              << " t=" << it.second.trustedValidations
-                              << ", n=" << it.second.nodesUsing;
+        uint256 const & currLedger =  it.first;
+        ValidationCount const & currCounts = it.second;
 
-        // Temporary logging to make sure tiebreaking isn't broken
-        if (it.second.trustedValidations > 0)
-            JLOG(m_journal.trace())
-                << "  TieBreakTV: " << it.first;
-        else
+        JLOG(m_journal.debug()) << "L: " << currLedger
+                              << " t=" << currCounts.trustedValidations
+                              << ", n=" << currCounts.nodesUsing;
+
+        bool const preferCurr = [&]()
         {
-            if (it.second.nodesUsing > 0)
+            // Prefer ledger with more trustedValidations
+            if (currCounts.trustedValidations > bestCounts.trustedValidations)
+                return true;
+            if (currCounts.trustedValidations < bestCounts.trustedValidations)
+                return false;
+            // If neither are trusted, prefer more nodesUsing
+            if (currCounts.trustedValidations == 0)
             {
-                JLOG(m_journal.trace())
-                    << "  TieBreakNU: " << it.first;
+                if (currCounts.nodesUsing > bestCounts.nodesUsing)
+                    return true;
+                if (currCounts.nodesUsing < bestCounts.nodesUsing)
+                    return false;
             }
-        }
+            // If tied trustedValidations (non-zero) or tied nodesUsing,
+            // prefer higher ledger hash
+            return currLedger > closedLedger;
+
+        }();
 
-        // 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))
+        // Switch to current ledger if it is preferred over best so far
+        if (preferCurr)
         {
-            bestVC = it.second;
-            closedLedger = it.first;
+            bestCounts = currCounts;
+            closedLedger = currLedger;
             switchLedgers = true;
         }
     }

src/ripple/overlay/impl/PeerImp.cpp

@@ -1305,7 +1305,7 @@ PeerImp::onMessage (std::shared_ptr <protocol::TMStatusChange> const& m)
     {
         if (!closedLedgerHash_.isZero ())
         {
-            JLOG(p_journal_.trace()) << "Status: Out of sync";
+            JLOG(p_journal_.debug()) << "Status: Out of sync";
             closedLedgerHash_.zero ();
         }
 
@@ -1318,11 +1318,11 @@ PeerImp::onMessage (std::shared_ptr <protocol::TMStatusChange> const& m)
         // a peer has changed ledgers
         memcpy (closedLedgerHash_.begin (), m->ledgerhash ().data (), 256 / 8);
         addLedger (closedLedgerHash_);
-        JLOG(p_journal_.trace()) << "LCL is " << closedLedgerHash_;
+        JLOG(p_journal_.debug()) << "LCL is " << closedLedgerHash_;
     }
     else
     {
-        JLOG(p_journal_.trace()) << "Status: No ledger";
+        JLOG(p_journal_.debug()) << "Status: No ledger";
         closedLedgerHash_.zero ();
     }