Distinguish ledger requests needed for preferred ledger analysis

ximinez · Oct 15, 2024 · 527ba85 · 527ba85
1 parent 5e9662e
commit 527ba85
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 30 deletions.
diff --git a/src/xrpld/app/consensus/RCLValidations.cpp b/src/xrpld/app/consensus/RCLValidations.cpp
@@ -146,7 +146,7 @@ RCLValidationsAdaptor::acquire(LedgerHash const& hash)
                 JLOG(j_.debug())
                     << "JOB advanceLedger getConsensusLedger2 started";
                 pApp->getInboundLedgers().acquireAsync(
-                    hash, 0, InboundLedger::Reason::CONSENSUS);
+                    hash, 0, InboundLedger::Reason::PREFERRED);
             });
         return std::nullopt;
     }

diff --git a/src/xrpld/app/ledger/InboundLedger.h b/src/xrpld/app/ledger/InboundLedger.h
@@ -41,9 +41,10 @@ class InboundLedger final : public TimeoutCounter,
 
     // These are the reasons we might acquire a ledger
     enum class Reason {
-        HISTORY,   // Acquiring past ledger
-        GENERIC,   // Generic other reasons
-        CONSENSUS  // We believe the consensus round requires this ledger
+        HISTORY,    // Acquiring past ledger
+        GENERIC,    // Generic other reasons
+        CONSENSUS,  // We believe the consensus round requires this ledger
+        PREFERRED   // We need this ledger for preferred ledger analysis
     };
 
     InboundLedger(
@@ -198,23 +199,8 @@ class InboundLedger final : public TimeoutCounter,
     std::unique_ptr<PeerSet> mPeerSet;
 };
 
-inline std::string
-to_string(InboundLedger::Reason reason)
-{
-    using enum InboundLedger::Reason;
-    switch (reason)
-    {
-        case HISTORY:
-            return "HISTORY";
-        case GENERIC:
-            return "GENERIC";
-        case CONSENSUS:
-            return "CONSENSUS";
-        default:
-            assert(false);
-            return "unknown";
-    }
-}
+std::string
+to_string(InboundLedger::Reason reason);
 
 }  // namespace ripple
 

diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp
@@ -73,6 +73,26 @@ enum {
 // millisecond for each ledger timeout
 auto constexpr ledgerAcquireTimeout = 3000ms;
 
+std::string
+to_string(InboundLedger::Reason reason)
+{
+    using enum InboundLedger::Reason;
+    switch (reason)
+    {
+        case HISTORY:
+            return "HISTORY";
+        case GENERIC:
+            return "GENERIC";
+        case CONSENSUS:
+            return "CONSENSUS";
+        case PREFERRED:
+            return "PREFERRED";
+        default:
+            assert(false);
+            return "unknown";
+    }
+}
+
 InboundLedger::InboundLedger(
     Application& app,
     uint256 const& hash,
@@ -141,7 +161,7 @@ InboundLedger::init(ScopedLockType& collectionLock, bool broadcast)
     app_.getLedgerMaster().storeLedger(mLedger);
 
     // Check if this could be a newer fully-validated ledger
-    if (mReason == Reason::CONSENSUS)
+    if (mReason >= Reason::CONSENSUS)
         app_.getLedgerMaster().checkAccept(mLedger);
 }
 

diff --git a/src/xrpld/app/ledger/detail/InboundLedgers.cpp b/src/xrpld/app/ledger/detail/InboundLedgers.cpp
@@ -80,15 +80,10 @@ class InboundLedgersImp : public InboundLedgers
                     return true;
                 if (reason == InboundLedger::Reason::GENERIC)
                     return true;
-                if (reason == InboundLedger::Reason::CONSENSUS)
+                if (reason >= InboundLedger::Reason::CONSENSUS)
                     return true;
                 return false;
             }();
-            assert(
-                shouldAcquire ==
-                !(app_.getOPs().isNeedNetworkLedger() &&
-                  (reason != InboundLedger::Reason::GENERIC) &&
-                  (reason != InboundLedger::Reason::CONSENSUS)));
 
             std::stringstream ss;
             ss << "InboundLedger::acquire: "
@@ -121,6 +116,11 @@ class InboundLedgersImp : public InboundLedgers
                 // ledger interval has passed, so the node is beginning to
                 // fall behind.
                 bool const fallingBehind = app_.getOPs().isFallingBehind();
+                // If the ledger is needed for preferred ledger analysis and we
+                // don't have it, chances are we're not going to build it,
+                // because someone else has built it, so download it.
+                bool const preferred =
+                    reason == InboundLedger::Reason::PREFERRED;
                 // If everything else is ok, don't try to acquire the ledger
                 // if the requested seq is in the near future relative to
                 // the validated ledger. Because validations lag behind
@@ -141,8 +141,9 @@ class InboundLedgersImp : public InboundLedgers
                 ss << " Evaluating whether to broadcast requests to peers"
                    << ". full: " << (isFull ? "true" : "false")
                    << ". falling behind: " << (fallingBehind ? "true" : "false")
-                   << ". ledger sequence " << seq
-                   << ". Valid sequence: " << validSeq
+                   << ". needed for preferred ledger analysis: "
+                   << (preferred ? "true" : "false") << ". ledger sequence "
+                   << seq << ". Valid sequence: " << validSeq
                    << ". Lag leeway: " << lagLeeway
                    << ". request for near future ledger: "
                    << (nearFuture ? "true" : "false")
@@ -154,6 +155,9 @@ class InboundLedgersImp : public InboundLedgers
                 // If the node is falling behind, send requests.
                 if (fallingBehind)
                     return true;
+                // If needed for preferred analysis, send requests.
+                if (preferred)
+                    return true;
                 // If the ledger is in the near future, do NOT send requests.
                 // This node is probably about to build it.
                 if (nearFuture)