[WIP] Checkpoint

include/xrpl/protocol/detail/features.macro

@@ -19,6 +19,7 @@ XRPL_FIX    (Cleanup3_2_0,                Supported::no, VoteBehavior::DefaultNo
 XRPL_FEATURE(MPTokensV2,                  Supported::no, VoteBehavior::DefaultNo)
 XRPL_FIX    (Security3_1_3,               Supported::no, VoteBehavior::DefaultNo)
 XRPL_FIX    (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
+XRPL_FIX    (ExpiredNFTokenOfferRemoval,  Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (BatchInnerSigs,              Supported::no, VoteBehavior::DefaultNo)
 XRPL_FEATURE(LendingProtocol,             Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionDelegationV1_1,    Supported::no,  VoteBehavior::DefaultNo)

src/libxrpl/basics/Number.cpp

@@ -222,9 +222,12 @@ Number::Guard::bringIntoRange(
     {
         mantissa *= 10;
         --exponent;
+        std::cout << "bringIntoRange. mantissa*=10: " << mantissa << ", exponent: " << exponent
+                  << std::endl;
     }
     if (exponent < minExponent)
     {
+        std::cout << "bringIntoRange. zero\n";
         constexpr Number zero = Number{};
 
         negative = zero.negative_;
@@ -246,13 +249,50 @@ Number::Guard::doRoundUp(
     auto r = round();
     if (r == 1 || (r == 0 && (mantissa & 1) == 1))
     {
-        ++mantissa;
-        // Ensure mantissa after incrementing fits within both the
-        // min/maxMantissa range and is a valid "rep".
-        if (mantissa > maxMantissa || mantissa > maxRep)
+        std::cout << "doRoundUp. r: " << r << std::endl;
+        if (isFeatureEnabled(fixCleanup3_2_0) || !getCurrentTransactionRules())
         {
-            mantissa /= 10;
-            ++exponent;
+            // Ensure mantissa after incrementing fits within both the
+            // min/maxMantissa range and is a valid "rep".
+            if (mantissa < maxMantissa && mantissa < maxRep)
+            {
+                // Nothing unusual here, just increment the mantissa
+                ++mantissa;
+                std::cout << "\tmantissa++: " << mantissa << std::endl;
+            }
+            else
+            {
+                // Incrementing the mantissa will require dividing, which will require rounding. So
+                // _don't_ increment the mantissa. Instead, divide and round recursively. It should
+                // be impossible to recurse more than once, because once the mantissa is divided by
+                // 10, it will be _well_ under maxMantissa and maxRep, so adding 1 will have no
+                // change of bringing it back over.
+                push(mantissa % 10);
+                mantissa /= 10;
+                ++exponent;
+                XRPL_ASSERT_PARTS(
+                    mantissa < maxMantissa && mantissa < maxRep,
+                    "xrpl::Number::Guard::doRoundUp",
+                    "can't recurse more than once");
+                std::cout << "\tmantissa/=10: " << mantissa << ", exponent: " << exponent
+                          << std::endl;
+                // Here be dragons
+                doRoundUp(negative, mantissa, exponent, minMantissa, maxMantissa, location);
+                return;
+            }
+        }
+        else
+        {
+            // Need to preserve the incorrect behavior until the fix amendment can be retired,
+            // because otherwise would risk an unplanned ledger fork.
+            ++mantissa;
+            // Ensure mantissa after incrementing fits within both the
+            // min/maxMantissa range and is a valid "rep".
+            if (mantissa > maxMantissa || mantissa > maxRep)
+            {
+                mantissa /= 10;
+                ++exponent;
+            }
         }
     }
     bringIntoRange(negative, mantissa, exponent, minMantissa);
@@ -667,6 +707,8 @@ Number::operator*=(Number const& y)
     auto const& minMantissa = range.min;
     auto const& maxMantissa = range.max;
 
+    std::cout << "zn: " << zn << ", zm: " << zm << ", ze: " << ze << std::endl;
+
     while (zm > maxMantissa || zm > maxRep)
     {
         // The following is optimization for:
@@ -675,6 +717,9 @@ Number::operator*=(Number const& y)
         g.push(divu10(zm));
         ++ze;
     }
+
+    std::cout << "zn: " << zn << ", zm: " << zm << ", ze: " << ze << std::endl;
+
     xm = static_cast<internalrep>(zm);
     xe = ze;
     g.doRoundUp(
@@ -787,8 +832,7 @@ Number::operator/=(Number const& y)
     return *this;
 }
 
-Number::
-operator rep() const
+Number::operator rep() const
 {
     rep drops = mantissa();
     int offset = exponent();

src/libxrpl/beast/core/CurrentThreadName.cpp

@@ -81,9 +81,12 @@ setCurrentThreadNameImpl(std::string_view name)
 {
     // truncate and set the thread name.
     char boundedName[maxThreadNameLength + 1];
-    auto const boundedSize = name.size() < maxThreadNameLength ? name.size() : maxThreadNameLength;
-    name.copy(boundedName, boundedSize);
-    boundedName[boundedSize] = '\0';
+    std::snprintf(
+        boundedName,
+        sizeof(boundedName),
+        "%.*s",
+        static_cast<int>(maxThreadNameLength),
+        name.data());  // NOLINT(bugprone-suspicious-stringview-data-usage)
 
     pthread_setname_np(pthread_self(), boundedName);
 

src/libxrpl/tx/transactors/nft/NFTokenAcceptOffer.cpp

@@ -68,12 +68,15 @@ NFTokenAcceptOffer::preclaim(PreclaimContext const& ctx)
 
             if (hasExpired(ctx.view, (*offerSLE)[~sfExpiration]))
             {
-                // Before fixSecurity3_1_3 amendment, expired offers caused tecEXPIRED in preclaim,
-                // leaving them on ledger forever. After the amendment, we allow expired offers to
-                // reach doApply() where they get deleted and tecEXPIRED is returned.
-                if (!ctx.view.rules().enabled(fixSecurity3_1_3))
+                // Before fixExpiredNFTokenOfferRemoval amendment, expired
+                // offers caused tecEXPIRED in preclaim, leaving them on ledger
+                // forever. After the amendment, we allow expired offers to
+                // reach doApply() where they get deleted and tecEXPIRED is
+                // returned.
+                if (!ctx.view.rules().enabled(fixExpiredNFTokenOfferRemoval))
                     return {nullptr, tecEXPIRED};
-                // Amendment enabled: return the expired offer to be handled in doApply.
+                // Amendment enabled: return the expired offer to be handled in
+                // doApply
             }
 
             if ((*offerSLE)[sfAmount].negative())
@@ -447,9 +450,10 @@ NFTokenAcceptOffer::doApply()
     auto bo = loadToken(ctx_.tx[~sfNFTokenBuyOffer]);
     auto so = loadToken(ctx_.tx[~sfNFTokenSellOffer]);
 
-    // With fixSecurity3_1_3 amendment, check for expired offers and delete them, returning
-    // tecEXPIRED. This ensures expired offers are properly cleaned up from the ledger.
-    if (view().rules().enabled(fixSecurity3_1_3))
+    // With fixExpiredNFTokenOfferRemoval amendment, check for expired offers
+    // and delete them, returning tecEXPIRED. This ensures expired offers
+    // are properly cleaned up from the ledger.
+    if (view().rules().enabled(fixExpiredNFTokenOfferRemoval))
     {
         bool foundExpired = false;
 

src/test/app/NFToken_test.cpp

@@ -1096,10 +1096,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
 
         // The buy offer must not have expired.
         // NOTE: this is only a preclaim check with the
-        // fixSecurity3_1_3 amendment disabled.
+        // fixExpiredNFTokenOfferRemoval amendment disabled.
         env(token::acceptBuyOffer(alice, buyerExpOfferIndex), ter(tecEXPIRED));
         env.close();
-        if (features[fixSecurity3_1_3])
+        if (features[fixExpiredNFTokenOfferRemoval])
         {
             buyerCount--;
         }
@@ -1117,12 +1117,12 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
 
         // The sell offer must not have expired.
         // NOTE: this is only a preclaim check with the
-        // fixSecurity3_1_3 amendment disabled.
+        // fixExpiredNFTokenOfferRemoval amendment disabled.
         env(token::acceptSellOffer(buyer, aliceExpOfferIndex), ter(tecEXPIRED));
         env.close();
         // Alice's count is decremented by one when the expired offer is
         // removed.
-        if (features[fixSecurity3_1_3])
+        if (features[fixExpiredNFTokenOfferRemoval])
         {
             aliceCount--;
         }
@@ -3101,10 +3101,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             // No one can accept an expired sell offer.
             env(token::acceptSellOffer(buyer, offer1), ter(tecEXPIRED));
 
-            // With fixSecurity3_1_3 amendment, the first accept
+            // With fixExpiredNFTokenOfferRemoval amendment, the first accept
             // attempt deletes the expired offer. Without the amendment,
             // the offer remains and we can try to accept it again.
-            if (features[fixSecurity3_1_3])
+            if (features[fixExpiredNFTokenOfferRemoval])
             {
                 // After amendment: offer was deleted by first accept attempt
                 minterCount--;
@@ -3123,7 +3123,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             BEAST_EXPECT(ownerCount(env, minter) == minterCount);
             BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
-            if (!features[fixSecurity3_1_3])
+            if (!features[fixExpiredNFTokenOfferRemoval])
             {
                 // Before amendment: expired offer still exists and needs to be
                 // cancelled
@@ -3189,10 +3189,10 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             // An expired buy offer cannot be accepted.
             env(token::acceptBuyOffer(minter, offer1), ter(tecEXPIRED));
 
-            // With fixSecurity3_1_3 amendment, the first accept
+            // With fixExpiredNFTokenOfferRemoval amendment, the first accept
             // attempt deletes the expired offer. Without the amendment,
             // the offer remains and we can try to accept it again.
-            if (features[fixSecurity3_1_3])
+            if (features[fixExpiredNFTokenOfferRemoval])
             {
                 // After amendment: offer was deleted by first accept attempt
                 buyerCount--;
@@ -3211,7 +3211,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             BEAST_EXPECT(ownerCount(env, minter) == minterCount);
             BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
-            if (!features[fixSecurity3_1_3])
+            if (!features[fixExpiredNFTokenOfferRemoval])
             {
                 // Before amendment: expired offer still exists and can be
                 // cancelled
@@ -3288,7 +3288,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             env(token::brokerOffers(issuer, buyOffer1, sellOffer1), ter(tecEXPIRED));
             env.close();
 
-            if (features[fixSecurity3_1_3])
+            if (features[fixExpiredNFTokenOfferRemoval])
             {
                 // With amendment: expired offers are deleted
                 minterCount--;
@@ -3298,7 +3298,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             BEAST_EXPECT(ownerCount(env, minter) == minterCount);
             BEAST_EXPECT(ownerCount(env, buyer) == buyerCount);
 
-            if (features[fixSecurity3_1_3])
+            if (features[fixExpiredNFTokenOfferRemoval])
             {
                 // The buy offer was deleted, so no need to cancel it
                 // The sell offer still exists, so we can cancel it
@@ -3377,7 +3377,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
             env.close();
 
             BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            if (features[fixSecurity3_1_3])
+            if (features[fixExpiredNFTokenOfferRemoval])
             {
                 // After amendment: expired offers were deleted during broker
                 // attempt
@@ -3463,7 +3463,7 @@ class NFTokenBaseUtil_test : public beast::unit_test::suite
 
             // The expired offers are still in the ledger.
             BEAST_EXPECT(ownerCount(env, issuer) == 0);
-            if (!features[fixSecurity3_1_3])
+            if (!features[fixExpiredNFTokenOfferRemoval])
             {
                 // Before amendment: expired offers still exist in ledger
                 BEAST_EXPECT(ownerCount(env, minter) == 2);
@@ -7190,7 +7190,7 @@ public:
     {
         testWithFeats(
             allFeatures - fixNFTokenReserve - featureNFTokenMintOffer - featureDynamicNFT -
-            fixSecurity3_1_3);
+            fixExpiredNFTokenOfferRemoval);
     }
 };
 
@@ -7227,7 +7227,7 @@ class NFTokenWOExpiredOfferRemoval_test : public NFTokenBaseUtil_test
     void
     run() override
     {
-        testWithFeats(allFeatures - fixSecurity3_1_3);
+        testWithFeats(allFeatures - fixExpiredNFTokenOfferRemoval);
     }
 };
 

src/test/basics/Number_test.cpp

@@ -1584,3 +1584,101 @@ public:
 BEAST_DEFINE_TESTSUITE(Number, basics, xrpl);
 
 }  // namespace xrpl
+#include <xrpl/basics/Number.h>
+#include <xrpl/beast/unit_test.h>
+#include <xrpl/protocol/AccountID.h>
+#include <xrpl/protocol/MPTIssue.h>
+#include <xrpl/protocol/STAmount.h>
+
+#include <boost/multiprecision/cpp_int.hpp>
+
+#include <cstdint>
+#include <limits>
+#include <sstream>
+#include <string>
+
+namespace xrpl {
+
+class NumberUpwardWrongDirection_test : public beast::unit_test::suite
+{
+    using BigInt = boost::multiprecision::cpp_int;
+
+    static std::string
+    fmt(BigInt const& value)
+    {
+        std::ostringstream os;
+        os << value;
+        auto s = os.str();
+        std::string out;
+        int count = 0;
+        for (auto it = s.rbegin(); it != s.rend(); ++it)
+        {
+            if (count && count % 3 == 0)
+                out.insert(out.begin(), '_');
+            out.insert(out.begin(), *it);
+            ++count;
+        }
+        return out;
+    }
+
+public:
+    void
+    testUpwardRoundsDown()
+    {
+        testcase << "upward rounding produces a value below exact at maxRep cusp";
+
+        auto const origScale = Number::getMantissaScale();
+        auto const origRound = Number::setround(Number::upward);
+        Number::setMantissaScale(MantissaRange::large);
+
+        constexpr std::int64_t aValue = 1'000'000'000'000'049'863LL;
+        constexpr std::int64_t bValue = 9'223'372'036'854'315'903LL;
+
+        // JSON -> STAmount -> Number
+        AccountID const dummyIssuer = AccountID{42u};
+        MPTIssue const issue(/*sequence=*/1u, dummyIssuer);
+
+        STAmount const amountA{MPTAmount{aValue}, issue};
+        STAmount const amountB{MPTAmount{bValue}, issue};
+
+        // Public conversion operator: STAmount::operator Number() const.
+        Number const a = amountA;
+        Number const b = amountB;
+        Number const product = a * b;
+
+        // Exact reference in BigInt.
+        BigInt const exactProduct = BigInt(aValue) * BigInt(bValue);
+
+        // What Number actually stored.
+        BigInt storedValue = BigInt(product.mantissa());
+        for (int i = 0; i < product.exponent(); ++i)
+            storedValue *= 10;
+
+        BigInt const signedDifference = storedValue - exactProduct;
+
+        log << "\n"
+            << "  a              = " << fmt(BigInt(aValue)) << "\n"
+            << "  b              = " << fmt(BigInt(bValue)) << "\n"
+            << "  exact a*b      = " << fmt(exactProduct) << "\n"
+            << "  stored         = " << fmt(storedValue) << "\n"
+            << "  stored - exact = " << fmt(signedDifference) << "\n"
+            << "  upward         = " << (signedDifference >= 0 ? "held" : "VIOLATED") << "\n";
+
+        BEAST_EXPECT(signedDifference >= 0);
+        BEAST_EXPECT(product.mantissa() == (std::numeric_limits<std::int64_t>::max() /10) + 1);
+        BEAST_EXPECT(product.exponent() == 19);
+
+        Number::setround(origRound);
+        Number::setMantissaScale(origScale);
+    }
+
+    void
+    run() override
+    {
+        testUpwardRoundsDown();
+    }
+};
+
+BEAST_DEFINE_TESTSUITE(NumberUpwardWrongDirection, basics, ripple);
+
+}  // namespace xrpl

src/test/rpc/LedgerEntry_test.cpp

@@ -1201,7 +1201,7 @@ class LedgerEntry_test : public beast::unit_test::suite
             checkErrorValue(
                 jrr[jss::result],
                 "malformedAuthorizedCredentials",
-                "Invalid field 'authorized_credentials', not array of objects.");
+                "Invalid field 'authorized_credentials', not array.");
         }
 
         {
@@ -1219,7 +1219,7 @@ class LedgerEntry_test : public beast::unit_test::suite
             checkErrorValue(
                 jrr[jss::result],
                 "malformedAuthorizedCredentials",
-                "Invalid field 'authorized_credentials', not array of objects.");
+                "Invalid field 'authorized_credentials', not array.");
         }
 
         {

src/xrpld/overlay/detail/PeerImp.cpp

@@ -2639,7 +2639,7 @@ PeerImp::onMessage(std::shared_ptr<protocol::TMGetObjectByHash> const& m)
                     {
                         fee_.update(
                             Resource::feeModerateBurdenPeer,
-                            "Reply limit reached. Truncating reply.");
+                            " Reply limit reached. Truncating reply.");
                         break;
                     }
                 }

src/xrpld/rpc/handlers/ledger/LedgerEntry.cpp

@@ -267,7 +267,7 @@ parseAuthorizeCredentials(Json::Value const& jv)
         if (!jo.isObject())
         {
             return LedgerEntryHelpers::invalidFieldError(
-                "malformedAuthorizedCredentials", jss::authorized_credentials, "array of objects");
+                "malformedAuthorizedCredentials", jss::authorized_credentials, "array");
         }
 
         if (auto const value = LedgerEntryHelpers::hasRequired(