Fix tests, remove unused functions

.github/actions/build-deps/action.yml

@@ -28,7 +28,6 @@ runs:
         BUILD_DIR: ${{ inputs.build_dir }}
         BUILD_OPTION: ${{ inputs.force_build == 'true' && '*' || 'missing' }}
         BUILD_TYPE: ${{ inputs.build_type }}
-        VERBOSITY: ${{ inputs.verbosity }}
       run: |
         echo 'Installing dependencies.'
         mkdir -p '${{ env.BUILD_DIR }}'
@@ -39,6 +38,7 @@ runs:
           --options:host='&:tests=True' \
           --options:host='&:xrpld=True' \
           --settings:all build_type='${{ env.BUILD_TYPE }}' \
-          --conf:all tools.build:verbosity='${{ env.VERBOSITY }}' \
-          --conf:all tools.compilation:verbosity='${{ env.VERBOSITY }}' \
+          --conf:all tools.build:verbosity='${{ inputs.verbosity }}' \
+          --conf:all tools.compilation:verbosity='${{ inputs.verbosity }}' \
+          --conf:all tools.build:jobs=$(nproc) \
           ..

.github/scripts/strategy-matrix/linux.json

@@ -73,61 +73,47 @@
       "compiler_version": "20",
       "image_sha": "6948666"
     },
-    {
-      "distro_name": "rhel",
-      "distro_version": "8",
-      "compiler_name": "gcc",
-      "compiler_version": "14",
-      "image_sha": "10e69b4"
-    },
-    {
-      "distro_name": "rhel",
-      "distro_version": "8",
-      "compiler_name": "clang",
-      "compiler_version": "any",
-      "image_sha": "10e69b4"
-    },
     {
       "distro_name": "rhel",
       "distro_version": "9",
       "compiler_name": "gcc",
       "compiler_version": "12",
-      "image_sha": "10e69b4"
+      "image_sha": "6948666"
     },
     {
       "distro_name": "rhel",
       "distro_version": "9",
       "compiler_name": "gcc",
       "compiler_version": "13",
-      "image_sha": "10e69b4"
+      "image_sha": "6948666"
     },
     {
       "distro_name": "rhel",
       "distro_version": "9",
       "compiler_name": "gcc",
       "compiler_version": "14",
-      "image_sha": "10e69b4"
+      "image_sha": "6948666"
     },
     {
       "distro_name": "rhel",
       "distro_version": "9",
       "compiler_name": "clang",
       "compiler_version": "any",
-      "image_sha": "10e69b4"
+      "image_sha": "6948666"
     },
     {
       "distro_name": "rhel",
       "distro_version": "10",
       "compiler_name": "gcc",
       "compiler_version": "14",
-      "image_sha": "10e69b4"
+      "image_sha": "6948666"
     },
     {
       "distro_name": "rhel",
       "distro_version": "10",
       "compiler_name": "clang",
       "compiler_version": "any",
-      "image_sha": "10e69b4"
+      "image_sha": "6948666"
     },
     {
       "distro_name": "ubuntu",

.github/workflows/on-pr.yml

@@ -103,7 +103,6 @@ jobs:
     if: ${{ needs.should-run.outputs.go == 'true' }}
     uses: ./.github/workflows/reusable-build-test.yml
     strategy:
-      fail-fast: false
       matrix:
         os: [linux, macos, windows]
     with:

.github/workflows/on-trigger.yml

@@ -65,7 +65,6 @@ jobs:
   build-test:
     uses: ./.github/workflows/reusable-build-test.yml
     strategy:
-      fail-fast: ${{ github.event_name == 'merge_group' }}
       matrix:
         os: [linux, macos, windows]
     with:

.github/workflows/pre-commit.yml

@@ -9,7 +9,7 @@ on:
 jobs:
   # Call the workflow in the XRPLF/actions repo that runs the pre-commit hooks.
   run-hooks:
-    uses: XRPLF/actions/.github/workflows/pre-commit.yml@a8d7472b450eb53a1e5228f64552e5974457a21a
+    uses: XRPLF/actions/.github/workflows/pre-commit.yml@af1b0f0d764cda2e5435f5ac97b240d4bd4d95d3
     with:
       runs_on: ubuntu-latest
       container: '{ "image": "ghcr.io/xrplf/ci/tools-rippled-pre-commit:sha-a8c7be1" }'

.github/workflows/reusable-build-test.yml

@@ -42,7 +42,7 @@ jobs:
       - generate-matrix
     uses: ./.github/workflows/reusable-build-test-config.yml
     strategy:
-      fail-fast: ${{ github.event_name == 'merge_group' }}
+      fail-fast: false
       matrix: ${{ fromJson(needs.generate-matrix.outputs.matrix) }}
       max-parallel: 10
     with:

.github/workflows/reusable-notify-clio.yml

@@ -64,9 +64,7 @@ jobs:
           conan_remote_name: ${{ inputs.conan_remote_name }}
           conan_remote_url: ${{ inputs.conan_remote_url }}
       - name: Log into Conan remote
-        env:
-          CONAN_REMOTE_NAME: ${{ inputs.conan_remote_name }}
-        run: conan remote login ${{ env.CONAN_REMOTE_NAME }} "${{ secrets.conan_remote_username }}" --password "${{ secrets.conan_remote_password }}"
+        run: conan remote login ${{ inputs.conan_remote_name }} "${{ secrets.conan_remote_username }}" --password "${{ secrets.conan_remote_password }}"
       - name: Upload package
         env:
           CONAN_REMOTE_NAME: ${{ inputs.conan_remote_name }}

cmake/RippledCore.cmake

@@ -181,6 +181,11 @@ if(xrpld)
     xrpl.libxrpl
   )
   exclude_if_included(rippled)
