Fix tests, remove unused functions

include/xrpl/server/detail/Door.h

@@ -279,7 +279,7 @@ Door<Handler>::reOpen()
     if (ec)
     {
         JLOG(j_.error()) << "Open port '" << port_.name
-                         << "' failed:" << ec.message();
+                         << "' failed: " << ec.message();
         Throw<std::exception>();
     }
 
@@ -288,7 +288,7 @@ Door<Handler>::reOpen()
     if (ec)
     {
         JLOG(j_.error()) << "Option for port '" << port_.name
-                         << "' failed:" << ec.message();
+                         << "' failed: " << ec.message();
         Throw<std::exception>();
     }
 
@@ -296,7 +296,7 @@ Door<Handler>::reOpen()
     if (ec)
     {
         JLOG(j_.error()) << "Bind port '" << port_.name
-                         << "' failed:" << ec.message();
+                         << "' failed: " << ec.message();
         Throw<std::exception>();
     }
 
@@ -304,7 +304,7 @@ Door<Handler>::reOpen()
     if (ec)
     {
         JLOG(j_.error()) << "Listen on port '" << port_.name
-                         << "' failed:" << ec.message();
+                         << "' failed: " << ec.message();
         Throw<std::exception>();
     }
 

src/test/jtx/envconfig.h

@@ -22,6 +22,9 @@
 
 #include <xrpld/core/Config.h>
 
