Merge branch 'develop' into fix-innerobj-templ

Author: gregtatcam

.github/workflows/windows.yml

@@ -91,11 +91,7 @@ jobs:
           # Hard code for now. Move to the matrix if varied options are needed
           cmake-args: '-Dassert=ON -Dreporting=OFF -Dunity=ON'
           cmake-target: install
-      - name: test (permitted to silently fail)
+      - name: test
         shell: bash
-        # Github runners are resource limited, which causes unit tests to fail
-        # (e.g. OOM). To allow forward progress until self-hosted runners are
-        # up and running reliably, allow the job to succeed even if tests fail.
-        continue-on-error: true
         run: |
           ${build_dir}/${{ matrix.configuration }}/rippled --unittest --unittest-jobs $(nproc)

conanfile.py

@@ -147,9 +147,9 @@ def package(self):
     def package_info(self):
         libxrpl = self.cpp_info.components['libxrpl']
         libxrpl.libs = [
-            'libxrpl_core.a',
-            'libed25519.a',
-            'libsecp256k1.a',
+            'xrpl_core',
+            'ed25519',
+            'secp256k1',
         ]
         # TODO: Fix the protobufs to include each other relative to
         # `include/`, not `include/ripple/proto/`.

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

@@ -301,6 +301,60 @@ NFTokenAcceptOffer::pay(
     return tesSUCCESS;
 }
 
+TER
+NFTokenAcceptOffer::transferNFToken(
+    AccountID const& buyer,
+    AccountID const& seller,
+    uint256 const& nftokenID)
+{
+    auto tokenAndPage = nft::findTokenAndPage(view(), seller, nftokenID);
+
+    if (!tokenAndPage)
+        return tecINTERNAL;
+
+    if (auto const ret = nft::removeToken(
+            view(), seller, nftokenID, std::move(tokenAndPage->page));
+        !isTesSuccess(ret))
+        return ret;
+
+    auto const sleBuyer = view().read(keylet::account(buyer));
+    if (!sleBuyer)
+        return tecINTERNAL;
+
+    std::uint32_t const buyerOwnerCountBefore =
+        sleBuyer->getFieldU32(sfOwnerCount);
+
+    auto const insertRet =
+        nft::insertToken(view(), buyer, std::move(tokenAndPage->token));
+
+    // if fixNFTokenReserve is enabled, check if the buyer has sufficient
+    // reserve to own a new object, if their OwnerCount changed.
+    //
+    // There was an issue where the buyer accepts a sell offer, the ledger
+    // didn't check if the buyer has enough reserve, meaning that buyer can get
+    // NFTs free of reserve.
+    if (view().rules().enabled(fixNFTokenReserve))
+    {
+        // To check if there is sufficient reserve, we cannot use mPriorBalance
+        // because NFT is sold for a price. So we must use the balance after
+        // the deduction of the potential offer price. A small caveat here is
+        // that the balance has already deducted the transaction fee, meaning
+        // that the reserve requirement is a few drops higher.
+        auto const buyerBalance = sleBuyer->getFieldAmount(sfBalance);
+
+        auto const buyerOwnerCountAfter = sleBuyer->getFieldU32(sfOwnerCount);
+        if (buyerOwnerCountAfter > buyerOwnerCountBefore)
+        {
+            if (auto const reserve =
+                    view().fees().accountReserve(buyerOwnerCountAfter);
+                buyerBalance < reserve)
+                return tecINSUFFICIENT_RESERVE;
+        }
+    }
+
+    return insertRet;
+}
+
 TER
 NFTokenAcceptOffer::acceptOffer(std::shared_ptr<SLE> const& offer)
 {
@@ -333,17 +387,7 @@ NFTokenAcceptOffer::acceptOffer(std::shared_ptr<SLE> const& offer)
     }
 
     // Now transfer the NFT:
