Clean up consensus logic:

* Simplify bow-out handling * Improve dispute updating * Filter partial validations
XRPLF · Jun 29, 2017 · a89be5b · a89be5b
1 parent 7b0d482
commit a89be5b
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 26 deletions.
diff --git a/src/ripple/app/misc/Validations.cpp b/src/ripple/app/misc/Validations.cpp
@@ -96,6 +96,19 @@ class ValidationsImp : public Validations
         if (!val->isTrusted() && pubKey)
             val->setTrusted();
 
+        // Do not process partial validations.
+        if(!val->isFull())
+        {
+            JLOG (j_.debug()) <<
+            "Val (partial) for " << hash <<
+            " from " << toBase58 (TokenType::TOKEN_NODE_PUBLIC, signer) <<
+            " ignored " << (val->isTrusted () ? "trusted/" : "UNtrusted/") <<
+            (isCurrent ? "current" : "stale");
+
+            // Only forward if current
+            return isCurrent && val->isTrusted();
+        }
+
         if (!val->isTrusted ())
         {
             JLOG (j_.trace()) <<

diff --git a/src/ripple/consensus/Consensus.h b/src/ripple/consensus/Consensus.h
@@ -371,7 +371,7 @@ class Consensus
     startRound(
         NetClock::time_point const& now,
         typename Ledger_t::ID const& prevLedgerID,
-        Ledger_t const& prevLedger,
+        Ledger_t prevLedger,
         bool proposing);
 
     /** A peer has proposed a new position, adjust our tracking.
@@ -638,7 +638,7 @@ void
 Consensus<Derived, Traits>::startRound(
     NetClock::time_point const& now,
     typename Ledger_t::ID const& prevLedgerID,
-    Ledger_t const& prevLedger,
+    Ledger_t prevLedger,
     bool proposing)
 {
     std::lock_guard<std::recursive_mutex> _(*lock_);
@@ -653,11 +653,31 @@ Consensus<Derived, Traits>::startRound(
     {
         prevCloseTime_ = rawCloseTimes_.self;
     }
+
+    Mode startMode = proposing ? Mode::proposing : Mode::observing;
+
+    // We were handed the wrong ledger
+    if (prevLedger.id() != prevLedgerID)
+    {
+        // try to acquire the correct one
+        if(auto newLedger = impl().acquireLedger(prevLedgerID))
+        {
+            prevLedger = *newLedger;
+        }
+        else // Unable to acquire the correct ledger
+        {
+            startMode = Mode::wrongLedger;
+            JLOG(j_.info())
+                << "Entering consensus with: " << previousLedger_.id();
+            JLOG(j_.info()) << "Correct LCL is: " << prevLedgerID;
+        }
+    }
+
     startRoundInternal(
         now,
         prevLedgerID,
         prevLedger,
-        proposing ? Mode::proposing : Mode::observing);
+        startMode);
 }
 template <class Derived, class Traits>
 void
@@ -687,19 +707,6 @@ Consensus<Derived, Traits>::startRoundInternal(
         previousLedger_.closeAgree(),
         previousLedger_.seq() + 1);
 
-    if (previousLedger_.id() != prevLedgerID_)
-    {
-        handleWrongLedger(prevLedgerID_);
-
-        // Unable to acquire the correct ledger
-        if (mode_ == Mode::wrongLedger)
-        {
-            JLOG(j_.info())
-                << "Entering consensus with: " << previousLedger_.id();
-            JLOG(j_.info()) << "Correct LCL is: " << prevLedgerID;
-        }
-    }
-
     playbackProposals();
     if (peerProposals_.size() > (prevProposers_ / 2))
     {
@@ -1005,14 +1012,15 @@ Consensus<Derived, Traits>::handleWrongLedger(
 {
     assert(lgrId != prevLedgerID_ || previousLedger_.id() != lgrId);
 
+    // Stop proposing because we are out of sync
+    leaveConsensus();
+
+    // First time switching to this ledger
     if (prevLedgerID_ != lgrId)
     {
-        // first time switching to this ledger
         prevLedgerID_ = lgrId;
 
-        // Stop proposing because we are out of sync
-        leaveConsensus();
-
+        // Clear out state
         if (result_)
         {
             result_->disputes.clear();
@@ -1382,16 +1390,26 @@ Consensus<Derived, Traits>::updateOurPositions()
                         << ", tx " << newID;
 
         result_->position.changePosition(newID, consensusCloseTime, now_);
-        if (!result_->position.isBowOut())
+
+        // Share our new transaction set and update disputes
+        // if we haven't already received it
+        if (acquired_.emplace(newID, result_->set).second)
         {
-            // Share our new transaction set if we haven't already received
-            // it from a peer
-            if (acquired_.emplace(newID, result_->set).second)
+            if (!result_->position.isBowOut())
                 impl().relay(result_->set);
 
-            if (mode_ == Mode::proposing)
-                impl().propose(result_->position);
+            for (auto const& p : peerProposals_)
+            {
+                if (p.second.position() == newID)
+                {
+                    updateDisputes(p.first, result_->set);
+                }
+            }
         }
+
+        // Share our new position if we are still participating this round
+        if (!result_->position.isBowOut() && (mode_ == Mode::proposing))
+            impl().propose(result_->position);
     }
 }