Merge branch 'develop' into gregtatcam/vault-lending-amount-range

VaultCreate and VaultSet round AssetsMaximum to the vault asset, and
LoanBrokerSet and LoanSet check DebtMaximum and PrincipalRequested
against it. Each builds an STAmount, which throws for a value out of
range for the asset, e.g. XRP above 1e17. The transaction fails with
tefEXCEPTION.

Add isRepresentable() and reject these values up front: VaultCreate
with temMALFORMED, the others with tecPRECISION_LOSS.

Gated on fixCleanup3_5_0.

include/xrpl/protocol/STAmount.h

@@ -766,6 +766,21 @@ roundToAsset(
     return roundToScale(ret, scale);
 }
 
+/**
+ * Check whether an arbitrary precision Number can be converted to an STAmount
+ * of a given Asset without exceeding the asset's range.
+ *
+ * Use this before `roundToAsset` or `STAmount{asset, value}` on unchecked
+ * input, since those throw for out-of-range values, e.g. XRP above
+ * `STAmount::kMaxNativeN`.
+ *
+ * @param asset The relevant asset
+ * @param value The value to be checked
+ * @return true if the value is in range for the asset; false otherwise.
+ */
+[[nodiscard]] bool
+isRepresentable(Asset const& asset, Number const& value);
+
 //------------------------------------------------------------------------------
 
 inline bool

src/libxrpl/protocol/STAmount.cpp

@@ -443,6 +443,22 @@ getRate(STAmount const& offerOut, STAmount const& offerIn)
     }
 }
 
