From 673fb06c7519c9b8c9e319f1699024b88faa3b7c Mon Sep 17 00:00:00 2001 From: Jingchen Date: Wed, 5 Nov 2025 14:56:20 +0000 Subject: [PATCH 1/7] refactor: Retire fixTrustLinesToSelf amendment (#5989) Amendments activated for more than 2 years can be retired. This change retires the fixTrustLinesToSelf amendment. --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/rpc/Feature_test.cpp | 3 +- src/xrpld/app/tx/detail/Change.cpp | 85 --------------------- src/xrpld/app/tx/detail/Change.h | 3 - src/xrpld/app/tx/detail/SetTrust.cpp | 40 +--------- 5 files changed, 5 insertions(+), 128 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 92f05f8149..6ac33f3e78 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -62,7 +62,6 @@ XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(DisallowIncoming, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FIX (TrustLinesToSelf, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(ExpandedSignerList, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(CheckCashMakesTrustLine, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) @@ -119,6 +118,7 @@ XRPL_RETIRE(fixReducedOffersV1) XRPL_RETIRE(fixRmSmallIncreasedQOffers) XRPL_RETIRE(fixSTAmountCanonicalize) XRPL_RETIRE(fixTakerDryOfferRemoval) +XRPL_RETIRE(fixTrustLinesToSelf) XRPL_RETIRE(CryptoConditions) XRPL_RETIRE(Escrow) XRPL_RETIRE(EnforceInvariants) diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index 0f5cf65f72..1a53f39f60 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -121,7 +121,8 @@ class Feature_test : public beast::unit_test::suite // Test a random sampling of the variables. If any of these get retired // or removed, swap out for any other feature. BEAST_EXPECT( - featureToName(fixTrustLinesToSelf) == "fixTrustLinesToSelf"); + featureToName(fixRemoveNFTokenAutoTrustLine) == + "fixRemoveNFTokenAutoTrustLine"); BEAST_EXPECT(featureToName(featureFlow) == "Flow"); BEAST_EXPECT(featureToName(featureNegativeUNL) == "NegativeUNL"); BEAST_EXPECT( diff --git a/src/xrpld/app/tx/detail/Change.cpp b/src/xrpld/app/tx/detail/Change.cpp index 92b0eb2bb2..4832287c2e 100644 --- a/src/xrpld/app/tx/detail/Change.cpp +++ b/src/xrpld/app/tx/detail/Change.cpp @@ -147,88 +147,6 @@ Change::preCompute() account_ == beast::zero, "ripple::Change::preCompute : zero account"); } -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() { @@ -305,9 +223,6 @@ 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/xrpld/app/tx/detail/Change.h b/src/xrpld/app/tx/detail/Change.h index d71c5baeb5..9ff37b1515 100644 --- a/src/xrpld/app/tx/detail/Change.h +++ b/src/xrpld/app/tx/detail/Change.h @@ -29,9 +29,6 @@ public: preclaim(PreclaimContext const& ctx); private: - void - activateTrustLinesToSelfFix(); - TER applyAmendment(); diff --git a/src/xrpld/app/tx/detail/SetTrust.cpp b/src/xrpld/app/tx/detail/SetTrust.cpp index 2b10b94374..dcbdbc013f 100644 --- a/src/xrpld/app/tx/detail/SetTrust.cpp +++ b/src/xrpld/app/tx/detail/SetTrust.cpp @@ -195,29 +195,8 @@ SetTrust::preclaim(PreclaimContext const& ctx) auto const currency = saLimitAmount.getCurrency(); auto const uDstAccountID = saLimitAmount.getIssuer(); - if (ctx.view.rules().enabled(fixTrustLinesToSelf)) - { - 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; - } - } - } + if (id == uDstAccountID) + return temDST_IS_SRC; // This might be nullptr auto const sleDst = ctx.view.read(keylet::account(uDstAccountID)); @@ -403,21 +382,6 @@ SetTrust::doApply() auto viewJ = ctx_.app.journal("View"); - // 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) - { - return trustDelete( - view(), - view().peek(keylet::line(account_, uDstAccountID, currency)), - account_, - uDstAccountID, - viewJ); - } - SLE::pointer sleDst = view().peek(keylet::account(uDstAccountID)); if (!sleDst) From 28a1f909383952c12273ba11dff58d125563bfac Mon Sep 17 00:00:00 2001 From: Shawn Xie <35279399+shawnxie999@users.noreply.github.com> Date: Wed, 5 Nov 2025 18:08:10 -0500 Subject: [PATCH 2/7] fix: domain order book insertion #5998 --- src/xrpld/app/ledger/OrderBookDB.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xrpld/app/ledger/OrderBookDB.cpp b/src/xrpld/app/ledger/OrderBookDB.cpp index da4bc1d28f..1a407d0d3d 100644 --- a/src/xrpld/app/ledger/OrderBookDB.cpp +++ b/src/xrpld/app/ledger/OrderBookDB.cpp @@ -106,7 +106,7 @@ OrderBookDB::update(std::shared_ptr const& ledger) book.domain = (*sle)[~sfDomainID]; if (book.domain) - domainBooks_[{book.in, *book.domain}].insert(book.out); + domainBooks[{book.in, *book.domain}].insert(book.out); else allBooks[book.in].insert(book.out); From d10a5786633124322f0cd0417c9ead8f9da9dfe3 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 4 Nov 2025 20:44:45 -0500 Subject: [PATCH 3/7] Add optional enforcement of valid integer range to Number --- include/xrpl/basics/Number.h | 116 ++++++++++++++++++++-- src/libxrpl/basics/Number.cpp | 40 ++++++++ src/test/basics/Number_test.cpp | 168 ++++++++++++++++++++++++++++++++ 3 files changed, 317 insertions(+), 7 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index e34cc61b5b..59f3a212a6 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -13,16 +13,47 @@ class Number; std::string to_string(Number const& amount); +template +constexpr bool +isPowerOfTen(T value) +{ + while (value >= 10 && value % 10 == 0) + value /= 10; + return value == 1; +} + class Number { +public: + /** Describes whether and how to enforce this number as an integer. + * + * - none: No enforcement. The value may vary freely. This is the default. + * - weak: If the absolute value is greater than maxIntValue, valid() will + * return false. + * - strong: Assignment operations will throw if the absolute value is above + * maxIntValue. + */ + enum EnforceInteger { none, weak, strong }; + +private: using rep = std::int64_t; rep mantissa_{0}; int exponent_{std::numeric_limits::lowest()}; + // The enforcement setting is not serialized, and does not affect the + // ledger. If not "none", the value is checked to be within the valid + // integer range. With "strong", the checks will be made as automatic as + // possible. + EnforceInteger enforceInteger_ = none; + public: // The range for the mantissa when normalized - constexpr static std::int64_t minMantissa = 1'000'000'000'000'000LL; - constexpr static std::int64_t maxMantissa = 9'999'999'999'999'999LL; + constexpr static rep minMantissa = 1'000'000'000'000'000LL; + static_assert(isPowerOfTen(minMantissa)); + constexpr static rep maxMantissa = minMantissa * 10 - 1; + static_assert(maxMantissa == 9'999'999'999'999'999LL); + + constexpr static rep maxIntValue = minMantissa / 10; // The range for the exponent when normalized constexpr static int minExponent = -32768; @@ -35,15 +66,33 @@ public: explicit constexpr Number() = default; - Number(rep mantissa); - explicit Number(rep mantissa, int exponent); + Number(rep mantissa, EnforceInteger enforce = none); + explicit Number(rep mantissa, int exponent, EnforceInteger enforce = none); explicit constexpr Number(rep mantissa, int exponent, unchecked) noexcept; + constexpr Number(Number const& other) = default; + constexpr Number(Number&& other) = default; + + ~Number() = default; + + constexpr Number& + operator=(Number const& other); + constexpr Number& + operator=(Number&& other); constexpr rep mantissa() const noexcept; constexpr int exponent() const noexcept; + void + setIntegerEnforcement(EnforceInteger enforce); + + EnforceInteger + integerEnforcement() const noexcept; + + bool + valid() const noexcept; + constexpr Number operator+() const noexcept; constexpr Number @@ -184,6 +233,9 @@ public: private: static thread_local rounding_mode mode_; + void + checkInteger(char const* what) const; + void normalize(); constexpr bool @@ -197,16 +249,52 @@ inline constexpr Number::Number(rep mantissa, int exponent, unchecked) noexcept { } -inline Number::Number(rep mantissa, int exponent) - : mantissa_{mantissa}, exponent_{exponent} +inline Number::Number(rep mantissa, int exponent, EnforceInteger enforce) + : mantissa_{mantissa}, exponent_{exponent}, enforceInteger_(enforce) { normalize(); + + checkInteger("Number::Number integer overflow"); } -inline Number::Number(rep mantissa) : Number{mantissa, 0} +inline Number::Number(rep mantissa, EnforceInteger enforce) + : Number{mantissa, 0, enforce} { } +constexpr Number& +Number::operator=(Number const& other) +{ + if (this != &other) + { + mantissa_ = other.mantissa_; + exponent_ = other.exponent_; + enforceInteger_ = std::max(enforceInteger_, other.enforceInteger_); + + checkInteger("Number::operator= integer overflow"); + } + + return *this; +} + +constexpr Number& +Number::operator=(Number&& other) +{ + if (this != &other) + { + // std::move doesn't really do anything for these types, but + // this is future-proof in case the types ever change + mantissa_ = std::move(other.mantissa_); + exponent_ = std::move(other.exponent_); + if (other.enforceInteger_ > enforceInteger_) + enforceInteger_ = std::move(other.enforceInteger_); + + checkInteger("Number::operator= integer overflow"); + } + + return *this; +} + inline constexpr Number::rep Number::mantissa() const noexcept { @@ -219,6 +307,20 @@ Number::exponent() const noexcept return exponent_; } +inline void +Number::setIntegerEnforcement(EnforceInteger enforce) +{ + enforceInteger_ = enforce; + + checkInteger("Number::setIntegerEnforcement integer overflow"); +} + +inline Number::EnforceInteger +Number::integerEnforcement() const noexcept +{ + return enforceInteger_; +} + inline constexpr Number Number::operator+() const noexcept { diff --git a/src/libxrpl/basics/Number.cpp b/src/libxrpl/basics/Number.cpp index 89f7937e06..e3789de90a 100644 --- a/src/libxrpl/basics/Number.cpp +++ b/src/libxrpl/basics/Number.cpp @@ -155,6 +155,13 @@ Number::Guard::round() noexcept constexpr Number one{1000000000000000, -15, Number::unchecked{}}; +void +Number::checkInteger(char const* what) const +{ + if (enforceInteger_ == strong && !valid()) + throw std::overflow_error(what); +} + void Number::normalize() { @@ -207,9 +214,27 @@ Number::normalize() mantissa_ = -mantissa_; } +bool +Number::valid() const noexcept +{ + if (enforceInteger_ != none) + { + static Number const max = maxIntValue; + static Number const maxNeg = -maxIntValue; + // Avoid making a copy + if (mantissa_ < 0) + return *this >= maxNeg; + return *this <= max; + } + return true; +} + Number& Number::operator+=(Number const& y) { + // The strictest setting prevails + enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); + if (y == Number{}) return *this; if (*this == Number{}) @@ -322,6 +347,9 @@ Number::operator+=(Number const& y) } mantissa_ = xm * xn; exponent_ = xe; + + checkInteger("Number::addition integer overflow"); + return *this; } @@ -356,6 +384,9 @@ divu10(uint128_t& u) Number& Number::operator*=(Number const& y) { + // The strictest setting prevails + enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); + if (*this == Number{}) return *this; if (y == Number{}) @@ -422,12 +453,18 @@ Number::operator*=(Number const& y) XRPL_ASSERT( isnormal() || *this == Number{}, "ripple::Number::operator*=(Number) : result is normal"); + + checkInteger("Number::multiplication integer overflow"); + return *this; } Number& Number::operator/=(Number const& y) { + // The strictest setting prevails + enforceInteger_ = std::max(enforceInteger_, y.enforceInteger_); + if (y == Number{}) throw std::overflow_error("Number: divide by 0"); if (*this == Number{}) @@ -455,6 +492,9 @@ Number::operator/=(Number const& y) exponent_ = ne - de - 17; mantissa_ *= np * dp; normalize(); + + checkInteger("Number::division integer overflow"); + return *this; } diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 06203a4c2a..78c9b28952 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -725,6 +726,172 @@ public: BEAST_EXPECT(Number(-100, -30000).truncate() == Number(0, 0)); } + void + testInteger() + { + testcase("Integer enforcement"); + + using namespace std::string_literals; + + { + Number a{100}; + BEAST_EXPECT(a.integerEnforcement() == Number::none); + BEAST_EXPECT(a.valid()); + a = Number{1, 30}; + BEAST_EXPECT(a.valid()); + a = -100; + BEAST_EXPECT(a.valid()); + } + { + Number a{100, Number::weak}; + BEAST_EXPECT(a.integerEnforcement() == Number::weak); + BEAST_EXPECT(a.valid()); + a = Number{1, 30, Number::none}; + BEAST_EXPECT(!a.valid()); + a = -100; + BEAST_EXPECT(a.integerEnforcement() == Number::weak); + BEAST_EXPECT(a.valid()); + a = Number{5, Number::strong}; + BEAST_EXPECT(a.integerEnforcement() == Number::strong); + BEAST_EXPECT(a.valid()); + } + { + Number a{100, Number::strong}; + BEAST_EXPECT(a.integerEnforcement() == Number::strong); + BEAST_EXPECT(a.valid()); + try + { + a = Number{1, 30}; + BEAST_EXPECT(false); + } + catch (std::overflow_error const& e) + { + BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s); + // The throw is done _after_ the number is updated. + BEAST_EXPECT((a == Number{1, 30})); + } + BEAST_EXPECT(!a.valid()); + a = -100; + BEAST_EXPECT(a.integerEnforcement() == Number::strong); + BEAST_EXPECT(a.valid()); + } + { + Number a{INITIAL_XRP.drops(), Number::weak}; + BEAST_EXPECT(!a.valid()); + a = -a; + BEAST_EXPECT(!a.valid()); + + try + { + a.setIntegerEnforcement(Number::strong); + BEAST_EXPECT(false); + } + catch (std::overflow_error const& e) + { + BEAST_EXPECT( + e.what() == + "Number::setIntegerEnforcement integer overflow"s); + // The throw is internal to the operator before the result is + // assigned to the Number + BEAST_EXPECT(a == -INITIAL_XRP); + BEAST_EXPECT(!a.valid()); + } + try + { + ++a; + BEAST_EXPECT(false); + } + catch (std::overflow_error const& e) + { + BEAST_EXPECT(e.what() == "Number::addition integer overflow"s); + // The throw is internal to the operator before the result is + // assigned to the Number + BEAST_EXPECT(a == -INITIAL_XRP); + BEAST_EXPECT(!a.valid()); + } + a = Number::maxIntValue; + try + { + ++a; + BEAST_EXPECT(false); + } + catch (std::overflow_error const& e) + { + BEAST_EXPECT(e.what() == "Number::addition integer overflow"s); + // This time, the throw is done _after_ the number is updated. + BEAST_EXPECT(a == Number::maxIntValue + 1); + BEAST_EXPECT(!a.valid()); + } + a = -Number::maxIntValue; + try + { + --a; + BEAST_EXPECT(false); + } + catch (std::overflow_error const& e) + { + BEAST_EXPECT(e.what() == "Number::addition integer overflow"s); + // This time, the throw is done _after_ the number is updated. + BEAST_EXPECT(a == -Number::maxIntValue - 1); + BEAST_EXPECT(!a.valid()); + } + a = Number(1, 10); + try + { + a *= Number(1, 10); + BEAST_EXPECT(false); + } + catch (std::overflow_error const& e) + { + BEAST_EXPECT( + e.what() == "Number::multiplication integer overflow"s); + // The throw is done _after_ the number is updated. + BEAST_EXPECT((a == Number{1, 20})); + BEAST_EXPECT(!a.valid()); + } + try + { + a = Number::maxIntValue * 2; + BEAST_EXPECT(false); + } + catch (std::overflow_error const& e) + { + BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s); + // The throw is done _after_ the number is updated. + BEAST_EXPECT((a == Number{2, 14})); + BEAST_EXPECT(!a.valid()); + } + try + { + a = Number(3, 15, Number::strong); + BEAST_EXPECT(false); + } + catch (std::overflow_error const& e) + { + BEAST_EXPECT(e.what() == "Number::Number integer overflow"s); + // The Number doesn't get updated because the ctor throws + BEAST_EXPECT((a == Number{2, 14})); + BEAST_EXPECT(!a.valid()); + } + a = Number(1, 10); + try + { + a /= Number(1, -10); + BEAST_EXPECT(false); + } + catch (std::overflow_error const& e) + { + BEAST_EXPECT(e.what() == "Number::division integer overflow"s); + // The throw is done _after_ the number is updated. + BEAST_EXPECT((a == Number{1, 20})); + BEAST_EXPECT(!a.valid()); + } + a /= Number(1, 15); + BEAST_EXPECT((a == Number{1, 5})); + BEAST_EXPECT(a.valid()); + } + } + void run() override { @@ -746,6 +913,7 @@ public: test_inc_dec(); test_toSTAmount(); test_truncate(); + testInteger(); } }; From 93d99a671cd83aa0e0ac2b4bcf0fa2a32fab9797 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 4 Nov 2025 21:02:39 -0500 Subject: [PATCH 4/7] Make all STNumber fields "soeDEFAULT" --- include/xrpl/protocol/detail/ledger_entries.macro | 6 +++--- src/test/app/Vault_test.cpp | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/xrpl/protocol/detail/ledger_entries.macro b/include/xrpl/protocol/detail/ledger_entries.macro index 53110f09f5..5aae9a9322 100644 --- a/include/xrpl/protocol/detail/ledger_entries.macro +++ b/include/xrpl/protocol/detail/ledger_entries.macro @@ -479,10 +479,10 @@ LEDGER_ENTRY(ltVAULT, 0x0084, Vault, vault, ({ {sfAccount, soeREQUIRED}, {sfData, soeOPTIONAL}, {sfAsset, soeREQUIRED}, - {sfAssetsTotal, soeREQUIRED}, - {sfAssetsAvailable, soeREQUIRED}, + {sfAssetsTotal, soeDEFAULT}, + {sfAssetsAvailable, soeDEFAULT}, {sfAssetsMaximum, soeDEFAULT}, - {sfLossUnrealized, soeREQUIRED}, + {sfLossUnrealized, soeDEFAULT}, {sfShareMPTID, soeREQUIRED}, {sfWithdrawalPolicy, soeREQUIRED}, {sfScale, soeDEFAULT}, diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index ccbf0fed42..58b929f1a2 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -4438,7 +4438,8 @@ class Vault_test : public beast::unit_test::suite BEAST_EXPECT(checkString(vault, sfAssetsAvailable, "50")); BEAST_EXPECT(checkString(vault, sfAssetsMaximum, "1000")); BEAST_EXPECT(checkString(vault, sfAssetsTotal, "50")); - BEAST_EXPECT(checkString(vault, sfLossUnrealized, "0")); + // Since this field is default, it is not returned. + BEAST_EXPECT(!vault.isMember(sfLossUnrealized.getJsonName())); auto const strShareID = strHex(sle->at(sfShareMPTID)); BEAST_EXPECT(checkString(vault, sfShareMPTID, strShareID)); From 16609ccaad52a140d4b2ce8a127243bc3ed206fe Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 4 Nov 2025 21:07:01 -0500 Subject: [PATCH 5/7] Add integer enforcement when converting to XRP/MPTAmount to Number --- include/xrpl/protocol/MPTAmount.h | 2 +- include/xrpl/protocol/XRPAmount.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/xrpl/protocol/MPTAmount.h b/include/xrpl/protocol/MPTAmount.h index af14786501..1c7c7a25e7 100644 --- a/include/xrpl/protocol/MPTAmount.h +++ b/include/xrpl/protocol/MPTAmount.h @@ -64,7 +64,7 @@ public: operator Number() const noexcept { - return value(); + return {value(), Number::strong}; } /** Return the sign of the amount */ diff --git a/include/xrpl/protocol/XRPAmount.h b/include/xrpl/protocol/XRPAmount.h index e102a3707f..f26bb7366f 100644 --- a/include/xrpl/protocol/XRPAmount.h +++ b/include/xrpl/protocol/XRPAmount.h @@ -143,7 +143,7 @@ public: operator Number() const noexcept { - return drops(); + return {drops(), Number::weak}; } /** Return the sign of the amount */ From 82f68496b8832a941394d60d24737de6cb5ea697 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 5 Nov 2025 09:21:12 -0500 Subject: [PATCH 6/7] Fix build error - avoid copy --- src/test/app/AMM_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index 5a1816ebae..66bceec327 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -1384,7 +1384,7 @@ private: // equal asset deposit: unit test to exercise the rounding-down of // LPTokens in the AMMHelpers.cpp: adjustLPTokens calculations // The LPTokens need to have 16 significant digits and a fractional part - for (Number const deltaLPTokens : + for (Number const& deltaLPTokens : {Number{UINT64_C(100000'0000000009), -10}, Number{UINT64_C(100000'0000000001), -10}}) { From bd196c7609ec49efe2ae9ef9a4b0517c00ec5060 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 5 Nov 2025 18:15:49 -0500 Subject: [PATCH 7/7] Catch up the consequences of Number changes - Change the Number::maxIntValue to all 9's. - Add integral() to Asset (copied from Lending) - Add toNumber() functions to STAmount, MPTAmount, XRPAmount to allow explicit conversions with enforcement options. - Add optional Number::EnforceInteger options to STAmount and STNumber ctors, conversions, etc. IOUs are never checked. - Update Vault transactors, and helper functions, to check restrictions. - Fix and add Vault tests. --- include/xrpl/basics/Number.h | 3 +- include/xrpl/protocol/Asset.h | 6 ++ include/xrpl/protocol/MPTAmount.h | 8 ++- include/xrpl/protocol/STAmount.h | 67 ++++++++++++++++++- include/xrpl/protocol/STNumber.h | 12 ++++ include/xrpl/protocol/SystemParameters.h | 1 + include/xrpl/protocol/XRPAmount.h | 6 ++ src/libxrpl/ledger/View.cpp | 23 +++++-- src/libxrpl/protocol/STAmount.cpp | 19 ++++++ src/libxrpl/protocol/STNumber.cpp | 18 ++++++ src/test/app/Vault_test.cpp | 78 +++++++++++++++++++++-- src/test/basics/Number_test.cpp | 4 +- src/xrpld/app/tx/detail/VaultClawback.cpp | 19 +++++- src/xrpld/app/tx/detail/VaultDeposit.cpp | 23 +++++-- src/xrpld/app/tx/detail/VaultWithdraw.cpp | 11 +++- 15 files changed, 274 insertions(+), 24 deletions(-) diff --git a/include/xrpl/basics/Number.h b/include/xrpl/basics/Number.h index 59f3a212a6..4f447d5f98 100644 --- a/include/xrpl/basics/Number.h +++ b/include/xrpl/basics/Number.h @@ -53,7 +53,8 @@ public: constexpr static rep maxMantissa = minMantissa * 10 - 1; static_assert(maxMantissa == 9'999'999'999'999'999LL); - constexpr static rep maxIntValue = minMantissa / 10; + constexpr static rep maxIntValue = maxMantissa / 10; + static_assert(maxIntValue == 999'999'999'999'999LL); // The range for the exponent when normalized constexpr static int minExponent = -32768; diff --git a/include/xrpl/protocol/Asset.h b/include/xrpl/protocol/Asset.h index d0efe814a9..6765f7cf7d 100644 --- a/include/xrpl/protocol/Asset.h +++ b/include/xrpl/protocol/Asset.h @@ -84,6 +84,12 @@ public: return holds() && get().native(); } + bool + integral() const + { + return !holds() || get().native(); + } + friend constexpr bool operator==(Asset const& lhs, Asset const& rhs); diff --git a/include/xrpl/protocol/MPTAmount.h b/include/xrpl/protocol/MPTAmount.h index 1c7c7a25e7..5f552d839b 100644 --- a/include/xrpl/protocol/MPTAmount.h +++ b/include/xrpl/protocol/MPTAmount.h @@ -62,11 +62,17 @@ public: explicit constexpr operator bool() const noexcept; - operator Number() const noexcept + operator Number() const { return {value(), Number::strong}; } + Number + toNumber(Number::EnforceInteger enforce) const + { + return {value(), enforce}; + } + /** Return the sign of the amount */ constexpr int signum() const noexcept; diff --git a/include/xrpl/protocol/STAmount.h b/include/xrpl/protocol/STAmount.h index 83493efcdd..d0092fdbec 100644 --- a/include/xrpl/protocol/STAmount.h +++ b/include/xrpl/protocol/STAmount.h @@ -40,6 +40,12 @@ private: exponent_type mOffset; bool mIsNegative; + // The Enforce integer setting is not stored or serialized. If set, it is + // used during automatic conversions to Number. If not set, the default + // behavior is used. It can also be overridden when coverting by using + // toNumber(). + std::optional enforceConversion_; + public: using value_type = STAmount; @@ -135,9 +141,28 @@ public: STAmount(A const& asset, int mantissa, int exponent = 0); template - STAmount(A const& asset, Number const& number) + STAmount( + A const& asset, + Number const& number, + std::optional enforce = std::nullopt) : STAmount(asset, number.mantissa(), number.exponent()) { + enforceConversion_ = enforce; + if (!enforce) + { + // Use the default conversion behavior + [[maybe_unused]] + Number const n = *this; + } + else if (enforce == Number::strong) + { + // Throw if it's not valid + if (!validNumber()) + { + Throw( + "STAmount::STAmount integer Number lost precision"); + } + } } // Legacy support for new-style amounts @@ -145,6 +170,17 @@ public: STAmount(XRPAmount const& amount); STAmount(MPTAmount const& amount, MPTIssue const& mptIssue); operator Number() const; + Number + toNumber(Number::EnforceInteger enforce) const; + + void + setIntegerEnforcement(std::optional enforce); + + std::optional + integerEnforcement() const noexcept; + + bool + validNumber() const noexcept; //-------------------------------------------------------------------------- // @@ -155,6 +191,9 @@ public: int exponent() const noexcept; + bool + integral() const noexcept; + bool native() const noexcept; @@ -435,6 +474,12 @@ STAmount::exponent() const noexcept return mOffset; } +inline bool +STAmount::integral() const noexcept +{ + return mAsset.integral(); +} + inline bool STAmount::native() const noexcept { @@ -510,6 +555,8 @@ inline STAmount::operator bool() const noexcept inline STAmount::operator Number() const { + if (enforceConversion_) + return toNumber(*enforceConversion_); if (native()) return xrp(); if (mAsset.holds()) @@ -517,6 +564,17 @@ inline STAmount::operator Number() const return iou(); } +inline Number +STAmount::toNumber(Number::EnforceInteger enforce) const +{ + if (native()) + return xrp().toNumber(enforce); + if (mAsset.holds()) + return mpt().toNumber(enforce); + // It doesn't make sense to enforce limits on IOUs + return iou(); +} + inline STAmount& STAmount::operator=(beast::Zero) { @@ -538,6 +596,11 @@ STAmount::operator=(Number const& number) mValue = mIsNegative ? -number.mantissa() : number.mantissa(); mOffset = number.exponent(); canonicalize(); + + // Convert it back to a Number to check that it's valid + [[maybe_unused]] + Number n = *this; + return *this; } @@ -553,7 +616,7 @@ STAmount::clear() { // The -100 is used to allow 0 to sort less than a small positive values // which have a negative exponent. - mOffset = native() ? 0 : -100; + mOffset = integral() ? 0 : -100; mValue = 0; mIsNegative = false; } diff --git a/include/xrpl/protocol/STNumber.h b/include/xrpl/protocol/STNumber.h index 2ec3d66fd1..43b96a2b46 100644 --- a/include/xrpl/protocol/STNumber.h +++ b/include/xrpl/protocol/STNumber.h @@ -56,6 +56,18 @@ public: bool isDefault() const override; + /// Sets the flag on the underlying number + void + setIntegerEnforcement(Number::EnforceInteger enforce); + + /// Gets the flag value on the underlying number + Number::EnforceInteger + integerEnforcement() const noexcept; + + /// Checks the underlying number + bool + valid() const noexcept; + operator Number() const { return value_; diff --git a/include/xrpl/protocol/SystemParameters.h b/include/xrpl/protocol/SystemParameters.h index de78b65265..7a2c5a7f63 100644 --- a/include/xrpl/protocol/SystemParameters.h +++ b/include/xrpl/protocol/SystemParameters.h @@ -23,6 +23,7 @@ systemName() /** Number of drops in the genesis account. */ constexpr XRPAmount INITIAL_XRP{100'000'000'000 * DROPS_PER_XRP}; +static_assert(INITIAL_XRP.drops() == 100'000'000'000'000'000); /** Returns true if the amount does not exceed the initial XRP in existence. */ inline bool diff --git a/include/xrpl/protocol/XRPAmount.h b/include/xrpl/protocol/XRPAmount.h index f26bb7366f..159174accc 100644 --- a/include/xrpl/protocol/XRPAmount.h +++ b/include/xrpl/protocol/XRPAmount.h @@ -146,6 +146,12 @@ public: return {drops(), Number::weak}; } + Number + toNumber(Number::EnforceInteger enforce) const + { + return {value(), enforce}; + } + /** Return the sign of the amount */ constexpr int signum() const noexcept diff --git a/src/libxrpl/ledger/View.cpp b/src/libxrpl/ledger/View.cpp index 0175b099ea..92fd5ccc97 100644 --- a/src/libxrpl/ledger/View.cpp +++ b/src/libxrpl/ledger/View.cpp @@ -2878,13 +2878,17 @@ assetsToSharesDeposit( Number const assetTotal = vault->at(sfAssetsTotal); STAmount shares{vault->at(sfShareMPTID)}; + shares.setIntegerEnforcement(Number::weak); if (assetTotal == 0) return STAmount{ shares.asset(), Number(assets.mantissa(), assets.exponent() + vault->at(sfScale)) - .truncate()}; + .truncate(), + Number::weak}; - Number const shareTotal = issuance->at(sfOutstandingAmount); + Number const shareTotal{ + unsafe_cast(issuance->at(sfOutstandingAmount)), + Number::strong}; shares = (shareTotal * (assets / assetTotal)).truncate(); return shares; } @@ -2906,6 +2910,7 @@ sharesToAssetsDeposit( Number const assetTotal = vault->at(sfAssetsTotal); STAmount assets{vault->at(sfAsset)}; + assets.setIntegerEnforcement(Number::weak); if (assetTotal == 0) return STAmount{ assets.asset(), @@ -2913,7 +2918,9 @@ sharesToAssetsDeposit( shares.exponent() - vault->at(sfScale), false}; - Number const shareTotal = issuance->at(sfOutstandingAmount); + Number const shareTotal{ + unsafe_cast(issuance->at(sfOutstandingAmount)), + Number::strong}; assets = assetTotal * (shares / shareTotal); return assets; } @@ -2937,9 +2944,12 @@ assetsToSharesWithdraw( Number assetTotal = vault->at(sfAssetsTotal); assetTotal -= vault->at(sfLossUnrealized); STAmount shares{vault->at(sfShareMPTID)}; + shares.setIntegerEnforcement(Number::weak); if (assetTotal == 0) return shares; - Number const shareTotal = issuance->at(sfOutstandingAmount); + Number const shareTotal{ + unsafe_cast(issuance->at(sfOutstandingAmount)), + Number::strong}; Number result = shareTotal * (assets / assetTotal); if (truncate == TruncateShares::yes) result = result.truncate(); @@ -2965,9 +2975,12 @@ sharesToAssetsWithdraw( Number assetTotal = vault->at(sfAssetsTotal); assetTotal -= vault->at(sfLossUnrealized); STAmount assets{vault->at(sfAsset)}; + assets.setIntegerEnforcement(Number::weak); if (assetTotal == 0) return assets; - Number const shareTotal = issuance->at(sfOutstandingAmount); + Number const shareTotal{ + unsafe_cast(issuance->at(sfOutstandingAmount)), + Number::strong}; assets = assetTotal * (shares / shareTotal); return assets; } diff --git a/src/libxrpl/protocol/STAmount.cpp b/src/libxrpl/protocol/STAmount.cpp index 4aac551003..6f6355a145 100644 --- a/src/libxrpl/protocol/STAmount.cpp +++ b/src/libxrpl/protocol/STAmount.cpp @@ -255,6 +255,25 @@ STAmount::move(std::size_t n, void* buf) return emplace(n, buf, std::move(*this)); } +void +STAmount::setIntegerEnforcement(std::optional enforce) +{ + enforceConversion_ = enforce; +} + +std::optional +STAmount::integerEnforcement() const noexcept +{ + return enforceConversion_; +} + +bool +STAmount::validNumber() const noexcept +{ + Number n = toNumber(Number::EnforceInteger::weak); + return n.valid(); +} + //------------------------------------------------------------------------------ // // Conversion diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index 889dd9ee68..5486864e95 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/src/libxrpl/protocol/STNumber.cpp @@ -94,6 +94,24 @@ STNumber::isDefault() const return value_ == Number(); } +void +STNumber::setIntegerEnforcement(Number::EnforceInteger enforce) +{ + value_.setIntegerEnforcement(enforce); +} + +Number::EnforceInteger +STNumber::integerEnforcement() const noexcept +{ + return value_.integerEnforcement(); +} + +bool +STNumber::valid() const noexcept +{ + return value_.valid(); +} + std::ostream& operator<<(std::ostream& out, STNumber const& rhs) { diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 58b929f1a2..5b36ffaf8e 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -3597,7 +3597,32 @@ class Vault_test : public beast::unit_test::suite }); testCase(18, [&, this](Env& env, Data d) { - testcase("Scale deposit overflow on second deposit"); + testcase("MPT scale deposit overflow"); + // The computed number of shares can not be represented as an MPT + // without truncation + + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(5)}); + env(tx, ter{tecPRECISION_LOSS}); + env.close(); + } + }); + + testCase(14, [&, this](Env& env, Data d) { + testcase("MPT scale deposit overflow on first deposit"); + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(10)}); + env(tx, ter{tecPRECISION_LOSS}); + env.close(); + }); + + testCase(14, [&, this](Env& env, Data d) { + testcase("MPT scale deposit overflow on second deposit"); { auto tx = d.vault.deposit( @@ -3618,8 +3643,8 @@ class Vault_test : public beast::unit_test::suite } }); - testCase(18, [&, this](Env& env, Data d) { - testcase("Scale deposit overflow on total shares"); + testCase(14, [&, this](Env& env, Data d) { + testcase("No MPT scale deposit overflow on total shares"); { auto tx = d.vault.deposit( @@ -3635,7 +3660,7 @@ class Vault_test : public beast::unit_test::suite {.depositor = d.depositor, .id = d.keylet.key, .amount = d.asset(5)}); - env(tx, ter{tecPATH_DRY}); + env(tx); env.close(); } }); @@ -3919,6 +3944,28 @@ class Vault_test : public beast::unit_test::suite testCase(18, [&, this](Env& env, Data d) { testcase("Scale withdraw overflow"); + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(5)}); + env(tx, ter{tecPRECISION_LOSS}); + env.close(); + } + + { + auto tx = d.vault.withdraw( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = STAmount(d.asset, Number(10, 0))}); + env(tx, ter{tecPRECISION_LOSS}); + env.close(); + } + }); + + testCase(14, [&, this](Env& env, Data d) { + testcase("MPT scale withdraw overflow"); + { auto tx = d.vault.deposit( {.depositor = d.depositor, @@ -4137,6 +4184,29 @@ class Vault_test : public beast::unit_test::suite testCase(18, [&, this](Env& env, Data d) { testcase("Scale clawback overflow"); + { + auto tx = d.vault.deposit( + {.depositor = d.depositor, + .id = d.keylet.key, + .amount = d.asset(5)}); + env(tx, ter(tecPRECISION_LOSS)); + env.close(); + } + + { + auto tx = d.vault.clawback( + {.issuer = d.issuer, + .id = d.keylet.key, + .holder = d.depositor, + .amount = STAmount(d.asset, Number(10, 0))}); + env(tx, ter{tecPRECISION_LOSS}); + env.close(); + } + }); + + testCase(14, [&, this](Env& env, Data d) { + testcase("MPT Scale clawback overflow"); + { auto tx = d.vault.deposit( {.depositor = d.depositor, diff --git a/src/test/basics/Number_test.cpp b/src/test/basics/Number_test.cpp index 78c9b28952..a5fba8ef7d 100644 --- a/src/test/basics/Number_test.cpp +++ b/src/test/basics/Number_test.cpp @@ -858,7 +858,7 @@ public: { BEAST_EXPECT(e.what() == "Number::operator= integer overflow"s); // The throw is done _after_ the number is updated. - BEAST_EXPECT((a == Number{2, 14})); + BEAST_EXPECT((a == Number::maxIntValue * 2)); BEAST_EXPECT(!a.valid()); } try @@ -870,7 +870,7 @@ public: { BEAST_EXPECT(e.what() == "Number::Number integer overflow"s); // The Number doesn't get updated because the ctor throws - BEAST_EXPECT((a == Number{2, 14})); + BEAST_EXPECT((a == Number::maxIntValue * 2)); BEAST_EXPECT(!a.valid()); } a = Number(1, 10); diff --git a/src/xrpld/app/tx/detail/VaultClawback.cpp b/src/xrpld/app/tx/detail/VaultClawback.cpp index 7c56ca1d60..1c73d36bb0 100644 --- a/src/xrpld/app/tx/detail/VaultClawback.cpp +++ b/src/xrpld/app/tx/detail/VaultClawback.cpp @@ -71,9 +71,13 @@ VaultClawback::preclaim(PreclaimContext const& ctx) } Asset const vaultAsset = vault->at(sfAsset); - if (auto const amount = ctx.tx[~sfAmount]; - amount && vaultAsset != amount->asset()) - return tecWRONG_ASSET; + if (auto const amount = ctx.tx[~sfAmount]) + { + if (vaultAsset != amount->asset()) + return tecWRONG_ASSET; + else if (!amount->validNumber()) + return tecPRECISION_LOSS; + } if (vaultAsset.native()) { @@ -157,6 +161,8 @@ VaultClawback::doApply() MPTIssue const share{mptIssuanceID}; STAmount sharesDestroyed = {share}; STAmount assetsRecovered; + assetsRecovered.setIntegerEnforcement(Number::weak); + sharesDestroyed.setIntegerEnforcement(Number::weak); try { if (amount == beast::zero) @@ -169,6 +175,9 @@ VaultClawback::doApply() AuthHandling::ahIGNORE_AUTH, j_); + if (!sharesDestroyed.validNumber()) + return tecPRECISION_LOSS; + auto const maybeAssets = sharesToAssetsWithdraw(vault, sleIssuance, sharesDestroyed); if (!maybeAssets) @@ -184,6 +193,8 @@ VaultClawback::doApply() if (!maybeShares) return tecINTERNAL; // LCOV_EXCL_LINE sharesDestroyed = *maybeShares; + if (!sharesDestroyed.validNumber()) + return tecPRECISION_LOSS; } auto const maybeAssets = @@ -192,6 +203,8 @@ VaultClawback::doApply() return tecINTERNAL; // LCOV_EXCL_LINE assetsRecovered = *maybeAssets; } + if (!assetsRecovered.validNumber()) + return tecPRECISION_LOSS; // Clamp to maximum. if (assetsRecovered > *assetsAvailable) diff --git a/src/xrpld/app/tx/detail/VaultDeposit.cpp b/src/xrpld/app/tx/detail/VaultDeposit.cpp index 3e5ae741e3..66e144312f 100644 --- a/src/xrpld/app/tx/detail/VaultDeposit.cpp +++ b/src/xrpld/app/tx/detail/VaultDeposit.cpp @@ -42,6 +42,9 @@ VaultDeposit::preclaim(PreclaimContext const& ctx) if (assets.asset() != vaultAsset) return tecWRONG_ASSET; + if (!assets.validNumber()) + return tecPRECISION_LOSS; + if (vaultAsset.native()) ; // No special checks for XRP else if (vaultAsset.holds()) @@ -217,6 +220,7 @@ VaultDeposit::doApply() } STAmount sharesCreated = {vault->at(sfShareMPTID)}, assetsDeposited; + sharesCreated.setIntegerEnforcement(Number::weak); try { // Compute exchange before transferring any amounts. @@ -227,14 +231,14 @@ VaultDeposit::doApply() return tecINTERNAL; // LCOV_EXCL_LINE sharesCreated = *maybeShares; } - if (sharesCreated == beast::zero) + if (sharesCreated == beast::zero || !sharesCreated.validNumber()) return tecPRECISION_LOSS; auto const maybeAssets = sharesToAssetsDeposit(vault, sleIssuance, sharesCreated); if (!maybeAssets) return tecINTERNAL; // LCOV_EXCL_LINE - else if (*maybeAssets > amount) + else if (*maybeAssets > amount || !maybeAssets->validNumber()) { // LCOV_EXCL_START JLOG(j_.error()) << "VaultDeposit: would take more than offered."; @@ -260,13 +264,22 @@ VaultDeposit::doApply() sharesCreated.asset() != assetsDeposited.asset(), "ripple::VaultDeposit::doApply : assets are not shares"); - vault->at(sfAssetsTotal) += assetsDeposited; - vault->at(sfAssetsAvailable) += assetsDeposited; + auto assetsTotalProxy = vault->at(sfAssetsTotal); + auto assetsAvailableProxy = vault->at(sfAssetsAvailable); + if (vault->at(sfAsset).value().integral()) + { + assetsTotalProxy.value().setIntegerEnforcement(Number::weak); + assetsAvailableProxy.value().setIntegerEnforcement(Number::weak); + } + assetsTotalProxy += assetsDeposited; + assetsAvailableProxy += assetsDeposited; + if (!assetsTotalProxy->valid() || !assetsAvailableProxy->valid()) + return tecLIMIT_EXCEEDED; view().update(vault); // A deposit must not push the vault over its limit. auto const maximum = *vault->at(sfAssetsMaximum); - if (maximum != 0 && *vault->at(sfAssetsTotal) > maximum) + if (maximum != 0 && *assetsTotalProxy > maximum) return tecLIMIT_EXCEEDED; // Transfer assets from depositor to vault. diff --git a/src/xrpld/app/tx/detail/VaultWithdraw.cpp b/src/xrpld/app/tx/detail/VaultWithdraw.cpp index 8cd3f5cd97..8806f7b236 100644 --- a/src/xrpld/app/tx/detail/VaultWithdraw.cpp +++ b/src/xrpld/app/tx/detail/VaultWithdraw.cpp @@ -50,6 +50,9 @@ VaultWithdraw::preclaim(PreclaimContext const& ctx) if (assets.asset() != vaultAsset && assets.asset() != vaultShare) return tecWRONG_ASSET; + if (!assets.validNumber()) + return tecPRECISION_LOSS; + if (vaultAsset.native()) ; // No special checks for XRP else if (vaultAsset.holds()) @@ -154,6 +157,8 @@ VaultWithdraw::doApply() MPTIssue const share{mptIssuanceID}; STAmount sharesRedeemed = {share}; STAmount assetsWithdrawn; + assetsWithdrawn.setIntegerEnforcement(Number::weak); + sharesRedeemed.setIntegerEnforcement(Number::weak); try { if (amount.asset() == vaultAsset) @@ -167,13 +172,15 @@ VaultWithdraw::doApply() sharesRedeemed = *maybeShares; } - if (sharesRedeemed == beast::zero) + if (sharesRedeemed == beast::zero || !sharesRedeemed.validNumber()) return tecPRECISION_LOSS; auto const maybeAssets = sharesToAssetsWithdraw(vault, sleIssuance, sharesRedeemed); if (!maybeAssets) return tecINTERNAL; // LCOV_EXCL_LINE assetsWithdrawn = *maybeAssets; + if (!assetsWithdrawn.validNumber()) + return tecPRECISION_LOSS; } else if (amount.asset() == share) { @@ -184,6 +191,8 @@ VaultWithdraw::doApply() if (!maybeAssets) return tecINTERNAL; // LCOV_EXCL_LINE assetsWithdrawn = *maybeAssets; + if (!assetsWithdrawn.validNumber()) + return tecPRECISION_LOSS; } else return tefINTERNAL; // LCOV_EXCL_LINE