+#include <random>
+#include <string>
+
 namespace ripple {
 namespace test {
 
@@ -32,10 +35,43 @@ namespace test {
 
 extern std::atomic<bool> envUseIPv4;
 
-inline char const*
+inline std::uint32_t
+getRandomIPv4Loopback()
+{
+    // When running multiple unit tests in parallel, especially on a shared
+    // machine used in CI pipelines, it is possible to run out of ports on the
+    // default loopback interface. While IPv6 only has a single loopback, ::1,
+    // for IPv4 the entire 127.0.0.0/8 block is available on Linux and Windows
+    // (except 127.0.0.0 and 127.255.255.255 that are reserved), which we should
+    // use as the majority of our tests use IPv4. On macOS only 127.0.0.1 is
+    // available as loopback address, unless the loopback interface is aliased
+    // to more addresses, e.g. `sudo ifconfig lo0 alias 127.0.0.2 up` etc.
+#if defined(__clang__) && defined(__APPLE__)
+    return 0x7F000001;  // 127.0.0.1
+#else
+    static std::random_device rd;
+    static std::mt19937 gen(rd());
+    static std::uniform_int_distribution<std::uint32_t> dist(
+        0x7F000001,  // 127.0.0.1
+        0x7FFFFFFE   // 127.255.255.254
+    );
+    return dist(gen);
+#endif
+}
+
+inline std::string
 getEnvLocalhostAddr()
 {
-    return envUseIPv4 ? "127.0.0.1" : "::1";
+    if (envUseIPv4)
+    {
+        auto const loopback_v4 = getRandomIPv4Loopback();
+        auto const& addr_v4 = std::to_string((loopback_v4 >> 24) & 0xFF) + "." +
+            std::to_string((loopback_v4 >> 16) & 0xFF) + "." +
+            std::to_string((loopback_v4 >> 8) & 0xFF) + "." +
+            std::to_string(loopback_v4 & 0xFF);
+        return addr_v4;
+    }
+    return "::1";
 }
 
 /// @brief initializes a config object for use with jtx::Env
@@ -106,27 +142,6 @@ std::unique_ptr<Config> secure_gateway_localnet(std::unique_ptr<Config>);
 std::unique_ptr<Config>
 validator(std::unique_ptr<Config>, std::string const&);
 
-/// @brief add a grpc address and port to config
-///
-/// This is intended for use with envconfig, for tests that require a grpc
-/// server. If this function is not called, grpc server will not start
-///
-///
-/// @param cfg config instance to be modified
-std::unique_ptr<Config> addGrpcConfig(std::unique_ptr<Config>);
-
-/// @brief add a grpc address, port and secure_gateway to config
-///
-/// This is intended for use with envconfig, for tests that require a grpc
-/// server. If this function is not called, grpc server will not start
-///
-///
-/// @param cfg config instance to be modified
-std::unique_ptr<Config>
-addGrpcConfigWithSecureGateway(
-    std::unique_ptr<Config>,
-    std::string const& secureGateway);
-
 std::unique_ptr<Config>
 makeConfig(
     std::map<std::string, std::string> extraTxQ = {},

src/test/jtx/impl/JSONRPCClient.cpp

@@ -18,6 +18,7 @@
 //==============================================================================
 
 #include <test/jtx/JSONRPCClient.h>
+#include <test/jtx/envconfig.h>
 
 #include <xrpl/json/json_reader.h>
 #include <xrpl/json/to_string.h>
@@ -54,8 +55,17 @@ class JSONRPCClient : public AbstractClient
                 continue;
             using namespace boost::asio::ip;
             if (pp.ip && pp.ip->is_unspecified())
-                *pp.ip = pp.ip->is_v6() ? address{address_v6::loopback()}
-                                        : address{address_v4::loopback()};
+            {
+                if (pp.ip->is_v6())
+                {
+                    *pp.ip = address{address_v6::loopback()};
+                }
+                else
+                {
+                    auto const loopback_v4 = getRandomIPv4Loopback();
+                    *pp.ip = address{address_v4(loopback_v4)};
+                }
+            }
 
             if (!pp.port)
                 Throw<std::runtime_error>("Use fixConfigPorts with auto ports");

src/test/jtx/impl/WSClient.cpp

@@ -19,6 +19,7 @@
 
 #include <test/jtx.h>
 #include <test/jtx/WSClient.h>
+#include <test/jtx/envconfig.h>
 
 #include <xrpl/json/json_reader.h>
 #include <xrpl/json/to_string.h>
@@ -68,8 +69,17 @@ class WSClientImpl : public WSClient
                 continue;
             using namespace boost::asio::ip;
             if (pp.ip && pp.ip->is_unspecified())
-                *pp.ip = pp.ip->is_v6() ? address{address_v6::loopback()}
-                                        : address{address_v4::loopback()};
+            {
+                if (pp.ip->is_v6())
+                {
+                    *pp.ip = address{address_v6::loopback()};
+                }
+                else
+                {
+                    auto const loopback_v4 = getRandomIPv4Loopback();
+                    *pp.ip = address{address_v4(loopback_v4)};
+                }
+            }
 
             if (!pp.port)
                 Throw<std::runtime_error>("Use fixConfigPorts with auto ports");

src/test/jtx/impl/envconfig.cpp

@@ -31,6 +31,10 @@ void
 setupConfigForUnitTests(Config& cfg)
 {
     using namespace jtx;
+
+    // Use the loopback address for IPv4 or IPv6, as appropriate.
+    auto const& localhost = getEnvLocalhostAddr();
+
     // Default fees to old values, so tests don't have to worry about changes in
     // Config.h
     cfg.FEES.reference_fee = UNIT_TEST_REFERENCE_FEE;
@@ -46,7 +50,7 @@ setupConfigForUnitTests(Config& cfg)
     cfg.legacy("database_path", "");
     cfg.setupControl(true, true, true);
     cfg["server"].append(PORT_PEER);
-    cfg[PORT_PEER].set("ip", getEnvLocalhostAddr());
+    cfg[PORT_PEER].set("ip", localhost);
 
     // Using port 0 asks the operating system to allocate an unused port, which
     // can be obtained after a "bind" call.
@@ -57,14 +61,14 @@ setupConfigForUnitTests(Config& cfg)
     cfg[PORT_PEER].set("protocol", "peer");
 
     cfg["server"].append(PORT_RPC);
-    cfg[PORT_RPC].set("ip", getEnvLocalhostAddr());
-    cfg[PORT_RPC].set("admin", getEnvLocalhostAddr());
+    cfg[PORT_RPC].set("ip", localhost);
+    cfg[PORT_RPC].set("admin", localhost);
     cfg[PORT_RPC].set("port", "0");
     cfg[PORT_RPC].set("protocol", "http,ws2");
 
     cfg["server"].append(PORT_WS);
-    cfg[PORT_WS].set("ip", getEnvLocalhostAddr());
-    cfg[PORT_WS].set("admin", getEnvLocalhostAddr());
+    cfg[PORT_WS].set("ip", localhost);
+    cfg[PORT_WS].set("admin", localhost);
     cfg[PORT_WS].set("port", "0");
     cfg[PORT_WS].set("protocol", "ws");
     cfg.SSL_VERIFY = false;
@@ -83,9 +87,11 @@ no_admin(std::unique_ptr<Config> cfg)
 std::unique_ptr<Config>
 secure_gateway(std::unique_ptr<Config> cfg)
 {
+    // Use the same IP address as assigned to the other IP fields.
+    auto const localhost = (*cfg)[PORT_PEER].get<std::string>("ip");
     (*cfg)[PORT_RPC].set("admin", "");
     (*cfg)[PORT_WS].set("admin", "");
-    (*cfg)[PORT_RPC].set("secure_gateway", getEnvLocalhostAddr());
+    (*cfg)[PORT_RPC].set("secure_gateway", *localhost);
     return cfg;
 }
 
@@ -118,28 +124,6 @@ validator(std::unique_ptr<Config> cfg, std::string const& seed)
     return cfg;
 }
 
-std::unique_ptr<Config>
-addGrpcConfig(std::unique_ptr<Config> cfg)
-{
-    (*cfg)[SECTION_PORT_GRPC].set("ip", getEnvLocalhostAddr());
-    (*cfg)[SECTION_PORT_GRPC].set("port", "0");
-    return cfg;
-}
-
-std::unique_ptr<Config>
-addGrpcConfigWithSecureGateway(
-    std::unique_ptr<Config> cfg,
-    std::string const& secureGateway)
-{
-    (*cfg)[SECTION_PORT_GRPC].set("ip", getEnvLocalhostAddr());
-
-    // Check https://man7.org/linux/man-pages/man7/ip.7.html
-    // "ip_local_port_range" section for using 0 ports
-    (*cfg)[SECTION_PORT_GRPC].set("port", "0");
-    (*cfg)[SECTION_PORT_GRPC].set("secure_gateway", secureGateway);
-    return cfg;
-}
-
 std::unique_ptr<Config>
 makeConfig(
     std::map<std::string, std::string> extraTxQ,

src/test/server/ServerStatus_test.cpp

@@ -80,14 +80,17 @@ class ServerStatus_test : public beast::unit_test::suite,
 
         if (proto == "https")
         {
+            // Use the same IP address as assigned to the other IP address
+            // fields.
+            auto const localhost = (*p)[PORT_PEER].get<std::string>("ip");
             // this port is here to allow the env to create its internal client,
             // which requires an http endpoint to talk to. In the connection
             // failure test, this endpoint should never be used
             (*p)["server"].append("port_alt");
-            (*p)["port_alt"].set("ip", getEnvLocalhostAddr());
+            (*p)["port_alt"].set("ip", *localhost);
             (*p)["port_alt"].set("port", "7099");
             (*p)["port_alt"].set("protocol", "http");
-            (*p)["port_alt"].set("admin", getEnvLocalhostAddr());
+            (*p)["port_alt"].set("admin", *localhost);
         }
 
         return p;

src/test/server/Server_test.cpp

@@ -466,37 +466,37 @@ public:
             messages.find("Missing 'protocol' in [port_rpc]") !=
             std::string::npos);
 
-        except(
-            [&]  // this creates a standard test config without the server
-                 // section
-            {
-                Env env{
-                    *this,
-                    envconfig([](std::unique_ptr<Config> cfg) {
-                        cfg = std::make_unique<Config>();
-                        cfg->overwrite(
-                            ConfigSection::nodeDatabase(), "type", "memory");
-                        cfg->overwrite(
-                            ConfigSection::nodeDatabase(), "path", "main");
-                        cfg->deprecatedClearSection(
-                            ConfigSection::importNodeDatabase());
-                        cfg->legacy("database_path", "");
-                        cfg->setupControl(true, true, true);
-                        (*cfg)["port_peer"].set("ip", getEnvLocalhostAddr());
-                        (*cfg)["port_peer"].set("port", "8080");
-                        (*cfg)["port_peer"].set("protocol", "peer");
-                        (*cfg)["port_rpc"].set("ip", getEnvLocalhostAddr());
-                        (*cfg)["port_rpc"].set("port", "8081");
-                        (*cfg)["port_rpc"].set("protocol", "http,ws2");
-                        (*cfg)["port_rpc"].set("admin", getEnvLocalhostAddr());
-                        (*cfg)["port_ws"].set("ip", getEnvLocalhostAddr());
-                        (*cfg)["port_ws"].set("port", "8082");
-                        (*cfg)["port_ws"].set("protocol", "ws");
-                        (*cfg)["port_ws"].set("admin", getEnvLocalhostAddr());
-                        return cfg;
-                    }),
-                    std::make_unique<CaptureLogs>(&messages)};
-            });
+        except([&]  // this creates a standard test config without the server
+                    // section
+               {
+                   Env env{
+                       *this,
+                       envconfig([](std::unique_ptr<Config> cfg) {
+                           auto const& localhost = getEnvLocalhostAddr();
+                           cfg = std::make_unique<Config>();
+                           cfg->overwrite(
+                               ConfigSection::nodeDatabase(), "type", "memory");
+                           cfg->overwrite(
+                               ConfigSection::nodeDatabase(), "path", "main");
+                           cfg->deprecatedClearSection(
+                               ConfigSection::importNodeDatabase());
+                           cfg->legacy("database_path", "");
+                           cfg->setupControl(true, true, true);
+                           (*cfg)["port_peer"].set("ip", localhost);
+                           (*cfg)["port_peer"].set("port", "8080");
+                           (*cfg)["port_peer"].set("protocol", "peer");
+                           (*cfg)["port_rpc"].set("ip", localhost);
+                           (*cfg)["port_rpc"].set("port", "8081");
+                           (*cfg)["port_rpc"].set("protocol", "http,ws2");
+                           (*cfg)["port_rpc"].set("admin", localhost);
+                           (*cfg)["port_ws"].set("ip", localhost);
+                           (*cfg)["port_ws"].set("port", "8082");
+                           (*cfg)["port_ws"].set("protocol", "ws");
+                           (*cfg)["port_ws"].set("admin", localhost);
+                           return cfg;
+                       }),
+                       std::make_unique<CaptureLogs>(&messages)};
+               });
         BEAST_EXPECT(
             messages.find("Required section [server] is missing") !=
             std::string::npos);