-    auto tokenAndPage = nft::findTokenAndPage(view(), seller, nftokenID);
-
-    if (!tokenAndPage)
-        return tecINTERNAL;
-
-    if (auto const ret = nft::removeToken(
-            view(), seller, nftokenID, std::move(tokenAndPage->page));
-        !isTesSuccess(ret))
-        return ret;
-
-    return nft::insertToken(view(), buyer, std::move(tokenAndPage->token));
+    return transferNFToken(buyer, seller, nftokenID);
 }
 
 TER
@@ -431,17 +475,8 @@ NFTokenAcceptOffer::doApply()
                 return r;
         }
 
-        auto tokenAndPage = nft::findTokenAndPage(view(), seller, nftokenID);
-
-        if (!tokenAndPage)
-            return tecINTERNAL;
-
-        if (auto const ret = nft::removeToken(
-                view(), seller, nftokenID, std::move(tokenAndPage->page));
-            !isTesSuccess(ret))
-            return ret;
-
-        return nft::insertToken(view(), buyer, std::move(tokenAndPage->token));
+        // Now transfer the NFT:
+        return transferNFToken(buyer, seller, nftokenID);
     }
 
     if (bo)

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

@@ -38,6 +38,12 @@ class NFTokenAcceptOffer : public Transactor
         std::shared_ptr<SLE> const& buy,
         std::shared_ptr<SLE> const& sell);
 
+    TER
+    transferNFToken(
+        AccountID const& buyer,
+        AccountID const& seller,
+        uint256 const& nfTokenID);
+
 public:
     static constexpr ConsequencesFactoryType ConsequencesFactory{Normal};
 

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 = 66;
+static constexpr std::size_t numFeatures = 67;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated
@@ -352,6 +352,7 @@ extern uint256 const featureXChainBridge;
 extern uint256 const fixDisallowIncomingV1;
 extern uint256 const featureDID;
 extern uint256 const fixFillOrKill;
+extern uint256 const fixNFTokenReserve;
 extern uint256 const fixInnerObjTemplate;
 
 }  // namespace ripple

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 = "2.0.1-rc1"
+char const* const versionString = "2.0.1"
 // clang-format on
 
 #if defined(DEBUG) || defined(SANITIZER)

src/ripple/protocol/impl/Feature.cpp

@@ -459,6 +459,7 @@ REGISTER_FEATURE(XChainBridge,                  Supported::yes, VoteBehavior::De
 REGISTER_FIX    (fixDisallowIncomingV1,         Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FEATURE(DID,                           Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX(fixFillOrKill,                     Supported::yes, VoteBehavior::DefaultNo);
+REGISTER_FIX    (fixNFTokenReserve,             Supported::yes, VoteBehavior::DefaultNo);
 REGISTER_FIX(fixInnerObjTemplate,               Supported::yes, VoteBehavior::DefaultNo);
 
 // The following amendments are obsolete, but must remain supported

src/secp256k1/src/CMakeLists.txt

@@ -64,9 +64,16 @@ elseif(APPLE)
   endif()
 elseif(CMAKE_SYSTEM_NAME STREQUAL "Windows")
   set(${PROJECT_NAME}_windows "secp256k1")
-  if(MSVC)
-    set(${PROJECT_NAME}_windows "${PROJECT_NAME}")
-  endif()
+  # This step is commented out from the original. It is bad practice to change
+  # the binary base name depending on the platform. CMake already manipulates
+  # the base name into a final name that fits the conventions of the platform.
+  # Linkers accept base names on the command line and then look for
+  # conventional names on disk. This way, developers can use base names
+  # everywhere (in the CMake and Conan they write) and the tools will do the
+  # right thing.
+  # if(MSVC)
+  #   set(${PROJECT_NAME}_windows "${PROJECT_NAME}")
+  # endif()
   set_target_properties(secp256k1 PROPERTIES
     ARCHIVE_OUTPUT_NAME "${${PROJECT_NAME}_windows}"
     RUNTIME_OUTPUT_NAME "${${PROJECT_NAME}_windows}-${${PROJECT_NAME}_soversion}"