+  # define a macro for tests that might need to
+  # be exluded or run differently in CI environment
+  if(is_ci)
+    target_compile_definitions(rippled PRIVATE RIPPLED_RUNNING_IN_CI)
+  endif ()
 
   if(voidstar)
     target_compile_options(rippled

cmake/RippledSanity.cmake

@@ -43,7 +43,6 @@ else ()
   set (is_linux FALSE)
 endif ()
 
-# The CI environment variable is set by GitHub Actions.
 if ("$ENV{CI}" STREQUAL "true" OR "$ENV{CONTINUOUS_INTEGRATION}" STREQUAL "true")
   set (is_ci TRUE)
 else ()

conan/global.conf

@@ -3,4 +3,4 @@
 core:non_interactive=True
 core.download:parallel={{ os.cpu_count() }}
 core.upload:parallel={{ os.cpu_count() }}
-tools.build:jobs={{ os.cpu_count() - 1 }}
+tools.build:jobs={{ (os.cpu_count() * 4/5) | int }}

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);

src/xrpld/rpc/handlers/Simulate.cpp

@@ -72,23 +72,39 @@ getAutofillSequence(Json::Value const& tx_json, RPC::JsonContext& context)
 }
 
 static std::optional<Json::Value>
-autofillSignature(Json::Value& sigObject)
+autofillTx(Json::Value& tx_json, RPC::JsonContext& context)
 {
-    if (!sigObject.isMember(jss::SigningPubKey))
+    if (!tx_json.isMember(jss::Fee))
     {
-        // autofill SigningPubKey
-        sigObject[jss::SigningPubKey] = "";
+        // autofill Fee
+        // Must happen after all the other autofills happen
+        // Error handling/messaging works better that way
+        auto feeOrError = RPC::getCurrentNetworkFee(
+            context.role,
+            context.app.config(),
+            context.app.getFeeTrack(),
+            context.app.getTxQ(),
+            context.app,
+            tx_json);
+        if (feeOrError.isMember(jss::error))
+            return feeOrError;
+        tx_json[jss::Fee] = feeOrError;
     }
 
-    if (sigObject.isMember(jss::Signers))
+    if (!tx_json.isMember(jss::SigningPubKey))
     {
-        if (!sigObject[jss::Signers].isArray())
+        // autofill SigningPubKey
+        tx_json[jss::SigningPubKey] = "";
+    }
+
+    if (tx_json.isMember(jss::Signers))
+    {
+        if (!tx_json[jss::Signers].isArray())
             return RPC::invalid_field_error("tx.Signers");
         // check multisigned signers
-        for (unsigned index = 0; index < sigObject[jss::Signers].size();
-             index++)
+        for (unsigned index = 0; index < tx_json[jss::Signers].size(); index++)
         {
-            auto& signer = sigObject[jss::Signers][index];
+            auto& signer = tx_json[jss::Signers][index];
             if (!signer.isObject() || !signer.isMember(jss::Signer) ||
                 !signer[jss::Signer].isObject())
                 return RPC::invalid_field_error(
@@ -113,41 +129,16 @@ autofillSignature(Json::Value& sigObject)
         }
     }
 
-    if (!sigObject.isMember(jss::TxnSignature))
+    if (!tx_json.isMember(jss::TxnSignature))
     {
         // autofill TxnSignature
-        sigObject[jss::TxnSignature] = "";
+        tx_json[jss::TxnSignature] = "";
     }
-    else if (sigObject[jss::TxnSignature] != "")
+    else if (tx_json[jss::TxnSignature] != "")
     {
         // Transaction must not be signed
         return rpcError(rpcTX_SIGNED);
     }
-    return std::nullopt;
-}
-
-static std::optional<Json::Value>
-autofillTx(Json::Value& tx_json, RPC::JsonContext& context)
-{
-    if (!tx_json.isMember(jss::Fee))
-    {
-        // autofill Fee
-        // Must happen after all the other autofills happen
-        // Error handling/messaging works better that way
-        auto feeOrError = RPC::getCurrentNetworkFee(
-            context.role,
-            context.app.config(),
-            context.app.getFeeTrack(),
-            context.app.getTxQ(),
-            context.app,
-            tx_json);
-        if (feeOrError.isMember(jss::error))
-            return feeOrError;
-        tx_json[jss::Fee] = feeOrError;
-    }
-
-    if (auto error = autofillSignature(tx_json))
-        return *error;
 
     if (!tx_json.isMember(jss::Sequence))
     {