From 21a3f4a5b5115bb344e85917be43722e66c76ce2 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Wed, 27 Jul 2022 12:32:56 -0400 Subject: [PATCH 01/19] Change by-value to by-reference to persist vote --- src/ripple/app/misc/impl/AmendmentTable.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/app/misc/impl/AmendmentTable.cpp b/src/ripple/app/misc/impl/AmendmentTable.cpp index 5f75d2954..93113af80 100644 --- a/src/ripple/app/misc/impl/AmendmentTable.cpp +++ b/src/ripple/app/misc/impl/AmendmentTable.cpp @@ -425,7 +425,7 @@ AmendmentTableImpl::AmendmentTableImpl( } else // up-vote { - auto s = add(amend_hash, lock); + AmendmentState& s = add(amend_hash, lock); JLOG(j_.debug()) << "Amendment {" << *amendment_name << ", " << amend_hash << "} is upvoted."; From 6a9c270776e7a71fb76a52d040f1cd28b257eec7 Mon Sep 17 00:00:00 2001 From: Crypto Brad Garlinghouse Date: Tue, 26 Jul 2022 04:41:01 +0000 Subject: [PATCH 02/19] Properly handle self-assignment of PublicKey --- src/ripple/protocol/impl/PublicKey.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/ripple/protocol/impl/PublicKey.cpp b/src/ripple/protocol/impl/PublicKey.cpp index 9fed78088..ac86634f1 100644 --- a/src/ripple/protocol/impl/PublicKey.cpp +++ b/src/ripple/protocol/impl/PublicKey.cpp @@ -24,7 +24,6 @@ #include #include #include -#include namespace ripple { @@ -186,14 +185,18 @@ PublicKey::PublicKey(PublicKey const& other) : size_(other.size_) { if (size_) std::memcpy(buf_, other.buf_, size_); -}; +} PublicKey& PublicKey::operator=(PublicKey const& other) { - size_ = other.size_; - if (size_) - std::memcpy(buf_, other.buf_, size_); + if (this != &other) + { + size_ = other.size_; + if (size_) + std::memcpy(buf_, other.buf_, size_); + } + return *this; } From cd3a6bf530563b0b739fd02fb9d0eee1b5d70264 Mon Sep 17 00:00:00 2001 From: Wo Jake <87929946+wojake@users.noreply.github.com> Date: Tue, 2 Aug 2022 08:36:12 +0000 Subject: [PATCH 03/19] Document the "DefaultVote::no" policy in the code --- src/ripple/protocol/impl/Feature.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index 4060067e3..fcd774ce9 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -52,7 +52,7 @@ enum class Supported : bool { no = false, yes }; // enabled using run-time conditionals based on the state of the amendment. // There is value in retaining that conditional code for some time after // the amendment is enabled to make it simple to replay old transactions. -// However, once an Amendment has been enabled for, say, more than two years +// However, once an amendment has been enabled for, say, more than two years // then retaining that conditional code has less value since it is // uncommon to replay such old transactions. // @@ -61,10 +61,15 @@ enum class Supported : bool { no = false, yes }; // 2018 needs to happen on an older version of the server code. There's // a log message in Application.cpp that warns about replaying old ledgers. // -// At some point in the future someone may wish to remove Amendment -// conditional code for Amendments that were enabled after January 2018. +// At some point in the future someone may wish to remove amendment +// conditional code for amendments that were enabled after January 2018. // When that happens then the log message in Application.cpp should be // updated. +// +// Generally, amendments which introduce new features should be set as +// "DefaultVote::no" whereas in rare cases, amendments that fix critical +// bugs should be set as "DefaultVote::yes", if off-chain consensus is +// reached amongst reviewers, validator operators, and other participants. class FeatureCollections { From 47dec467ea659c1b64c7b5f4eb8a1bfa9759ff91 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Fri, 19 Aug 2022 14:40:53 -0700 Subject: [PATCH 04/19] Set version to 1.9.3 --- RELEASENOTES.md | 32 ++++++++++++++++++++++++++ src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 61ac91631..5f5810fc6 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -7,6 +7,38 @@ This document contains the release notes for `rippled`, the reference server imp Have new ideas? Need help with setting up your node? Come visit us [here](https://github.com/xrplf/rippled/issues/new/choose) +# Introducing XRP Ledger version 1.9.3 + +Version 1.9.3 of `rippled`, the reference server implementation of the XRP Ledger protocol is now available. This release corrects minor technical flaws with the code that loads configured amendment votes after a startup and the copy constructor of `PublicKey`. + +## Install / Upgrade + +On supported platforms, see the [instructions on installing or updating `rippled`](https://xrpl.org/install-rippled.html). + +## Changelog + +## Contributions + +This releases contains the following bug fixes: + +- **Change by-value to by-reference to persist vote**: A minor technical flaw, caused by use of a copy instead of a reference, resulted in operator-configured "yes" votes to not be properly loaded after a restart. ([#4256](https://github.com/XRPLF/rippled/pull/4256)) +- **Properly handle self-assignment of PublicKey**: The `PublicKey` copy assignment operator mishandled the case where a `PublicKey` would be assigned to itself, and could result in undefined behavior. + +### GitHub + +The public source code repository for `rippled` is hosted on GitHub at . + +We welcome contributions, big and small, and invite everyone to join the community of XRP Ledger developers and help us build the Internet of Value. + +### Credits + +The following people contributed directly to this release: + +- Howard Hinnant +- Crypto Brad Garlinghouse +- Wo Jake <87929946+wojake@users.noreply.github.com> + + # Introducing XRP Ledger version 1.9.2 Version 1.9.2 of `rippled`, the reference server implementation of the XRP Ledger protocol, is now available. This release includes several fixes and improvements, including a second new fix amendment to correct a bug in Non-Fungible Tokens (NFTs) code, a new API method for order book changes, less noisy logging, and other small fixes. diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index c51e0d595..734dc11cb 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -33,7 +33,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "1.9.2" +char const* const versionString = "1.9.3" // clang-format on #if defined(DEBUG) || defined(SANITIZER) From 92d35e54c7de6bbe44ff6c7c52cc0765b3f78258 Mon Sep 17 00:00:00 2001 From: seelabs Date: Tue, 16 Nov 2021 22:31:34 -0500 Subject: [PATCH 05/19] Switch from C++17 to C++20 --- CMakeLists.txt | 2 +- src/ripple/app/consensus/RCLConsensus.cpp | 4 +- src/ripple/app/paths/impl/BookStep.cpp | 8 +-- src/ripple/basics/TaggedCache.h | 2 +- src/ripple/beast/utility/PropertyStream.h | 12 ---- .../utility/src/beast_PropertyStream.cpp | 36 ----------- src/ripple/consensus/Validations.h | 4 +- src/ripple/ledger/OpenView.h | 2 +- src/ripple/ledger/detail/RawStateTable.h | 2 +- .../nodestore/backend/RocksDBFactory.cpp | 5 +- src/ripple/overlay/Slot.h | 39 +++++------- src/ripple/overlay/impl/PeerImp.cpp | 2 +- src/ripple/protocol/Feature.h | 1 - src/ripple/protocol/Serializer.h | 8 +-- src/ripple/rpc/impl/ShardArchiveHandler.cpp | 63 ++++++++++--------- src/test/app/Path_test.cpp | 2 +- src/test/core/Coroutine_test.cpp | 2 +- src/test/csf/Peer.h | 2 +- 18 files changed, 75 insertions(+), 121 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c0c0fff94..d3b494c10 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,7 +6,7 @@ endif () project (rippled) set(CMAKE_CXX_EXTENSIONS OFF) -set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) # make GIT_COMMIT_HASH define available to all sources diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index 79d41581a..a4a4e0989 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -421,7 +421,9 @@ RCLConsensus::Adaptor::onAccept( Json::Value&& consensusJson) { app_.getJobQueue().addJob( - jtACCEPT, "acceptLedger", [=, cj = std::move(consensusJson)]() mutable { + jtACCEPT, + "acceptLedger", + [=, this, cj = std::move(consensusJson)]() mutable { // Note that no lock is held or acquired during this job. // This is because generic Consensus guarantees that once a ledger // is accepted, the consensus results and capture by reference state diff --git a/src/ripple/app/paths/impl/BookStep.cpp b/src/ripple/app/paths/impl/BookStep.cpp index a80ee13f8..a6b2c5961 100644 --- a/src/ripple/app/paths/impl/BookStep.cpp +++ b/src/ripple/app/paths/impl/BookStep.cpp @@ -1125,10 +1125,10 @@ bookStepEqual(Step const& step, ripple::Book const& book) bool const inXRP = isXRP(book.in.currency); bool const outXRP = isXRP(book.out.currency); if (inXRP && outXRP) - return equalHelper< - XRPAmount, - XRPAmount, - BookPaymentStep>(step, book); + { + assert(0); + return false; // no such thing as xrp/xrp book step + } if (inXRP && !outXRP) return equalHelper< XRPAmount, diff --git a/src/ripple/basics/TaggedCache.h b/src/ripple/basics/TaggedCache.h index 548d21dc7..d9a1b542e 100644 --- a/src/ripple/basics/TaggedCache.h +++ b/src/ripple/basics/TaggedCache.h @@ -684,7 +684,7 @@ private: { // strong, expired ++cacheRemovals; - if (cit->second.ptr.unique()) + if (cit->second.ptr.use_count() == 1) { stuffToSweep.push_back(cit->second.ptr); ++mapRemovals; diff --git a/src/ripple/beast/utility/PropertyStream.h b/src/ripple/beast/utility/PropertyStream.h index bfedb39ec..dbcc8a2d7 100644 --- a/src/ripple/beast/utility/PropertyStream.h +++ b/src/ripple/beast/utility/PropertyStream.h @@ -77,12 +77,6 @@ protected: add(std::string const& key, signed char value); virtual void add(std::string const& key, unsigned char value); - virtual void - add(std::string const& key, wchar_t value); -#if 0 - virtual void add (std::string const& key, char16_t value); - virtual void add (std::string const& key, char32_t value); -#endif virtual void add(std::string const& key, short value); virtual void @@ -139,12 +133,6 @@ protected: add(signed char value); virtual void add(unsigned char value); - virtual void - add(wchar_t value); -#if 0 - virtual void add (char16_t value); - virtual void add (char32_t value); -#endif virtual void add(short value); virtual void diff --git a/src/ripple/beast/utility/src/beast_PropertyStream.cpp b/src/ripple/beast/utility/src/beast_PropertyStream.cpp index 70c5ab9a8..ecd707e95 100644 --- a/src/ripple/beast/utility/src/beast_PropertyStream.cpp +++ b/src/ripple/beast/utility/src/beast_PropertyStream.cpp @@ -414,24 +414,6 @@ PropertyStream::add(std::string const& key, unsigned char value) lexical_add(key, value); } -void -PropertyStream::add(std::string const& key, wchar_t value) -{ - lexical_add(key, value); -} - -#if 0 -void PropertyStream::add (std::string const& key, char16_t value) -{ - lexical_add (key, value); -} - -void PropertyStream::add (std::string const& key, char32_t value) -{ - lexical_add (key, value); -} -#endif - void PropertyStream::add(std::string const& key, short value) { @@ -525,24 +507,6 @@ PropertyStream::add(unsigned char value) lexical_add(value); } -void -PropertyStream::add(wchar_t value) -{ - lexical_add(value); -} - -#if 0 -void PropertyStream::add (char16_t value) -{ - lexical_add (value); -} - -void PropertyStream::add (char32_t value) -{ - lexical_add (value); -} -#endif - void PropertyStream::add(short value) { diff --git a/src/ripple/consensus/Validations.h b/src/ripple/consensus/Validations.h index 9200ac883..46bf4322a 100644 --- a/src/ripple/consensus/Validations.h +++ b/src/ripple/consensus/Validations.h @@ -27,8 +27,10 @@ #include #include #include + #include #include +#include #include #include @@ -294,7 +296,7 @@ class Validations using NodeKey = typename Validation::NodeKey; using WrappedValidationType = std::decay_t< - std::result_of_t>; + std::invoke_result_t>; // Manages concurrent access to members mutable Mutex mutex_; diff --git a/src/ripple/ledger/OpenView.h b/src/ripple/ledger/OpenView.h index 8467e4abc..98b783e3a 100644 --- a/src/ripple/ledger/OpenView.h +++ b/src/ripple/ledger/OpenView.h @@ -77,7 +77,7 @@ private: }; // List of tx, key order - // Use the boost pmr functionality instead of the c++-17 standard pmr + // Use boost::pmr functionality instead of std::pmr // functions b/c clang does not support pmr yet (as-of 9/2020) using txs_map = std::map< key_type, diff --git a/src/ripple/ledger/detail/RawStateTable.h b/src/ripple/ledger/detail/RawStateTable.h index 2bee9e2a9..2bb38dc49 100644 --- a/src/ripple/ledger/detail/RawStateTable.h +++ b/src/ripple/ledger/detail/RawStateTable.h @@ -119,7 +119,7 @@ private: } }; - // Use the boost pmr functionality instead of the c++-17 standard pmr + // Use boost::pmr functionality instead of the std::pmr // functions b/c clang does not support pmr yet (as-of 9/2020) using items_t = std::map< key_type, diff --git a/src/ripple/nodestore/backend/RocksDBFactory.cpp b/src/ripple/nodestore/backend/RocksDBFactory.cpp index e17dc55de..1a9e529e1 100644 --- a/src/ripple/nodestore/backend/RocksDBFactory.cpp +++ b/src/ripple/nodestore/backend/RocksDBFactory.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include // VFALCO Bad dependency #include @@ -30,6 +31,7 @@ #include #include #include + #include #include @@ -310,7 +312,8 @@ public: } else { - status = Status(customCode + getStatus.code()); + status = + Status(customCode + unsafe_cast(getStatus.code())); JLOG(m_journal.error()) << getStatus.ToString(); } diff --git a/src/ripple/overlay/Slot.h b/src/ripple/overlay/Slot.h index b7a2129ed..1197eff56 100644 --- a/src/ripple/overlay/Slot.h +++ b/src/ripple/overlay/Slot.h @@ -494,16 +494,11 @@ template std::set Slot::getSelected() const { - std::set init; - return std::accumulate( - peers_.begin(), peers_.end(), init, [](auto& init, auto const& it) { - if (it.second.state == PeerState::Selected) - { - init.insert(it.first); - return init; - } - return init; - }); + std::set r; + for (auto const& [id, info] : peers_) + if (info.state == PeerState::Selected) + r.insert(id); + return r; } template @@ -513,20 +508,20 @@ std::unordered_map< Slot::getPeers() const { using namespace std::chrono; - auto init = std::unordered_map< + auto r = std::unordered_map< id_t, std::tuple>(); - return std::accumulate( - peers_.begin(), peers_.end(), init, [](auto& init, auto const& it) { - init.emplace(std::make_pair( - it.first, - std::move(std::make_tuple( - it.second.state, - it.second.count, - epoch(it.second.expire).count(), - epoch(it.second.lastMessage).count())))); - return init; - }); + + for (auto const& [id, info] : peers_) + r.emplace(std::make_pair( + id, + std::move(std::make_tuple( + info.state, + info.count, + epoch(info.expire).count(), + epoch(info.lastMessage).count())))); + + return r; } /** Slots is a container for validator's Slot and handles Slot update diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 60870c90a..5962ab0df 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -2119,7 +2119,7 @@ PeerImp::onMessage(std::shared_ptr const& m) m->ledgerseq(), app_.getLedgerMaster().getValidLedgerIndex()); } - app_.getOPs().pubPeerStatus([=]() -> Json::Value { + app_.getOPs().pubPeerStatus([=, this]() -> Json::Value { Json::Value j = Json::objectValue; if (m->has_newstatus()) diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index f0d0c8efb..b46e4d588 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -126,7 +126,6 @@ class FeatureBitset : private std::bitset public: using base::bitset; using base::operator==; - using base::operator!=; using base::all; using base::any; diff --git a/src/ripple/protocol/Serializer.h b/src/ripple/protocol/Serializer.h index 37058f562..7c3ccf958 100644 --- a/src/ripple/protocol/Serializer.h +++ b/src/ripple/protocol/Serializer.h @@ -251,22 +251,22 @@ public: } bool - operator==(Blob const& v) + operator==(Blob const& v) const { return v == mData; } bool - operator!=(Blob const& v) + operator!=(Blob const& v) const { return v != mData; } bool - operator==(const Serializer& v) + operator==(const Serializer& v) const { return v.mData == mData; } bool - operator!=(const Serializer& v) + operator!=(const Serializer& v) const { return v.mData != mData; } diff --git a/src/ripple/rpc/impl/ShardArchiveHandler.cpp b/src/ripple/rpc/impl/ShardArchiveHandler.cpp index 5e5635475..2284780c2 100644 --- a/src/ripple/rpc/impl/ShardArchiveHandler.cpp +++ b/src/ripple/rpc/impl/ShardArchiveHandler.cpp @@ -416,41 +416,42 @@ ShardArchiveHandler::complete(path dstPath) } // Make lambdas mutable captured vars can be moved from - auto wrapper = jobCounter_.wrap([=, - dstPath = std::move(dstPath)]() mutable { - if (stopping_) - return; + auto wrapper = + jobCounter_.wrap([=, this, dstPath = std::move(dstPath)]() mutable { + if (stopping_) + return; - // If not synced then defer and retry - auto const mode{app_.getOPs().getOperatingMode()}; - if (mode != OperatingMode::FULL) - { - std::lock_guard lock(m_); - timer_.expires_from_now(static_cast( - (static_cast(OperatingMode::FULL) - - static_cast(mode)) * - 10)); + // If not synced then defer and retry + auto const mode{app_.getOPs().getOperatingMode()}; + if (mode != OperatingMode::FULL) + { + std::lock_guard lock(m_); + timer_.expires_from_now(static_cast( + (static_cast(OperatingMode::FULL) - + static_cast(mode)) * + 10)); - auto wrapper = timerCounter_.wrap( - [=, dstPath = std::move(dstPath)]( - boost::system::error_code const& ec) mutable { - if (ec != boost::asio::error::operation_aborted) - complete(std::move(dstPath)); - }); + auto wrapper = timerCounter_.wrap( + [=, this, dstPath = std::move(dstPath)]( + boost::system::error_code const& ec) mutable { + if (ec != boost::asio::error::operation_aborted) + complete(std::move(dstPath)); + }); - if (!wrapper) - onClosureFailed( - "failed to wrap closure for operating mode timer", lock); + if (!wrapper) + onClosureFailed( + "failed to wrap closure for operating mode timer", + lock); + else + timer_.async_wait(*wrapper); + } else - timer_.async_wait(*wrapper); - } - else - { - process(dstPath); - std::lock_guard lock(m_); - removeAndProceed(lock); - } - }); + { + process(dstPath); + std::lock_guard lock(m_); + removeAndProceed(lock); + } + }); if (!wrapper) { diff --git a/src/test/app/Path_test.cpp b/src/test/app/Path_test.cpp index 05d23e829..ef290393d 100644 --- a/src/test/app/Path_test.cpp +++ b/src/test/app/Path_test.cpp @@ -203,7 +203,7 @@ public: wait_for(std::chrono::duration const& rel_time) { std::unique_lock lk(mutex_); - auto b = cv_.wait_for(lk, rel_time, [=] { return signaled_; }); + auto b = cv_.wait_for(lk, rel_time, [this] { return signaled_; }); signaled_ = false; return b; } diff --git a/src/test/core/Coroutine_test.cpp b/src/test/core/Coroutine_test.cpp index 8937255a7..6d1e5e333 100644 --- a/src/test/core/Coroutine_test.cpp +++ b/src/test/core/Coroutine_test.cpp @@ -44,7 +44,7 @@ public: wait_for(std::chrono::duration const& rel_time) { std::unique_lock lk(mutex_); - auto b = cv_.wait_for(lk, rel_time, [=] { return signaled_; }); + auto b = cv_.wait_for(lk, rel_time, [this] { return signaled_; }); signaled_ = false; return b; } diff --git a/src/test/csf/Peer.h b/src/test/csf/Peer.h index 3a61b853c..6d3008f73 100644 --- a/src/test/csf/Peer.h +++ b/src/test/csf/Peer.h @@ -538,7 +538,7 @@ struct Peer ConsensusMode const& mode, Json::Value&& consensusJson) { - schedule(delays.ledgerAccept, [=]() { + schedule(delays.ledgerAccept, [=, this]() { const bool proposing = mode == ConsensusMode::proposing; const bool consensusFail = result.state == ConsensusState::MovedOn; From 69bb2be446e3cc24c694c0835b48bd2ecd3d119e Mon Sep 17 00:00:00 2001 From: Crypto Brad Garlinghouse Date: Tue, 19 Jul 2022 18:51:31 +0000 Subject: [PATCH 06/19] Introduce amendment to handle trustlines to self: Trustlines must be between two different accounts but two trustlines exist where an account extends trust to itself. They were created in the early days, likely because of bugs that have been fixed. The new fixTrustLinesToSelf amendment will remove those trustlines when it activates. --- src/ripple/app/tx/impl/Change.cpp | 87 ++++++++++++++++++++++++++++ src/ripple/app/tx/impl/Change.h | 3 + src/ripple/app/tx/impl/SetTrust.cpp | 53 ++++++++++------- src/ripple/protocol/Feature.h | 3 +- src/ripple/protocol/impl/Feature.cpp | 1 + 5 files changed, 124 insertions(+), 23 deletions(-) diff --git a/src/ripple/app/tx/impl/Change.cpp b/src/ripple/app/tx/impl/Change.cpp index bd66d7d58..93ed1a04f 100644 --- a/src/ripple/app/tx/impl/Change.cpp +++ b/src/ripple/app/tx/impl/Change.cpp @@ -23,9 +23,11 @@ #include #include #include +#include #include #include #include +#include namespace ripple { @@ -120,6 +122,88 @@ Change::preCompute() assert(account_ == beast::zero); } +void +Change::activateTrustLinesToSelfFix() +{ + JLOG(j_.warn()) << "fixTrustLinesToSelf amendment activation code starting"; + + auto removeTrustLineToSelf = [this](Sandbox& sb, uint256 id) { + auto tl = sb.peek(keylet::child(id)); + + if (tl == nullptr) + { + JLOG(j_.warn()) << id << ": Unable to locate trustline"; + return true; + } + + if (tl->getType() != ltRIPPLE_STATE) + { + JLOG(j_.warn()) << id << ": Unexpected type " + << static_cast(tl->getType()); + return true; + } + + auto const& lo = tl->getFieldAmount(sfLowLimit); + auto const& hi = tl->getFieldAmount(sfHighLimit); + + if (lo != hi) + { + JLOG(j_.warn()) << id << ": Trustline doesn't meet requirements"; + return true; + } + + if (auto const page = tl->getFieldU64(sfLowNode); !sb.dirRemove( + keylet::ownerDir(lo.getIssuer()), page, tl->key(), false)) + { + JLOG(j_.error()) << id << ": failed to remove low entry from " + << toBase58(lo.getIssuer()) << ":" << page + << " owner directory"; + return false; + } + + if (auto const page = tl->getFieldU64(sfHighNode); !sb.dirRemove( + keylet::ownerDir(hi.getIssuer()), page, tl->key(), false)) + { + JLOG(j_.error()) << id << ": failed to remove high entry from " + << toBase58(hi.getIssuer()) << ":" << page + << " owner directory"; + return false; + } + + if (tl->getFlags() & lsfLowReserve) + adjustOwnerCount( + sb, sb.peek(keylet::account(lo.getIssuer())), -1, j_); + + if (tl->getFlags() & lsfHighReserve) + adjustOwnerCount( + sb, sb.peek(keylet::account(hi.getIssuer())), -1, j_); + + sb.erase(tl); + + JLOG(j_.warn()) << "Successfully deleted trustline " << id; + + return true; + }; + + using namespace std::literals; + + Sandbox sb(&view()); + + if (removeTrustLineToSelf( + sb, + uint256{ + "2F8F21EFCAFD7ACFB07D5BB04F0D2E18587820C7611305BB674A64EAB0FA71E1"sv}) && + removeTrustLineToSelf( + sb, + uint256{ + "326035D5C0560A9DA8636545DD5A1B0DFCFF63E68D491B5522B767BB00564B1A"sv})) + { + JLOG(j_.warn()) << "fixTrustLinesToSelf amendment activation code " + "executed successfully"; + sb.apply(ctx_.rawView()); + } +} + TER Change::applyAmendment() { @@ -196,6 +280,9 @@ Change::applyAmendment() amendments.push_back(amendment); amendmentObject->setFieldV256(sfAmendments, amendments); + if (amendment == fixTrustLinesToSelf) + activateTrustLinesToSelfFix(); + ctx_.app.getAmendmentTable().enable(amendment); if (!ctx_.app.getAmendmentTable().isSupported(amendment)) diff --git a/src/ripple/app/tx/impl/Change.h b/src/ripple/app/tx/impl/Change.h index acd21837e..0ee7067b3 100644 --- a/src/ripple/app/tx/impl/Change.h +++ b/src/ripple/app/tx/impl/Change.h @@ -56,6 +56,9 @@ public: preclaim(PreclaimContext const& ctx); private: + void + activateTrustLinesToSelfFix(); + TER applyAmendment(); diff --git a/src/ripple/app/tx/impl/SetTrust.cpp b/src/ripple/app/tx/impl/SetTrust.cpp index 5f268f2c2..23af19c7b 100644 --- a/src/ripple/app/tx/impl/SetTrust.cpp +++ b/src/ripple/app/tx/impl/SetTrust.cpp @@ -104,19 +104,27 @@ SetTrust::preclaim(PreclaimContext const& ctx) auto const currency = saLimitAmount.getCurrency(); auto const uDstAccountID = saLimitAmount.getIssuer(); - if (id == uDstAccountID) + if (ctx.view.rules().enabled(fixTrustLinesToSelf)) { - // Prevent trustline to self from being created, - // unless one has somehow already been created - // (in which case doApply will clean it up). - auto const sleDelete = - ctx.view.read(keylet::line(id, uDstAccountID, currency)); - - if (!sleDelete) - { - JLOG(ctx.j.trace()) - << "Malformed transaction: Can not extend credit to self."; + if (id == uDstAccountID) return temDST_IS_SRC; + } + else + { + if (id == uDstAccountID) + { + // Prevent trustline to self from being created, + // unless one has somehow already been created + // (in which case doApply will clean it up). + auto const sleDelete = + ctx.view.read(keylet::line(id, uDstAccountID, currency)); + + if (!sleDelete) + { + JLOG(ctx.j.trace()) + << "Malformed transaction: Can not extend credit to self."; + return temDST_IS_SRC; + } } } @@ -183,18 +191,19 @@ SetTrust::doApply() auto viewJ = ctx_.app.journal("View"); - if (account_ == uDstAccountID) + // Trust lines to self are impossible but because of the old bug there are + // two on 19-02-2022. This code was here to allow those trust lines to be + // deleted. The fixTrustLinesToSelf fix amendment will remove them when it + // enables so this code will no longer be needed. + if (!view().rules().enabled(fixTrustLinesToSelf) && + account_ == uDstAccountID) { - // The only purpose here is to allow a mistakenly created - // trust line to oneself to be deleted. If no such trust - // lines exist now, why not remove this code and simply - // return an error? - SLE::pointer sleDelete = - view().peek(keylet::line(account_, uDstAccountID, currency)); - - JLOG(j_.warn()) << "Clearing redundant line."; - - return trustDelete(view(), sleDelete, account_, uDstAccountID, viewJ); + return trustDelete( + view(), + view().peek(keylet::line(account_, uDstAccountID, currency)), + account_, + uDstAccountID, + viewJ); } SLE::pointer sleDst = view().peek(keylet::account(uDstAccountID)); diff --git a/src/ripple/protocol/Feature.h b/src/ripple/protocol/Feature.h index b46e4d588..fc3256d16 100644 --- a/src/ripple/protocol/Feature.h +++ b/src/ripple/protocol/Feature.h @@ -74,7 +74,7 @@ namespace detail { // Feature.cpp. Because it's only used to reserve storage, and determine how // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than // the actual number of amendments. A LogicError on startup will verify this. -static constexpr std::size_t numFeatures = 50; +static constexpr std::size_t numFeatures = 51; /** Amendments that this server supports and the default voting behavior. Whether they are enabled depends on the Rules defined in the validated @@ -337,6 +337,7 @@ extern uint256 const featureExpandedSignerList; extern uint256 const fixNFTokenDirV1; extern uint256 const fixNFTokenNegOffer; extern uint256 const featureNonFungibleTokensV1_1; +extern uint256 const fixTrustLinesToSelf; } // namespace ripple diff --git a/src/ripple/protocol/impl/Feature.cpp b/src/ripple/protocol/impl/Feature.cpp index fcd774ce9..e33b9efeb 100644 --- a/src/ripple/protocol/impl/Feature.cpp +++ b/src/ripple/protocol/impl/Feature.cpp @@ -447,6 +447,7 @@ REGISTER_FEATURE(ExpandedSignerList, Supported::yes, DefaultVote::no) REGISTER_FIX (fixNFTokenDirV1, Supported::yes, DefaultVote::no); REGISTER_FIX (fixNFTokenNegOffer, Supported::yes, DefaultVote::no); REGISTER_FEATURE(NonFungibleTokensV1_1, Supported::yes, DefaultVote::no); +REGISTER_FIX (fixTrustLinesToSelf, Supported::yes, DefaultVote::no); // The following amendments have been active for at least two years. Their // pre-amendment code has been removed and the identifiers are deprecated. From e67f90588a9050162881389d7e7d1d0fb31066b0 Mon Sep 17 00:00:00 2001 From: Crypto Brad Garlinghouse Date: Sun, 24 Jul 2022 19:59:51 +0000 Subject: [PATCH 07/19] Use constexpr to check memo validity --- src/ripple/protocol/impl/STTx.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ripple/protocol/impl/STTx.cpp b/src/ripple/protocol/impl/STTx.cpp index 66d20f316..1ce4ddb64 100644 --- a/src/ripple/protocol/impl/STTx.cpp +++ b/src/ripple/protocol/impl/STTx.cpp @@ -481,11 +481,10 @@ isMemoOkay(STObject const& st, std::string& reason) // The only allowed characters for MemoType and MemoFormat are the // characters allowed in URLs per RFC 3986: alphanumerics and the // following symbols: -._~:/?#[]@!$&'()*+,;=% - static std::array const allowedSymbols = [] { - std::array a; - a.fill(0); + static constexpr std::array const allowedSymbols = []() { + std::array a{}; - std::string symbols( + std::string_view symbols( "0123456789" "-._~:/?#[]@!$&'()*+,;=%" "ABCDEFGHIJKLMNOPQRSTUVWXYZ" From 83ac141f656b1a95b5661853951ebd95b3ffba99 Mon Sep 17 00:00:00 2001 From: Crypto Brad Garlinghouse Date: Tue, 9 Aug 2022 19:05:24 +0000 Subject: [PATCH 08/19] Remove charUnHex --- Builds/CMake/RippledCore.cmake | 1 - src/ripple/basics/StringUtilities.h | 28 +++++++++++++---- src/ripple/basics/impl/strHex.cpp | 49 ----------------------------- src/ripple/basics/strHex.h | 15 --------- 4 files changed, 22 insertions(+), 71 deletions(-) delete mode 100644 src/ripple/basics/impl/strHex.cpp diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index ec9e23663..10af3b310 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -50,7 +50,6 @@ target_sources (xrpl_core PRIVATE src/ripple/basics/impl/FileUtilities.cpp src/ripple/basics/impl/IOUAmount.cpp src/ripple/basics/impl/Log.cpp - src/ripple/basics/impl/strHex.cpp src/ripple/basics/impl/StringUtilities.cpp #[===============================[ main sources: diff --git a/src/ripple/basics/StringUtilities.h b/src/ripple/basics/StringUtilities.h index 81e9ae826..48de772ca 100644 --- a/src/ripple/basics/StringUtilities.h +++ b/src/ripple/basics/StringUtilities.h @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -48,6 +49,24 @@ template std::optional strUnHex(std::size_t strSize, Iterator begin, Iterator end) { + static constexpr std::array const unxtab = []() { + std::array t{}; + + for (auto& x : t) + x = -1; + + for (int i = 0; i < 10; ++i) + t['0' + i] = i; + + for (int i = 0; i < 6; ++i) + { + t['A' + i] = 10 + i; + t['a' + i] = 10 + i; + } + + return t; + }(); + Blob out; out.reserve((strSize + 1) / 2); @@ -56,25 +75,22 @@ strUnHex(std::size_t strSize, Iterator begin, Iterator end) if (strSize & 1) { - int c = charUnHex(*iter); + int c = unxtab[*iter++]; if (c < 0) return {}; out.push_back(c); - ++iter; } while (iter != end) { - int cHigh = charUnHex(*iter); - ++iter; + int cHigh = unxtab[*iter++]; if (cHigh < 0) return {}; - int cLow = charUnHex(*iter); - ++iter; + int cLow = unxtab[*iter++]; if (cLow < 0) return {}; diff --git a/src/ripple/basics/impl/strHex.cpp b/src/ripple/basics/impl/strHex.cpp deleted file mode 100644 index 084493af5..000000000 --- a/src/ripple/basics/impl/strHex.cpp +++ /dev/null @@ -1,49 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012, 2013 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#include -#include - -namespace ripple { - -int -charUnHex(unsigned char c) -{ - static constexpr std::array const xtab = []() { - std::array t{}; - - for (auto& x : t) - x = -1; - - for (int i = 0; i < 10; ++i) - t['0' + i] = i; - - for (int i = 0; i < 6; ++i) - { - t['A' + i] = 10 + i; - t['a' + i] = 10 + i; - } - - return t; - }(); - - return xtab[c]; -} - -} // namespace ripple diff --git a/src/ripple/basics/strHex.h b/src/ripple/basics/strHex.h index e48ea9215..257fb540b 100644 --- a/src/ripple/basics/strHex.h +++ b/src/ripple/basics/strHex.h @@ -25,21 +25,6 @@ namespace ripple { -/** @{ */ -/** Converts a hex digit to the corresponding integer - @param cDigit one of '0'-'9', 'A'-'F' or 'a'-'f' - @return an integer from 0 to 15 on success; -1 on failure. -*/ -int -charUnHex(unsigned char c); - -inline int -charUnHex(char c) -{ - return charUnHex(static_cast(c)); -} -/** @} */ - template std::string strHex(FwdIt begin, FwdIt end) From 3d0c14f3e3108347e85889843cb90912700dacf0 Mon Sep 17 00:00:00 2001 From: Howard Hinnant Date: Thu, 18 Aug 2022 20:29:36 -0400 Subject: [PATCH 09/19] Remove use of deprecated std::iterator --- .../detail/aged_container_iterator.h | 59 ++++++++----------- .../container/detail/aged_ordered_container.h | 40 ++++++------- .../detail/aged_unordered_container.h | 42 ++++++------- src/ripple/beast/core/List.h | 3 +- src/ripple/beast/core/LockFreeStack.h | 15 +---- 5 files changed, 69 insertions(+), 90 deletions(-) diff --git a/src/ripple/beast/container/detail/aged_container_iterator.h b/src/ripple/beast/container/detail/aged_container_iterator.h index cd8677ce1..7467ad33c 100644 --- a/src/ripple/beast/container/detail/aged_container_iterator.h +++ b/src/ripple/beast/container/detail/aged_container_iterator.h @@ -30,23 +30,21 @@ class aged_ordered_container; namespace detail { -// Idea for Base template argument to prevent having to repeat -// the base class declaration comes from newbiz on ##c++/Freenode -// // If Iterator is SCARY then this iterator will be as well. -template < - bool is_const, - class Iterator, - class Base = std::iterator< - typename std::iterator_traits::iterator_category, - typename std::conditional< - is_const, - typename Iterator::value_type::stashed::value_type const, - typename Iterator::value_type::stashed::value_type>::type, - typename std::iterator_traits::difference_type>> -class aged_container_iterator : public Base +template +class aged_container_iterator { public: + using iterator_category = + typename std::iterator_traits::iterator_category; + using value_type = typename std::conditional< + is_const, + typename Iterator::value_type::stashed::value_type const, + typename Iterator::value_type::stashed::value_type>::type; + using difference_type = + typename std::iterator_traits::difference_type; + using pointer = value_type*; + using reference = value_type&; using time_point = typename Iterator::value_type::stashed::time_point; aged_container_iterator() = default; @@ -56,13 +54,11 @@ public: template < bool other_is_const, class OtherIterator, - class OtherBase, class = typename std::enable_if< (other_is_const == false || is_const == true) && std::is_same::value == false>::type> explicit aged_container_iterator( - aged_container_iterator const& - other) + aged_container_iterator const& other) : m_iter(other.m_iter) { } @@ -70,22 +66,19 @@ public: // Disable constructing a const_iterator from a non-const_iterator. template < bool other_is_const, - class OtherBase, class = typename std::enable_if< other_is_const == false || is_const == true>::type> aged_container_iterator( - aged_container_iterator const& - other) + aged_container_iterator const& other) : m_iter(other.m_iter) { } // Disable assigning a const_iterator to a non-const iterator - template + template auto operator=( - aged_container_iterator const& - other) -> + aged_container_iterator const& other) -> typename std::enable_if< other_is_const == false || is_const == true, aged_container_iterator&>::type @@ -94,20 +87,18 @@ public: return *this; } - template + template bool - operator==( - aged_container_iterator const& - other) const + operator==(aged_container_iterator const& + other) const { return m_iter == other.m_iter; } - template + template bool - operator!=( - aged_container_iterator const& - other) const + operator!=(aged_container_iterator const& + other) const { return m_iter != other.m_iter; } @@ -142,13 +133,13 @@ public: return prev; } - typename Base::reference + reference operator*() const { return m_iter->value; } - typename Base::pointer + pointer operator->() const { return &m_iter->value; @@ -167,7 +158,7 @@ private: template friend class aged_unordered_container; - template + template friend class aged_container_iterator; template diff --git a/src/ripple/beast/container/detail/aged_ordered_container.h b/src/ripple/beast/container/detail/aged_ordered_container.h index ed6585dd5..9da5f20a0 100644 --- a/src/ripple/beast/container/detail/aged_ordered_container.h +++ b/src/ripple/beast/container/detail/aged_ordered_container.h @@ -989,22 +989,20 @@ public: template < bool is_const, class Iterator, - class Base, class = std::enable_if_t::value>> - beast::detail::aged_container_iterator - erase(beast::detail::aged_container_iterator pos); + beast::detail::aged_container_iterator + erase(beast::detail::aged_container_iterator pos); // enable_if prevents erase (reverse_iterator first, reverse_iterator last) // from compiling template < bool is_const, class Iterator, - class Base, class = std::enable_if_t::value>> - beast::detail::aged_container_iterator + beast::detail::aged_container_iterator erase( - beast::detail::aged_container_iterator first, - beast::detail::aged_container_iterator last); + beast::detail::aged_container_iterator first, + beast::detail::aged_container_iterator last); template auto @@ -1019,10 +1017,9 @@ public: template < bool is_const, class Iterator, - class Base, class = std::enable_if_t::value>> void - touch(beast::detail::aged_container_iterator pos) + touch(beast::detail::aged_container_iterator pos) { touch(pos, clock().now()); } @@ -1264,11 +1261,10 @@ private: template < bool is_const, class Iterator, - class Base, class = std::enable_if_t::value>> void touch( - beast::detail::aged_container_iterator pos, + beast::detail::aged_container_iterator pos, typename clock_type::time_point const& now); template < @@ -2010,13 +2006,13 @@ template < class Clock, class Compare, class Allocator> -template -beast::detail::aged_container_iterator +template +beast::detail::aged_container_iterator aged_ordered_container:: - erase(beast::detail::aged_container_iterator pos) + erase(beast::detail::aged_container_iterator pos) { unlink_and_delete_element(&*((pos++).iterator())); - return beast::detail::aged_container_iterator( + return beast::detail::aged_container_iterator( pos.iterator()); } @@ -2028,17 +2024,17 @@ template < class Clock, class Compare, class Allocator> -template -beast::detail::aged_container_iterator +template +beast::detail::aged_container_iterator aged_ordered_container:: erase( - beast::detail::aged_container_iterator first, - beast::detail::aged_container_iterator last) + beast::detail::aged_container_iterator first, + beast::detail::aged_container_iterator last) { for (; first != last;) unlink_and_delete_element(&*((first++).iterator())); - return beast::detail::aged_container_iterator( + return beast::detail::aged_container_iterator( first.iterator()); } @@ -2173,11 +2169,11 @@ template < class Clock, class Compare, class Allocator> -template +template void aged_ordered_container:: touch( - beast::detail::aged_container_iterator pos, + beast::detail::aged_container_iterator pos, typename clock_type::time_point const& now) { auto& e(*pos.iterator()); diff --git a/src/ripple/beast/container/detail/aged_unordered_container.h b/src/ripple/beast/container/detail/aged_unordered_container.h index 8bc2330fa..920e6196b 100644 --- a/src/ripple/beast/container/detail/aged_unordered_container.h +++ b/src/ripple/beast/container/detail/aged_unordered_container.h @@ -1205,15 +1205,15 @@ public: return emplace(std::forward(args)...); } - template - beast::detail::aged_container_iterator - erase(beast::detail::aged_container_iterator pos); + template + beast::detail::aged_container_iterator + erase(beast::detail::aged_container_iterator pos); - template - beast::detail::aged_container_iterator + template + beast::detail::aged_container_iterator erase( - beast::detail::aged_container_iterator first, - beast::detail::aged_container_iterator last); + beast::detail::aged_container_iterator first, + beast::detail::aged_container_iterator last); template auto @@ -1222,9 +1222,9 @@ public: void swap(aged_unordered_container& other) noexcept; - template + template void - touch(beast::detail::aged_container_iterator pos) + touch(beast::detail::aged_container_iterator pos) { touch(pos, clock().now()); } @@ -1541,10 +1541,10 @@ private: insert_unchecked(first, last); } - template + template void touch( - beast::detail::aged_container_iterator pos, + beast::detail::aged_container_iterator pos, typename clock_type::time_point const& now) { auto& e(*pos.iterator()); @@ -3044,8 +3044,8 @@ template < class Hash, class KeyEqual, class Allocator> -template -beast::detail::aged_container_iterator +template +beast::detail::aged_container_iterator aged_unordered_container< IsMulti, IsMap, @@ -3054,11 +3054,11 @@ aged_unordered_container< Clock, Hash, KeyEqual, - Allocator>:: - erase(beast::detail::aged_container_iterator pos) + Allocator>::erase(beast::detail::aged_container_iterator + pos) { unlink_and_delete_element(&*((pos++).iterator())); - return beast::detail::aged_container_iterator( + return beast::detail::aged_container_iterator( pos.iterator()); } @@ -3071,8 +3071,8 @@ template < class Hash, class KeyEqual, class Allocator> -template -beast::detail::aged_container_iterator +template +beast::detail::aged_container_iterator aged_unordered_container< IsMulti, IsMap, @@ -3083,13 +3083,13 @@ aged_unordered_container< KeyEqual, Allocator>:: erase( - beast::detail::aged_container_iterator first, - beast::detail::aged_container_iterator last) + beast::detail::aged_container_iterator first, + beast::detail::aged_container_iterator last) { for (; first != last;) unlink_and_delete_element(&*((first++).iterator())); - return beast::detail::aged_container_iterator( + return beast::detail::aged_container_iterator( first.iterator()); } diff --git a/src/ripple/beast/core/List.h b/src/ripple/beast/core/List.h index 1daf5cda7..9b3c889d6 100644 --- a/src/ripple/beast/core/List.h +++ b/src/ripple/beast/core/List.h @@ -72,11 +72,12 @@ private: template class ListIterator - : public std::iterator { public: + using iterator_category = std::bidirectional_iterator_tag; using value_type = typename beast::detail::CopyConst::type; + using difference_type = std::ptrdiff_t; using pointer = value_type*; using reference = value_type&; using size_type = std::size_t; diff --git a/src/ripple/beast/core/LockFreeStack.h b/src/ripple/beast/core/LockFreeStack.h index ff022b96a..107564415 100644 --- a/src/ripple/beast/core/LockFreeStack.h +++ b/src/ripple/beast/core/LockFreeStack.h @@ -29,18 +29,7 @@ namespace beast { //------------------------------------------------------------------------------ template -class LockFreeStackIterator : public std::iterator< - std::forward_iterator_tag, - typename Container::value_type, - typename Container::difference_type, - typename std::conditional< - IsConst, - typename Container::const_pointer, - typename Container::pointer>::type, - typename std::conditional< - IsConst, - typename Container::const_reference, - typename Container::reference>::type> +class LockFreeStackIterator { protected: using Node = typename Container::Node; @@ -48,7 +37,9 @@ protected: typename std::conditional::type; public: + using iterator_category = std::forward_iterator_tag; using value_type = typename Container::value_type; + using difference_type = typename Container::difference_type; using pointer = typename std::conditional< IsConst, typename Container::const_pointer, From 5e1cb09b8892e650f6c34a66521b6b1673bd6b65 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S Date: Mon, 25 Jul 2022 13:25:01 -0700 Subject: [PATCH 10/19] Update broken link to hosted Doxygen content (fixes #4251) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f4cb4e2a1..20ce719d7 100644 --- a/README.md +++ b/README.md @@ -55,5 +55,5 @@ git-subtree. See those directories' README files for more details. * [XRP Ledger Dev Portal](https://xrpl.org/) * [Setup and Installation](https://xrpl.org/install-rippled.html) -* [Source Documentation (Doxygen)](https://ripple.github.io/rippled) +* [Source Documentation (Doxygen)](https://xrplf.github.io/rippled/) * [Learn more about the XRP Ledger (YouTube)](https://www.youtube.com/playlist?list=PLJQ55Tj1hIVZtJ_JdTvSum2qMTsedWkNi) From ce64f7a90f99c6b5e68d3c3d913443023de061a6 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S Date: Wed, 20 Jul 2022 15:48:41 -0700 Subject: [PATCH 11/19] Remove deprecated AccountTxOld.cpp (fixes #2926) --- Builds/CMake/RippledCore.cmake | 1 - src/ripple/app/main/Main.cpp | 5 +- src/ripple/rpc/handlers/AccountTxOld.cpp | 255 ----------------------- 3 files changed, 3 insertions(+), 258 deletions(-) delete mode 100644 src/ripple/rpc/handlers/AccountTxOld.cpp diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 10af3b310..041044b86 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -580,7 +580,6 @@ target_sources (rippled PRIVATE src/ripple/rpc/handlers/AccountObjects.cpp src/ripple/rpc/handlers/AccountOffers.cpp src/ripple/rpc/handlers/AccountTx.cpp - src/ripple/rpc/handlers/AccountTxOld.cpp src/ripple/rpc/handlers/BlackList.cpp src/ripple/rpc/handlers/BookOffers.cpp src/ripple/rpc/handlers/CanDelete.cpp diff --git a/src/ripple/app/main/Main.cpp b/src/ripple/app/main/Main.cpp index 64b6464a4..1d8169217 100644 --- a/src/ripple/app/main/Main.cpp +++ b/src/ripple/app/main/Main.cpp @@ -133,8 +133,9 @@ printHelp(const po::options_description& desc) " account_objects [] [strict]\n" " account_offers | [] " "[strict]\n" - " account_tx accountID [ledger_min [ledger_max [limit " - "[offset]]]] [binary] [count] [descending]\n" + " account_tx accountID [ledger_index_min [ledger_index_max " + "[limit " + "]]] [binary]\n" " book_changes []\n" " book_offers [ " "[ [ []]]]]\n" diff --git a/src/ripple/rpc/handlers/AccountTxOld.cpp b/src/ripple/rpc/handlers/AccountTxOld.cpp deleted file mode 100644 index 1bc64247c..000000000 --- a/src/ripple/rpc/handlers/AccountTxOld.cpp +++ /dev/null @@ -1,255 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of rippled: https://github.com/ripple/rippled - Copyright (c) 2012-2014 Ripple Labs Inc. - - Permission to use, copy, modify, and/or distribute this software for any - purpose with or without fee is hereby granted, provided that the above - copyright notice and this permission notice appear in all copies. - - THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES - WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF - MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR - ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES - WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN - ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF - OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -*/ -//============================================================================== - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -namespace ripple { - -// { -// account: account, -// ledger_index_min: ledger_index, -// ledger_index_max: ledger_index, -// binary: boolean, // optional, defaults to false -// count: boolean, // optional, defaults to false -// descending: boolean, // optional, defaults to false -// offset: integer, // optional, defaults to 0 -// limit: integer // optional -// } -Json::Value -doAccountTxOld(RPC::JsonContext& context) -{ - std::uint32_t offset = context.params.isMember(jss::offset) - ? context.params[jss::offset].asUInt() - : 0; - std::uint32_t limit = context.params.isMember(jss::limit) - ? context.params[jss::limit].asUInt() - : UINT32_MAX; - bool bBinary = context.params.isMember(jss::binary) && - context.params[jss::binary].asBool(); - bool bDescending = context.params.isMember(jss::descending) && - context.params[jss::descending].asBool(); - bool bCount = context.params.isMember(jss::count) && - context.params[jss::count].asBool(); - std::uint32_t uLedgerMin; - std::uint32_t uLedgerMax; - std::uint32_t uValidatedMin; - std::uint32_t uValidatedMax; - bool bValidated = - context.ledgerMaster.getValidatedRange(uValidatedMin, uValidatedMax); - - if (!context.params.isMember(jss::account)) - return rpcError(rpcINVALID_PARAMS); - - auto const raAccount = - parseBase58(context.params[jss::account].asString()); - if (!raAccount) - return rpcError(rpcACT_MALFORMED); - - if (offset > 3000) - return rpcError(rpcATX_DEPRECATED); - - context.loadType = Resource::feeHighBurdenRPC; - - // DEPRECATED - if (context.params.isMember(jss::ledger_min)) - { - context.params[jss::ledger_index_min] = context.params[jss::ledger_min]; - bDescending = true; - } - - // DEPRECATED - if (context.params.isMember(jss::ledger_max)) - { - context.params[jss::ledger_index_max] = context.params[jss::ledger_max]; - bDescending = true; - } - - if (context.params.isMember(jss::ledger_index_min) || - context.params.isMember(jss::ledger_index_max)) - { - std::int64_t iLedgerMin = context.params.isMember(jss::ledger_index_min) - ? context.params[jss::ledger_index_min].asInt() - : -1; - std::int64_t iLedgerMax = context.params.isMember(jss::ledger_index_max) - ? context.params[jss::ledger_index_max].asInt() - : -1; - - if (!bValidated && (iLedgerMin == -1 || iLedgerMax == -1)) - { - // Don't have a validated ledger range. - if (context.apiVersion == 1) - return rpcError(rpcLGR_IDXS_INVALID); - return rpcError(rpcNOT_SYNCED); - } - - uLedgerMin = iLedgerMin == -1 ? uValidatedMin : iLedgerMin; - uLedgerMax = iLedgerMax == -1 ? uValidatedMax : iLedgerMax; - - if (uLedgerMax < uLedgerMin) - { - if (context.apiVersion == 1) - return rpcError(rpcLGR_IDXS_INVALID); - return rpcError(rpcNOT_SYNCED); - } - } - else - { - std::shared_ptr ledger; - auto ret = RPC::lookupLedger(ledger, context); - - if (!ledger) - return ret; - - if (!ret[jss::validated].asBool() || - (ledger->info().seq > uValidatedMax) || - (ledger->info().seq < uValidatedMin)) - { - return rpcError(rpcLGR_NOT_VALIDATED); - } - - uLedgerMin = uLedgerMax = ledger->info().seq; - } - - int count = 0; - -#ifndef DEBUG - - try - { -#endif - - Json::Value ret(Json::objectValue); - - ret[jss::account] = context.app.accountIDCache().toBase58(*raAccount); - Json::Value& jvTxns = (ret[jss::transactions] = Json::arrayValue); - - RelationalDatabase::AccountTxOptions options = { - *raAccount, - uLedgerMin, - uLedgerMax, - offset, - limit, - isUnlimited(context.role)}; - - if (bBinary) - { - std::vector txns; - - if (bDescending) - txns = dynamic_cast( - &context.app.getRelationalDatabase()) - ->getNewestAccountTxsB(options); - else - txns = dynamic_cast( - &context.app.getRelationalDatabase()) - ->getOldestAccountTxsB(options); - - for (auto it = txns.begin(), end = txns.end(); it != end; ++it) - { - ++count; - Json::Value& jvObj = jvTxns.append(Json::objectValue); - - std::uint32_t uLedgerIndex = std::get<2>(*it); - jvObj[jss::tx_blob] = strHex(std::get<0>(*it)); - jvObj[jss::meta] = strHex(std::get<1>(*it)); - jvObj[jss::ledger_index] = uLedgerIndex; - jvObj[jss::validated] = bValidated && - uValidatedMin <= uLedgerIndex && - uValidatedMax >= uLedgerIndex; - } - } - else - { - RelationalDatabase::AccountTxs txns; - - if (bDescending) - txns = dynamic_cast( - &context.app.getRelationalDatabase()) - ->getNewestAccountTxs(options); - else - txns = dynamic_cast( - &context.app.getRelationalDatabase()) - ->getOldestAccountTxs(options); - - for (auto it = txns.begin(), end = txns.end(); it != end; ++it) - { - ++count; - Json::Value& jvObj = jvTxns.append(Json::objectValue); - - if (it->first) - jvObj[jss::tx] = - it->first->getJson(JsonOptions::include_date); - - if (it->second) - { - std::uint32_t uLedgerIndex = it->second->getLgrSeq(); - - auto meta = it->second->getJson(JsonOptions::none); - insertDeliveredAmount( - meta, context, it->first, *it->second); - jvObj[jss::meta] = std::move(meta); - - jvObj[jss::validated] = bValidated && - uValidatedMin <= uLedgerIndex && - uValidatedMax >= uLedgerIndex; - } - } - } - - // Add information about the original query - ret[jss::ledger_index_min] = uLedgerMin; - ret[jss::ledger_index_max] = uLedgerMax; - ret[jss::validated] = bValidated && uValidatedMin <= uLedgerMin && - uValidatedMax >= uLedgerMax; - ret[jss::offset] = offset; - - // We no longer return the full count but only the count of returned - // transactions. Computing this count was two expensive and this API is - // deprecated anyway. - if (bCount) - ret[jss::count] = count; - - if (context.params.isMember(jss::limit)) - ret[jss::limit] = limit; - - return ret; -#ifndef DEBUG - } - catch (std::exception const&) - { - return rpcError(rpcINTERNAL); - } - -#endif -} - -} // namespace ripple From b88ed5a8ec2a0735031ca23dc6569d54787dc2f2 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S Date: Fri, 15 Jul 2022 13:23:03 -0700 Subject: [PATCH 12/19] Update command-line usage help message (fixes #3318) --- src/ripple/app/main/Main.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ripple/app/main/Main.cpp b/src/ripple/app/main/Main.cpp index 1d8169217..70127afe0 100644 --- a/src/ripple/app/main/Main.cpp +++ b/src/ripple/app/main/Main.cpp @@ -161,6 +161,7 @@ printHelp(const po::options_description& desc) " ledger_request \n" " log_level [[] ]\n" " logrotate\n" + " manifest \n" " node_to_shard [status|start|stop]\n" " peers\n" " ping\n" @@ -180,6 +181,7 @@ printHelp(const po::options_description& desc) " submit_multisigned \n" " tx \n" " validation_create [||]\n" + " validator_info\n" " validators\n" " validator_list_sites\n" " version\n" From 9aaa0dff5fd422e5f6880df8e20a1fd5ad3b4424 Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S Date: Fri, 15 Jul 2022 16:26:48 -0700 Subject: [PATCH 13/19] Build the command map at compile time (fixes #3298): We profiled different algorithms and data structures to understand which strategy is best from a performance standpoint: - Linear search on an array; - Binary search on a sorted array; - Using `std::map`; and - Using `std::unordered_map`. Both linear search and std::unordered_map outperformed the other alternatives so no change to the existing data structure is justified. If more handers are added, this should be revisited. For some additional details and timings, please see: https://github.com/XRPLF/rippled/issues/3298#issuecomment-1185946010 --- src/ripple/net/impl/RPCCall.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ripple/net/impl/RPCCall.cpp b/src/ripple/net/impl/RPCCall.cpp index 334ca8693..eb4906f3a 100644 --- a/src/ripple/net/impl/RPCCall.cpp +++ b/src/ripple/net/impl/RPCCall.cpp @@ -1227,10 +1227,7 @@ public: int maxParams; }; - // FIXME: replace this with a function-static std::map and the lookup - // code with std::map::find when the problem with magic statics on - // Visual Studio is fixed. - static Command const commands[] = { + static constexpr Command commands[] = { // Request-response methods // - Returns an error, or the request. // - To modify the method, provide a new method in the request. From d318ab612adc86f1fd8527a50af232f377ca89ef Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Sun, 31 Jul 2022 23:10:03 -0700 Subject: [PATCH 14/19] Avoid unnecessary copying and dynamic memory allocations Co-authored-by: Chenna Keshava B S --- src/ripple/app/consensus/RCLConsensus.cpp | 11 ++----- src/ripple/app/consensus/RCLCxPeerPos.cpp | 38 ++++++----------------- src/ripple/app/consensus/RCLCxPeerPos.h | 32 ++++++------------- src/ripple/consensus/ConsensusProposal.h | 29 +++++++++++++++-- 4 files changed, 49 insertions(+), 61 deletions(-) diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index a4a4e0989..31c851eb8 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -211,15 +211,10 @@ RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal) prop.set_nodepubkey( validatorKeys_.publicKey.data(), validatorKeys_.publicKey.size()); - auto signingHash = sha512Half( - HashPrefix::proposal, - std::uint32_t(proposal.proposeSeq()), - proposal.closeTime().time_since_epoch().count(), - proposal.prevLedger(), - proposal.position()); - auto sig = signDigest( - validatorKeys_.publicKey, validatorKeys_.secretKey, signingHash); + validatorKeys_.publicKey, + validatorKeys_.secretKey, + proposal.signingHash()); prop.set_signature(sig.data(), sig.size()); diff --git a/src/ripple/app/consensus/RCLCxPeerPos.cpp b/src/ripple/app/consensus/RCLCxPeerPos.cpp index 709e78988..ee5a45b94 100644 --- a/src/ripple/app/consensus/RCLCxPeerPos.cpp +++ b/src/ripple/app/consensus/RCLCxPeerPos.cpp @@ -32,29 +32,23 @@ RCLCxPeerPos::RCLCxPeerPos( Slice const& signature, uint256 const& suppression, Proposal&& proposal) - : data_{std::make_shared( - publicKey, - signature, - suppression, - std::move(proposal))} + : publicKey_(publicKey) + , suppression_(suppression) + , proposal_(std::move(proposal)) { -} + // The maximum allowed size of a signature is 72 bytes; we verify + // this elsewhere, but we want to be extra careful here: + assert(signature.size() != 0 && signature.size() <= signature_.capacity()); -uint256 -RCLCxPeerPos::signingHash() const -{ - return sha512Half( - HashPrefix::proposal, - std::uint32_t(proposal().proposeSeq()), - proposal().closeTime().time_since_epoch().count(), - proposal().prevLedger(), - proposal().position()); + if (signature.size() != 0 && signature.size() <= signature_.capacity()) + signature_.assign(signature.begin(), signature.end()); } bool RCLCxPeerPos::checkSign() const { - return verifyDigest(publicKey(), signingHash(), signature(), false); + return verifyDigest( + publicKey(), proposal_.signingHash(), signature(), false); } Json::Value @@ -88,16 +82,4 @@ proposalUniqueId( return s.getSHA512Half(); } -RCLCxPeerPos::Data::Data( - PublicKey const& publicKey, - Slice const& signature, - uint256 const& suppress, - Proposal&& proposal) - : publicKey_{publicKey} - , signature_{signature} - , suppression_{suppress} - , proposal_{std::move(proposal)} -{ -} - } // namespace ripple diff --git a/src/ripple/app/consensus/RCLCxPeerPos.h b/src/ripple/app/consensus/RCLCxPeerPos.h index 9d448aac4..e82a85d42 100644 --- a/src/ripple/app/consensus/RCLCxPeerPos.h +++ b/src/ripple/app/consensus/RCLCxPeerPos.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -61,10 +62,6 @@ public: uint256 const& suppress, Proposal&& proposal); - //! Create the signing hash for the proposal - uint256 - signingHash() const; - //! Verify the signing hash of the proposal bool checkSign() const; @@ -73,27 +70,27 @@ public: Slice signature() const { - return data_->signature_; + return {signature_.data(), signature_.size()}; } //! Public key of peer that sent the proposal PublicKey const& publicKey() const { - return data_->publicKey_; + return publicKey_; } //! Unique id used by hash router to suppress duplicates uint256 const& suppressionID() const { - return data_->suppression_; + return suppression_; } Proposal const& proposal() const { - return data_->proposal_; + return proposal_; } //! JSON representation of proposal @@ -101,21 +98,10 @@ public: getJson() const; private: - struct Data : public CountedObject - { - PublicKey publicKey_; - Buffer signature_; - uint256 suppression_; - Proposal proposal_; - - Data( - PublicKey const& publicKey, - Slice const& signature, - uint256 const& suppress, - Proposal&& proposal); - }; - - std::shared_ptr data_; + PublicKey publicKey_; + uint256 suppression_; + Proposal proposal_; + boost::container::static_vector signature_; template void diff --git a/src/ripple/consensus/ConsensusProposal.h b/src/ripple/consensus/ConsensusProposal.h index a3eccbb01..c5103cfe0 100644 --- a/src/ripple/consensus/ConsensusProposal.h +++ b/src/ripple/consensus/ConsensusProposal.h @@ -16,13 +16,16 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ //============================================================================== -#ifndef RIPPLE_CONSENSUS_ConsensusProposal_H_INCLUDED -#define RIPPLE_CONSENSUS_ConsensusProposal_H_INCLUDED +#ifndef RIPPLE_CONSENSUS_CONSENSUSPROPOSAL_H_INCLUDED +#define RIPPLE_CONSENSUS_CONSENSUSPROPOSAL_H_INCLUDED +#include #include #include +#include #include #include +#include namespace ripple { /** Represents a proposed position taken during a round of consensus. @@ -169,6 +172,7 @@ public: NetClock::time_point newCloseTime, NetClock::time_point now) { + signingHash_.reset(); position_ = newPosition; closeTime_ = newCloseTime; time_ = now; @@ -185,6 +189,7 @@ public: void bowOut(NetClock::time_point now) { + signingHash_.reset(); time_ = now; proposeSeq_ = seqLeave; } @@ -210,6 +215,23 @@ public: return ret; } + //! The digest for this proposal, used for signing purposes. + uint256 const& + signingHash() const + { + if (!signingHash_) + { + signingHash_ = sha512Half( + HashPrefix::proposal, + std::uint32_t(proposeSeq()), + closeTime().time_since_epoch().count(), + prevLedger(), + position()); + } + + return signingHash_.value(); + } + private: //! Unique identifier of prior ledger this proposal is based on LedgerID_t previousLedger_; @@ -228,6 +250,9 @@ private: //! The identifier of the node taking this position NodeID_t nodeID_; + + //! The signing hash for this proposal + mutable std::optional signingHash_; }; template From 5a15229eeb13b69c8adf1f653b88a8f8b9480546 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Fri, 3 Jun 2022 18:09:37 -0700 Subject: [PATCH 15/19] Improve detection & handling of duplicate Node ID: Each node on the network is supposed to have a unique cryptographic identity. Typically, this identity is generated randomly at startup and stored for later reuse in the (poorly named) file `wallet.db`. If the file is copied, it is possible for two nodes to share the same node identity. This is generally not desirable and existing servers will detect and reject connections to other servers that have the same key. This commit achives three things: 1. It improves the detection code to pinpoint instances where two distinct servers with the same key connect with each other. In that case, servers will log an appropriate error and shut down pending intervention by the server's operator. 2. It makes it possible for server administrators to securely and easily generate new cryptographic identities for servers using the new `--newnodeid` command line arguments. When a server is started using this command, it will generate and save a random secure identity. 3. It makes it possible to configure the identity using a command line option, which makes it possible to derive it from data or parameters associated with the container or hardware where the instance is running by passing the `--nodeid` option, followed by a single argument identifying the infomation from which the node's identity is derived. For example, the following command will result in nodes with different hostnames having different node identities: `rippled --nodeid $HOSTNAME` The last option is particularly useful for automated cloud-based deployments that minimize the need for storing state and provide unique deployment identifiers. **Important note for server operators:** Depending on variables outside of the the control of this code, such as operating system version or configuration, permissions, and more, it may be possible for other users or programs to be able to access the command line arguments of other processes on the system. If you are operating in a shared environment, you should avoid using this option, preferring instead to use the `[node_seed]` option in the configuration file, and use permissions to limit exposure of the node seed. A user who gains access to the value used to derive the node's unique identity could impersonate that node. The commit also updates the minimum supported server protocol version to `XRPL/2.1`, which has been supported since version 1.5.0 and eliminates support for `XPRL/2.0`. --- src/ripple/app/consensus/RCLConsensus.cpp | 8 ++-- src/ripple/app/main/Application.cpp | 38 ++++++++++++++--- src/ripple/app/main/Application.h | 10 ++++- src/ripple/app/main/Main.cpp | 6 ++- src/ripple/app/main/NodeIdentity.cpp | 31 ++++++++++---- src/ripple/app/main/NodeIdentity.h | 11 ++++- src/ripple/app/rdb/Wallet.h | 17 ++++++-- src/ripple/app/rdb/impl/Wallet.cpp | 6 +++ src/ripple/overlay/README.md | 4 +- src/ripple/overlay/impl/Handshake.cpp | 39 +++++++++++++---- src/ripple/overlay/impl/ProtocolVersion.cpp | 1 - src/ripple/protocol/Seed.h | 8 +++- src/ripple/protocol/impl/Seed.cpp | 3 +- src/test/jtx/impl/Env.cpp | 2 +- src/test/overlay/ProtocolVersion_test.cpp | 47 +++++++++++---------- 15 files changed, 166 insertions(+), 65 deletions(-) diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index 31c851eb8..aec747e09 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -86,9 +86,11 @@ RCLConsensus::Adaptor::Adaptor( , inboundTransactions_{inboundTransactions} , j_(journal) , validatorKeys_(validatorKeys) - , valCookie_{rand_int( - 1, - std::numeric_limits::max())} + , valCookie_( + 1 + + rand_int( + crypto_prng(), + std::numeric_limits::max() - 1)) , nUnlVote_(validatorKeys_.nodeID, j_) { assert(valCookie_ != 0); diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index b7f18ba34..99f3b060b 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -52,10 +52,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include #include @@ -165,6 +167,8 @@ public: std::unique_ptr logs_; std::unique_ptr timeKeeper_; + std::uint64_t const instanceCookie_; + beast::Journal m_journal; std::unique_ptr perfLog_; Application::MutexType m_masterMutex; @@ -274,6 +278,11 @@ public: , config_(std::move(config)) , logs_(std::move(logs)) , timeKeeper_(std::move(timeKeeper)) + , instanceCookie_( + 1 + + rand_int( + crypto_prng(), + std::numeric_limits::max() - 1)) , m_journal(logs_->journal("Application")) // PerfLog must be started before any other threads are launched. @@ -508,13 +517,13 @@ public: //-------------------------------------------------------------------------- bool - setup() override; + setup(boost::program_options::variables_map const& cmdline) override; void start(bool withTimers) override; void run() override; void - signalStop() override; + signalStop(std::string msg = "") override; bool checkSigs() const override; void @@ -526,6 +535,12 @@ public: //-------------------------------------------------------------------------- + std::uint64_t + instanceID() const override + { + return instanceCookie_; + } + Logs& logs() override { @@ -1108,7 +1123,7 @@ private: // TODO Break this up into smaller, more digestible initialization segments. bool -ApplicationImp::setup() +ApplicationImp::setup(boost::program_options::variables_map const& cmdline) { // We want to intercept CTRL-C and the standard termination signal SIGTERM // and terminate the process. This handler will NEVER be invoked twice. @@ -1146,8 +1161,10 @@ ApplicationImp::setup() if (logs_->threshold() > kDebug) logs_->threshold(kDebug); } - JLOG(m_journal.info()) << "process starting: " - << BuildInfo::getFullVersionString(); + + JLOG(m_journal.info()) << "Process starting: " + << BuildInfo::getFullVersionString() + << ", Instance Cookie: " << instanceCookie_; if (numberOfThreads(*config_) < 2) { @@ -1265,7 +1282,7 @@ ApplicationImp::setup() if (!config().reporting()) m_orderBookDB.setup(getLedgerMaster().getCurrentLedger()); - nodeIdentity_ = getNodeIdentity(*this); + nodeIdentity_ = getNodeIdentity(*this, cmdline); if (!cluster_->load(config().section(SECTION_CLUSTER_NODES))) { @@ -1627,10 +1644,17 @@ ApplicationImp::run() } void -ApplicationImp::signalStop() +ApplicationImp::signalStop(std::string msg) { 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 diff --git a/src/ripple/app/main/Application.h b/src/ripple/app/main/Application.h index 53155ca4f..3b357deef 100644 --- a/src/ripple/app/main/Application.h +++ b/src/ripple/app/main/Application.h @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -136,13 +137,14 @@ public: virtual ~Application() = default; virtual bool - setup() = 0; + setup(boost::program_options::variables_map const& options) = 0; + virtual void start(bool withTimers) = 0; virtual void run() = 0; virtual void - signalStop() = 0; + signalStop(std::string msg = "") = 0; virtual bool checkSigs() const = 0; virtual void @@ -154,6 +156,10 @@ public: // --- // + /** Returns a 64-bit instance identifier, generated at startup */ + virtual std::uint64_t + instanceID() const = 0; + virtual Logs& logs() = 0; virtual Config& diff --git a/src/ripple/app/main/Main.cpp b/src/ripple/app/main/Main.cpp index 70127afe0..f25b83fd5 100644 --- a/src/ripple/app/main/Main.cpp +++ b/src/ripple/app/main/Main.cpp @@ -372,6 +372,10 @@ run(int argc, char** argv) "conf", po::value(), "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(), + "Specify the node identity for this server.")( "quorum", po::value(), "Override the minimum validation quorum.")( @@ -756,7 +760,7 @@ run(int argc, char** argv) auto app = make_Application( std::move(config), std::move(logs), std::move(timeKeeper)); - if (!app->setup()) + if (!app->setup(vm)) return -1; // With our configuration parsed, ensure we have diff --git a/src/ripple/app/main/NodeIdentity.cpp b/src/ripple/app/main/NodeIdentity.cpp index a2051bbb6..e66b9e840 100644 --- a/src/ripple/app/main/NodeIdentity.cpp +++ b/src/ripple/app/main/NodeIdentity.cpp @@ -20,27 +20,38 @@ #include #include #include -#include #include #include -#include #include namespace ripple { std::pair -getNodeIdentity(Application& app) +getNodeIdentity( + Application& app, + boost::program_options::variables_map const& cmdline) { - // If a seed is specified in the configuration file use that directly. - if (app.config().exists(SECTION_NODE_SEED)) + std::optional seed; + + if (cmdline.count("nodeid")) { - auto const seed = parseBase58( + seed = parseGenericSeed(cmdline["nodeid"].as(), false); + + if (!seed) + Throw("Invalid 'nodeid' in command line"); + } + else if (app.config().exists(SECTION_NODE_SEED)) + { + seed = parseBase58( app.config().section(SECTION_NODE_SEED).lines().front()); if (!seed) - Throw("NodeIdentity: Bad [" SECTION_NODE_SEED - "] specified"); + Throw("Invalid [" SECTION_NODE_SEED + "] in configuration file"); + } + if (seed) + { auto secretKey = generateSecretKey(KeyType::secp256k1, *seed); auto publicKey = derivePublicKey(KeyType::secp256k1, secretKey); @@ -48,6 +59,10 @@ getNodeIdentity(Application& app) } auto db = app.getWalletDB().checkoutDb(); + + if (cmdline.count("newnodeid") != 0) + clearNodeIdentity(*db); + return getNodeIdentity(*db); } diff --git a/src/ripple/app/main/NodeIdentity.h b/src/ripple/app/main/NodeIdentity.h index 60deeed85..b82b3657a 100644 --- a/src/ripple/app/main/NodeIdentity.h +++ b/src/ripple/app/main/NodeIdentity.h @@ -23,13 +23,20 @@ #include #include #include +#include #include namespace ripple { -/** The cryptographic credentials identifying this server instance. */ +/** The cryptographic credentials identifying this server instance. + + @param app The application object + @param cmdline The command line parameters passed into the application. + */ std::pair -getNodeIdentity(Application& app); +getNodeIdentity( + Application& app, + boost::program_options::variables_map const& cmdline); } // namespace ripple diff --git a/src/ripple/app/rdb/Wallet.h b/src/ripple/app/rdb/Wallet.h index 6bf6ca9ea..2769e459a 100644 --- a/src/ripple/app/rdb/Wallet.h +++ b/src/ripple/app/rdb/Wallet.h @@ -87,10 +87,19 @@ saveManifests( void addValidatorManifest(soci::session& session, std::string const& serialized); -/** - * @brief getNodeIdentity Returns the public and private keys of this node. - * @param session Session with the database. - * @return Pair of public and private keys. +/** 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. */ std::pair getNodeIdentity(soci::session& session); diff --git a/src/ripple/app/rdb/impl/Wallet.cpp b/src/ripple/app/rdb/impl/Wallet.cpp index 24715404c..25a06bbd9 100644 --- a/src/ripple/app/rdb/impl/Wallet.cpp +++ b/src/ripple/app/rdb/impl/Wallet.cpp @@ -119,6 +119,12 @@ addValidatorManifest(soci::session& session, std::string const& serialized) tr.commit(); } +void +clearNodeIdentity(soci::session& session) +{ + session << "DELETE FROM NodeIdentity;"; +} + std::pair getNodeIdentity(soci::session& session) { diff --git a/src/ripple/overlay/README.md b/src/ripple/overlay/README.md index 289c5f707..8be890ef7 100644 --- a/src/ripple/overlay/README.md +++ b/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 presently encoded using **Base64** encoding, but implementations -should support both **Base64** and **HEX** encoding for this value. +The value is encoded as **HEX**, but implementations should support both +**Base64** and **HEX** encoding for this value for legacy purposes. | Field Name | Request | Response | |--------------------- |:-----------------: |:-----------------: | diff --git a/src/ripple/overlay/impl/Handshake.cpp b/src/ripple/overlay/impl/Handshake.cpp index 2ea208f55..793dec19e 100644 --- a/src/ripple/overlay/impl/Handshake.cpp +++ b/src/ripple/overlay/impl/Handshake.cpp @@ -204,6 +204,8 @@ 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); @@ -215,14 +217,8 @@ buildHandshake( if (auto const cl = app.getLedgerMaster().getClosedLedger()) { - // 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())); + h.insert("Closed-Ledger", strHex(cl->info().hash)); + h.insert("Previous-Ledger", strHex(cl->info().parentHash)); } } @@ -306,7 +302,34 @@ 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: // diff --git a/src/ripple/overlay/impl/ProtocolVersion.cpp b/src/ripple/overlay/impl/ProtocolVersion.cpp index 4931dacb4..9a549b563 100644 --- a/src/ripple/overlay/impl/ProtocolVersion.cpp +++ b/src/ripple/overlay/impl/ProtocolVersion.cpp @@ -36,7 +36,6 @@ namespace ripple { // clang-format off constexpr ProtocolVersion const supportedProtocolList[] { - {2, 0}, {2, 1}, {2, 2} }; diff --git a/src/ripple/protocol/Seed.h b/src/ripple/protocol/Seed.h index c1768d205..2ebc64970 100644 --- a/src/ripple/protocol/Seed.h +++ b/src/ripple/protocol/Seed.h @@ -116,9 +116,13 @@ template <> std::optional parseBase58(std::string const& s); -/** Attempt to parse a string as a seed */ +/** 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) + * */ std::optional -parseGenericSeed(std::string const& str); +parseGenericSeed(std::string const& str, bool rfc1751 = true); /** Encode a Seed in RFC1751 format */ std::string diff --git a/src/ripple/protocol/impl/Seed.cpp b/src/ripple/protocol/impl/Seed.cpp index f4c6ee52b..49da20a42 100644 --- a/src/ripple/protocol/impl/Seed.cpp +++ b/src/ripple/protocol/impl/Seed.cpp @@ -87,7 +87,7 @@ parseBase58(std::string const& s) } std::optional -parseGenericSeed(std::string const& str) +parseGenericSeed(std::string const& str, bool rfc1751) { if (str.empty()) return std::nullopt; @@ -111,6 +111,7 @@ parseGenericSeed(std::string const& str) if (auto seed = parseBase58(str)) return seed; + if (rfc1751) { std::string key; if (RFC1751::getKeyFromEnglish(key, str) == 1) diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 3445fd1c9..900b9812d 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -83,7 +83,7 @@ Env::AppBundle::AppBundle( std::move(config), std::move(logs), std::move(timeKeeper_)); app = owned.get(); app->logs().threshold(thresh); - if (!app->setup()) + if (!app->setup({})) Throw("Env::AppBundle: setup failed"); timeKeeper->set(app->getLedgerMaster().getClosedLedger()->info().closeTime); app->start(false /*don't start timers*/); diff --git a/src/test/overlay/ProtocolVersion_test.cpp b/src/test/overlay/ProtocolVersion_test.cpp index 81845345d..a5a26fe74 100644 --- a/src/test/overlay/ProtocolVersion_test.cpp +++ b/src/test/overlay/ProtocolVersion_test.cpp @@ -25,22 +25,21 @@ namespace ripple { class ProtocolVersion_test : public beast::unit_test::suite { private: - template - static std::string - join(FwdIt first, FwdIt last, char const* sep = ",") - { - std::string result; - if (first == last) - return result; - result = to_string(*first++); - while (first != last) - result += sep + to_string(*first++); - return result; - } - void check(std::string const& s, std::string const& answer) { + auto join = [](auto first, auto last) { + std::string result; + if (first != last) + { + result = to_string(*first++); + + while (first != last) + result += "," + to_string(*first++); + } + return result; + }; + auto const result = parseProtocolVersions(s); BEAST_EXPECT(join(result.begin(), result.end()) == answer); } @@ -60,20 +59,21 @@ public: // Empty string check("", ""); + + // clang-format off check( - "RTXP/1.1,RTXP/1.2,RTXP/1.3,XRPL/2.1,XRPL/2.0", + "RTXP/1.1,RTXP/1.2,RTXP/1.3,XRPL/2.1,XRPL/2.0,/XRPL/3.0", "XRPL/2.0,XRPL/2.1"); check( - "RTXP/0.9,RTXP/1.01,XRPL/0.3,XRPL/2.01,XRPL/19.04,Oscar/" - "123,NIKB", + "RTXP/0.9,RTXP/1.01,XRPL/0.3,XRPL/2.01,websocket", ""); check( - "XRPL/2.0,RTXP/1.2,XRPL/2.0,XRPL/19.4,XRPL/7.89,XRPL/" - "A.1,XRPL/2.01", + "XRPL/2.0,XRPL/2.0,XRPL/19.4,XRPL/7.89,XRPL/XRPL/3.0,XRPL/2.01", "XRPL/2.0,XRPL/7.89,XRPL/19.4"); check( "XRPL/2.0,XRPL/3.0,XRPL/4,XRPL/,XRPL,OPT XRPL/2.2,XRPL/5.67", "XRPL/2.0,XRPL/3.0,XRPL/5.67"); + // clang-format on } { @@ -81,13 +81,14 @@ public: BEAST_EXPECT(negotiateProtocolVersion("RTXP/1.2") == std::nullopt); BEAST_EXPECT( - negotiateProtocolVersion("RTXP/1.2, XRPL/2.0") == - make_protocol(2, 0)); + negotiateProtocolVersion("RTXP/1.2, XRPL/2.0, XRPL/2.1") == + make_protocol(2, 1)); BEAST_EXPECT( - negotiateProtocolVersion("XRPL/2.0") == make_protocol(2, 0)); + negotiateProtocolVersion("XRPL/2.2") == make_protocol(2, 2)); BEAST_EXPECT( - negotiateProtocolVersion("RTXP/1.2, XRPL/2.0, XRPL/999.999") == - make_protocol(2, 0)); + negotiateProtocolVersion( + "RTXP/1.2, XRPL/2.2, XRPL/2.3, XRPL/999.999") == + make_protocol(2, 2)); BEAST_EXPECT( negotiateProtocolVersion("XRPL/999.999, WebSocket/1.0") == std::nullopt); From e2eed966b0ecb6445027e6a023b48d702c5f4832 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Wed, 25 May 2022 11:22:47 -0700 Subject: [PATCH 16/19] Improve AccountID string conversion caching: Caching the base58check encoded version of an `AccountID` has performance advantages, because because of the computationally heavy cost associated with the conversion, which requires the application of SHA-256 twice. This commit makes the cache significantly more efficient in terms of memory used: it eliminates the map, using a vector with a size that is determined by the configured size of the node, and a hash function to directly map any given `AccountID` to a specific slot in the cache; the eviction policy is simple: in case of collision the existing entry is removed and replaced with the new data. Previously, use of the cache was optional and required additional effort by the programmer. Now the cache is automatic and does not require any additional work or information. The new cache also utilizes a 64-way spinlock, to help reduce any contention that the pressure on the cache would impose. --- src/ripple/app/ledger/AcceptedLedger.cpp | 4 +- src/ripple/app/ledger/AcceptedLedgerTx.cpp | 5 +- src/ripple/app/ledger/AcceptedLedgerTx.h | 3 +- src/ripple/app/main/Application.cpp | 11 +- src/ripple/app/main/Application.h | 3 - src/ripple/app/paths/PathRequest.cpp | 19 ++- src/ripple/app/rdb/backend/detail/Node.h | 4 - .../app/rdb/backend/detail/impl/Node.cpp | 21 +-- .../app/rdb/backend/impl/SQLiteDatabase.cpp | 60 +++------ src/ripple/core/Config.h | 3 +- src/ripple/core/impl/Config.cpp | 29 ++-- src/ripple/protocol/AccountID.h | 43 ++---- src/ripple/protocol/impl/AccountID.cpp | 126 +++++++++++------- src/ripple/rpc/handlers/AccountChannels.cpp | 2 +- src/ripple/rpc/handlers/AccountInfo.cpp | 2 +- src/ripple/rpc/handlers/AccountLines.cpp | 2 +- src/ripple/rpc/handlers/AccountObjects.cpp | 4 +- src/ripple/rpc/handlers/AccountOffers.cpp | 2 +- src/ripple/rpc/handlers/GatewayBalances.cpp | 2 +- src/ripple/rpc/handlers/NFTOffers.cpp | 6 +- src/ripple/rpc/handlers/NoRippleCheck.cpp | 2 +- 21 files changed, 157 insertions(+), 196 deletions(-) diff --git a/src/ripple/app/ledger/AcceptedLedger.cpp b/src/ripple/app/ledger/AcceptedLedger.cpp index 526704d18..4f308653d 100644 --- a/src/ripple/app/ledger/AcceptedLedger.cpp +++ b/src/ripple/app/ledger/AcceptedLedger.cpp @@ -31,11 +31,9 @@ AcceptedLedger::AcceptedLedger( transactions_.reserve(256); auto insertAll = [&](auto const& txns) { - auto const& idcache = app.accountIDCache(); - for (auto const& item : txns) transactions_.emplace_back(std::make_unique( - ledger, item.first, item.second, idcache)); + ledger, item.first, item.second)); }; if (app.config().reporting()) diff --git a/src/ripple/app/ledger/AcceptedLedgerTx.cpp b/src/ripple/app/ledger/AcceptedLedgerTx.cpp index f0408b0c0..613a91e43 100644 --- a/src/ripple/app/ledger/AcceptedLedgerTx.cpp +++ b/src/ripple/app/ledger/AcceptedLedgerTx.cpp @@ -28,8 +28,7 @@ namespace ripple { AcceptedLedgerTx::AcceptedLedgerTx( std::shared_ptr const& ledger, std::shared_ptr const& txn, - std::shared_ptr const& met, - AccountIDCache const& accountCache) + std::shared_ptr const& met) : mTxn(txn) , mMeta(txn->getTransactionID(), ledger->seq(), *met) , mAffected(mMeta.getAffectedAccounts()) @@ -52,7 +51,7 @@ AcceptedLedgerTx::AcceptedLedgerTx( { Json::Value& affected = (mJson[jss::affected] = Json::arrayValue); for (auto const& account : mAffected) - affected.append(accountCache.toBase58(account)); + affected.append(toBase58(account)); } if (mTxn->getTxnType() == ttOFFER_CREATE) diff --git a/src/ripple/app/ledger/AcceptedLedgerTx.h b/src/ripple/app/ledger/AcceptedLedgerTx.h index 7d6897857..2995d447b 100644 --- a/src/ripple/app/ledger/AcceptedLedgerTx.h +++ b/src/ripple/app/ledger/AcceptedLedgerTx.h @@ -46,8 +46,7 @@ public: AcceptedLedgerTx( std::shared_ptr const& ledger, std::shared_ptr const&, - std::shared_ptr const&, - AccountIDCache const&); + std::shared_ptr const&); std::shared_ptr const& getTxn() const diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 99f3b060b..dce11bc38 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -181,7 +181,6 @@ public: NodeStoreScheduler m_nodeStoreScheduler; std::unique_ptr m_shaMapStore; PendingSaves pendingSaves_; - AccountIDCache accountIDCache_; std::optional openLedger_; NodeCache m_tempNodeCache; @@ -336,8 +335,6 @@ public: m_nodeStoreScheduler, logs_->journal("SHAMapStore"))) - , accountIDCache_(128000) - , m_tempNodeCache( "NodeCache", 16384, @@ -494,6 +491,8 @@ public: config_->reporting() ? std::make_unique(*this) : nullptr) { + initAccountIdCache(config_->getValueFor(SizedItem::accountIdCacheSize)); + add(m_resourceManager.get()); // @@ -856,12 +855,6 @@ public: return pendingSaves_; } - AccountIDCache const& - accountIDCache() const override - { - return accountIDCache_; - } - OpenLedger& openLedger() override { diff --git a/src/ripple/app/main/Application.h b/src/ripple/app/main/Application.h index 3b357deef..d8cb7d318 100644 --- a/src/ripple/app/main/Application.h +++ b/src/ripple/app/main/Application.h @@ -90,7 +90,6 @@ class PathRequests; class PendingSaves; class PublicKey; class SecretKey; -class AccountIDCache; class STLedgerEntry; class TimeKeeper; class TransactionMaster; @@ -251,8 +250,6 @@ public: getSHAMapStore() = 0; virtual PendingSaves& pendingSaves() = 0; - virtual AccountIDCache const& - accountIDCache() const = 0; virtual OpenLedger& openLedger() = 0; virtual OpenLedger const& diff --git a/src/ripple/app/paths/PathRequest.cpp b/src/ripple/app/paths/PathRequest.cpp index d1acb3ac1..02b46c81e 100644 --- a/src/ripple/app/paths/PathRequest.cpp +++ b/src/ripple/app/paths/PathRequest.cpp @@ -552,9 +552,16 @@ PathRequest::findPaths( continueCallback); mContext[issue] = ps; - auto& sourceAccount = !isXRP(issue.account) - ? issue.account - : isXRP(issue.currency) ? xrpAccount() : *raSrcAccount; + auto const& sourceAccount = [&] { + if (!isXRP(issue.account)) + return issue.account; + + if (isXRP(issue.currency)) + return xrpAccount(); + + return *raSrcAccount; + }(); + STAmount saMaxAmount = saSendMax.value_or( STAmount({issue.currency, sourceAccount}, 1u, 0, true)); @@ -675,10 +682,8 @@ PathRequest::doUpdate( destCurrencies.append(to_string(c)); } - newStatus[jss::source_account] = - app_.accountIDCache().toBase58(*raSrcAccount); - newStatus[jss::destination_account] = - app_.accountIDCache().toBase58(*raDstAccount); + newStatus[jss::source_account] = toBase58(*raSrcAccount); + newStatus[jss::destination_account] = toBase58(*raDstAccount); newStatus[jss::destination_amount] = saDstAmount.getJson(JsonOptions::none); newStatus[jss::full_reply] = !fast; diff --git a/src/ripple/app/rdb/backend/detail/Node.h b/src/ripple/app/rdb/backend/detail/Node.h index fa7e39c83..0ebae76dc 100644 --- a/src/ripple/app/rdb/backend/detail/Node.h +++ b/src/ripple/app/rdb/backend/detail/Node.h @@ -389,7 +389,6 @@ getNewestAccountTxsB( * account which match given criteria starting from given marker * and calls callback for each found transaction. * @param session Session with database. - * @param idCache Account ID cache. * @param onUnsavedLedger Callback function to call on each found unsaved * ledger within given range. * @param onTransaction Callback function to call on each found transaction. @@ -408,7 +407,6 @@ getNewestAccountTxsB( std::pair, int> oldestAccountTxPage( soci::session& session, - AccountIDCache const& idCache, std::function const& onUnsavedLedger, std::function< void(std::uint32_t, std::string const&, Blob&&, Blob&&)> const& @@ -422,7 +420,6 @@ oldestAccountTxPage( * account which match given criteria starting from given marker * and calls callback for each found transaction. * @param session Session with database. - * @param idCache Account ID cache. * @param onUnsavedLedger Callback function to call on each found unsaved * ledger within given range. * @param onTransaction Callback function to call on each found transaction. @@ -441,7 +438,6 @@ oldestAccountTxPage( std::pair, int> newestAccountTxPage( soci::session& session, - AccountIDCache const& idCache, std::function const& onUnsavedLedger, std::function< void(std::uint32_t, std::string const&, Blob&&, Blob&&)> const& diff --git a/src/ripple/app/rdb/backend/detail/impl/Node.cpp b/src/ripple/app/rdb/backend/detail/impl/Node.cpp index 0c9f3b017..b3b354ebe 100644 --- a/src/ripple/app/rdb/backend/detail/impl/Node.cpp +++ b/src/ripple/app/rdb/backend/detail/impl/Node.cpp @@ -307,7 +307,7 @@ saveValidatedLedger( sql += txnId; sql += "','"; - sql += app.accountIDCache().toBase58(account); + sql += toBase58(account); sql += "',"; sql += ledgerSeq; sql += ","; @@ -760,8 +760,7 @@ transactionsSQL( sql = boost::str( boost::format("SELECT %s FROM AccountTransactions " "WHERE Account = '%s' %s %s LIMIT %u, %u;") % - selection % app.accountIDCache().toBase58(options.account) % - maxClause % minClause % + selection % toBase58(options.account) % maxClause % minClause % beast::lexicalCastThrow(options.offset) % beast::lexicalCastThrow(numberOfResults)); else @@ -774,9 +773,9 @@ transactionsSQL( "ORDER BY AccountTransactions.LedgerSeq %s, " "AccountTransactions.TxnSeq %s, AccountTransactions.TransID %s " "LIMIT %u, %u;") % - selection % app.accountIDCache().toBase58(options.account) % - maxClause % minClause % (descending ? "DESC" : "ASC") % + selection % toBase58(options.account) % maxClause % minClause % (descending ? "DESC" : "ASC") % (descending ? "DESC" : "ASC") % + (descending ? "DESC" : "ASC") % beast::lexicalCastThrow(options.offset) % beast::lexicalCastThrow(numberOfResults)); JLOG(j.trace()) << "txSQL query: " << sql; @@ -1049,7 +1048,6 @@ getNewestAccountTxsB( * account that matches the given criteria starting from the provided * marker and invokes the callback parameter for each found transaction. * @param session Session with the database. - * @param idCache Account ID cache. * @param onUnsavedLedger Callback function to call on each found unsaved * ledger within the given range. * @param onTransaction Callback function to call on each found transaction. @@ -1069,7 +1067,6 @@ getNewestAccountTxsB( static std::pair, int> accountTxPage( soci::session& session, - AccountIDCache const& idCache, std::function const& onUnsavedLedger, std::function< void(std::uint32_t, std::string const&, Blob&&, Blob&&)> const& @@ -1135,8 +1132,8 @@ accountTxPage( ORDER BY AccountTransactions.LedgerSeq %s, AccountTransactions.TxnSeq %s LIMIT %u;)")) % - idCache.toBase58(options.account) % options.minLedger % - options.maxLedger % order % order % queryLimit); + toBase58(options.account) % options.minLedger % options.maxLedger % + order % order % queryLimit); } else { @@ -1146,7 +1143,7 @@ accountTxPage( const std::uint32_t maxLedger = forward ? options.maxLedger : findLedger - 1; - auto b58acct = idCache.toBase58(options.account); + auto b58acct = toBase58(options.account); sql = boost::str( boost::format(( R"(SELECT AccountTransactions.LedgerSeq,AccountTransactions.TxnSeq, @@ -1250,7 +1247,6 @@ accountTxPage( std::pair, int> oldestAccountTxPage( soci::session& session, - AccountIDCache const& idCache, std::function const& onUnsavedLedger, std::function< void(std::uint32_t, std::string const&, Blob&&, Blob&&)> const& @@ -1261,7 +1257,6 @@ oldestAccountTxPage( { return accountTxPage( session, - idCache, onUnsavedLedger, onTransaction, options, @@ -1273,7 +1268,6 @@ oldestAccountTxPage( std::pair, int> newestAccountTxPage( soci::session& session, - AccountIDCache const& idCache, std::function const& onUnsavedLedger, std::function< void(std::uint32_t, std::string const&, Blob&&, Blob&&)> const& @@ -1284,7 +1278,6 @@ newestAccountTxPage( { return accountTxPage( session, - idCache, onUnsavedLedger, onTransaction, options, diff --git a/src/ripple/app/rdb/backend/impl/SQLiteDatabase.cpp b/src/ripple/app/rdb/backend/impl/SQLiteDatabase.cpp index e6ec44399..547ab843b 100644 --- a/src/ripple/app/rdb/backend/impl/SQLiteDatabase.cpp +++ b/src/ripple/app/rdb/backend/impl/SQLiteDatabase.cpp @@ -1322,7 +1322,6 @@ SQLiteDatabaseImp::oldestAccountTxPage(AccountTxPageOptions const& options) return {}; static std::uint32_t const page_length(200); - auto& idCache = app_.accountIDCache(); auto onUnsavedLedger = std::bind(saveLedgerAsync, std::ref(app_), std::placeholders::_1); AccountTxs ret; @@ -1338,15 +1337,10 @@ SQLiteDatabaseImp::oldestAccountTxPage(AccountTxPageOptions const& options) if (existsTransaction()) { auto db = checkoutTransaction(); - auto newmarker = detail::oldestAccountTxPage( - *db, - idCache, - onUnsavedLedger, - onTransaction, - options, - 0, - page_length) - .first; + auto newmarker = + detail::oldestAccountTxPage( + *db, onUnsavedLedger, onTransaction, options, 0, page_length) + .first; return {ret, newmarker}; } @@ -1363,7 +1357,6 @@ SQLiteDatabaseImp::oldestAccountTxPage(AccountTxPageOptions const& options) return false; auto [marker, total] = detail::oldestAccountTxPage( session, - idCache, onUnsavedLedger, onTransaction, opt, @@ -1391,7 +1384,6 @@ SQLiteDatabaseImp::newestAccountTxPage(AccountTxPageOptions const& options) return {}; static std::uint32_t const page_length(200); - auto& idCache = app_.accountIDCache(); auto onUnsavedLedger = std::bind(saveLedgerAsync, std::ref(app_), std::placeholders::_1); AccountTxs ret; @@ -1407,15 +1399,10 @@ SQLiteDatabaseImp::newestAccountTxPage(AccountTxPageOptions const& options) if (existsTransaction()) { auto db = checkoutTransaction(); - auto newmarker = detail::newestAccountTxPage( - *db, - idCache, - onUnsavedLedger, - onTransaction, - options, - 0, - page_length) - .first; + auto newmarker = + detail::newestAccountTxPage( + *db, onUnsavedLedger, onTransaction, options, 0, page_length) + .first; return {ret, newmarker}; } @@ -1432,7 +1419,6 @@ SQLiteDatabaseImp::newestAccountTxPage(AccountTxPageOptions const& options) return false; auto [marker, total] = detail::newestAccountTxPage( session, - idCache, onUnsavedLedger, onTransaction, opt, @@ -1460,7 +1446,6 @@ SQLiteDatabaseImp::oldestAccountTxPageB(AccountTxPageOptions const& options) return {}; static std::uint32_t const page_length(500); - auto& idCache = app_.accountIDCache(); auto onUnsavedLedger = std::bind(saveLedgerAsync, std::ref(app_), std::placeholders::_1); MetaTxsList ret; @@ -1475,15 +1460,10 @@ SQLiteDatabaseImp::oldestAccountTxPageB(AccountTxPageOptions const& options) if (existsTransaction()) { auto db = checkoutTransaction(); - auto newmarker = detail::oldestAccountTxPage( - *db, - idCache, - onUnsavedLedger, - onTransaction, - options, - 0, - page_length) - .first; + auto newmarker = + detail::oldestAccountTxPage( + *db, onUnsavedLedger, onTransaction, options, 0, page_length) + .first; return {ret, newmarker}; } @@ -1500,7 +1480,6 @@ SQLiteDatabaseImp::oldestAccountTxPageB(AccountTxPageOptions const& options) return false; auto [marker, total] = detail::oldestAccountTxPage( session, - idCache, onUnsavedLedger, onTransaction, opt, @@ -1528,7 +1507,6 @@ SQLiteDatabaseImp::newestAccountTxPageB(AccountTxPageOptions const& options) return {}; static std::uint32_t const page_length(500); - auto& idCache = app_.accountIDCache(); auto onUnsavedLedger = std::bind(saveLedgerAsync, std::ref(app_), std::placeholders::_1); MetaTxsList ret; @@ -1543,15 +1521,10 @@ SQLiteDatabaseImp::newestAccountTxPageB(AccountTxPageOptions const& options) if (existsTransaction()) { auto db = checkoutTransaction(); - auto newmarker = detail::newestAccountTxPage( - *db, - idCache, - onUnsavedLedger, - onTransaction, - options, - 0, - page_length) - .first; + auto newmarker = + detail::newestAccountTxPage( + *db, onUnsavedLedger, onTransaction, options, 0, page_length) + .first; return {ret, newmarker}; } @@ -1568,7 +1541,6 @@ SQLiteDatabaseImp::newestAccountTxPageB(AccountTxPageOptions const& options) return false; auto [marker, total] = detail::newestAccountTxPage( session, - idCache, onUnsavedLedger, onTransaction, opt, diff --git a/src/ripple/core/Config.h b/src/ripple/core/Config.h index c4a5076e7..2d440a1af 100644 --- a/src/ripple/core/Config.h +++ b/src/ripple/core/Config.h @@ -57,7 +57,8 @@ enum class SizedItem : std::size_t { lgrDBCache, openFinalLimit, burstSize, - ramSizeGB + ramSizeGB, + accountIdCacheSize, }; // This entire derived class is deprecated. diff --git a/src/ripple/core/impl/Config.cpp b/src/ripple/core/impl/Config.cpp index e53d96883..f8d8878a7 100644 --- a/src/ripple/core/impl/Config.cpp +++ b/src/ripple/core/impl/Config.cpp @@ -108,26 +108,27 @@ namespace ripple { // clang-format off // The configurable node sizes are "tiny", "small", "medium", "large", "huge" -inline constexpr std::array>, 12> +inline constexpr std::array>, 13> sizedItems {{ // FIXME: We should document each of these items, explaining exactly // what they control and whether there exists an explicit // config option that can be used to override the default. - // tiny small medium large huge - {SizedItem::sweepInterval, {{ 10, 30, 60, 90, 120 }}}, - {SizedItem::treeCacheSize, {{ 262144, 524288, 2097152, 4194304, 8388608 }}}, - {SizedItem::treeCacheAge, {{ 30, 60, 90, 120, 900 }}}, - {SizedItem::ledgerSize, {{ 32, 32, 64, 256, 384 }}}, - {SizedItem::ledgerAge, {{ 30, 60, 180, 300, 600 }}}, - {SizedItem::ledgerFetch, {{ 2, 3, 4, 5, 8 }}}, - {SizedItem::hashNodeDBCache, {{ 4, 12, 24, 64, 128 }}}, - {SizedItem::txnDBCache, {{ 4, 12, 24, 64, 128 }}}, - {SizedItem::lgrDBCache, {{ 4, 8, 16, 32, 128 }}}, - {SizedItem::openFinalLimit, {{ 8, 16, 32, 64, 128 }}}, - {SizedItem::burstSize, {{ 4, 8, 16, 32, 48 }}}, - {SizedItem::ramSizeGB, {{ 8, 12, 16, 24, 32 }}}, + // tiny small medium large huge + {SizedItem::sweepInterval, {{ 10, 30, 60, 90, 120 }}}, + {SizedItem::treeCacheSize, {{ 262144, 524288, 2097152, 4194304, 8388608 }}}, + {SizedItem::treeCacheAge, {{ 30, 60, 90, 120, 900 }}}, + {SizedItem::ledgerSize, {{ 32, 32, 64, 256, 384 }}}, + {SizedItem::ledgerAge, {{ 30, 60, 180, 300, 600 }}}, + {SizedItem::ledgerFetch, {{ 2, 3, 4, 5, 8 }}}, + {SizedItem::hashNodeDBCache, {{ 4, 12, 24, 64, 128 }}}, + {SizedItem::txnDBCache, {{ 4, 12, 24, 64, 128 }}}, + {SizedItem::lgrDBCache, {{ 4, 8, 16, 32, 128 }}}, + {SizedItem::openFinalLimit, {{ 8, 16, 32, 64, 128 }}}, + {SizedItem::burstSize, {{ 4, 8, 16, 32, 48 }}}, + {SizedItem::ramSizeGB, {{ 8, 12, 16, 24, 32 }}}, + {SizedItem::accountIdCacheSize, {{ 20047, 50053, 77081, 150061, 300007 }}} }}; // Ensure that the order of entries in the table corresponds to the diff --git a/src/ripple/protocol/AccountID.h b/src/ripple/protocol/AccountID.h index e514cfb74..79768eefd 100644 --- a/src/ripple/protocol/AccountID.h +++ b/src/ripple/protocol/AccountID.h @@ -106,41 +106,20 @@ operator<<(std::ostream& os, AccountID const& x) return os; } -//------------------------------------------------------------------------------ +/** Initialize the global cache used to map AccountID to base58 conversions. -/** Caches the base58 representations of AccountIDs + The cache is optional and need not be initialized. But because conversion + is expensive (it requires a SHA-256 operation) in most cases the overhead + of the cache is worth the benefit. - This operation occurs with sufficient frequency to - justify having a cache. In the future, rippled should - require clients to receive "binary" results, where - AccountIDs are hex-encoded. -*/ -class AccountIDCache -{ -private: - std::mutex mutable mutex_; - std::size_t capacity_; - hash_map mutable m0_; - hash_map mutable m1_; + @param count The number of entries the cache should accomodate. Zero will + disable the cache, releasing any memory associated with it. -public: - AccountIDCache(AccountIDCache const&) = delete; - AccountIDCache& - operator=(AccountIDCache const&) = delete; - - explicit AccountIDCache(std::size_t capacity); - - /** Return ripple::toBase58 for the AccountID - - Thread Safety: - Safe to call from any thread concurrently - - @note This function intentionally returns a - copy for correctness. - */ - std::string - toBase58(AccountID const&) const; -}; + @note The function will only initialize the cache the first time it is + invoked. Subsequent invocations do nothing. + */ +void +initAccountIdCache(std::size_t count); } // namespace ripple diff --git a/src/ripple/protocol/impl/AccountID.cpp b/src/ripple/protocol/impl/AccountID.cpp index 8ca8d1d15..c615807cf 100644 --- a/src/ripple/protocol/impl/AccountID.cpp +++ b/src/ripple/protocol/impl/AccountID.cpp @@ -17,17 +17,95 @@ */ //============================================================================== +#include +#include #include #include #include #include +#include #include +#include namespace ripple { +namespace detail { + +/** Caches the base58 representations of AccountIDs */ +class AccountIdCache +{ +private: + struct CachedAccountID + { + AccountID id; + char encoding[40] = {0}; + }; + + // The actual cache + std::vector cache_; + + // We use a hash function designed to resist algorithmic complexity attacks + hardened_hash<> hasher_; + + // 64 spinlocks, packed into a single 64-bit value + std::atomic locks_ = 0; + +public: + AccountIdCache(std::size_t count) : cache_(count) + { + // This is non-binding, but we try to avoid wasting memory that + // is caused by overallocation. + cache_.shrink_to_fit(); + } + + std::string + toBase58(AccountID const& id) + { + auto const index = hasher_(id) % cache_.size(); + + packed_spinlock sl(locks_, index % 64); + + { + std::lock_guard lock(sl); + + // The check against the first character of the encoding ensures + // that we don't mishandle the case of the all-zero account: + if (cache_[index].encoding[0] != 0 && cache_[index].id == id) + return cache_[index].encoding; + } + + auto ret = + encodeBase58Token(TokenType::AccountID, id.data(), id.size()); + + assert(ret.size() <= 38); + + { + std::lock_guard lock(sl); + cache_[index].id = id; + std::strcpy(cache_[index].encoding, ret.c_str()); + } + + return ret; + } +}; + +} // namespace detail + +static std::unique_ptr accountIdCache; + +void +initAccountIdCache(std::size_t count) +{ + if (!accountIdCache && count != 0) + accountIdCache = std::make_unique(count); +} + std::string toBase58(AccountID const& v) { + if (accountIdCache) + return accountIdCache->toBase58(v); + return encodeBase58Token(TokenType::AccountID, v.data(), v.size()); } @@ -112,52 +190,4 @@ to_issuer(AccountID& issuer, std::string const& s) return true; } -//------------------------------------------------------------------------------ - -/* VFALCO NOTE - An alternate implementation could use a pair of insert-only - hash maps that each use a single large memory allocation - to store a fixed size hash table and all of the AccountID/string - pairs laid out in memory (wouldn't use std::string here just a - length prefixed or zero terminated array). Possibly using - boost::intrusive as the basis for the unordered container. - This would cut down to one allocate/free cycle per swap of - the map. -*/ - -AccountIDCache::AccountIDCache(std::size_t capacity) : capacity_(capacity) -{ - m1_.reserve(capacity_); -} - -std::string -AccountIDCache::toBase58(AccountID const& id) const -{ - std::lock_guard lock(mutex_); - auto iter = m1_.find(id); - if (iter != m1_.end()) - return iter->second; - iter = m0_.find(id); - std::string result; - if (iter != m0_.end()) - { - result = iter->second; - // Can use insert-only hash maps if - // we didn't erase from here. - m0_.erase(iter); - } - else - { - result = ripple::toBase58(id); - } - if (m1_.size() >= capacity_) - { - m0_ = std::move(m1_); - m1_.clear(); - m1_.reserve(capacity_); - } - m1_.emplace(id, result); - return result; -} - } // namespace ripple diff --git a/src/ripple/rpc/handlers/AccountChannels.cpp b/src/ripple/rpc/handlers/AccountChannels.cpp index cc79173e5..e5059d3ff 100644 --- a/src/ripple/rpc/handlers/AccountChannels.cpp +++ b/src/ripple/rpc/handlers/AccountChannels.cpp @@ -202,7 +202,7 @@ doAccountChannels(RPC::JsonContext& context) to_string(*marker) + "," + std::to_string(nextHint); } - result[jss::account] = context.app.accountIDCache().toBase58(accountID); + result[jss::account] = toBase58(accountID); for (auto const& item : visitData.items) addChannel(jsonChannels, *item); diff --git a/src/ripple/rpc/handlers/AccountInfo.cpp b/src/ripple/rpc/handlers/AccountInfo.cpp index 417a3ffcd..f5432dc65 100644 --- a/src/ripple/rpc/handlers/AccountInfo.cpp +++ b/src/ripple/rpc/handlers/AccountInfo.cpp @@ -215,7 +215,7 @@ doAccountInfo(RPC::JsonContext& context) } else { - result[jss::account] = context.app.accountIDCache().toBase58(accountID); + result[jss::account] = toBase58(accountID); RPC::inject_error(rpcACT_NOT_FOUND, result); } diff --git a/src/ripple/rpc/handlers/AccountLines.cpp b/src/ripple/rpc/handlers/AccountLines.cpp index 843b9ddea..364d40673 100644 --- a/src/ripple/rpc/handlers/AccountLines.cpp +++ b/src/ripple/rpc/handlers/AccountLines.cpp @@ -252,7 +252,7 @@ doAccountLines(RPC::JsonContext& context) to_string(*marker) + "," + std::to_string(nextHint); } - result[jss::account] = context.app.accountIDCache().toBase58(accountID); + result[jss::account] = toBase58(accountID); for (auto const& item : visitData.items) addLine(jsonLines, item); diff --git a/src/ripple/rpc/handlers/AccountObjects.cpp b/src/ripple/rpc/handlers/AccountObjects.cpp index 4dcb3aba7..6424c3afd 100644 --- a/src/ripple/rpc/handlers/AccountObjects.cpp +++ b/src/ripple/rpc/handlers/AccountObjects.cpp @@ -160,7 +160,7 @@ doAccountNFTs(RPC::JsonContext& context) cp = nullptr; } - result[jss::account] = context.app.accountIDCache().toBase58(accountID); + result[jss::account] = toBase58(accountID); context.loadType = Resource::feeMediumBurdenRPC; return result; } @@ -275,7 +275,7 @@ doAccountObjects(RPC::JsonContext& context) result[jss::account_objects] = Json::arrayValue; } - result[jss::account] = context.app.accountIDCache().toBase58(accountID); + result[jss::account] = toBase58(accountID); context.loadType = Resource::feeMediumBurdenRPC; return result; } diff --git a/src/ripple/rpc/handlers/AccountOffers.cpp b/src/ripple/rpc/handlers/AccountOffers.cpp index d31787563..e957fe8a8 100644 --- a/src/ripple/rpc/handlers/AccountOffers.cpp +++ b/src/ripple/rpc/handlers/AccountOffers.cpp @@ -77,7 +77,7 @@ doAccountOffers(RPC::JsonContext& context) } // Get info on account. - result[jss::account] = context.app.accountIDCache().toBase58(accountID); + result[jss::account] = toBase58(accountID); if (!ledger->exists(keylet::account(accountID))) return rpcError(rpcACT_NOT_FOUND); diff --git a/src/ripple/rpc/handlers/GatewayBalances.cpp b/src/ripple/rpc/handlers/GatewayBalances.cpp index 825a74ab8..d0770f31e 100644 --- a/src/ripple/rpc/handlers/GatewayBalances.cpp +++ b/src/ripple/rpc/handlers/GatewayBalances.cpp @@ -80,7 +80,7 @@ doGatewayBalances(RPC::JsonContext& context) context.loadType = Resource::feeHighBurdenRPC; - result[jss::account] = context.app.accountIDCache().toBase58(accountID); + result[jss::account] = toBase58(accountID); // Parse the specified hotwallet(s), if any std::set hotWallets; diff --git a/src/ripple/rpc/handlers/NFTOffers.cpp b/src/ripple/rpc/handlers/NFTOffers.cpp index 34bbc8446..69a090e27 100644 --- a/src/ripple/rpc/handlers/NFTOffers.cpp +++ b/src/ripple/rpc/handlers/NFTOffers.cpp @@ -41,12 +41,10 @@ appendNftOfferJson( obj[jss::nft_offer_index] = to_string(offer->key()); obj[jss::flags] = (*offer)[sfFlags]; - obj[jss::owner] = - app.accountIDCache().toBase58(offer->getAccountID(sfOwner)); + obj[jss::owner] = toBase58(offer->getAccountID(sfOwner)); if (offer->isFieldPresent(sfDestination)) - obj[jss::destination] = - app.accountIDCache().toBase58(offer->getAccountID(sfDestination)); + obj[jss::destination] = toBase58(offer->getAccountID(sfDestination)); if (offer->isFieldPresent(sfExpiration)) obj[jss::expiration] = offer->getFieldU32(sfExpiration); diff --git a/src/ripple/rpc/handlers/NoRippleCheck.cpp b/src/ripple/rpc/handlers/NoRippleCheck.cpp index 2a6ab7ca4..a2af9845f 100644 --- a/src/ripple/rpc/handlers/NoRippleCheck.cpp +++ b/src/ripple/rpc/handlers/NoRippleCheck.cpp @@ -40,7 +40,7 @@ fillTransaction( ReadView const& ledger) { txArray["Sequence"] = Json::UInt(sequence++); - txArray["Account"] = context.app.accountIDCache().toBase58(accountID); + txArray["Account"] = toBase58(accountID); auto& fees = ledger.fees(); // Convert the reference transaction cost in fee units to drops // scaled to represent the current fee load. From 0ecfc7cb1a958b731e5f184876ea89ae2d4214ee Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Mon, 30 May 2022 21:05:10 -0700 Subject: [PATCH 17/19] Improve self-signed certificate generation: When starting, the code generates a new ephemeral private key and a corresponding certificate to go along with it. This process can take time and, while this is unlikely to matter for normal server operations, it can have a significant impact for unit testing and development. Profiling data suggests that ~20% of the time needed for a unit test run can be attributed to this. This commit does several things: 1. It restructures the code so that a new self-signed certificate and its corresponding private key are only initialized once at startup; this has minimal impact on the operation of a regular server. 2. It provides new default DH parameters. This doesn't impact the security of the connection, but those who compile from scratch can generate new parameters if they so choose. 3. It properly sets the version number in the certificate, fixing issue #4007; thanks to @donovanhide for the report. 4. It uses SHA-256 instead of SHA-1 as the hash algorithm for the certificate and adds some X.509 extensions as well as a random 128-bit serial number. 5. It rounds the certificate's "start of validity" period so that the server's precise startup time cannot be easily deduced and limits the validity period to two years, down from ten years. 6. It removes some CBC-based ciphers from the default cipher list to avoid some potential security issues, such as CVE-2016-2107 and CVE-2013-0169. --- src/ripple/basics/impl/make_SSLContext.cpp | 427 ++++++++++----------- 1 file changed, 194 insertions(+), 233 deletions(-) diff --git a/src/ripple/basics/impl/make_SSLContext.cpp b/src/ripple/basics/impl/make_SSLContext.cpp index 67beb79d3..79a0e9009 100644 --- a/src/ripple/basics/impl/make_SSLContext.cpp +++ b/src/ripple/basics/impl/make_SSLContext.cpp @@ -17,18 +17,55 @@ */ //============================================================================== -#include #include #include -#include -#include -#include +#include #include namespace ripple { namespace openssl { namespace detail { +/** The default strength of self-signed RSA certifices. + + Per NIST Special Publication 800-57 Part 3, 2048-bit RSA is still + considered acceptably secure. Generally, we would want to go above + and beyond such recommendations (e.g. by using 3072 or 4096 bits) + but there is a computational cost associated with that may not + be worth paying, considering that: + + - We regenerate a new ephemeral certificate and a securely generated + random private key every time the server is started; and + - There should not be any truly secure information (e.g. seeds or private + keys) that gets relayed to the server anyways over these RPCs. + + @note If you increase the number of bits you need to generate new + default DH parameters and update defaultDH accordingly. + * */ +int defaultRSAKeyBits = 2048; + +/** The default DH parameters. + + These were generated using the OpenSSL command: `openssl dhparam 2048` + by Nik Bougalis on May, 29, 2022. + + It is safe to use this, but if you want you can generate different + parameters and put them here. There's no easy way to change this + via the config file at this time. + + @note If you increase the number of bits you need to update + defaultRSAKeyBits accordingly. + */ +static constexpr char const defaultDH[] = + "-----BEGIN DH PARAMETERS-----\n" + "MIIBCAKCAQEApKSWfR7LKy0VoZ/SDCObCvJ5HKX2J93RJ+QN8kJwHh+uuA8G+t8Q\n" + "MDRjL5HanlV/sKN9HXqBc7eqHmmbqYwIXKUt9MUZTLNheguddxVlc2IjdP5i9Ps8\n" + "l7su8tnP0l1JvC6Rfv3epRsEAw/ZW/lC2IwkQPpOmvnENQhQ6TgrUzcGkv4Bn0X6\n" + "pxrDSBpZ+45oehGCUAtcbY8b02vu8zPFoxqo6V/+MIszGzldlik5bVqrJpVF6E8C\n" + "tRqHjj6KuDbPbjc+pRGvwx/BSO3SULxmYu9J1NOk090MU1CMt6IJY7TpEc9Xrac9\n" + "9yqY3xXZID240RRcaJ25+U4lszFPqP+CEwIBAg==\n" + "-----END DH PARAMETERS-----"; + /** The default list of ciphers we accept over TLS. Generally we include cipher suites that are part of TLS v1.2, but @@ -43,188 +80,148 @@ namespace detail { global or per-port basis, using the `ssl_ciphers` directive in the config file. */ -std::string const defaultCipherList = "TLSv1.2:!DSS:!PSK:!eNULL:!aNULL"; - -template -struct custom_delete; - -template <> -struct custom_delete -{ - explicit custom_delete() = default; - - void - operator()(RSA* rsa) const - { - RSA_free(rsa); - } -}; - -template <> -struct custom_delete -{ - explicit custom_delete() = default; - - void - operator()(EVP_PKEY* evp_pkey) const - { - EVP_PKEY_free(evp_pkey); - } -}; - -template <> -struct custom_delete -{ - explicit custom_delete() = default; - - void - operator()(X509* x509) const - { - X509_free(x509); - } -}; - -template <> -struct custom_delete -{ - explicit custom_delete() = default; - - void - operator()(DH* dh) const - { - DH_free(dh); - } -}; - -template -using custom_delete_unique_ptr = std::unique_ptr>; - -// RSA - -using rsa_ptr = custom_delete_unique_ptr; - -static rsa_ptr -rsa_generate_key(int n_bits) -{ -#if OPENSSL_VERSION_NUMBER >= 0x00908000L - BIGNUM* bn = BN_new(); - BN_set_word(bn, RSA_F4); - - RSA* rsa = RSA_new(); - if (RSA_generate_key_ex(rsa, n_bits, bn, nullptr) != 1) - { - RSA_free(rsa); - rsa = nullptr; - } - - BN_free(bn); -#else - RSA* rsa = RSA_generate_key(n_bits, RSA_F4, nullptr, nullptr); -#endif - - if (rsa == nullptr) - LogicError("RSA_generate_key failed"); - - return rsa_ptr(rsa); -} - -// EVP_PKEY - -using evp_pkey_ptr = custom_delete_unique_ptr; - -static evp_pkey_ptr -evp_pkey_new() -{ - EVP_PKEY* evp_pkey = EVP_PKEY_new(); - - if (evp_pkey == nullptr) - LogicError("EVP_PKEY_new failed"); - - return evp_pkey_ptr(evp_pkey); -} - -static void -evp_pkey_assign_rsa(EVP_PKEY* evp_pkey, rsa_ptr rsa) -{ - if (!EVP_PKEY_assign_RSA(evp_pkey, rsa.get())) - LogicError("EVP_PKEY_assign_RSA failed"); - - rsa.release(); -} - -// X509 - -using x509_ptr = custom_delete_unique_ptr; - -static x509_ptr -x509_new() -{ - X509* x509 = X509_new(); - - if (x509 == nullptr) - LogicError("X509_new failed"); - - X509_set_version(x509, NID_X509); - - int const margin = 60 * 60; // 3600, one hour - int const length = 10 * 365.25 * 24 * 60 * 60; // 315576000, ten years - - X509_gmtime_adj(X509_get_notBefore(x509), -margin); - X509_gmtime_adj(X509_get_notAfter(x509), length); - - return x509_ptr(x509); -} - -static void -x509_set_pubkey(X509* x509, EVP_PKEY* evp_pkey) -{ - X509_set_pubkey(x509, evp_pkey); -} - -static void -x509_sign(X509* x509, EVP_PKEY* evp_pkey) -{ - if (!X509_sign(x509, evp_pkey, EVP_sha1())) - LogicError("X509_sign failed"); -} - -static void -ssl_ctx_use_certificate(SSL_CTX* const ctx, x509_ptr cert) -{ - if (SSL_CTX_use_certificate(ctx, cert.get()) <= 0) - LogicError("SSL_CTX_use_certificate failed"); -} - -static void -ssl_ctx_use_privatekey(SSL_CTX* const ctx, evp_pkey_ptr key) -{ - if (SSL_CTX_use_PrivateKey(ctx, key.get()) <= 0) - LogicError("SSL_CTX_use_PrivateKey failed"); -} - -static std::string -error_message(std::string const& what, boost::system::error_code const& ec) -{ - std::stringstream ss; - ss << what << ": " << ec.message() << " (" << ec.value() << ")"; - return ss.str(); -} +std::string const defaultCipherList = "TLSv1.2:!CBC:!DSS:!PSK:!eNULL:!aNULL"; static void initAnonymous(boost::asio::ssl::context& context) { using namespace openssl; - evp_pkey_ptr pkey = evp_pkey_new(); - evp_pkey_assign_rsa(pkey.get(), rsa_generate_key(2048)); + static auto defaultRSA = []() { + BIGNUM* bn = BN_new(); + BN_set_word(bn, RSA_F4); - x509_ptr cert = x509_new(); - x509_set_pubkey(cert.get(), pkey.get()); - x509_sign(cert.get(), pkey.get()); + auto rsa = RSA_new(); + + if (!rsa) + LogicError("RSA_new failed"); + + if (RSA_generate_key_ex(rsa, defaultRSAKeyBits, bn, nullptr) != 1) + LogicError("RSA_generate_key_ex failure"); + + BN_clear_free(bn); + + return rsa; + }(); + + static auto defaultEphemeralPrivateKey = []() { + auto pkey = EVP_PKEY_new(); + + if (!pkey) + LogicError("EVP_PKEY_new failed"); + + // We need to up the reference count of here, since we are retaining a + // copy of the key for (potential) reuse. + if (RSA_up_ref(defaultRSA) != 1) + LogicError( + "EVP_PKEY_assign_RSA: incrementing reference count failed"); + + if (!EVP_PKEY_assign_RSA(pkey, defaultRSA)) + LogicError("EVP_PKEY_assign_RSA failed"); + + return pkey; + }(); + + static auto defaultCert = []() { + auto x509 = X509_new(); + + if (x509 == nullptr) + LogicError("X509_new failed"); + + // According to the standards (X.509 et al), the value should be one + // less than the actualy certificate version we want. Since we want + // version 3, we must use a 2. + X509_set_version(x509, 2); + + // To avoid leaking information about the precise time that the + // server started up, we adjust the validity period: + char buf[16] = {0}; + + auto const ts = std::time(nullptr) - (25 * 60 * 60); + + int ret = std::strftime( + buf, sizeof(buf) - 1, "%y%m%d000000Z", std::gmtime(&ts)); + + buf[ret] = 0; + + if (ASN1_TIME_set_string_X509(X509_get_notBefore(x509), buf) != 1) + LogicError("Unable to set certificate validity date"); + + // And make it valid for two years + X509_gmtime_adj(X509_get_notAfter(x509), 2 * 365 * 24 * 60 * 60); + + // Set a serial number + if (auto b = BN_new(); b != nullptr) + { + if (BN_rand(b, 128, BN_RAND_TOP_ANY, BN_RAND_BOTTOM_ANY)) + { + if (auto a = ASN1_INTEGER_new(); a != nullptr) + { + if (BN_to_ASN1_INTEGER(b, a)) + X509_set_serialNumber(x509, a); + + ASN1_INTEGER_free(a); + } + } + + BN_clear_free(b); + } + + // Some certificate details + { + X509V3_CTX ctx; + + X509V3_set_ctx_nodb(&ctx); + X509V3_set_ctx(&ctx, x509, x509, nullptr, nullptr, 0); + + if (auto ext = X509V3_EXT_conf_nid( + nullptr, &ctx, NID_basic_constraints, "critical,CA:FALSE")) + { + X509_add_ext(x509, ext, -1); + X509_EXTENSION_free(ext); + } + + if (auto ext = X509V3_EXT_conf_nid( + nullptr, + &ctx, + NID_ext_key_usage, + "critical,serverAuth,clientAuth")) + { + X509_add_ext(x509, ext, -1); + X509_EXTENSION_free(ext); + } + + if (auto ext = X509V3_EXT_conf_nid( + nullptr, &ctx, NID_key_usage, "critical,digitalSignature")) + { + X509_add_ext(x509, ext, -1); + X509_EXTENSION_free(ext); + } + + if (auto ext = X509V3_EXT_conf_nid( + nullptr, &ctx, NID_subject_key_identifier, "hash")) + { + X509_add_ext(x509, ext, -1); + X509_EXTENSION_free(ext); + } + } + + // And a private key + X509_set_pubkey(x509, defaultEphemeralPrivateKey); + + if (!X509_sign(x509, defaultEphemeralPrivateKey, EVP_sha256())) + LogicError("X509_sign failed"); + + return x509; + }(); SSL_CTX* const ctx = context.native_handle(); - ssl_ctx_use_certificate(ctx, std::move(cert)); - ssl_ctx_use_privatekey(ctx, std::move(pkey)); + + if (SSL_CTX_use_certificate(ctx, defaultCert) <= 0) + LogicError("SSL_CTX_use_certificate failed"); + + if (SSL_CTX_use_PrivateKey(ctx, defaultEphemeralPrivateKey) <= 0) + LogicError("SSL_CTX_use_PrivateKey failed"); } static void @@ -234,6 +231,10 @@ initAuthenticated( std::string const& cert_file, std::string const& chain_file) { + auto fmt_error = [](boost::system::error_code ec) -> std::string { + return " [" + std::to_string(ec.value()) + ": " + ec.message() + "]"; + }; + SSL_CTX* const ssl = context.native_handle(); bool cert_set = false; @@ -246,10 +247,7 @@ initAuthenticated( cert_file, boost::asio::ssl::context::pem, ec); if (ec) - { - LogicError(error_message("Problem with SSL certificate file.", ec) - .c_str()); - } + LogicError("Problem with SSL certificate file" + fmt_error(ec)); cert_set = true; } @@ -261,11 +259,10 @@ initAuthenticated( if (!f) { - LogicError(error_message( - "Problem opening SSL chain file.", - boost::system::error_code( - errno, boost::system::generic_category())) - .c_str()); + LogicError( + "Problem opening SSL chain file" + + fmt_error(boost::system::error_code( + errno, boost::system::generic_category()))); } try @@ -312,8 +309,7 @@ initAuthenticated( if (ec) { LogicError( - error_message("Problem using the SSL private key file.", ec) - .c_str()); + "Problem using the SSL private key file" + fmt_error(ec)); } } @@ -324,7 +320,7 @@ initAuthenticated( } std::shared_ptr -get_context(std::string const& cipherList) +get_context(std::string cipherList) { auto c = std::make_shared( boost::asio::ssl::context::sslv23); @@ -338,55 +334,20 @@ get_context(std::string const& cipherList) boost::asio::ssl::context::single_dh_use | boost::asio::ssl::context::no_compression); - { - auto const& l = !cipherList.empty() ? cipherList : defaultCipherList; - auto result = SSL_CTX_set_cipher_list(c->native_handle(), l.c_str()); - if (result != 1) - LogicError("SSL_CTX_set_cipher_list failed"); - } + if (cipherList.empty()) + cipherList = defaultCipherList; - // These are the raw DH parameters that Ripple Labs has - // chosen for Ripple, in the binary format needed by - // d2i_DHparams. - // - unsigned char const params[] = { - 0x30, 0x82, 0x01, 0x08, 0x02, 0x82, 0x01, 0x01, 0x00, 0x8f, 0xca, 0x66, - 0x85, 0x33, 0xcb, 0xcf, 0x36, 0x27, 0xb2, 0x4c, 0xb8, 0x50, 0xb8, 0xf9, - 0x53, 0xf8, 0xb9, 0x2d, 0x1c, 0xa2, 0xad, 0x86, 0x58, 0x29, 0x3b, 0x88, - 0x3e, 0xf5, 0x65, 0xb8, 0xda, 0x22, 0xf4, 0x8b, 0x21, 0x12, 0x18, 0xf7, - 0x16, 0xcd, 0x7c, 0xc7, 0x3a, 0x2d, 0x61, 0xb7, 0x11, 0xf6, 0xb0, 0x65, - 0xa0, 0x5b, 0xa4, 0x06, 0x95, 0x28, 0xa4, 0x4f, 0x76, 0xc0, 0xeb, 0xfa, - 0x95, 0xdf, 0xbf, 0x19, 0x90, 0x64, 0x8f, 0x60, 0xd5, 0x36, 0xba, 0xab, - 0x0d, 0x5a, 0x5c, 0x94, 0xd5, 0xf7, 0x32, 0xd6, 0x2a, 0x76, 0x77, 0x83, - 0x10, 0xc4, 0x2f, 0x10, 0x96, 0x3e, 0x37, 0x84, 0x45, 0x9c, 0xef, 0x33, - 0xf6, 0xd0, 0x2a, 0xa7, 0xce, 0x0a, 0xce, 0x0d, 0xa1, 0xa7, 0x44, 0x5d, - 0x18, 0x3f, 0x4f, 0xa4, 0x23, 0x9c, 0x5d, 0x74, 0x4f, 0xee, 0xdf, 0xaa, - 0x0d, 0x0a, 0x52, 0x57, 0x73, 0xb1, 0xe4, 0xc5, 0x72, 0x93, 0x9d, 0x03, - 0xe9, 0xf5, 0x48, 0x8c, 0xd1, 0xe6, 0x7c, 0x21, 0x65, 0x4e, 0x16, 0x51, - 0xa3, 0x16, 0x51, 0x10, 0x75, 0x60, 0x37, 0x93, 0xb8, 0x15, 0xd6, 0x14, - 0x41, 0x4a, 0x61, 0xc9, 0x1a, 0x4e, 0x9f, 0x38, 0xd8, 0x2c, 0xa5, 0x31, - 0xe1, 0x87, 0xda, 0x1f, 0xa4, 0x31, 0xa2, 0xa4, 0x42, 0x1e, 0xe0, 0x30, - 0xea, 0x2f, 0x9b, 0x77, 0x91, 0x59, 0x3e, 0xd5, 0xd0, 0xc5, 0x84, 0x45, - 0x17, 0x19, 0x74, 0x8b, 0x18, 0xb0, 0xc1, 0xe0, 0xfc, 0x1c, 0xaf, 0xe6, - 0x2a, 0xef, 0x4e, 0x0e, 0x8a, 0x5c, 0xc2, 0x91, 0xb9, 0x2b, 0xf8, 0x17, - 0x8d, 0xed, 0x44, 0xaa, 0x47, 0xaa, 0x52, 0xa2, 0xdb, 0xb6, 0xf5, 0xa1, - 0x88, 0x85, 0xa1, 0xd5, 0x87, 0xb8, 0x07, 0xd3, 0x97, 0xbe, 0x37, 0x74, - 0x72, 0xf1, 0xa8, 0x29, 0xf1, 0xa7, 0x7d, 0x19, 0xc3, 0x27, 0x09, 0xcf, - 0x23, 0x02, 0x01, 0x02}; + if (auto result = + SSL_CTX_set_cipher_list(c->native_handle(), cipherList.c_str()); + result != 1) + LogicError("SSL_CTX_set_cipher_list failed"); - unsigned char const* data = ¶ms[0]; - - custom_delete_unique_ptr const dh{ - d2i_DHparams(nullptr, &data, sizeof(params))}; - if (!dh) - LogicError("d2i_DHparams returned nullptr."); - - SSL_CTX_set_tmp_dh(c->native_handle(), dh.get()); + c->use_tmp_dh({std::addressof(detail::defaultDH), sizeof(defaultDH)}); // Disable all renegotiation support in TLS v1.2. This can help prevent // exploitation of the bug described in CVE-2021-3499 (for details see - // https://www.openssl.org/news/secadv/20210325.txt) when linking against - // OpenSSL versions prior to 1.1.1k. + // https://www.openssl.org/news/secadv/20210325.txt) when linking + // against OpenSSL versions prior to 1.1.1k. SSL_CTX_set_options(c->native_handle(), SSL_OP_NO_RENEGOTIATION); return c; From 7b3507bb873495a974db33c57a888221ddabcacc Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Sat, 21 May 2022 20:42:30 -0700 Subject: [PATCH 18/19] Improve wrapper around OpenSSL RAND --- src/ripple/crypto/csprng.h | 3 -- src/ripple/crypto/impl/csprng.cpp | 75 ++++++++++++++----------------- 2 files changed, 33 insertions(+), 45 deletions(-) diff --git a/src/ripple/crypto/csprng.h b/src/ripple/crypto/csprng.h index 395063249..3ad5d7000 100644 --- a/src/ripple/crypto/csprng.h +++ b/src/ripple/crypto/csprng.h @@ -39,9 +39,6 @@ class csprng_engine private: std::mutex mutex_; - void - mix(void* buffer, std::size_t count, double bitsPerByte); - public: using result_type = std::uint64_t; diff --git a/src/ripple/crypto/impl/csprng.cpp b/src/ripple/crypto/impl/csprng.cpp index a166fe288..04b3b3fc3 100644 --- a/src/ripple/crypto/impl/csprng.cpp +++ b/src/ripple/crypto/impl/csprng.cpp @@ -17,7 +17,6 @@ */ //============================================================================== -#include #include #include #include @@ -28,25 +27,19 @@ namespace ripple { -void -csprng_engine::mix(void* data, std::size_t size, double bitsPerByte) -{ - assert(data != nullptr); - assert(size != 0); - assert(bitsPerByte != 0); - - std::lock_guard lock(mutex_); - RAND_add(data, size, (size * bitsPerByte) / 8.0); -} - csprng_engine::csprng_engine() { - mix_entropy(); + // This is not strictly necessary + if (RAND_poll() != 1) + Throw("CSPRNG: Initial polling failed"); } csprng_engine::~csprng_engine() { + // This cleanup function is not needed in newer versions of OpenSSL +#if (OPENSSL_VERSION_NUMBER < 0x10100000L) RAND_cleanup(); +#endif } void @@ -64,46 +57,44 @@ csprng_engine::mix_entropy(void* buffer, std::size_t count) e = rd(); } - // Assume 2 bits per byte for the system entropy: - mix(entropy.data(), - entropy.size() * sizeof(std::random_device::result_type), - 2.0); + std::lock_guard lock(mutex_); + + // We add data to the pool, but we conservatively assume that + // it contributes no actual entropy. + RAND_add( + entropy.data(), + entropy.size() * sizeof(std::random_device::result_type), + 0); - // We want to be extremely conservative about estimating - // how much entropy the buffer the user gives us contains - // and assume only 0.5 bits of entropy per byte: if (buffer != nullptr && count != 0) - mix(buffer, count, 0.5); + RAND_add(buffer, count, 0); +} + +void +csprng_engine::operator()(void* ptr, std::size_t count) +{ + // RAND_bytes is thread-safe on OpenSSL 1.1.0 and later when compiled + // with thread support, so we don't need to grab a mutex. + // https://mta.openssl.org/pipermail/openssl-users/2020-November/013146.html +#if (OPENSSL_VERSION_NUMBER < 0x10100000L) || !defined(OPENSSL_THREADS) + std::lock_guard lock(mutex_); +#endif + + auto const result = + RAND_bytes(reinterpret_cast(ptr), count); + + if (result != 1) + Throw("CSPRNG: Insufficient entropy"); } csprng_engine::result_type csprng_engine::operator()() { result_type ret; - - std::lock_guard lock(mutex_); - - auto const result = - RAND_bytes(reinterpret_cast(&ret), sizeof(ret)); - - if (result == 0) - Throw("Insufficient entropy"); - + (*this)(&ret, sizeof(result_type)); return ret; } -void -csprng_engine::operator()(void* ptr, std::size_t count) -{ - std::lock_guard lock(mutex_); - - auto const result = - RAND_bytes(reinterpret_cast(ptr), count); - - if (result != 1) - Throw("Insufficient entropy"); -} - csprng_engine& crypto_prng() { From fe05b8c4feb815e1aa6afd7ff8c85dbc92bcd651 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Thu, 4 Aug 2022 11:16:10 -0700 Subject: [PATCH 19/19] Set version to 1.10.0-b1 --- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index 734dc11cb..43f0eedff 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -33,7 +33,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "1.9.3" +char const* const versionString = "1.10.0-b1" // clang-format on #if defined(DEBUG) || defined(SANITIZER)