+bool
+isRepresentable(Asset const& asset, Number const& value)
+{
+    try
+    {
+        // Out-of-range values throw std::overflow_error from Number or
+        // IOUAmount, or std::runtime_error from STAmount::canonicalize.
+        [[maybe_unused]] STAmount const amount{asset, value};
+        return true;
+    }
+    catch (std::runtime_error const&)
+    {
+        return false;
+    }
+}
+
 /**
  * @brief Safely checks if two STAmount values can be added without overflow,
  * underflow, or precision loss.

src/libxrpl/tx/transactors/lending/LoanBrokerSet.cpp

@@ -14,6 +14,7 @@
 #include <xrpl/protocol/Indexes.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STLedgerEntry.h>
 #include <xrpl/protocol/STNumber.h>
 #include <xrpl/protocol/STTakesAsset.h>
@@ -171,10 +172,15 @@ LoanBrokerSet::preclaim(PreclaimContext const& ctx)
     }
 
     // Check that relevant values can be represented as the vault asset
-    // type. This is mostly only relevant for integral (non-IOU) types
+    // type. This is mostly only relevant for integral (non-IOU) types.
+    // fixCleanup3_5_0 also rejects out-of-range values, e.g. XRP above
+    // STAmount::kMaxNativeN, which would otherwise throw.
+    bool const fix350Enabled = ctx.view.rules().enabled(fixCleanup3_5_0);
     for (auto const& field : getValueFields())
     {
-        if (auto const value = tx[field]; value && STAmount{asset, *value} != *value)
+        if (auto const value = tx[field]; value &&
+            ((fix350Enabled && !isRepresentable(asset, *value)) ||
+             STAmount{asset, *value} != *value))
         {
             JLOG(ctx.j.warn()) << field.f->getName() << " (" << *value
                                << ") can not be represented as a(n) " << to_string(asset) << ".";

src/libxrpl/tx/transactors/lending/LoanSet.cpp

@@ -19,6 +19,7 @@
 #include <xrpl/protocol/LedgerFormats.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STLedgerEntry.h>
 #include <xrpl/protocol/STNumber.h>
 #include <xrpl/protocol/STObject.h>
@@ -365,10 +366,14 @@ LoanSet::preclaim(PreclaimContext const& ctx)
     // Check that relevant values can be represented as the vault asset type.
     // This check is almost duplicated in doApply, but that check is done after
     // the overall loan scale is known. This is mostly only relevant for
-    // integral (non-IOU) types
+    // integral (non-IOU) types. fixCleanup3_5_0 also rejects out-of-range
+    // values, e.g. XRP above STAmount::kMaxNativeN, which would otherwise throw.
+    bool const fix350Enabled = ctx.view.rules().enabled(fixCleanup3_5_0);
     for (auto const& field : getValueFields())
     {
-        if (auto const value = tx[field]; value && STAmount{asset, *value} != *value)
+        if (auto const value = tx[field]; value &&
+            ((fix350Enabled && !isRepresentable(asset, *value)) ||
+             STAmount{asset, *value} != *value))
         {
             JLOG(ctx.j.warn()) << field.f->getName() << " (" << *value
                                << ") can not be represented as a(n) " << to_string(asset) << ".";

src/libxrpl/tx/transactors/vault/VaultCreate.cpp

@@ -18,6 +18,7 @@
 #include <xrpl/protocol/MPTIssue.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STLedgerEntry.h>
 #include <xrpl/protocol/STNumber.h>  // IWYU pragma: keep
 #include <xrpl/protocol/STTakesAsset.h>
@@ -87,6 +88,11 @@ VaultCreate::preflight(PreflightContext const& ctx)
     {
         if (*assetMax < beast::kZero)
             return temMALFORMED;
+
+        // An AssetsMaximum out of range for the asset, e.g. XRP above
+        // STAmount::kMaxNativeN, would throw when rounded to the asset.
+        if (ctx.rules.enabled(fixCleanup3_5_0) && !isRepresentable(ctx.tx[sfAsset], *assetMax))
+            return temMALFORMED;
     }
 
     if (auto const metadata = ctx.tx[~sfMPTokenMetadata])

src/libxrpl/tx/transactors/vault/VaultSet.cpp

@@ -7,6 +7,7 @@
 #include <xrpl/protocol/LedgerFormats.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/STLedgerEntry.h>
 #include <xrpl/protocol/STNumber.h>  // IWYU pragma: keep
 #include <xrpl/protocol/STTakesAsset.h>
@@ -74,6 +75,16 @@ VaultSet::preclaim(PreclaimContext const& ctx)
         return tecNO_PERMISSION;
     }
 
+    // An AssetsMaximum out of range for the vault asset, e.g. XRP above
+    // STAmount::kMaxNativeN, would throw when rounded to the asset.
+    if (auto const assetMax = ctx.tx[~sfAssetsMaximum]; assetMax &&
+        ctx.view.rules().enabled(fixCleanup3_5_0) &&
+        !isRepresentable(vault->at(sfAsset), *assetMax))
+    {
+        JLOG(ctx.j.debug()) << "VaultSet: max assets out of range for the asset.";
+        return tecPRECISION_LOSS;
+    }
+
     auto const mptIssuanceID = (*vault)[sfShareMPTID];
     auto const sleIssuance = ctx.view.read(keylet::mptokenIssuance(mptIssuanceID));
     if (!sleIssuance)

src/test/app/lending/LoanBroker_test.cpp

@@ -3082,6 +3082,42 @@ class LoanBroker_test : public beast::unit_test::Suite
     }
 
 public:
+    void
+    testDebtMaximumOutOfRange(FeatureBitset features)
+    {
+        using namespace jtx;
+        using namespace loan_broker;
+
+        bool const withFix = features[fixCleanup3_5_0];
+        testcase(
+            std::string("DebtMaximum out of range for the asset (") + (withFix ? "post" : "pre") +
+            "-fixCleanup3_5_0)");
+
+        Env env(*this, features);
+        Account const alice{"alice"};
+        env.fund(XRP(100'000), alice);
+        env.close();
+
+        Vault const vault{env};
+        auto const [tx, keylet] = vault.create({.owner = alice, .asset = xrpIssue()});
+        env(tx);
+        env.close();
+
+        // Without the fix, the representability check in preclaim throws for
+        // XRP above STAmount::kMaxNativeN.
+        auto const ownerCount = env.ownerCount(alice);
+        Number const maxXrp{static_cast<std::int64_t>(STAmount::kMaxNativeN)};
+        env(set(alice, keylet.key),
+            kDebtMaximum(maxXrp + 1),
+            Ter(withFix ? TER{tecPRECISION_LOSS} : TER{tefEXCEPTION}));
+        env.close();
+        BEAST_EXPECT(env.ownerCount(alice) == ownerCount);
+
+        env(set(alice, keylet.key), kDebtMaximum(maxXrp));
+        env.close();
+        BEAST_EXPECT(env.ownerCount(alice) > ownerCount);
+    }
+
     void
     run() override
     {
@@ -3120,6 +3156,9 @@ public:
         testLoanBrokerDeleteConfidentialCOA(all_);
         testLoanBrokerDeleteConfidentialCOA(all_ - fixCleanup3_5_0);
 
+        testDebtMaximumOutOfRange(all_);
+        testDebtMaximumOutOfRange(all_ - fixCleanup3_5_0);
+
         // featureMPTokensV2 independently makes ValidMPTTransfer enforcing,
         // but it's Supported::No (never enabled on real networks); exclude
         // it here so fixCleanup3_4_0 alone is the deciding amendment, as it

src/test/app/lending/LoanSet_test.cpp

@@ -22,6 +22,7 @@
 #include <xrpl/protocol/LedgerFormats.h>
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
+#include <xrpl/protocol/STAmount.h>
 #include <xrpl/protocol/SeqProxy.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>
@@ -928,6 +929,38 @@ private:
         run(all_, tesSUCCESS);
     }
 
+    void
+    testPrincipalRequestedOutOfRange(FeatureBitset features)
+    {
+        using namespace jtx;
+        using namespace loan;
+
+        bool const withFix = features[fixCleanup3_5_0];
+        testcase(
+            std::string("LoanSet PrincipalRequested out of range for the asset (") +
+            (withFix ? "post" : "pre") + "-fixCleanup3_5_0)");
+
+        Env env(*this, features);
+        Account const lender{"lender"};
+        Account const borrower{"borrower"};
+        env.fund(XRP(10'000'000), lender, borrower);
+        env.close();
+
+        PrettyAsset const xrp{xrpIssue(), 1'000'000};
+        auto const broker = createVaultAndBroker(env, xrp, lender);
+
+        // Without the fix, the representability check in preclaim throws for
+        // XRP above STAmount::kMaxNativeN.
+        auto const ownerCount = env.ownerCount(borrower);
+        Number const maxXrp{static_cast<std::int64_t>(STAmount::kMaxNativeN)};
+        env(set(borrower, broker.brokerID, maxXrp + 1),
+            Sig(sfCounterpartySignature, lender),
+            Fee(env.current()->fees().base * 2),
+            Ter(withFix ? TER{tecPRECISION_LOSS} : TER{tefEXCEPTION}));
+        env.close();
+        BEAST_EXPECT(env.ownerCount(borrower) == ownerCount);
+    }
+
 public:
     void
     run() override
@@ -940,6 +973,9 @@ public:
         testLoanSetExistingLineAfterIssuerClearsDefaultRipple();
         testLoanSetOriginationFeeTwoMptCreates(all_);
         testLoanSetOriginationFeeTwoMptCreates(all_ - fixCleanup3_4_0);
+
+        testPrincipalRequestedOutOfRange(all_);
+        testPrincipalRequestedOutOfRange(all_ - fixCleanup3_5_0);
     }
 };
 

src/test/app/vault/VaultScale_test.cpp

@@ -1012,13 +1012,15 @@ private:
     }
 
     void
-    testAssetsMaximum()
+    testAssetsMaximum(FeatureBitset features)
     {
-        testcase("Assets Maximum");
+        bool const withFix = features[fixCleanup3_5_0];
+        testcase(
+            std::string("Assets Maximum (") + (withFix ? "post" : "pre") + "-fixCleanup3_5_0)");
 
         using namespace test::jtx;
 
-        Env env{*this, testableAmendments()};
+        Env env{*this, features};
         Account const owner{"owner"};
         Account const issuer{"issuer"};
 
@@ -1044,6 +1046,10 @@ private:
         auto const initialXRPPlus1 = to_string(kInitialXrp + 1);
         BEAST_EXPECT(initialXRPPlus1 == "100000000000000001");
 
+        // Without the fix, rounding an out-of-range AssetsMaximum to the vault
+        // asset throws.
+        TER const outOfRangeErr = withFix ? TER{temMALFORMED} : TER{tefEXCEPTION};
+
         {
             testcase("Assets Maximum: XRP");
 
@@ -1053,11 +1059,11 @@ private:
             tx[sfData] = "4D65746144617461";
 
             tx[sfAssetsMaximum] = maxInt64;
-            env(tx, Ter(tefEXCEPTION));
+            env(tx, Ter(outOfRangeErr));
             env.close();
 
             tx[sfAssetsMaximum] = initialXRPPlus1;
-            env(tx, Ter(tefEXCEPTION));
+            env(tx, Ter(outOfRangeErr));
             env.close();
 
             tx[sfAssetsMaximum] = initialXRP;
@@ -1306,7 +1312,7 @@ private:
             // What _can't_ IOUs do?
             // 1. Exceed maximum exponent / offset
             tx[sfAssetsMaximum] = "1000000000000000e81";
-            env(tx, Ter(tefEXCEPTION));
+            env(tx, Ter(outOfRangeErr));
             env.close();
 
             // 2. Mantissa larger than uint64 max
@@ -1331,7 +1337,8 @@ public:
     run() override
     {
         testScaleIOU();
-        testAssetsMaximum();
+        testAssetsMaximum(all_ - fixCleanup3_5_0);
+        testAssetsMaximum(all_);
     }
 };
 

src/test/app/vault/VaultValidation_test.cpp

@@ -30,11 +30,13 @@
 #include <xrpl/protocol/Protocol.h>
 #include <xrpl/protocol/SField.h>
 #include <xrpl/protocol/STAmount.h>
+#include <xrpl/protocol/STNumber.h>  // IWYU pragma: keep
 #include <xrpl/protocol/SeqProxy.h>
 #include <xrpl/protocol/TER.h>
 #include <xrpl/protocol/TxFlags.h>
 #include <xrpl/protocol/jss.h>
 
+#include <cstdint>
 #include <functional>
 #include <string>
 #include <tuple>
@@ -1177,6 +1179,62 @@ private:
         }
     }
 
+    void
+    testAssetsMaximumOutOfRange(FeatureBitset features)
+    {
+        using namespace test::jtx;
+
+        bool const withFix = features[fixCleanup3_5_0];
+        testcase(
+            std::string("AssetsMaximum out of range for the asset (") + (withFix ? "post" : "pre") +
+            "-fixCleanup3_5_0)");
+
+        Env env{*this, features};
+        Account const issuer{"issuer"};
+        Account const owner{"owner"};
+        env.fund(XRP(1'000'000), issuer, owner);
+        env.close();
+
+        Vault const vault{env};
+
+        // Without the fix, rounding an out-of-range AssetsMaximum to the vault
+        // asset throws.
+        TER const createErr = withFix ? TER{temMALFORMED} : TER{tefEXCEPTION};
+        TER const setErr = withFix ? TER{tecPRECISION_LOSS} : TER{tefEXCEPTION};
+
+        auto test = [&](Asset const& asset, Number const& maxValid, Number const& overMax) {
+            {
+                auto [tx, keylet] = vault.create({.owner = owner, .asset = asset});
+                tx[sfAssetsMaximum] = to_string(overMax);
+                env(tx, Ter(createErr));
+                env.close();
+                BEAST_EXPECT(!env.le(keylet));
+            }
+
+            auto [tx, keylet] = vault.create({.owner = owner, .asset = asset});
+            tx[sfAssetsMaximum] = to_string(maxValid);
+            env(tx);
+            env.close();
+
+            auto setTx = vault.set({.owner = owner, .id = keylet.key});
+            setTx[sfAssetsMaximum] = to_string(overMax);
+            env(setTx, Ter(setErr));
+            env.close();
+
+            if (auto const sle = env.le(keylet); BEAST_EXPECT(sle))
+                BEAST_EXPECT(sle->at(sfAssetsMaximum) == maxValid);
+        };
+
+        Number const maxXrp{static_cast<std::int64_t>(STAmount::kMaxNativeN)};
+        test(xrpIssue(), maxXrp, maxXrp + 1);
+
+        MPT const btc = MPTTester({.env = env, .issuer = issuer, .holders = {owner}});
+        test(btc.asset(), Number{static_cast<std::int64_t>(kMaxMpTokenAmount)}, Number{1, 19});
+
+        Number const maxIou{static_cast<std::int64_t>(STAmount::kMaxValue), STAmount::kMaxOffset};
+        test(issuer["USD"], maxIou, Number{1, STAmount::kMaxOffset + 16});
+    }
+
 public:
     void
     run() override
@@ -1190,6 +1248,9 @@ public:
 
         testVaultWithdrawPseudoAccountDestination(all_ - fixCleanup3_4_0);
         testVaultWithdrawPseudoAccountDestination(all_);
+
+        testAssetsMaximumOutOfRange(all_ - fixCleanup3_5_0);
+        testAssetsMaximumOutOfRange(all_);
     }
 };