Automatically allocated tcp ports for UT

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 @@ class Section
 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
-                if (*port.port == 0)
+                // Port 0 is not supported for [server]
+                if ((*port.port == 0) && (port.name == "server"))
                     Throw<std::exception>();
             }
             catch (std::exception const&)

src/test/app/LedgerReplay_test.cpp

@@ -583,10 +583,7 @@ class LedgerReplayClient
         PeerSetBehavior behavior = PeerSetBehavior::Good,
         InboundLedgersBehavior inboundBhvr = InboundLedgersBehavior::Good,
         PeerFeature peerFeature = PeerFeature::LedgerReplayEnabled)
-        : env(suite,
-              jtx::envconfig(jtx::port_increment, 3),
-              nullptr,
-              beast::severities::kDisabled)
+        : env(suite, jtx::envconfig(), 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)
-        : SEnv<T>(
-              s,
-              jtx::envconfig(jtx::port_increment, side ? 3 : 0),
-              features)
+    XEnv(T& s, bool side = false) : SEnv<T>(s, jtx::envconfig(), features)
     {
         using namespace jtx;
         STAmount xrp_funds{XRP(10000)};

src/test/core/Config_test.cpp

@@ -23,10 +23,13 @@
 #include <xrpld/core/ConfigSections.h>
 #include <xrpl/basics/contract.h>
 #include <xrpl/server/Port.h>
+
 #include <boost/filesystem.hpp>
 #include <boost/format.hpp>
+
 #include <fstream>
 #include <iostream>
+#include <regex>
 
 namespace ripple {
 namespace detail {
@@ -139,12 +142,15 @@ class RippledCfgGuard : public ripple::test::detail::FileDirGuard
         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)
     {
@@ -879,6 +885,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()
     {
@@ -1289,6 +1316,7 @@ r.ripple.com:51235
         testSetup(false);
         testSetup(true);
         testPort();
+        testZeroPort();
         testWhitespace();
         testColons();
         testComments();

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/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,19 @@ 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);
+    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 +112,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 +125,8 @@ 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);
+    (*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{
-            *this,
-            envconfig(port_increment, 3),
-            nullptr,
-            beast::severities::kDisabled};
+        Env eB{*this, envconfig(), 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

@@ -717,7 +717,7 @@ class AccountObjects_test : public beast::unit_test::suite
         {
             // 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) {
@@ -758,7 +758,7 @@ class AccountObjects_test : public beast::unit_test::suite
             // 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(
@@ -803,7 +803,7 @@ class AccountObjects_test : public beast::unit_test::suite
         }
         {
             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);