Fix CI unit tests (#5196)

- Add retries for rpc client
- Add dynamic port allocation for rpc servers

.github/workflows/macos.yml

@@ -56,6 +56,9 @@ jobs:
           else
               brew install cmake
           fi
+      - name: install nproc
+        run: |
+          brew install coreutils
       - name: check environment
         run: |
           env | sort
@@ -63,6 +66,9 @@ jobs:
           python --version
           conan --version
           cmake --version
+          nproc --version
+          echo -n "nproc returns: "
+          nproc
       - name: configure Conan
         run : |
           conan profile new default --detect || true
@@ -81,6 +87,9 @@ jobs:
         with:
           generator: ${{ matrix.generator }}
           configuration: ${{ matrix.configuration }}
+          cmake-args: ${{ matrix.cmake-args }}
       - name: test
         run: |
-          ${build_dir}/rippled --unittest
+          n=$(nproc)
+          echo "Using $n test jobs"
+          ${build_dir}/rippled --unittest --unittest-jobs $n

include/xrpl/basics/BasicConfig.h

@@ -21,18 +21,21 @@
 #define RIPPLE_BASICS_BASICCONFIG_H_INCLUDED
 
 #include <xrpl/basics/contract.h>
+
 #include <boost/beast/core/string.hpp>
 #include <boost/lexical_cast.hpp>
+
 #include <algorithm>
-#include <map>
 #include <optional>
 #include <ostream>
 #include <string>
+#include <unordered_map>
 #include <vector>
 
 namespace ripple {
 
-using IniFileSections = std::map<std::string, std::vector<std::string>>;
+using IniFileSections =
+    std::unordered_map<std::string, std::vector<std::string>>;
 
 //------------------------------------------------------------------------------
 
@@ -43,7 +46,7 @@ class Section
 {
 private:
     std::string name_;
-    std::map<std::string, std::string> lookup_;
+    std::unordered_map<std::string, std::string> lookup_;
     std::vector<std::string> lines_;
     std::vector<std::string> values_;
     bool had_trailing_comments_ = false;
@@ -215,7 +218,7 @@ public:
 class BasicConfig
 {
 private:
-    std::map<std::string, Section, boost::beast::iless> map_;
+    std::unordered_map<std::string, Section> map_;
 
 public:
     /** Returns `true` if a section with the given name exists. */

include/xrpl/server/detail/ServerImpl.h

@@ -22,18 +22,21 @@
 
 #include <xrpl/basics/chrono.h>
 #include <xrpl/beast/core/List.h>
-#include <xrpl/server/Server.h>
 #include <xrpl/server/detail/Door.h>
 #include <xrpl/server/detail/io_list.h>
+
 #include <boost/asio.hpp>
+
 #include <array>
 #include <chrono>
 #include <mutex>
 #include <optional>
+#include <unordered_map>
 
 namespace ripple {
 
-using Endpoints = std::vector<boost::asio::ip::tcp::endpoint>;
+using Endpoints =
+    std::unordered_map<std::string, boost::asio::ip::tcp::endpoint>;
 
 /** A multi-protocol server.
 
@@ -168,11 +171,17 @@ ServerImpl<Handler>::ports(std::vector<Port> const& ports)
     for (auto const& port : ports)
     {
         ports_.push_back(port);
+        auto& internalPort = ports_.back();
         if (auto sp = ios_.emplace<Door<Handler>>(
-                handler_, io_service_, ports_.back(), j_))
+                handler_, io_service_, internalPort, j_))
         {
             list_.push_back(sp);
-            eps.push_back(sp->get_endpoint());
+
+            auto ep = sp->get_endpoint();
+            if (!internalPort.port)
+                internalPort.port = ep.port();
+            eps.emplace(port.name, std::move(ep));
+
             sp->run();
         }
     }

src/libxrpl/basics/BasicConfig.cpp

@@ -104,7 +104,7 @@ Section::append(std::vector<std::string> const& lines)
 bool
 Section::exists(std::string const& name) const
 {
-    return lookup_.find(name) != lookup_.end();
+    return lookup_.contains(name);
 }
 
 std::ostream&
@@ -120,13 +120,13 @@ operator<<(std::ostream& os, Section const& section)
 bool
 BasicConfig::exists(std::string const& name) const
 {
-    return map_.find(name) != map_.end();
+    return map_.contains(name);
 }
 
 Section&
 BasicConfig::section(std::string const& name)
 {
-    return map_[name];
+    return map_.emplace(name, name).first->second;
 }
 
 Section const&
@@ -163,7 +163,7 @@ BasicConfig::deprecatedClearSection(std::string const& section)
 void
 BasicConfig::legacy(std::string const& section, std::string value)
 {
-    map_[section].legacy(std::move(value));
+    map_.emplace(section, section).first->second.legacy(std::move(value));
 }
 
 std::string

src/libxrpl/server/Port.cpp

@@ -198,6 +198,7 @@ populate(
 void
 parse_Port(ParsedPort& port, Section const& section, std::ostream& log)
 {
+    port.name = section.name();
     {
         auto const optResult = section.get("ip");
         if (optResult)
@@ -223,8 +224,8 @@ parse_Port(ParsedPort& port, Section const& section, std::ostream& log)
             {
                 port.port = beast::lexicalCastThrow<std::uint16_t>(*optResult);
 
-                // Port 0 is not supported
+                // Port 0 is not supported for [server]
-                if (*port.port == 0)
+                if ((*port.port == 0) && (port.name == "server"))
                     Throw<std::exception>();
             }
             catch (std::exception const&)

src/test/app/LedgerReplay_test.cpp

@@ -584,10 +584,7 @@ public:
         PeerSetBehavior behavior = PeerSetBehavior::Good,
         InboundLedgersBehavior inboundBhvr = InboundLedgersBehavior::Good,
         PeerFeature peerFeature = PeerFeature::LedgerReplayEnabled)
-        : env(suite,
+        : env(suite, jtx::envconfig(), nullptr, beast::severities::kDisabled)
-              jtx::envconfig(jtx::port_increment, 3),
-              nullptr,
-              beast::severities::kDisabled)
         , app(env.app())
         , ledgerMaster(env.app().getLedgerMaster())
         , inboundLedgers(

src/test/app/XChain_test.cpp

@@ -201,11 +201,7 @@ struct SEnv
 template <class T>
 struct XEnv : public jtx::XChainBridgeObjects, public SEnv<T>
 {
-    XEnv(T& s, bool side = false)
+    XEnv(T& s, bool side = false) : SEnv<T>(s, jtx::envconfig(), features)
-        : SEnv<T>(
-              s,
-              jtx::envconfig(jtx::port_increment, side ? 3 : 0),
-              features)
     {
         using namespace jtx;
         STAmount xrp_funds{XRP(10000)};

src/test/core/Config_test.cpp

@@ -24,10 +24,13 @@
 #include <xrpl/basics/contract.h>
 #include <xrpl/beast/unit_test/suite.h>
 #include <xrpl/server/Port.h>
+
 #include <boost/filesystem.hpp>
 #include <boost/format.hpp>
+
 #include <fstream>
 #include <iostream>
+#include <regex>
 
 namespace ripple {
 namespace detail {
@@ -140,12 +143,15 @@ public:
         path subDir,
         path const& dbPath,
         path const& validatorsFile,
-        bool useCounter = true)
+        bool useCounter = true,
+        std::string confContents = "")
         : FileDirGuard(
               test,
               std::move(subDir),
               path(Config::configFileName),
-              configContents(dbPath.string(), validatorsFile.string()),
+              confContents.empty()
+                  ? configContents(dbPath.string(), validatorsFile.string())
+                  : confContents,
               useCounter)
         , dataDir_(dbPath)
     {
@@ -1068,6 +1074,27 @@ trustthesevalidators.gov
         BEAST_EXPECT(wss.admin_nets_v4.size() + wss.admin_nets_v6.size() == 1);
     }
 
+    void
+    testZeroPort()
+    {
+        auto const contents = std::regex_replace(
+            detail::configContents("", ""),
+            std::regex("port\\s*=\\s*\\d+"),
+            "port = 0");
+
+        try
+        {
+            detail::RippledCfgGuard const cfg(
+                *this, "testPort", "", "", true, contents);
+            BEAST_EXPECT(false);
+        }
+        catch (std::exception const& ex)
+        {
+            BEAST_EXPECT(std::string_view(ex.what()).starts_with(
+                "Invalid value '0' for key 'port'"));
+        }
+    }
+
     void
     testWhitespace()
     {
@@ -1478,6 +1505,7 @@ r.ripple.com:51235
         testSetup(false);
         testSetup(true);
         testPort();
+        testZeroPort();
         testWhitespace();
         testColons();
         testComments();

src/test/jtx/Env.h

@@ -419,6 +419,20 @@ public:
         app().checkSigs(false);
     }
 
+    // set rpc retries
+    void
+    set_retries(unsigned r = 5)
+    {
+        retries_ = r;
+    }
+
+    // get rpc retries
+    unsigned
+    retries() const
+    {
+        return retries_;
+    }
+
     /** Associate AccountID with account. */
     void
     memoize(Account const& account);
@@ -700,6 +714,7 @@ protected:
     uint256 txid_;
     TER ter_ = tesSUCCESS;
     bool parseFailureExpected_ = false;
+    unsigned retries_ = 5;
 
     Json::Value
     do_rpc(

src/test/jtx/envconfig.h

@@ -106,19 +106,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 adjust the default configured server ports by a specified value
-///
-/// This is intended for use with envconfig, as in
-/// envconfig(port_increment, 5)
-///
-/// @param cfg config instance to be modified
-/// @param int amount by which to increment the existing server port
-/// values in the config
-///
-/// @return unique_ptr to Config instance
-std::unique_ptr<Config>
-port_increment(std::unique_ptr<Config>, int);
-
 /// @brief add a grpc address and port to config
 ///
 /// This is intended for use with envconfig, for tests that require a grpc

src/test/jtx/impl/Env.cpp

@@ -325,11 +325,6 @@ Env::submit(JTx const& jt)
     ParsedResult parsedResult;
     auto const jr = [&]() {
         if (jt.stx)
-        {
-            // We shouldn't need to retry, but it fixes the test on macOS for
-            // the moment.
-            int retries = 3;
-            do
         {
             txid_ = jt.stx->getTransactionID();
             Serializer s;
@@ -339,11 +334,8 @@ Env::submit(JTx const& jt)
             parsedResult = parseResult(jr);
             test.expect(parsedResult.ter, "ter uninitialized!");
             ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED);
-                if (ter_ != telENV_RPC_FAILED ||
+
-                    parsedResult.rpcCode != rpcINTERNAL ||
-                    jt.ter == telENV_RPC_FAILED || --retries <= 0)
             return jr;
-            } while (true);
         }
         else
         {
@@ -576,8 +568,21 @@ Env::do_rpc(
     std::vector<std::string> const& args,
     std::unordered_map<std::string, std::string> const& headers)
 {
-    return rpcClient(args, app().config(), app().logs(), apiVersion, headers)
+    auto response =
-        .second;
+        rpcClient(args, app().config(), app().logs(), apiVersion, headers);
+
+    for (unsigned ctr = 0; (ctr < retries_) and (response.first == rpcINTERNAL);
+         ++ctr)
+    {
+        JLOG(journal.error())
+            << "Env::do_rpc error, retrying, attempt #" << ctr + 1 << " ...";
+        std::this_thread::sleep_for(std::chrono::milliseconds(500));
+
+        response =
+            rpcClient(args, app().config(), app().logs(), apiVersion, headers);
+    }
+
+    return response.second;
 }
 
 void

src/test/jtx/impl/JSONRPCClient.cpp

@@ -52,6 +52,10 @@ class JSONRPCClient : public AbstractClient
             if (pp.ip && pp.ip->is_unspecified())
                 *pp.ip = pp.ip->is_v6() ? address{address_v6::loopback()}
                                         : address{address_v4::loopback()};
+
+            if (!pp.port)
+                Throw<std::runtime_error>("Use fixConfigPorts with auto ports");
+
             return {*pp.ip, *pp.port};
         }
         Throw<std::runtime_error>("Missing HTTP port");

src/test/jtx/impl/WSClient.cpp

@@ -69,6 +69,10 @@ class WSClientImpl : public WSClient
             if (pp.ip && pp.ip->is_unspecified())
                 *pp.ip = pp.ip->is_v6() ? address{address_v6::loopback()}
                                         : address{address_v4::loopback()};
+
+            if (!pp.port)
+                Throw<std::runtime_error>("Use fixConfigPorts with auto ports");
+
             return {*pp.ip, *pp.port};
         }
         Throw<std::runtime_error>("Missing WebSocket port");

src/test/jtx/impl/envconfig.cpp

@@ -25,22 +25,11 @@
 namespace ripple {
 namespace test {
 
-int port_base = 8000;
-void
-incPorts(int times)
-{
-    port_base += (4 * times);
-}
-
 std::atomic<bool> envUseIPv4{false};
 
 void
 setupConfigForUnitTests(Config& cfg)
 {
-    std::string const port_peer = std::to_string(port_base);
-    std::string port_rpc = std::to_string(port_base + 1);
-    std::string port_ws = std::to_string(port_base + 2);
-
     using namespace jtx;
     // Default fees to old values, so tests don't have to worry about changes in
     // Config.h
@@ -58,19 +47,25 @@ setupConfigForUnitTests(Config& cfg)
     cfg.setupControl(true, true, true);
     cfg["server"].append(PORT_PEER);
     cfg[PORT_PEER].set("ip", getEnvLocalhostAddr());
-    cfg[PORT_PEER].set("port", port_peer);
+
+    // Using port 0 asks the operating system to allocate an unused port, which
+    // can be obtained after a "bind" call.
+    // Works for all system (Linux, Windows, Unix, Mac).
+    // Check https://man7.org/linux/man-pages/man7/ip.7.html
+    // "ip_local_port_range" section for more info
+    cfg[PORT_PEER].set("port", "0");
     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("port", port_rpc);
+    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("port", port_ws);
+    cfg[PORT_WS].set("port", "0");
     cfg[PORT_WS].set("protocol", "ws");
     cfg.SSL_VERIFY = false;
 }
@@ -123,27 +118,11 @@ validator(std::unique_ptr<Config> cfg, std::string const& seed)
     return cfg;
 }
 
-std::unique_ptr<Config>
-port_increment(std::unique_ptr<Config> cfg, int increment)
-{
-    for (auto const sectionName : {PORT_PEER, PORT_RPC, PORT_WS})
-    {
-        Section& s = (*cfg)[sectionName];
-        auto const port = s.get<std::int32_t>("port");
-        if (port)
-        {
-            s.set("port", std::to_string(*port + increment));
-        }
-    }
-    return cfg;
-}
-
 std::unique_ptr<Config>
 addGrpcConfig(std::unique_ptr<Config> cfg)
 {
-    std::string port_grpc = std::to_string(port_base + 3);
     (*cfg)[SECTION_PORT_GRPC].set("ip", getEnvLocalhostAddr());
-    (*cfg)[SECTION_PORT_GRPC].set("port", port_grpc);
+    (*cfg)[SECTION_PORT_GRPC].set("port", "0");
     return cfg;
 }
 
@@ -152,9 +131,11 @@ addGrpcConfigWithSecureGateway(
     std::unique_ptr<Config> cfg,
     std::string const& secureGateway)
 {
-    std::string port_grpc = std::to_string(port_base + 3);
     (*cfg)[SECTION_PORT_GRPC].set("ip", getEnvLocalhostAddr());
-    (*cfg)[SECTION_PORT_GRPC].set("port", port_grpc);
+
+    // 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;
 }

src/test/ledger/View_test.cpp

@@ -1012,11 +1012,7 @@ class View_test : public beast::unit_test::suite
 
         // The two Env's can't share the same ports, so modify the config
         // of the second Env to use higher port numbers
-        Env eB{
+        Env eB{*this, envconfig(), nullptr, beast::severities::kDisabled};
-            *this,
-            envconfig(port_increment, 3),
-            nullptr,
-            beast::severities::kDisabled};
 
         // Make ledgers that are incompatible with the first ledgers.  Note
         // that bob is funded before alice.

src/test/rpc/AccountObjects_test.cpp

@@ -759,7 +759,7 @@ public:
         {
             // Create a bridge
             test::jtx::XChainBridgeObjects x;
-            Env scEnv(*this, envconfig(port_increment, 3), features);
+            Env scEnv(*this, envconfig(), features);
             x.createScBridgeObjects(scEnv);
 
             auto scEnvAcctObjs = [&](Account const& acct, char const* type) {
@@ -800,7 +800,7 @@ public:
             // Alice and Bob create a xchain sequence number that we can look
             // for in the ledger.
             test::jtx::XChainBridgeObjects x;
-            Env scEnv(*this, envconfig(port_increment, 3), features);
+            Env scEnv(*this, envconfig(), features);
             x.createScBridgeObjects(scEnv);
 
             scEnv(
@@ -845,7 +845,7 @@ public:
         }
         {
             test::jtx::XChainBridgeObjects x;
-            Env scEnv(*this, envconfig(port_increment, 3), features);
+            Env scEnv(*this, envconfig(), features);
             x.createScBridgeObjects(scEnv);
             auto const amt = XRP(1000);
 

src/test/rpc/LedgerRPC_test.cpp

@@ -64,7 +64,7 @@ class LedgerRPC_XChain_test : public beast::unit_test::suite,
         using namespace test::jtx;
 
         Env mcEnv{*this, features};
-        Env scEnv(*this, envconfig(port_increment, 3), features);
+        Env scEnv(*this, envconfig(), features);
 
         createBridgeObjects(mcEnv, scEnv);
 
@@ -157,7 +157,7 @@ class LedgerRPC_XChain_test : public beast::unit_test::suite,
         using namespace test::jtx;
 
         Env mcEnv{*this, features};
-        Env scEnv(*this, envconfig(port_increment, 3), features);
+        Env scEnv(*this, envconfig(), features);
 
         createBridgeObjects(mcEnv, scEnv);
 
@@ -218,7 +218,7 @@ class LedgerRPC_XChain_test : public beast::unit_test::suite,
         using namespace test::jtx;
 
         Env mcEnv{*this, features};
-        Env scEnv(*this, envconfig(port_increment, 3), features);
+        Env scEnv(*this, envconfig(), features);
 
         // note: signers.size() and quorum are both 5 in createBridgeObjects
         createBridgeObjects(mcEnv, scEnv);

src/test/rpc/LedgerRequestRPC_test.cpp

@@ -347,6 +347,7 @@ public:
         auto const USD = gw["USD"];
         env.fund(XRP(100000), gw);
 
+        env.set_retries(0);
         auto const result = env.rpc("ledger_request", "1")[jss::result];
         // The current HTTP/S ServerHandler returns an HTTP 403 error code here
         // rather than a noPermission JSON error.  The JSONRPCClient just eats

src/test/rpc/ServerInfo_test.cpp

@@ -104,17 +104,17 @@ admin = 127.0.0.1
         }
 
         {
-            auto config = makeValidatorConfig();
+            Env env(*this, makeValidatorConfig());
-            auto const rpc_port =
+            auto const& config = env.app().config();
-                (*config)["port_rpc"].get<unsigned int>("port");
+
+            auto const rpc_port = config["port_rpc"].get<unsigned int>("port");
             auto const grpc_port =
-                (*config)[SECTION_PORT_GRPC].get<unsigned int>("port");
+                config[SECTION_PORT_GRPC].get<unsigned int>("port");
-            auto const ws_port = (*config)["port_ws"].get<unsigned int>("port");
+            auto const ws_port = config["port_ws"].get<unsigned int>("port");
             BEAST_EXPECT(grpc_port);
             BEAST_EXPECT(rpc_port);
             BEAST_EXPECT(ws_port);
 
-            Env env(*this, std::move(config));
             auto const result = env.rpc("server_info");
             BEAST_EXPECT(!result[jss::result].isMember(jss::error));
             BEAST_EXPECT(result[jss::result][jss::status] == "success");

src/test/rpc/Subscribe_test.cpp

@@ -614,7 +614,7 @@ public:
         }
 
         {
-            Env env_nonadmin{*this, no_admin(envconfig(port_increment, 3))};
+            Env env_nonadmin{*this, no_admin(envconfig())};
             Json::Value jv;
             jv[jss::url] = "no-url";
             auto jr =

src/test/rpc/ValidatorInfo_test.cpp

@@ -50,6 +50,7 @@ public:
     {
         using namespace test::jtx;
         Env env{*this, envconfig(no_admin)};
+        env.set_retries(0);
         auto const info = env.rpc("validator_info")[jss::result];
         BEAST_EXPECT(info.isNull());
     }

src/test/rpc/ValidatorRPC_test.cpp

@@ -49,6 +49,7 @@ public:
             for (std::string cmd : {"validators", "validator_list_sites"})
             {
                 Env env{*this, isAdmin ? envconfig() : envconfig(no_admin)};
+                env.set_retries(isAdmin ? 5 : 0);
                 auto const jrr = env.rpc(cmd)[jss::result];
                 if (isAdmin)
                 {

src/test/server/Server_test.cpp

@@ -299,8 +299,8 @@ public:
         serverPort.back().port = 0;
         serverPort.back().protocol.insert("http");
         auto eps = s->ports(serverPort);
-        test_request(eps[0]);
+        test_request(eps.begin()->second);
-        test_keepalive(eps[0]);
+        test_keepalive(eps.begin()->second);
         // s->close();
         s = nullptr;
         pass();
@@ -423,7 +423,20 @@ public:
                 std::make_unique<CaptureLogs>(&messages)};
         });
         BEAST_EXPECT(
-            messages.find("Invalid value '0' for key 'port' in [port_rpc]") !=
+            messages.find("Invalid value '0' for key 'port' in [port_rpc]") ==
+            std::string::npos);
+
+        except([&] {
+            Env env{
+                *this,
+                envconfig([](std::unique_ptr<Config> cfg) {
+                    (*cfg)["server"].set("port", "0");
+                    return cfg;
+                }),
+                std::make_unique<CaptureLogs>(&messages)};
+        });
+        BEAST_EXPECT(
+            messages.find("Invalid value '0' for key 'port' in [server]") !=
             std::string::npos);
 
         except([&] {

src/test/unit_test/multi_runner.cpp

@@ -32,9 +32,6 @@
 namespace ripple {
 namespace test {
 
-extern void
-incPorts(int times);
-
 namespace detail {
 
 std::string
@@ -510,9 +507,6 @@ multi_runner_child::multi_runner_child(
     , quiet_{quiet}
     , print_log_{!quiet || print_log}
 {
-    // incPort twice (2*jobIndex_) because some tests need two envs
-    test::incPorts(2 * job_index_);
-
     if (num_jobs_ > 1)
     {
         keep_alive_thread_ = std::thread([this] {

src/xrpld/app/main/Application.cpp

@@ -90,6 +90,9 @@
 
 namespace ripple {
 
+static void
+fixConfigPorts(Config& config, Endpoints const& endpoints);
+
 // VFALCO TODO Move the function definitions into the class declaration
 class ApplicationImp : public Application, public BasicApp
 {
@@ -1417,6 +1420,7 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline)
                 *config_, beast::logstream{m_journal.error()});
             setup.makeContexts();
             serverHandler_->setup(setup, m_journal);
+            fixConfigPorts(*config_, serverHandler_->endpoints());
         }
         catch (std::exception const& e)
         {
@@ -1538,7 +1542,11 @@ ApplicationImp::start(bool withTimers)
     m_shaMapStore->start();
     if (overlay_)
         overlay_->start();
-    grpcServer_->start();
+
+    if (grpcServer_->start())
+        fixConfigPorts(
+            *config_, {{SECTION_PORT_GRPC, grpcServer_->getEndpoint()}});
+
     ledgerCleaner_->start();
     perfLog_->start();
 }
@@ -2189,4 +2197,24 @@ make_Application(
         std::move(config), std::move(logs), std::move(timeKeeper));
 }
 
+void
+fixConfigPorts(Config& config, Endpoints const& endpoints)
+{
+    for (auto const& [name, ep] : endpoints)
+    {
+        if (!config.exists(name))
+            continue;
+
+        auto& section = config[name];
+        auto const optPort = section.get("port");
+        if (optPort)
+        {
+            std::uint16_t const port =
+                beast::lexicalCast<std::uint16_t>(*optPort);
+            if (!port)
+                section.set("port", std::to_string(ep.port()));
+        }
+    }
+}
+
 }  // namespace ripple

src/xrpld/app/main/GRPCServer.cpp

@@ -268,7 +268,7 @@ template <class Request, class Response>
 std::optional<boost::asio::ip::tcp::endpoint>
 GRPCServerImpl::CallData<Request, Response>::getClientEndpoint()
 {
-    return getEndpoint(ctx_.peer());
+    return ripple::getEndpoint(ctx_.peer());
 }
 
 template <class Request, class Response>
@@ -447,8 +447,8 @@ GRPCServerImpl::handleRpcs()
 
         if (!ok)
         {
-            JLOG(journal_.debug())
+            JLOG(journal_.debug()) << "Request listener cancelled. "
-                << "Request listener cancelled. " << "Destroying object";
+                                   << "Destroying object";
             erase(ptr);
         }
         else
@@ -564,8 +564,12 @@ GRPCServerImpl::start()
     JLOG(journal_.info()) << "Starting gRPC server at " << serverAddress_;
 
     grpc::ServerBuilder builder;
+
     // Listen on the given address without any authentication mechanism.
-    builder.AddListeningPort(serverAddress_, grpc::InsecureServerCredentials());
+    // Actually binded port will be returned into "port" variable.
+    int port = 0;
+    builder.AddListeningPort(
+        serverAddress_, grpc::InsecureServerCredentials(), &port);
     // Register "service_" as the instance through which we'll communicate with
     // clients. In this case it corresponds to an *asynchronous* service.
     builder.RegisterService(&service_);
@@ -574,11 +578,21 @@ GRPCServerImpl::start()
     cq_ = builder.AddCompletionQueue();
     // Finally assemble the server.
     server_ = builder.BuildAndStart();
+    serverPort_ = static_cast<std::uint16_t>(port);
 
-    return true;
+    return static_cast<bool>(serverPort_);
 }
 
-void
+boost::asio::ip::tcp::endpoint
+GRPCServerImpl::getEndpoint() const
+{
+    std::string const addr =
+        serverAddress_.substr(0, serverAddress_.find_last_of(':'));
+    return boost::asio::ip::tcp::endpoint(
+        boost::asio::ip::make_address(addr), serverPort_);
+}
+
+bool
 GRPCServer::start()
 {
     // Start the server and setup listeners
@@ -591,6 +605,7 @@ GRPCServer::start()
             this->impl_.handleRpcs();
         });
     }
+    return running_;
 }
 
 void
@@ -609,4 +624,10 @@ GRPCServer::~GRPCServer()
     XRPL_ASSERT(!running_, "ripple::GRPCServer::~GRPCServer : is not running");
 }
 
+boost::asio::ip::tcp::endpoint
+GRPCServer::getEndpoint() const
+{
+    return impl_.getEndpoint();
+}
+
 }  // namespace ripple

src/xrpld/app/main/GRPCServer.h

@@ -84,6 +84,7 @@ private:
     Application& app_;
 
     std::string serverAddress_;
+    std::uint16_t serverPort_ = 0;
 
     std::vector<boost::asio::ip::address> secureGatewayIPs_;
 
@@ -141,6 +142,10 @@ public:
     std::vector<std::shared_ptr<Processor>>
     setupListeners();
 
+    // Obtaining actually binded endpoint (if port 0 was used for server setup).
+    boost::asio::ip::tcp::endpoint
+    getEndpoint() const;
+
 private:
     // Class encompasing the state and logic needed to serve a request.
     template <class Request, class Response>
@@ -305,7 +310,7 @@ public:
     GRPCServer&
     operator=(const GRPCServer&) = delete;
 
-    void
+    bool
     start();
 
     void
@@ -313,6 +318,9 @@ public:
 
     ~GRPCServer();
 
+    boost::asio::ip::tcp::endpoint
+    getEndpoint() const;
+
 private:
     GRPCServerImpl impl_;
     std::thread thread_;

src/xrpld/core/detail/Config.cpp

@@ -420,6 +420,35 @@ Config::setup(
     get_if_exists(nodeDbSection, "fast_load", FAST_LOAD);
 }
 
+// 0 ports are allowed for unit tests, but still not allowed to be present in
+// config file
+static void
+checkZeroPorts(Config const& config)
+{
+    if (!config.exists("server"))
+        return;
+
+    for (auto const& name : config.section("server").values())
+    {
+        if (!config.exists(name))
+            return;
+
+        auto const& section = config[name];
+        auto const optResult = section.get("port");
+        if (optResult)
+        {
+            auto const port = beast::lexicalCast<std::uint16_t>(*optResult);
+            if (!port)
+            {
+                std::stringstream ss;
+                ss << "Invalid value '" << *optResult << "' for key 'port' in ["
+                   << name << "]";
+                Throw<std::runtime_error>(ss.str());
+            }
+        }
+    }
+}
+
 void
 Config::load()
 {
@@ -440,6 +469,7 @@ Config::load()
     }
 
     loadFromString(fileContents);
+    checkZeroPorts(*this);
 }
 
 void

src/xrpld/overlay/detail/OverlayImpl.cpp

@@ -478,7 +478,7 @@ OverlayImpl::start()
 {
     PeerFinder::Config config = PeerFinder::Config::makeConfig(
         app_.config(),
-        serverHandler_.setup().overlay.port,
+        serverHandler_.setup().overlay.port(),
         app_.getValidationPublicKey().has_value(),
         setup_.ipLimit);
 

src/xrpld/rpc/ServerHandler.h

@@ -28,9 +28,11 @@
 #include <xrpl/server/Server.h>
 #include <xrpl/server/Session.h>
 #include <xrpl/server/WSSession.h>
+
 #include <boost/beast/core/tcp_stream.hpp>
 #include <boost/beast/ssl/ssl_stream.hpp>
 #include <boost/utility/string_view.hpp>
+
 #include <condition_variable>
 #include <map>
 #include <mutex>
@@ -71,15 +73,7 @@ public:
         client_t client;
 
         // Configuration for the Overlay
-        struct overlay_t
+        boost::asio::ip::tcp::endpoint overlay;
-        {
-            explicit overlay_t() = default;
-
-            boost::asio::ip::address ip;
-            std::uint16_t port = 0;
-        };
-
-        overlay_t overlay;
 
         void
         makeContexts();
@@ -95,6 +89,7 @@ private:
     NetworkOPs& m_networkOPs;
     std::unique_ptr<Server> m_server;
     Setup setup_;
+    Endpoints endpoints_;
     JobQueue& m_jobQueue;
     beast::insight::Counter rpc_requests_;
     beast::insight::Event rpc_size_;
@@ -145,6 +140,12 @@ public:
         return setup_;
     }
 
+    Endpoints const&
+    endpoints() const
+    {
+        return endpoints_;
+    }
+
     void
     stop();
 

src/xrpld/rpc/detail/ServerHandler.cpp

@@ -44,9 +44,11 @@
 #include <xrpl/server/Server.h>
 #include <xrpl/server/SimpleWriter.h>
 #include <xrpl/server/detail/JSONRPCUtil.h>
+
 #include <boost/algorithm/string.hpp>
 #include <boost/beast/http/fields.hpp>
 #include <boost/beast/http/string_body.hpp>
+
 #include <algorithm>
 #include <stdexcept>
 
@@ -130,7 +132,26 @@ void
 ServerHandler::setup(Setup const& setup, beast::Journal journal)
 {
     setup_ = setup;
-    m_server->ports(setup.ports);
+    endpoints_ = m_server->ports(setup.ports);
+
+    // fix auto ports
+    for (auto& port : setup_.ports)
+    {
+        if (auto it = endpoints_.find(port.name); it != endpoints_.end())
+        {
+            auto const endpointPort = it->second.port();
+            if (!port.port)
+                port.port = endpointPort;
+
+            if (!setup_.client.port &&
+                (port.protocol.count("http") > 0 ||
+                 port.protocol.count("https") > 0))
+                setup_.client.port = endpointPort;
+
+            if (!setup_.overlay.port() && (port.protocol.count("peer") > 0))
+                setup_.overlay.port(endpointPort);
+        }
+    }
 }
 
 //------------------------------------------------------------------------------
@@ -1094,11 +1115,6 @@ to_Port(ParsedPort const& parsed, std::ostream& log)
         log << "Missing 'port' in [" << p.name << "]";
         Throw<std::exception>();
     }
-    else if (*parsed.port == 0)
-    {
-        log << "Port " << *parsed.port << "in [" << p.name << "] is invalid";
-        Throw<std::exception>();
-    }
     p.port = *parsed.port;
 
     if (parsed.protocol.empty())
@@ -1157,7 +1173,6 @@ parse_Ports(Config const& config, std::ostream& log)
             continue;
 
         ParsedPort parsed = common;
-        parsed.name = name;
         parse_Port(parsed, config[name], log);
         result.push_back(to_Port(parsed, log));
     }
@@ -1232,11 +1247,10 @@ setup_Overlay(ServerHandler::Setup& setup)
         });
     if (iter == setup.ports.cend())
     {
-        setup.overlay.port = 0;
+        setup.overlay = {};
         return;
     }
-    setup.overlay.ip = iter->ip;
+    setup.overlay = {iter->ip, iter->port};
-    setup.overlay.port = iter->port;
 }
 
 ServerHandler::Setup