From 9f2fd23575804bbb292adb63d425ea52faab2170 Mon Sep 17 00:00:00 2001
From: Scott Schurr <scott@ripple.com>
Date: Tue, 17 Oct 2023 22:52:33 -0700
Subject: [PATCH] refactor(peerfinder): use LogicError in PeerFinder::Logic
 (#4562)

It might be possible for the server code to indirect through certain
`end()` iterators. While a debug build would catch this problem with
`assert()`s, a release build would crash. If there are problems in this
area in the future, it is best to get a definitive indication of the
nature of the error regardless of whether it's a debug or release build.
To accomplish this, these `assert`s are converted into `LogicError`s
that will produce a reasonable error message when they fire.
---
 src/ripple/peerfinder/impl/Logic.h | 35 +++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/src/ripple/peerfinder/impl/Logic.h b/src/ripple/peerfinder/impl/Logic.h
index ca14a5111..dc325cc48 100644
--- a/src/ripple/peerfinder/impl/Logic.h
+++ b/src/ripple/peerfinder/impl/Logic.h
@@ -416,10 +416,9 @@ public:
         // assert later when erasing the key.
         slot->public_key(key);
         {
-            auto const result = keys_.insert(key);
+            [[maybe_unused]] bool const inserted = keys_.insert(key).second;
             // Public key must not already exist
-            assert(result.second);
-            (void)result.second;
+            assert(inserted);
         }
 
         // Change state and update counts
@@ -434,7 +433,11 @@ public:
         if (slot->fixed() && !slot->inbound())
         {
             auto iter(fixed_.find(slot->remote_endpoint()));
-            assert(iter != fixed_.end());
+            if (iter == fixed_.end())
+                LogicError(
+                    "PeerFinder::Logic::activate(): remote_endpoint "
+                    "missing from fixed_");
+
             iter->second.success(m_clock.now());
             JLOG(m_journal.trace()) << beast::leftw(18) << "Logic fixed "
                                     << slot->remote_endpoint() << " success";
@@ -858,7 +861,11 @@ public:
         {
             auto const iter = slots_.find(slot->remote_endpoint());
             // The slot must exist in the table
-            assert(iter != slots_.end());
+            if (iter == slots_.end())
+                LogicError(
+                    "PeerFinder::Logic::remove(): remote_endpoint "
+                    "missing from slots_");
+
             // Remove from slot by IP table
             slots_.erase(iter);
         }
@@ -867,7 +874,11 @@ public:
         {
             auto const iter = keys_.find(*slot->public_key());
             // Key must exist
-            assert(iter != keys_.end());
+            if (iter == keys_.end())
+                LogicError(
+                    "PeerFinder::Logic::remove(): public_key missing "
+                    "from keys_");
+
             keys_.erase(iter);
         }
         // Remove from connected address table
@@ -875,7 +886,11 @@ public:
             auto const iter(
                 connectedAddresses_.find(slot->remote_endpoint().address()));
             // Address must exist
-            assert(iter != connectedAddresses_.end());
+            if (iter == connectedAddresses_.end())
+                LogicError(
+                    "PeerFinder::Logic::remove(): remote_endpont "
+                    "address missing from connectedAddresses_");
+
             connectedAddresses_.erase(iter);
         }
 
@@ -894,7 +909,11 @@ public:
         if (slot->fixed() && !slot->inbound() && slot->state() != Slot::active)
         {
             auto iter(fixed_.find(slot->remote_endpoint()));
-            assert(iter != fixed_.end());
+            if (iter == fixed_.end())
+                LogicError(
+                    "PeerFinder::Logic::on_closed(): remote_endpont "
+                    "missing from fixed_");
+
             iter->second.failure(m_clock.now());
             JLOG(m_journal.debug()) << beast::leftw(18) << "Logic fixed "
                                     << slot->remote_endpoint() << " failed";