Revert "Improve detection & handling of duplicate Node ID:"

src/ripple/app/consensus/RCLConsensus.cpp

@@ -86,11 +86,9 @@ RCLConsensus::Adaptor::Adaptor(
     , inboundTransactions_{inboundTransactions}
     , j_(journal)
     , validatorKeys_(validatorKeys)
-    , valCookie_(
-          1 +
-          rand_int(
-              crypto_prng(),
-              std::numeric_limits<std::uint64_t>::max() - 1))
+    , valCookie_{rand_int<std::uint64_t>(
+          1,
+          std::numeric_limits<std::uint64_t>::max())}
     , nUnlVote_(validatorKeys_.nodeID, j_)
 {
     assert(valCookie_ != 0);

src/ripple/app/main/Application.cpp

@@ -52,12 +52,10 @@
 #include <ripple/basics/ByteUtilities.h>
 #include <ripple/basics/PerfLog.h>
 #include <ripple/basics/ResolverAsio.h>
-#include <ripple/basics/random.h>
 #include <ripple/basics/safe_cast.h>
 #include <ripple/beast/asio/io_latency_probe.h>
 #include <ripple/beast/core/LexicalCast.h>
 #include <ripple/core/DatabaseCon.h>
-#include <ripple/crypto/csprng.h>
 #include <ripple/json/json_reader.h>
 #include <ripple/nodestore/DatabaseShard.h>
 #include <ripple/nodestore/DummyScheduler.h>
@@ -167,8 +165,6 @@ class ApplicationImp : public Application, public BasicApp
     std::unique_ptr<Logs> logs_;
     std::unique_ptr<TimeKeeper> timeKeeper_;
 
-    std::uint64_t const instanceCookie_;
-
     beast::Journal m_journal;
     std::unique_ptr<perf::PerfLog> perfLog_;
     Application::MutexType m_masterMutex;
@@ -277,11 +273,6 @@ class ApplicationImp : public Application, public BasicApp
         , config_(std::move(config))
         , logs_(std::move(logs))
         , timeKeeper_(std::move(timeKeeper))
-        , instanceCookie_(
-              1 +
-              rand_int(
-                  crypto_prng(),
-                  std::numeric_limits<std::uint64_t>::max() - 1))
         , m_journal(logs_->journal("Application"))
 
         // PerfLog must be started before any other threads are launched.
@@ -516,13 +507,13 @@ class ApplicationImp : public Application, public BasicApp
     //--------------------------------------------------------------------------
 
     bool
-    setup(boost::program_options::variables_map const& cmdline) override;
+    setup() override;
     void
     start(bool withTimers) override;
     void
     run() override;
     void
-    signalStop(std::string msg = "") override;
+    signalStop() override;
     bool
     checkSigs() const override;
     void
@@ -534,12 +525,6 @@ class ApplicationImp : public Application, public BasicApp
 
     //--------------------------------------------------------------------------
 
-    std::uint64_t
-    instanceID() const override
-    {
-        return instanceCookie_;
-    }
-
     Logs&
     logs() override
     {
@@ -1116,7 +1101,7 @@ class ApplicationImp : public Application, public BasicApp
 
 // TODO Break this up into smaller, more digestible initialization segments.
 bool
-ApplicationImp::setup(boost::program_options::variables_map const& cmdline)
+ApplicationImp::setup()
 {
     // We want to intercept CTRL-C and the standard termination signal SIGTERM
     // and terminate the process. This handler will NEVER be invoked twice.
@@ -1154,10 +1139,8 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline)
         if (logs_->threshold() > kDebug)
             logs_->threshold(kDebug);
     }
-
-    JLOG(m_journal.info()) << "Process starting: "
-                           << BuildInfo::getFullVersionString()
-                           << ", Instance Cookie: " << instanceCookie_;
+    JLOG(m_journal.info()) << "process starting: "
+                           << BuildInfo::getFullVersionString();
 
     if (numberOfThreads(*config_) < 2)
     {
@@ -1275,7 +1258,7 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline)
     if (!config().reporting())
         m_orderBookDB.setup(getLedgerMaster().getCurrentLedger());
 
-    nodeIdentity_ = getNodeIdentity(*this, cmdline);
+    nodeIdentity_ = getNodeIdentity(*this);
 
     if (!cluster_->load(config().section(SECTION_CLUSTER_NODES)))
     {
@@ -1637,17 +1620,10 @@ ApplicationImp::run()
 }
 
 void
-ApplicationImp::signalStop(std::string msg)
+ApplicationImp::signalStop()
 {
     if (!isTimeToStop.exchange(true))
-    {
-        if (msg.empty())
-            JLOG(m_journal.warn()) << "Server stopping";
-        else
-            JLOG(m_journal.warn()) << "Server stopping: " << msg;
-
         stoppingCondition_.notify_all();
-    }
 }
 
 bool

src/ripple/app/main/Application.h

@@ -28,7 +28,6 @@
 #include <ripple/shamap/FullBelowCache.h>
 #include <ripple/shamap/TreeNodeCache.h>
 #include <boost/asio.hpp>
-#include <boost/program_options.hpp>
 #include <memory>
 #include <mutex>
 
@@ -136,14 +135,13 @@ class Application : public beast::PropertyStream::Source
     virtual ~Application() = default;
 
     virtual bool
-    setup(boost::program_options::variables_map const& options) = 0;
-
+    setup() = 0;
     virtual void
     start(bool withTimers) = 0;
     virtual void
     run() = 0;
     virtual void
-    signalStop(std::string msg = "") = 0;
+    signalStop() = 0;
     virtual bool
     checkSigs() const = 0;
     virtual void
@@ -155,10 +153,6 @@ class Application : public beast::PropertyStream::Source
     // ---
     //
 
-    /** Returns a 64-bit instance identifier, generated at startup */
-    virtual std::uint64_t
-    instanceID() const = 0;
-
     virtual Logs&
     logs() = 0;
     virtual Config&

src/ripple/app/main/Main.cpp

@@ -372,10 +372,6 @@ run(int argc, char** argv)
         "conf", po::value<std::string>(), "Specify the configuration file.")(
         "debug", "Enable normally suppressed debug logging")(
         "help,h", "Display this message.")(
-        "newnodeid", "Generate a new node identity for this server.")(
-        "nodeid",
-        po::value<std::string>(),
-        "Specify the node identity for this server.")(
         "quorum",
         po::value<std::size_t>(),
         "Override the minimum validation quorum.")(
@@ -760,7 +756,7 @@ run(int argc, char** argv)
         auto app = make_Application(
             std::move(config), std::move(logs), std::move(timeKeeper));
 
-        if (!app->setup(vm))
+        if (!app->setup())
             return -1;
 
         // With our configuration parsed, ensure we have

src/ripple/app/main/NodeIdentity.cpp

@@ -20,49 +20,34 @@
 #include <ripple/app/main/Application.h>
 #include <ripple/app/main/NodeIdentity.h>
 #include <ripple/app/rdb/Wallet.h>
+#include <ripple/basics/Log.h>
 #include <ripple/core/Config.h>
 #include <ripple/core/ConfigSections.h>
+#include <boost/format.hpp>
 #include <boost/optional.hpp>
 
 namespace ripple {
 
 std::pair<PublicKey, SecretKey>
-getNodeIdentity(
-    Application& app,
-    boost::program_options::variables_map const& cmdline)
+getNodeIdentity(Application& app)
 {
-    std::optional<Seed> seed;
-
-    if (cmdline.count("nodeid"))
-    {
-        seed = parseGenericSeed(cmdline["nodeid"].as<std::string>(), false);
-
-        if (!seed)
-            Throw<std::runtime_error>("Invalid 'nodeid' in command line");
-    }
-    else if (app.config().exists(SECTION_NODE_SEED))
+    // If a seed is specified in the configuration file use that directly.
+    if (app.config().exists(SECTION_NODE_SEED))
     {
-        seed = parseBase58<Seed>(
+        auto const seed = parseBase58<Seed>(
             app.config().section(SECTION_NODE_SEED).lines().front());
 
         if (!seed)
-            Throw<std::runtime_error>("Invalid [" SECTION_NODE_SEED
-                                      "] in configuration file");
-    }
+            Throw<std::runtime_error>("NodeIdentity: Bad [" SECTION_NODE_SEED
+                                      "] specified");
 
-    if (seed)
-    {
         auto secretKey = generateSecretKey(KeyType::secp256k1, *seed);
         auto publicKey = derivePublicKey(KeyType::secp256k1, secretKey);
 
         return {publicKey, secretKey};
     }
 
     auto db = app.getWalletDB().checkoutDb();
-
-    if (cmdline.count("newnodeid") != 0)
-        clearNodeIdentity(*db);
-
     return getNodeIdentity(*db);
 }
 

src/ripple/app/main/NodeIdentity.h

@@ -23,20 +23,13 @@
 #include <ripple/app/main/Application.h>
 #include <ripple/protocol/PublicKey.h>
 #include <ripple/protocol/SecretKey.h>
-#include <boost/program_options.hpp>
 #include <utility>
 
 namespace ripple {
 
-/** The cryptographic credentials identifying this server instance.
-
-    @param app The application object
-    @param cmdline The command line parameters passed into the application.
- */
+/** The cryptographic credentials identifying this server instance. */
 std::pair<PublicKey, SecretKey>
-getNodeIdentity(
-    Application& app,
-    boost::program_options::variables_map const& cmdline);
+getNodeIdentity(Application& app);
 
 }  // namespace ripple
 

src/ripple/app/rdb/Wallet.h

@@ -87,19 +87,10 @@ saveManifests(
 void
 addValidatorManifest(soci::session& session, std::string const& serialized);
 
-/** Delete any saved public/private key associated with this node. */
-void
-clearNodeIdentity(soci::session& session);
-
-/** Returns a stable public and private key for this node.
-
-    The node's public identity is defined by a secp256k1 keypair
-    that is (normally) randomly generated. This function will
-    return such a keypair, securely generating one if needed.
-
-    @param session Session with the database.
-
-    @return Pair of public and private secp256k1 keys.
+/**
+ * @brief getNodeIdentity Returns the public and private keys of this node.
+ * @param session Session with the database.
+ * @return Pair of public and private keys.
  */
 std::pair<PublicKey, SecretKey>
 getNodeIdentity(soci::session& session);

src/ripple/app/rdb/impl/Wallet.cpp

@@ -119,12 +119,6 @@ addValidatorManifest(soci::session& session, std::string const& serialized)
     tr.commit();
 }
 
-void
-clearNodeIdentity(soci::session& session)
-{
-    session << "DELETE FROM NodeIdentity;";
-}
-
 std::pair<PublicKey, SecretKey>
 getNodeIdentity(soci::session& session)
 {

src/ripple/overlay/README.md

@@ -296,8 +296,8 @@ For more on the Peer Crawler, please visit https://xrpl.org/peer-crawler.html.
 If present, identifies the hash of the last ledger that the sending server
 considers to be closed.
 
-The value is encoded as **HEX**, but implementations should support both
-**Base64** and **HEX** encoding for this value for legacy purposes.
+The value is presently encoded using **Base64** encoding, but implementations
+should support both **Base64** and **HEX** encoding for this value.
 
 | Field Name          	|  Request          	| Response          	|
 |---------------------	|:-----------------:	|:-----------------:	|

src/ripple/overlay/impl/Handshake.cpp

@@ -204,8 +204,6 @@ buildHandshake(
         h.insert("Session-Signature", base64_encode(sig.data(), sig.size()));
     }
 
-    h.insert("Instance-Cookie", std::to_string(app.instanceID()));
-
     if (!app.config().SERVER_DOMAIN.empty())
         h.insert("Server-Domain", app.config().SERVER_DOMAIN);
 
@@ -217,8 +215,14 @@ buildHandshake(
 
     if (auto const cl = app.getLedgerMaster().getClosedLedger())
     {
-        h.insert("Closed-Ledger", strHex(cl->info().hash));
-        h.insert("Previous-Ledger", strHex(cl->info().parentHash));
+        // TODO: Use hex for these
+        h.insert(
+            "Closed-Ledger",
+            base64_encode(cl->info().hash.begin(), cl->info().hash.size()));
+        h.insert(
+            "Previous-Ledger",
+            base64_encode(
+                cl->info().parentHash.begin(), cl->info().parentHash.size()));
     }
 }
 
@@ -302,34 +306,7 @@ verifyHandshake(
     }();
 
     if (publicKey == app.nodeIdentity().first)
-    {
-        auto const peerInstanceID = [&headers]() {
-            std::uint64_t iid = 0;
-
-            if (auto const iter = headers.find("Instance-Cookie");
-                iter != headers.end())
-            {
-                if (!beast::lexicalCastChecked(iid, iter->value().to_string()))
-                    throw std::runtime_error("Invalid instance cookie");
-
-                if (iid == 0)
-                    throw std::runtime_error("Invalid instance cookie");
-            }
-
-            return iid;
-        }();
-
-        // Attempt to differentiate self-connections as opposed to accidental
-        // node identity reuse caused by accidental misconfiguration. When we
-        // detect this, we stop the process and log an error message.
-        if (peerInstanceID != app.instanceID())
-        {
-            app.signalStop("Remote server is using our node identity");
-            throw std::runtime_error("Node identity reuse detected");
-        }
-
         throw std::runtime_error("Self connection");
-    }
 
     // This check gets two birds with one stone:
     //

src/ripple/overlay/impl/ProtocolVersion.cpp

@@ -36,6 +36,7 @@ namespace ripple {
 // clang-format off
 constexpr ProtocolVersion const supportedProtocolList[]
 {
+    {2, 0},
     {2, 1},
     {2, 2}
 };

src/ripple/protocol/Seed.h

@@ -116,13 +116,9 @@ template <>
 std::optional<Seed>
 parseBase58(std::string const& s);
 
-/** Attempt to parse a string as a seed.
-
-    @param str the string to parse
-    @param rfc1751 true if we should attempt RFC1751 style parsing (deprecated)
- * */
+/** Attempt to parse a string as a seed */
 std::optional<Seed>
-parseGenericSeed(std::string const& str, bool rfc1751 = true);
+parseGenericSeed(std::string const& str);
 
 /** Encode a Seed in RFC1751 format */
 std::string