Proposed 1.9.2-rc1

Builds/CMake/RippledCore.cmake

@@ -156,7 +156,9 @@ install (
     src/ripple/basics/MathUtilities.h
     src/ripple/basics/safe_cast.h
     src/ripple/basics/Slice.h
+    src/ripple/basics/spinlock.h
     src/ripple/basics/StringUtilities.h
+    src/ripple/basics/ThreadSafetyAnalysis.h
     src/ripple/basics/ToString.h
     src/ripple/basics/UnorderedContainers.h
     src/ripple/basics/XRPAmount.h

Builds/CMake/RippledDocs.cmake

@@ -31,7 +31,7 @@ if (tests)
     # find_path sets a CACHE variable, so don't try using a "local" variable.
     find_path (${variable} "${name}" ${ARGN})
     if (NOT ${variable})
-      message (WARNING "could not find ${name}")
+      message (NOTICE "could not find ${name}")
     else ()
       message (STATUS "found ${name}: ${${variable}}/${name}")
     endif ()

Builds/linux/README.md

@@ -239,3 +239,32 @@ change the `/opt/local` module path above to match your chosen installation pref
 `rippled` builds a set of unit tests into the server executable. To run these unit
 tests after building, pass the `--unittest` option to the compiled `rippled`
 executable. The executable will exit with summary info after running the unit tests.
+
+## Workaround for a compile error in soci
+
+Compilation errors have been observed with Apple Clang 13.1.6+ and soci v4.x. soci compiles with the `-Werror` flag which causes warnings to be treated as errors. These warnings pertain to style (not correctness). However, they cause the cmake process to fail.
+
+Here's an example of how this looks:
+```
+.../rippled/.nih_c/unix_makefiles/AppleClang_13.1.6.13160021/Debug/src/soci/src/core/session.cpp:450:66: note: in instantiation of function template specialization 'soci::use<std::string>' requested here
+    return prepare << backEnd_->get_column_descriptions_query(), use(table_name, "t");
+                                                                 ^
+1 error generated.
+```
+
+Please apply the below patch (courtesy of Scott Determan) to remove these errors. `.nih_c/unix_makefiles/AppleClang_13.1.6.13160021/Debug/src/soci/cmake/SociConfig.cmake` file needs to be edited. This file is an example for Mac OS and it might be slightly different for other OS/Architectures.
+
+```
+diff --git a/cmake/SociConfig.cmake b/cmake/SociConfig.cmake
+index 97d907e4..11bcd1f3 100644
+--- a/cmake/SociConfig.cmake
++++ b/cmake/SociConfig.cmake
+@@ -58,8 +58,8 @@ if (MSVC)
+
+ else()
+
+-  set(SOCI_GCC_CLANG_COMMON_FLAGS
+-    "-pedantic -Werror -Wno-error=parentheses -Wall -Wextra -Wpointer-arith -Wcast-align -Wcast-qual -Wfloat-equal -Woverloaded-virtual -Wredundant-decls -Wno-long-long")
++  set(SOCI_GCC_CLANG_COMMON_FLAGS "")
++    # "-pedantic -Werror -Wno-error=parentheses -Wall -Wextra -Wpointer-arith -Wcast-align -Wcast-qual -Wfloat-equal -Woverloaded-virtual -Wredundant-decls -Wno-long-long")
+```

CMakeLists.txt

@@ -21,6 +21,12 @@ if(Git_FOUND)
     endif()
 endif() #git
 
+if (thread_safety_analysis)
+  add_compile_options(-Wthread-safety -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -DRIPPLE_ENABLE_THREAD_SAFETY_ANNOTATIONS)
+  add_compile_options("-stdlib=libc++")
+  add_link_options("-stdlib=libc++")
+endif()
+
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/Builds/CMake")
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/Builds/CMake/deps")
 

src/ripple/app/misc/NetworkOPs.cpp

@@ -919,7 +919,10 @@ void
 NetworkOPsImp::setStateTimer()
 {
     setHeartbeatTimer();
-    setClusterTimer();
+
+    // Only do this work if a cluster is configured
+    if (app_.cluster().size() != 0)
+        setClusterTimer();
 }
 
 void
@@ -972,6 +975,7 @@ void
 NetworkOPsImp::setClusterTimer()
 {
     using namespace std::chrono_literals;
+
     setTimer(
         clusterTimer_,
         10s,
@@ -1057,7 +1061,11 @@ NetworkOPsImp::processHeartbeatTimer()
 void
 NetworkOPsImp::processClusterTimer()
 {
+    if (app_.cluster().size() == 0)
+        return;
+
     using namespace std::chrono_literals;
+
     bool const update = app_.cluster().update(
         app_.nodeIdentity().first,
         "",
@@ -2323,10 +2331,6 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters)
     if (!app_.config().SERVER_DOMAIN.empty())
         info[jss::server_domain] = app_.config().SERVER_DOMAIN;
 
-    if (!app_.config().reporting())
-        if (auto const netid = app_.overlay().networkID())
-            info[jss::network_id] = static_cast<Json::UInt>(*netid);
-
     info[jss::build_version] = BuildInfo::getVersionString();
 
     info[jss::server_state] = strOperatingMode(admin);
@@ -2469,6 +2473,9 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters)
 
     if (!app_.config().reporting())
     {
+        if (auto const netid = app_.overlay().networkID())
+            info[jss::network_id] = static_cast<Json::UInt>(*netid);
+
         auto const escalationMetrics =
             app_.getTxQ().getMetrics(*app_.openLedger().current());
 

src/ripple/app/misc/SHAMapStoreImp.cpp

@@ -17,17 +17,18 @@
 */
 //==============================================================================
 
+#include <ripple/app/misc/SHAMapStoreImp.h>
+
 #include <ripple/app/ledger/TransactionMaster.h>
 #include <ripple/app/misc/NetworkOPs.h>
-#include <ripple/app/misc/SHAMapStoreImp.h>
 #include <ripple/app/rdb/State.h>
 #include <ripple/app/rdb/backend/SQLiteDatabase.h>
 #include <ripple/beast/core/CurrentThreadName.h>
 #include <ripple/core/ConfigSections.h>
 #include <ripple/core/Pg.h>
-#include <ripple/nodestore/impl/DatabaseRotatingImp.h>
-
 #include <ripple/nodestore/Scheduler.h>
+#include <ripple/nodestore/impl/DatabaseRotatingImp.h>
+#include <ripple/shamap/SHAMapMissingNode.h>
 
 #include <boost/algorithm/string/predicate.hpp>
 
@@ -363,11 +364,24 @@ SHAMapStoreImp::run()
 
             JLOG(journal_.debug()) << "copying ledger " << validatedSeq;
             std::uint64_t nodeCount = 0;
-            validatedLedger->stateMap().snapShot(false)->visitNodes(std::bind(
-                &SHAMapStoreImp::copyNode,
-                this,
-                std::ref(nodeCount),
-                std::placeholders::_1));
+
+            try
+            {
+                validatedLedger->stateMap().snapShot(false)->visitNodes(
+                    std::bind(
+                        &SHAMapStoreImp::copyNode,
+                        this,
+                        std::ref(nodeCount),
+                        std::placeholders::_1));
+            }
+            catch (SHAMapMissingNode const& e)
+            {
+                JLOG(journal_.error())
+                    << "Missing node while copying ledger before rotate: "
+                    << e.what();
+                continue;
+            }
+
             if (stopping())
                 return;
             // Only log if we completed without a "health" abort

src/ripple/app/paths/Pathfinder.cpp

@@ -796,8 +796,8 @@ Pathfinder::addPathsForType(
     PathType const& pathType,
     std::function<bool(void)> const& continueCallback)
 {
-    JLOG(j_.warn()) << "addPathsForType "
-                    << CollectionAndDelimiter(pathType, ", ");
+    JLOG(j_.debug()) << "addPathsForType "
+                     << CollectionAndDelimiter(pathType, ", ");
     // See if the set of paths for this type already exists.
     auto it = mPaths.find(pathType);
     if (it != mPaths.end())

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

@@ -254,7 +254,10 @@ readAmendments(
 
     soci::transaction tr(session);
     std::string sql =
-        "SELECT AmendmentHash, AmendmentName, Veto FROM FeatureVotes";
+        "SELECT AmendmentHash, AmendmentName, Veto FROM "
+        "( SELECT AmendmentHash, AmendmentName, Veto, RANK() OVER "
+        "(  PARTITION BY AmendmentHash ORDER BY ROWID DESC ) "
+        "as rnk FROM FeatureVotes ) WHERE rnk = 1";
     // SOCI requires boost::optional (not std::optional) as parameters.
     boost::optional<std::string> amendment_hash;
     boost::optional<std::string> amendment_name;

src/ripple/app/reporting/P2pProxy.cpp

@@ -72,7 +72,7 @@ shouldForwardToP2p(RPC::JsonContext& context)
     if (params.isMember(jss::ledger_index))
     {
         auto indexValue = params[jss::ledger_index];
-        if (!indexValue.isNumeric())
+        if (indexValue.isString())
         {
             auto index = indexValue.asString();
             return index == "current" || index == "closed";

src/ripple/app/tx/impl/CreateOffer.cpp

@@ -728,7 +728,7 @@ CreateOffer::flowCross(
         // additional path with XRP as the intermediate between two books.
         // This second path we have to build ourselves.
         STPathSet paths;
-        if (!takerAmount.in.native() & !takerAmount.out.native())
+        if (!takerAmount.in.native() && !takerAmount.out.native())
         {
             STPath path;
             path.emplace_back(std::nullopt, xrpCurrency(), std::nullopt);

src/ripple/app/tx/impl/NFTokenAcceptOffer.cpp

@@ -75,6 +75,12 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
             if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration]))
                 return {nullptr, tecEXPIRED};
 
+            // The initial implementation had a bug that allowed a negative
+            // amount.  The fixNFTokenNegOffer amendment fixes that.
+            if ((*offerSLE)[sfAmount].negative() &&
+                ctx.view.rules().enabled(fixNFTokenNegOffer))
+                return {nullptr, temBAD_OFFER};
+
             return {std::move(offerSLE), tesSUCCESS};
         }
         return {nullptr, tesSUCCESS};
@@ -103,6 +109,14 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
         if ((*so)[sfAmount] > (*bo)[sfAmount])
             return tecINSUFFICIENT_PAYMENT;
 
+        // If the buyer specified a destination, that destination must be
+        // the seller or the broker.
+        if (auto const dest = bo->at(~sfDestination))
+        {
+            if (*dest != so->at(sfOwner) && *dest != ctx.tx[sfAccount])
+                return tecNFTOKEN_BUY_SELL_MISMATCH;
+        }
+
         // If the seller specified a destination, that destination must be
         // the buyer or the broker.
         if (auto const dest = so->at(~sfDestination))
@@ -142,6 +156,15 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
             !nft::findToken(ctx.view, ctx.tx[sfAccount], (*bo)[sfNFTokenID]))
             return tecNO_PERMISSION;
 
+        // If not in bridged mode...
+        if (!so)
+        {
+            // If the offer has a Destination field, the acceptor must be the
+            // Destination.
+            if (auto const dest = bo->at(~sfDestination);
+                dest.has_value() && *dest != ctx.tx[sfAccount])
+                return tecNO_PERMISSION;
+        }
         // The account offering to buy must have funds:
         auto const needed = bo->at(sfAmount);
 

src/ripple/app/tx/impl/NFTokenCreateOffer.cpp

@@ -46,7 +46,11 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx)
     auto const nftFlags = nft::getFlags(ctx.tx[sfNFTokenID]);
 
     {
-        auto const amount = ctx.tx[sfAmount];
+        STAmount const amount = ctx.tx[sfAmount];
+
+        if (amount.negative() && ctx.rules.enabled(fixNFTokenNegOffer))
+            // An offer for a negative amount makes no sense.
+            return temBAD_AMOUNT;
 
         if (!isXRP(amount))
         {
@@ -78,9 +82,14 @@ NFTokenCreateOffer::preflight(PreflightContext const& ctx)
 
     if (auto dest = ctx.tx[~sfDestination])
     {
-        // The destination field is only valid on a sell offer; it makes no
-        // sense in a buy offer.
-        if (!isSellOffer)
+        // Some folks think it makes sense for a buy offer to specify a
+        // specific broker using the Destination field.  This change doesn't
+        // deserve it's own amendment, so we're piggy-backing on
+        // fixNFTokenNegOffer.
+        //
+        // Prior to fixNFTokenNegOffer any use of the Destination field on
+        // a buy offer was malformed.
+        if (!isSellOffer && !ctx.rules.enabled(fixNFTokenNegOffer))
             return temMALFORMED;
 
         // The destination can't be the account executing the transaction.

src/ripple/basics/Slice.h

@@ -48,7 +48,8 @@ class Slice
     std::size_t size_ = 0;
 
 public:
-    using const_iterator = std::uint8_t const*;
+    using value_type = std::uint8_t;
+    using const_iterator = value_type const*;
 
     /** Default constructed Slice has length 0. */
     Slice() noexcept = default;
@@ -75,13 +76,13 @@ class Slice
         This may be zero for an empty range.
     */
     /** @{ */
-    std::size_t
+    [[nodiscard]] std::size_t
     size() const noexcept
     {
         return size_;
     }
 
-    std::size_t
+    [[nodiscard]] std::size_t
     length() const noexcept
     {
         return size_;

src/ripple/basics/ThreadSafetyAnalysis.h

@@ -0,0 +1,63 @@
+#ifndef RIPPLE_BASICS_THREAD_SAFTY_ANALYSIS_H_INCLUDED
+#define RIPPLE_BASICS_THREAD_SAFTY_ANALYSIS_H_INCLUDED
+
+#ifdef RIPPLE_ENABLE_THREAD_SAFETY_ANNOTATIONS
+#define THREAD_ANNOTATION_ATTRIBUTE__(x) __attribute__((x))
+#else
+#define THREAD_ANNOTATION_ATTRIBUTE__(x)  // no-op
+#endif
+
+#define CAPABILITY(x) THREAD_ANNOTATION_ATTRIBUTE__(capability(x))
+
+#define SCOPED_CAPABILITY THREAD_ANNOTATION_ATTRIBUTE__(scoped_lockable)
+
+#define GUARDED_BY(x) THREAD_ANNOTATION_ATTRIBUTE__(guarded_by(x))
+
+#define PT_GUARDED_BY(x) THREAD_ANNOTATION_ATTRIBUTE__(pt_guarded_by(x))
+
+#define ACQUIRED_BEFORE(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(acquired_before(__VA_ARGS__))
+
+#define ACQUIRED_AFTER(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(acquired_after(__VA_ARGS__))
+
+#define REQUIRES(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(requires_capability(__VA_ARGS__))
+
+#define REQUIRES_SHARED(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(requires_shared_capability(__VA_ARGS__))
+
+#define ACQUIRE(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(acquire_capability(__VA_ARGS__))
+
+#define ACQUIRE_SHARED(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(acquire_shared_capability(__VA_ARGS__))
+
+#define RELEASE(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(release_capability(__VA_ARGS__))
+
+#define RELEASE_SHARED(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(release_shared_capability(__VA_ARGS__))
+
+#define RELEASE_GENERIC(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(release_generic_capability(__VA_ARGS__))
+
+#define TRY_ACQUIRE(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_capability(__VA_ARGS__))
+
+#define TRY_ACQUIRE_SHARED(...) \
+    THREAD_ANNOTATION_ATTRIBUTE__(try_acquire_shared_capability(__VA_ARGS__))
+
+#define EXCLUDES(...) THREAD_ANNOTATION_ATTRIBUTE__(locks_excluded(__VA_ARGS__))
+
+#define ASSERT_CAPABILITY(x) THREAD_ANNOTATION_ATTRIBUTE__(assert_capability(x))
+
+#define ASSERT_SHARED_CAPABILITY(x) \
+    THREAD_ANNOTATION_ATTRIBUTE__(assert_shared_capability(x))
+
+#define RETURN_CAPABILITY(x) THREAD_ANNOTATION_ATTRIBUTE__(lock_returned(x))
+
+#define NO_THREAD_SAFETY_ANALYSIS \
+    THREAD_ANNOTATION_ATTRIBUTE__(no_thread_safety_analysis)
+
+#endif

src/ripple/basics/base_uint.h

@@ -26,6 +26,7 @@
 #define RIPPLE_BASICS_BASE_UINT_H_INCLUDED
 
 #include <ripple/basics/Expected.h>
+#include <ripple/basics/Slice.h>
 #include <ripple/basics/contract.h>
 #include <ripple/basics/hardened_hash.h>
 #include <ripple/basics/strHex.h>
@@ -56,6 +57,11 @@ struct is_contiguous_container<
 {
 };
 
+template <>
+struct is_contiguous_container<Slice> : std::true_type
+{
+};
+
 }  // namespace detail
 
 /** Integers of any length that is a multiple of 32-bits