diff --git a/include/xrpl/protocol/Asset.h b/include/xrpl/protocol/Asset.h index 173b4e9f75c..04e161a0da0 100644 --- a/include/xrpl/protocol/Asset.h +++ b/include/xrpl/protocol/Asset.h @@ -27,6 +27,15 @@ namespace ripple { +template + requires( + std::is_same_v || std::is_same_v || + std::is_same_v) +struct AmountType +{ + using amount_type = T; +}; + /* Asset is an abstraction of three different issue types: XRP, IOU, MPT. * For historical reasons, two issue types XRP and IOU are wrapped in Issue * type. Many functions and classes there were first written for Issue @@ -92,14 +101,14 @@ class Asset return holds() && get().native(); } - friend constexpr bool - operator==(Asset const& lhs, Asset const& rhs); - - friend constexpr bool - operator!=(Asset const& lhs, Asset const& rhs); + std::variant< + AmountType, + AmountType, + AmountType> + getAmountType() const; friend constexpr bool - operator<(Asset const& lhs, Asset const& rhs); + operator==(Asset const& lhs, Asset const& rhs); friend constexpr std::weak_ordering operator<=>(Asset const& lhs, Asset const& rhs); @@ -160,27 +169,6 @@ operator==(Asset const& lhs, Asset const& rhs) rhs.issue_); } -constexpr bool -operator!=(Asset const& lhs, Asset const& rhs) -{ - return !(lhs == rhs); -} - -constexpr bool -operator<(Asset const& lhs, Asset const& rhs) -{ - return std::visit( - [&]( - TLhs const& issLhs, TRhs const& issRhs) { - if constexpr (std::is_same_v) - return issLhs < issRhs; - else - return false; - }, - lhs.issue_, - rhs.issue_); -} - constexpr bool operator==(Currency const& lhs, Asset const& rhs) { diff --git a/include/xrpl/protocol/MPTAmount.h b/include/xrpl/protocol/MPTAmount.h index 0371fba0719..761e52da99b 100644 --- a/include/xrpl/protocol/MPTAmount.h +++ b/include/xrpl/protocol/MPTAmount.h @@ -94,9 +94,6 @@ class MPTAmount : private boost::totally_ordered, constexpr int signum() const noexcept; - Json::Value - jsonClipped() const; - /** Returns the underlying value. Code SHOULD NOT call this function unless the type has been abstracted away, e.g. in a templated function. diff --git a/include/xrpl/protocol/MPTIssue.h b/include/xrpl/protocol/MPTIssue.h index 6b20d6d586b..13f03283398 100644 --- a/include/xrpl/protocol/MPTIssue.h +++ b/include/xrpl/protocol/MPTIssue.h @@ -53,7 +53,7 @@ class MPTIssue void setJson(Json::Value& jv) const; - auto + constexpr std::weak_ordering operator<=>(MPTIssue const&) const = default; bool @@ -61,31 +61,8 @@ class MPTIssue { return false; } - - friend constexpr std::weak_ordering - operator<=>(MPTIssue const& lhs, MPTIssue const& rhs); }; -constexpr bool -operator==(MPTIssue const& lhs, MPTIssue const& rhs) -{ - return lhs.mptID_ == rhs.mptID_; -} - -constexpr bool -operator!=(MPTIssue const& lhs, MPTIssue const& rhs) -{ - return !(lhs == rhs); -} - -constexpr std::weak_ordering -operator<=>(MPTIssue const& lhs, MPTIssue const& rhs) -{ - if (auto const c{lhs.mptID_ <=> rhs.mptID_}; c != 0) - return c; - return lhs.mptID_ <=> rhs.mptID_; -} - /** MPT is a non-native token. */ inline bool diff --git a/include/xrpl/protocol/PathAsset.h b/include/xrpl/protocol/PathAsset.h index 9a148ad2b17..79b1a687136 100644 --- a/include/xrpl/protocol/PathAsset.h +++ b/include/xrpl/protocol/PathAsset.h @@ -25,6 +25,8 @@ namespace ripple { +/* Represent STPathElement's asset, which can be Currency or MPTID. + */ class PathAsset { private: @@ -32,6 +34,7 @@ class PathAsset public: PathAsset() = default; + // Enables comparing Asset and PathAsset PathAsset(Asset const& asset); PathAsset(Currency const& currency) : easset_(currency) { @@ -54,12 +57,6 @@ class PathAsset constexpr std::variant const& value() const; - static PathAsset - toPathAsset(Asset const& asset); - - static std::optional - toPathAsset(std::optional const& asset); - friend constexpr bool operator==(PathAsset const& lhs, PathAsset const& rhs); }; @@ -101,7 +98,9 @@ PathAsset::value() const constexpr bool PathAsset::isXRP() const { - return holds() && get() == xrpCurrency(); + return std::visit( + [&](A const& a) { return ripple::isXRP(a); }, + easset_); } constexpr bool @@ -139,12 +138,6 @@ to_string(PathAsset const& asset); std::ostream& operator<<(std::ostream& os, PathAsset const& x); -bool -equalAssets(PathAsset const& asset1, Asset const& asset2); - -bool -equalAssets(Asset const& asset1, PathAsset const& asset2); - } // namespace ripple #endif // RIPPLE_APP_PATHASSET_H_INCLUDED diff --git a/include/xrpl/protocol/STPathSet.h b/include/xrpl/protocol/STPathSet.h index a9cc9c889aa..9e7afdeee96 100644 --- a/include/xrpl/protocol/STPathSet.h +++ b/include/xrpl/protocol/STPathSet.h @@ -44,9 +44,6 @@ class STPathElement final : public CountedObject std::size_t hash_value_; public: - struct PathAssetTag - { - }; enum Type { typeNone = 0x00, typeAccount = @@ -65,34 +62,16 @@ class STPathElement final : public CountedObject STPathElement& operator=(STPathElement const&) = default; - STPathElement( - std::optional const& account, - std::optional const& asset, - std::optional const& issuer); - STPathElement( std::optional const& account, std::optional const& asset, - std::optional const& issuer, - PathAssetTag); - - STPathElement( - AccountID const& account, - Asset const& asset, - AccountID const& issuer, - bool forceCurrency = false); + std::optional const& issuer); STPathElement( AccountID const& account, PathAsset const& asset, AccountID const& issuer, - bool forceCurrency = false); - - STPathElement( - unsigned int uType, - AccountID const& account, - Asset const& asset, - AccountID const& issuer); + bool forceAsset = false); STPathElement( unsigned int uType, @@ -174,12 +153,6 @@ class STPath final : public CountedObject void emplace_back(Args&&... args); - bool - hasSeen( - AccountID const& account, - Asset const& asset, - AccountID const& issuer) const; - bool hasSeen( AccountID const& account, @@ -285,23 +258,10 @@ inline STPathElement::STPathElement() : mType(typeNone), is_offer_(true) hash_value_ = get_hash(*this); } -inline STPathElement::STPathElement( - std::optional const& account, - std::optional const& asset, - std::optional const& issuer) - : STPathElement( - account, - PathAsset::toPathAsset(asset), - issuer, - PathAssetTag{}) -{ -} - inline STPathElement::STPathElement( std::optional const& account, std::optional const& asset, - std::optional const& issuer, - PathAssetTag) + std::optional const& issuer) : mType(typeNone) { if (!account) @@ -336,24 +296,11 @@ inline STPathElement::STPathElement( hash_value_ = get_hash(*this); } -inline STPathElement::STPathElement( - AccountID const& account, - Asset const& asset, - AccountID const& issuer, - bool forceCurrency) - : STPathElement( - account, - PathAsset::toPathAsset(asset), - issuer, - forceCurrency) -{ -} - inline STPathElement::STPathElement( AccountID const& account, PathAsset const& asset, AccountID const& issuer, - bool forceCurrency) + bool forceAsset) : mType(typeNone) , mAccountID(account) , mAssetID(asset) @@ -363,28 +310,15 @@ inline STPathElement::STPathElement( if (!is_offer_) mType |= typeAccount; - if (!asset.holds() && - (forceCurrency || !isXRP(mAssetID.get()))) - mType |= typeCurrency; + if (forceAsset || !isXRP(mAssetID)) + mType |= asset.holds() ? typeMPT : typeCurrency; if (!isXRP(issuer)) mType |= typeIssuer; - if (asset.holds()) - mType |= typeMPT; - hash_value_ = get_hash(*this); } -inline STPathElement::STPathElement( - unsigned int uType, - AccountID const& account, - Asset const& asset, - AccountID const& issuer) - : STPathElement(uType, account, PathAsset::toPathAsset(asset), issuer) -{ -} - inline STPathElement::STPathElement( unsigned int uType, AccountID const& account, @@ -396,6 +330,8 @@ inline STPathElement::STPathElement( , mIssuerID(issuer) , is_offer_(isXRP(mAccountID)) { + // uType could be assetType; i.e. either Currency or MPTID. + // Get the actual type. if (!asset.holds()) mType = mType & (~Type::typeMPT); else if (mAssetID.holds() && isXRP(mAssetID.get())) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index aa1100f8f01..9bf9ac683f5 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -236,8 +236,8 @@ TRANSACTION(ttCLAWBACK, 30, Clawback, ({ /** This transaction claws back tokens from an AMM pool. */ TRANSACTION(ttAMM_CLAWBACK, 31, AMMClawback, ({ {sfHolder, soeREQUIRED}, - {sfAsset, soeREQUIRED}, - {sfAsset2, soeREQUIRED}, + {sfAsset, soeREQUIRED, soeMPTSupported}, + {sfAsset2, soeREQUIRED, soeMPTSupported}, {sfAmount, soeOPTIONAL, soeMPTSupported}, })) @@ -250,8 +250,8 @@ TRANSACTION(ttAMM_CREATE, 35, AMMCreate, ({ /** This transaction type deposits into an AMM instance */ TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, ({ - {sfAsset, soeREQUIRED}, - {sfAsset2, soeREQUIRED}, + {sfAsset, soeREQUIRED, soeMPTSupported}, + {sfAsset2, soeREQUIRED, soeMPTSupported}, {sfAmount, soeOPTIONAL, soeMPTSupported}, {sfAmount2, soeOPTIONAL, soeMPTSupported}, {sfEPrice, soeOPTIONAL}, @@ -261,8 +261,8 @@ TRANSACTION(ttAMM_DEPOSIT, 36, AMMDeposit, ({ /** This transaction type withdraws from an AMM instance */ TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, ({ - {sfAsset, soeREQUIRED}, - {sfAsset2, soeREQUIRED}, + {sfAsset, soeREQUIRED, soeMPTSupported}, + {sfAsset2, soeREQUIRED, soeMPTSupported}, {sfAmount, soeOPTIONAL, soeMPTSupported}, {sfAmount2, soeOPTIONAL, soeMPTSupported}, {sfEPrice, soeOPTIONAL}, @@ -271,15 +271,15 @@ TRANSACTION(ttAMM_WITHDRAW, 37, AMMWithdraw, ({ /** This transaction type votes for the trading fee */ TRANSACTION(ttAMM_VOTE, 38, AMMVote, ({ - {sfAsset, soeREQUIRED}, - {sfAsset2, soeREQUIRED}, + {sfAsset, soeREQUIRED, soeMPTSupported}, + {sfAsset2, soeREQUIRED, soeMPTSupported}, {sfTradingFee, soeREQUIRED}, })) /** This transaction type bids for the auction slot */ TRANSACTION(ttAMM_BID, 39, AMMBid, ({ - {sfAsset, soeREQUIRED}, - {sfAsset2, soeREQUIRED}, + {sfAsset, soeREQUIRED, soeMPTSupported}, + {sfAsset2, soeREQUIRED, soeMPTSupported}, {sfBidMin, soeOPTIONAL}, {sfBidMax, soeOPTIONAL}, {sfAuthAccounts, soeOPTIONAL}, @@ -287,8 +287,8 @@ TRANSACTION(ttAMM_BID, 39, AMMBid, ({ /** This transaction type deletes AMM in the empty state */ TRANSACTION(ttAMM_DELETE, 40, AMMDelete, ({ - {sfAsset, soeREQUIRED}, - {sfAsset2, soeREQUIRED}, + {sfAsset, soeREQUIRED, soeMPTSupported}, + {sfAsset2, soeREQUIRED, soeMPTSupported}, })) /** This transactions creates a crosschain sequence number */ diff --git a/src/libxrpl/protocol/Asset.cpp b/src/libxrpl/protocol/Asset.cpp index 4162eb70c26..b152c68e38b 100644 --- a/src/libxrpl/protocol/Asset.cpp +++ b/src/libxrpl/protocol/Asset.cpp @@ -43,6 +43,20 @@ Asset::setJson(Json::Value& jv) const std::visit([&](auto&& issue) { issue.setJson(jv); }, issue_); } +std:: + variant, AmountType, AmountType> + Asset::getAmountType() const +{ + static AmountType xrp; + static AmountType iou; + static AmountType mpt; + if (holds()) + return mpt; + if (native()) + return xrp; + return iou; +} + std::string to_string(Asset const& asset) { diff --git a/src/libxrpl/protocol/MPTAmount.cpp b/src/libxrpl/protocol/MPTAmount.cpp index 1eceef197e1..17bbedea451 100644 --- a/src/libxrpl/protocol/MPTAmount.cpp +++ b/src/libxrpl/protocol/MPTAmount.cpp @@ -59,23 +59,6 @@ MPTAmount::operator<(MPTAmount const& other) const return value_ < other.value_; } -Json::Value -MPTAmount::jsonClipped() const -{ - static_assert( - std::is_signed_v && std::is_integral_v, - "Expected MPTAmount to be a signed integral type"); - - constexpr auto min = std::numeric_limits::min(); - constexpr auto max = std::numeric_limits::max(); - - if (value_ < min) - return min; - if (value_ > max) - return max; - return static_cast(value_); -} - MPTAmount MPTAmount::minPositiveAmount() { diff --git a/src/libxrpl/protocol/MPTIssue.cpp b/src/libxrpl/protocol/MPTIssue.cpp index 14f2f5fe796..d1a7a09e533 100644 --- a/src/libxrpl/protocol/MPTIssue.cpp +++ b/src/libxrpl/protocol/MPTIssue.cpp @@ -18,6 +18,7 @@ //============================================================================== #include +#include #include #include diff --git a/src/libxrpl/protocol/PathAsset.cpp b/src/libxrpl/protocol/PathAsset.cpp index 47966a1a3e5..77aa6cbfd0a 100644 --- a/src/libxrpl/protocol/PathAsset.cpp +++ b/src/libxrpl/protocol/PathAsset.cpp @@ -22,27 +22,6 @@ namespace ripple { -PathAsset -PathAsset::toPathAsset(const Asset& asset) -{ - return std::visit( - [&](TIss const& issue) { - if constexpr (std::is_same_v) - return PathAsset{issue.currency}; - else - return PathAsset{issue.getMptID()}; - }, - asset.value()); -} - -std::optional -PathAsset::toPathAsset(std::optional const& asset) -{ - if (asset) - return toPathAsset(*asset); - return std::nullopt; -} - std::string to_string(PathAsset const& asset) { @@ -57,29 +36,4 @@ operator<<(std::ostream& os, PathAsset const& x) return os; } -bool -equalAssets(PathAsset const& asset1, Asset const& asset2) -{ - return std::visit( - [&]( - TPa const& element, TIss const& issue) { - if constexpr ( - std::is_same_v && std::is_same_v) - return element == issue.currency; - else if constexpr ( - std::is_same_v && std::is_same_v) - return element == issue.getMptID(); - else - return false; - }, - asset1.value(), - asset2.value()); -} - -bool -equalAssets(Asset const& asset1, PathAsset const& asset2) -{ - return equalAssets(asset2, asset1); -} - } // namespace ripple diff --git a/src/libxrpl/protocol/STParsedJSON.cpp b/src/libxrpl/protocol/STParsedJSON.cpp index 3c1bf09dee0..cf800dc908b 100644 --- a/src/libxrpl/protocol/STParsedJSON.cpp +++ b/src/libxrpl/protocol/STParsedJSON.cpp @@ -657,7 +657,7 @@ parseLeaf( Json::Value const& account = pathEl[jss::account]; Json::Value const& asset = pathEl[assetName]; Json::Value const& issuer = pathEl[jss::issuer]; - bool hasCurrency = false; + bool hasAsset = false; AccountID uAccount, uIssuer; PathAsset uAsset; @@ -697,7 +697,7 @@ parseLeaf( return ret; } - hasCurrency = true; + hasAsset = true; if (isMPT) { @@ -757,7 +757,7 @@ parseLeaf( } } - p.emplace_back(uAccount, uAsset, uIssuer, hasCurrency); + p.emplace_back(uAccount, uAsset, uIssuer, hasAsset); } tail.push_back(p); diff --git a/src/libxrpl/protocol/STPathSet.cpp b/src/libxrpl/protocol/STPathSet.cpp index 5be549a2091..cbbb16447e9 100644 --- a/src/libxrpl/protocol/STPathSet.cpp +++ b/src/libxrpl/protocol/STPathSet.cpp @@ -95,7 +95,7 @@ STPathSet::STPathSet(SerialIter& sit, SField const& name) : STBase(name) auto hasMPT = iType & STPathElement::typeMPT; AccountID account; - PathAsset asset{}; + PathAsset asset; AccountID issuer; if (hasAccount) @@ -164,15 +164,6 @@ STPathSet::isDefault() const return value.empty(); } -bool -STPath::hasSeen( - AccountID const& account, - Asset const& asset, - AccountID const& issuer) const -{ - return hasSeen(account, PathAsset::toPathAsset(asset), issuer); -} - bool STPath::hasSeen( AccountID const& account, diff --git a/src/test/app/AMMCalc_test.cpp b/src/test/app/AMMCalc_test.cpp index 058cdfd1d2d..9fb578dcec9 100644 --- a/src/test/app/AMMCalc_test.cpp +++ b/src/test/app/AMMCalc_test.cpp @@ -176,9 +176,8 @@ class AMMCalc_test : public beast::unit_test::suite std::string toString(STAmount const& a) { - std::stringstream str; - str << a.getText() << "/" << to_string(a.issue().currency); - return str.str(); + return std::format( + "{}/{}", a.getText(), to_string(a.get().currency)); } STAmount @@ -203,8 +202,8 @@ class AMMCalc_test : public beast::unit_test::suite STAmount sin{}; int limitingStep = vp.size(); STAmount limitStepOut{}; - auto trate = [&](auto const& amt) { - auto const currency = to_string(amt.issue().currency); + auto trate = [&](STAmount const& amt) { + auto const currency = to_string(amt.get().currency); return rates.find(currency) != rates.end() ? rates.at(currency) : QUALITY_ONE; }; @@ -268,8 +267,8 @@ class AMMCalc_test : public beast::unit_test::suite STAmount sout{}; int limitingStep = 0; STAmount limitStepIn{}; - auto trate = [&](auto const& amt) { - auto const currency = to_string(amt.issue().currency); + auto trate = [&](STAmount const& amt) { + auto const currency = to_string(amt.get().currency); return rates.find(currency) != rates.end() ? rates.at(currency) : QUALITY_ONE; }; diff --git a/src/test/app/Check_test.cpp b/src/test/app/Check_test.cpp index 2c4f44ce79f..621c20edb15 100644 --- a/src/test/app/Check_test.cpp +++ b/src/test/app/Check_test.cpp @@ -2146,8 +2146,7 @@ class Check_test : public beast::unit_test::suite return; BEAST_EXPECT( - offerAmount.issue().account == - checkAmount.issue().account); + offerAmount.getIssuer() == checkAmount.getIssuer()); BEAST_EXPECT( offerAmount.negative() == checkAmount.negative()); BEAST_EXPECT( diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index c004c0097f2..376020f7628 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -1544,9 +1544,6 @@ class MPToken_test : public beast::unit_test::suite jrr = env.rpc("json", "sign", to_string(jv1)); BEAST_EXPECT(jrr[jss::result][jss::error] == "invalidParams"); }; - auto toSFieldRef = [](SField const& field) { - return std::ref(field); - }; auto setMPTFields = [&](SField const& field, Json::Value& jv, bool withAmount = true) { @@ -3130,16 +3127,28 @@ class MPToken_test : public beast::unit_test::suite auto const USD = gw["USD"]; Env env(*this, features); fund(env, gw, {alice}, XRP(1'000), {USD(1'000)}); - MPTTester mpt(env, gw, {.holders = {alice}, .fund = false}); - mpt.create( - {.ownerCount = 1, - .holderCount = 0, - .flags = tfMPTCanTransfer | tfMPTCanTrade}); + MPTTester mpt(env, gw, {.fund = false}); + mpt.create({.flags = tfMPTCanTransfer | tfMPTCanTrade}); auto const MPT = mpt["MPT"]; + AMM amm(env, gw, MPT(100), XRP(100)); + amm.deposit(DepositArg{.account = alice, .asset1In = XRP(10)}); + amm::ammClawback( + gw, alice, MPTIssue(mpt.issuanceID()), xrpIssue(), MPT(10)); + } + + { + Account const gw{"gw"}; + Account const alice{"alice"}; + auto const USD = gw["USD"]; + Env env(*this, features); + fund(env, gw, {alice}, XRP(1'000), {USD(1'000)}); + MPTTester mpt(env, gw, {.fund = false}); + mpt.create({.flags = tfMPTCanTransfer | tfMPTCanTrade}); mpt.authorize({.account = alice}); mpt.pay(gw, alice, 1'000); + auto const MPT = mpt["MPT"]; AMM amm(env, gw, MPT(100), XRP(100)); - amm.deposit(DepositArg{.account = alice, .tokens = 100}); + amm.deposit(DepositArg{.account = alice, .tokens = 10'000}); amm::ammClawback( gw, alice, MPTIssue(mpt.issuanceID()), xrpIssue(), MPT(10)); } diff --git a/src/test/app/PayStrand_test.cpp b/src/test/app/PayStrand_test.cpp index 1a3bad1aa04..8f1f646eb5f 100644 --- a/src/test/app/PayStrand_test.cpp +++ b/src/test/app/PayStrand_test.cpp @@ -1006,11 +1006,7 @@ struct PayStrand_test : public beast::unit_test::suite // alice -> USD/XRP -> bob STPath path; - path.emplace_back( - std::nullopt, - xrpCurrency(), - std::nullopt, - STPathElement::PathAssetTag{}); + path.emplace_back(std::nullopt, xrpCurrency(), std::nullopt); auto [ter, strand] = toStrand( *env.current(), diff --git a/src/test/app/Taker_test.cpp b/src/test/app/Taker_test.cpp index 89e44b2b98b..b294e9f827b 100644 --- a/src/test/app/Taker_test.cpp +++ b/src/test/app/Taker_test.cpp @@ -180,10 +180,13 @@ class Taker_test : public beast::unit_test::suite std::string format_amount(STAmount const& amount) { - std::string txt = amount.getText(); - txt += "/"; - txt += to_string(amount.issue().currency); - return txt; + if (amount.holds()) + return std::format( + "{}/{}", + amount.getText(), + to_string(amount.get().currency)); + return std::format( + "{}/{}", amount.getText(), to_string(amount.get())); } void diff --git a/src/test/app/TheoreticalQuality_test.cpp b/src/test/app/TheoreticalQuality_test.cpp index eaa6eb1a67f..917d23377bf 100644 --- a/src/test/app/TheoreticalQuality_test.cpp +++ b/src/test/app/TheoreticalQuality_test.cpp @@ -71,8 +71,7 @@ struct RippleCalcTestParams *parseBase58( pe[jss::account].asString()), std::nullopt, - std::nullopt, - STPathElement::PathAssetTag{}); + std::nullopt); } else if ( pe.isMember(jss::currency) && pe.isMember(jss::issuer)) @@ -86,11 +85,7 @@ struct RippleCalcTestParams else assert(isXRP(*parseBase58( pe[jss::issuer].asString()))); - p.emplace_back( - std::nullopt, - currency, - issuer, - STPathElement::PathAssetTag{}); + p.emplace_back(std::nullopt, currency, issuer); } else { diff --git a/src/test/jtx/impl/AMMTest.cpp b/src/test/jtx/impl/AMMTest.cpp index 575e2e1d889..fb7753c6ad4 100644 --- a/src/test/jtx/impl/AMMTest.cpp +++ b/src/test/jtx/impl/AMMTest.cpp @@ -63,7 +63,7 @@ fund( for (auto const& amt : amts) { env.trust(amt + amt, account); - env(pay(amt.issue().account, account, amt)); + env(pay(amt.getIssuer(), account, amt)); } } env.close(); diff --git a/src/test/jtx/impl/TestHelpers.cpp b/src/test/jtx/impl/TestHelpers.cpp index a04d957e304..8f08d37acbe 100644 --- a/src/test/jtx/impl/TestHelpers.cpp +++ b/src/test/jtx/impl/TestHelpers.cpp @@ -64,11 +64,7 @@ ownerCount(Env const& env, Account const& account) void stpath_append_one(STPath& st, Account const& account) { - st.push_back(STPathElement( - {account.id(), - std::nullopt, - std::nullopt, - STPathElement::PathAssetTag{}})); + st.push_back(STPathElement({account.id(), std::nullopt, std::nullopt})); } void @@ -80,7 +76,7 @@ stpath_append_one(STPath& st, STPathElement const& pe) bool equal(STAmount const& sa1, STAmount const& sa2) { - return sa1 == sa2 && sa1.issue().account == sa2.issue().account; + return sa1 == sa2 && sa1.getIssuer() == sa2.getIssuer(); } // Issue path element @@ -289,7 +285,7 @@ expectLine( } auto amount = sle->getFieldAmount(sfBalance); - amount.setIssuer(value.issue().account); + amount.setIssuer(value.getIssuer()); if (!accountLow) amount.negate(); return amount == value && expectDefaultTrustLine; diff --git a/src/test/jtx/impl/amount.cpp b/src/test/jtx/impl/amount.cpp index ac0a6553ec7..450f4d7b634 100644 --- a/src/test/jtx/impl/amount.cpp +++ b/src/test/jtx/impl/amount.cpp @@ -90,10 +90,16 @@ operator<<(std::ostream& os, PrettyAmount const& amount) os << to_places(d, 6) << " XRP"; } + else if (amount.value().holds()) + { + os << amount.value().getText() << "/" + << to_string(amount.value().get().currency) << "(" + << amount.name() << ")"; + } else { os << amount.value().getText() << "/" - << to_string(amount.value().issue().currency) << "(" << amount.name() + << to_string(amount.value().get()) << "(" << amount.name() << ")"; } return os; diff --git a/src/test/jtx/impl/balance.cpp b/src/test/jtx/impl/balance.cpp index 42330658eb0..afc6acfb18e 100644 --- a/src/test/jtx/impl/balance.cpp +++ b/src/test/jtx/impl/balance.cpp @@ -38,7 +38,7 @@ balance::operator()(Env& env) const env.test.expect(sle->getFieldAmount(sfBalance) == value_); } } - else + else if (value_.holds()) { auto const sle = env.le(keylet::line(account_.id(), value_.issue())); if (none_) @@ -48,12 +48,28 @@ balance::operator()(Env& env) const else if (env.test.expect(sle)) { auto amount = sle->getFieldAmount(sfBalance); - amount.setIssuer(value_.issue().account); - if (account_.id() > value_.issue().account) + amount.setIssuer(value_.getIssuer()); + if (account_.id() > value_.getIssuer()) amount.negate(); env.test.expect(amount == value_); } } + else + { + auto const issuanceKey = + keylet::mptIssuance(value_.get().getMptID()); + auto const mptokenKey = keylet::mptoken(issuanceKey.key, account_); + auto const sle = env.le(mptokenKey); + if (none_) + { + env.test.expect(!sle); + } + else if (env.test.expect(sle)) + { + auto amount = sle->getFieldU64(sfMPTAmount); + env.test.expect(amount == value_.mpt().value()); + } + } } } // namespace jtx diff --git a/src/xrpld/app/misc/AMMHelpers.h b/src/xrpld/app/misc/AMMHelpers.h index d1abf7799ab..dd393bb74fa 100644 --- a/src/xrpld/app/misc/AMMHelpers.h +++ b/src/xrpld/app/misc/AMMHelpers.h @@ -148,11 +148,11 @@ withinRelativeDistance( * @param dist requested relative distance * @return true if within dist, false otherwise */ -// clang-format off template requires( std::is_same_v || std::is_same_v || - std::is_same_v || std::is_same_v) + std::is_same_v || std::is_same_v || + std::is_same_v) bool withinRelativeDistance(Amt const& calc, Amt const& req, Number const& dist) { @@ -161,7 +161,6 @@ withinRelativeDistance(Amt const& calc, Amt const& req, Number const& dist) auto const [min, max] = std::minmax(calc, req); return ((max - min) / max) < dist; } -// clang-format on /** Solve quadratic equation to find takerGets or takerPays. Round * to minimize the amount in order to maximize the quality. diff --git a/src/xrpld/app/misc/detail/MPTUtils.cpp b/src/xrpld/app/misc/detail/MPTUtils.cpp index e57ae6da800..c99a4b925ab 100644 --- a/src/xrpld/app/misc/detail/MPTUtils.cpp +++ b/src/xrpld/app/misc/detail/MPTUtils.cpp @@ -42,7 +42,7 @@ isMPTAllowed( auto const allMPTTx = ammTx || txType == ttOFFER_CREATE || txType == ttCHECK_CREATE || txType == ttCHECK_CASH || txType == ttPAYMENT; - assert(allMPTTx || isDEX); + ASSERT(allMPTTx || isDEX, "ripple::isMPTAllowed : all MPT tx or DEX"); auto const issuanceKey = keylet::mptIssuance(issuanceID); auto const issuanceSle = view.read(issuanceKey); diff --git a/src/xrpld/app/paths/AssetCache.cpp b/src/xrpld/app/paths/AssetCache.cpp index 35e5a73443f..f3c0fb1a224 100644 --- a/src/xrpld/app/paths/AssetCache.cpp +++ b/src/xrpld/app/paths/AssetCache.cpp @@ -77,7 +77,9 @@ AssetCache::getRippleLines(AccountID const& accountID, LineDirection direction) // to be replaced by the full set. The full set will be built // below, and will be returned, if needed, on subsequent calls // for either value of outgoing. - assert(size <= totalLineCount_); + ASSERT( + size <= totalLineCount_, + "ripple::RippleLineCache::getRippleLines : maximum lines"); totalLineCount_ -= size; lines_.erase(otheriter); } @@ -97,7 +99,9 @@ AssetCache::getRippleLines(AccountID const& accountID, LineDirection direction) if (inserted) { - assert(it->second == nullptr); + ASSERT( + it->second == nullptr, + "ripple::RippleLineCache::getRippleLines : null lines"); auto lines = PathFindTrustLine::getItems(accountID, *ledger_, direction); if (lines.size()) @@ -108,7 +112,9 @@ AssetCache::getRippleLines(AccountID const& accountID, LineDirection direction) } } - assert(!it->second || (it->second->size() > 0)); + ASSERT( + !it->second || (it->second->size() > 0), + "ripple::RippleLineCache::getRippleLines : null or nonempty lines"); auto const size = it->second ? it->second->size() : 0; JLOG(journal_.trace()) << "getRippleLines for ledger " << ledger_->info().seq << " found " << size diff --git a/src/xrpld/app/paths/Flow.cpp b/src/xrpld/app/paths/Flow.cpp index 5cae4ab76eb..02472bda247 100644 --- a/src/xrpld/app/paths/Flow.cpp +++ b/src/xrpld/app/paths/Flow.cpp @@ -130,33 +130,22 @@ flow( } } - using Var = - std::variant; - auto getTypedAmt = [&](Asset const& iss) -> Var { - static auto xrp = XRPAmount{}; - static auto mpt = MPTAmount{}; - static auto iou = IOUAmount{}; - if (isXRP(iss)) - return &xrp; - if (iss.holds()) - return &mpt; - return &iou; - }; - // The src account may send either xrp,iou,mpt. The dst account may receive // either xrp,iou,mpt. Since XRP, IOU, and MPT amounts are represented by // different types, use templates to tell `flow` about the amount types. return std::visit( [&, &strands_ = strands]( - TIn const*&&, TOut const*&&) { + TIn const&, TOut const&) { + using TIn_ = typename TIn::amount_type; + using TOut_ = typename TOut::amount_type; return finishFlow( sb, srcAsset, dstAsset, - flow( + flow( sb, strands_, - get(deliver), + get(deliver), partialPayment, offerCrossing, limitQuality, @@ -165,8 +154,8 @@ flow( ammContext, flowDebugInfo)); }, - getTypedAmt(srcAsset), - getTypedAmt(dstAsset)); + srcAsset.getAmountType(), + dstAsset.getAmountType()); } } // namespace ripple diff --git a/src/xrpld/app/paths/PathRequest.cpp b/src/xrpld/app/paths/PathRequest.cpp index 63dbe67c556..6d2c04a29d8 100644 --- a/src/xrpld/app/paths/PathRequest.cpp +++ b/src/xrpld/app/paths/PathRequest.cpp @@ -424,7 +424,7 @@ PathRequest::parseJson(Json::Value const& jvParams) if (saSendMax) { // If the assets don't match, ignore the source asset. - if (equalAssets(srcPathAsset, saSendMax->asset())) + if (srcPathAsset == saSendMax->asset()) { // If neither is the source and they are not equal, then the // source issuer is illegal. @@ -578,12 +578,7 @@ PathRequest::findPaths( << " Trying to find paths: " << STAmount(asset, 1).getFullText(); auto& pathfinder = getPathFinder( - cache, - pathasset_map, - PathAsset::toPathAsset(asset), - dst_amount, - level, - continueCallback); + cache, pathasset_map, asset, dst_amount, level, continueCallback); if (!pathfinder) { JLOG(m_journal.debug()) << iIdentifier << " No paths found"; diff --git a/src/xrpld/app/paths/Pathfinder.cpp b/src/xrpld/app/paths/Pathfinder.cpp index fd2749ad353..ca3d81c65f5 100644 --- a/src/xrpld/app/paths/Pathfinder.cpp +++ b/src/xrpld/app/paths/Pathfinder.cpp @@ -333,7 +333,7 @@ Pathfinder::findPaths( JLOG(j_.debug()) << "non-XRP to XRP payment"; paymentType = pt_nonXRP_to_XRP; } - else if (equalAssets(mSrcPathAsset, mDstAmount.asset())) + else if (mSrcPathAsset == mDstAmount.asset()) { // non-XRP -> non-XRP - Same currency JLOG(j_.debug()) << "non-XRP to non-XRP - same currency"; @@ -729,7 +729,7 @@ Pathfinder::getBestPaths( bool Pathfinder::issueMatchesOrigin(Asset const& asset) { - bool matchingAsset = equalAssets(asset, mSrcPathAsset); + bool matchingAsset = asset == mSrcPathAsset; bool matchingAccount = isXRP(asset) || (mSrcIssuer && asset.getIssuer() == mSrcIssuer) || asset.getIssuer() == mSrcAccount; @@ -1051,8 +1051,7 @@ Pathfinder::addLink( { bool const bRequireAuth( sleEnd->getFieldU32(sfFlags) & lsfRequireAuth); - bool const bIsEndAsset( - equalAssets(uEndPathAsset, mDstAmount.asset())); + bool const bIsEndAsset(uEndPathAsset == mDstAmount.asset()); bool const bIsNoRippleOut(isNoRippleOut(currentPath)); bool const bDestOnly(addFlags & afAC_LAST); diff --git a/src/xrpld/app/paths/detail/AmountSpec.h b/src/xrpld/app/paths/detail/AmountSpec.h index 67d9134ae20..fb167b267bc 100644 --- a/src/xrpld/app/paths/detail/AmountSpec.h +++ b/src/xrpld/app/paths/detail/AmountSpec.h @@ -260,8 +260,8 @@ toAmountSpec(STAmount const& amt) else { result.amount = IOUAmount(sMant, amt.exponent()); - result.issuer = amt.issue().account; - result.currency = amt.issue().currency; + result.issuer = amt.get().account; + result.currency = amt.get().currency; } return result; diff --git a/src/xrpld/app/paths/detail/BookStep.cpp b/src/xrpld/app/paths/detail/BookStep.cpp index 0e77c94acb9..b156d6af793 100644 --- a/src/xrpld/app/paths/detail/BookStep.cpp +++ b/src/xrpld/app/paths/detail/BookStep.cpp @@ -198,6 +198,10 @@ class BookStep : public StepImp> return ostr.str(); } + Rate + rate(ReadView const& view, Asset const& asset, AccountID const& dstAccount) + const; + private: friend bool operator==(BookStep const& lhs, BookStep const& rhs) @@ -339,21 +343,14 @@ class BookPaymentStep : public BookStep> // (the old code does not charge a fee) // Calculate amount that goes to the taker and the amount charged the // offer owner - auto rate = [&](Asset const& asset) { - if (isXRP(asset) || asset.getIssuer() == this->strandDst_) - return parityRate; - if (asset.holds()) - return transferRate(v, asset.getIssuer()); - return transferRate(v, asset.get().getMptID()); - }; - - auto const trIn = - redeems(prevStepDir) ? rate(this->book_.in) : parityRate; + auto const trIn = redeems(prevStepDir) + ? this->rate(v, this->book_.in, this->strandDst_) + : parityRate; // Always charge the transfer fee, even if the owner is the issuer, // unless the fee is waived auto const trOut = (this->ownerPaysTransferFee_ && waiveFee == WaiveTransferFee::No) - ? rate(this->book_.out) + ? this->rate(v, this->book_.out, this->strandDst_) : parityRate; Quality const q1{getRate(STAmount(trOut.value), STAmount(trIn.value))}; @@ -544,14 +541,8 @@ class BookOfferCrossingStep (this->ammLiquidity_ && this->ammLiquidity_->multiPath())) return ofrQ; - auto rate = [&](AccountID const& id) { - if (isXRP(id) || id == this->strandDst_) - return parityRate; - return transferRate(v, id); - }; - auto const trIn = redeems(prevStepDir) - ? rate(this->book_.in.getIssuer()) + ? this->rate(v, this->book_.in, this->strandDst_) : parityRate; // AMM doesn't pay the transfer fee on the out amount auto const trOut = parityRate; @@ -728,19 +719,13 @@ BookStep::forEachOffer( // (the old code does not charge a fee) // Calculate amount that goes to the taker and the amount charged the offer // owner - auto rate = [this, &sb](Asset const& asset) -> std::uint32_t { - if (isXRP(asset) || asset.getIssuer() == this->strandDst_) - return QUALITY_ONE; - if (asset.holds()) - return transferRate(sb, asset.getIssuer()).value; - return transferRate(sb, asset.get().getMptID()).value; - }; - - std::uint32_t const trIn = - redeems(prevStepDir) ? rate(book_.in) : QUALITY_ONE; + std::uint32_t const trIn = redeems(prevStepDir) + ? rate(sb, book_.in, this->strandDst_).value + : QUALITY_ONE; // Always charge the transfer fee, even if the owner is the issuer - std::uint32_t const trOut = - ownerPaysTransferFee_ ? rate(book_.out) : QUALITY_ONE; + std::uint32_t const trOut = ownerPaysTransferFee_ + ? rate(sb, book_.out, this->strandDst_).value + : QUALITY_ONE; typename FlowOfferStream::StepCounter counter( maxOffersToConsume_, j_); @@ -762,8 +747,8 @@ BookStep::forEachOffer( strandSrc_, strandDst_, offer, ofrQ, offers, offerAttempted)) return true; - // Make sure offer owner has authorization to own IOUs from issuer. - // An account can always own XRP or their own IOUs. + // Make sure offer owner has authorization to own Assets from issuer. + // An account can always own XRP or their own Assets. if (requireAuth(afView, offer.assetIn(), offer.owner()) != tesSUCCESS) { // Offer owner not authorized to hold IOU/MPT from issuer. @@ -1386,9 +1371,9 @@ BookStep::check(StrandContext const& ctx) const } else { - auto const mptID = + auto const issuanceID = keylet::mptIssuance(book_.in.get().getMptID()); - if (!view.exists(mptID)) + if (!view.exists(issuanceID)) return tecOBJECT_NOT_FOUND; if (auto const ter = isMPTDEXAllowed( @@ -1405,6 +1390,20 @@ BookStep::check(StrandContext const& ctx) const return tesSUCCESS; } +template +Rate +BookStep::rate( + ReadView const& view, + Asset const& asset, + AccountID const& dstAccount) const +{ + if (isXRP(asset) || asset.getIssuer() == dstAccount) + return parityRate; + if (asset.holds()) + return transferRate(view, asset.getIssuer()); + return transferRate(view, asset.get().getMptID()); +}; + //------------------------------------------------------------------------------ namespace test { @@ -1419,21 +1418,6 @@ equalHelper(Step const& step, ripple::Book const& book) return false; } -static std::variant -getTypedAmt(Asset const& asset) -{ - static auto xrp = XRPAmount{}; - static auto mpt = MPTAmount{}; - static auto iou = IOUAmount{}; - if (asset.holds()) - { - if (isXRP(asset.get().currency)) - return &xrp; - return &iou; - } - return &mpt; -} - bool bookStepEqual(Step const& step, ripple::Book const& book) { @@ -1442,15 +1426,15 @@ bookStepEqual(Step const& step, ripple::Book const& book) UNREACHABLE("ripple::test::bookStepEqual : no XRP to XRP book step"); return false; // no such thing as xrp/xrp book step } - bool ret = false; - std::visit( - [&](TIn*&&, TOut*&&) { - ret = - equalHelper>(step, book); + return std::visit( + [&](TIn const&, TOut const&) { + using TIn_ = typename TIn::amount_type; + using TOut_ = typename TOut::amount_type; + return equalHelper>( + step, book); }, - getTypedAmt(book.in), - getTypedAmt(book.out)); - return ret; + book.in.getAmountType(), + book.out.getAmountType()); } } // namespace test diff --git a/src/xrpld/app/paths/detail/DirectStep.cpp b/src/xrpld/app/paths/detail/DirectStep.cpp index a9504d1cd48..8fdd550efec 100644 --- a/src/xrpld/app/paths/detail/DirectStep.cpp +++ b/src/xrpld/app/paths/detail/DirectStep.cpp @@ -939,8 +939,7 @@ DirectStepI::check(StrandContext const& ctx) const // issue if (auto book = ctx.prevStep->bookStepBook()) { - if (book->out.holds() && - book->out.get() != srcIssue) + if (book->out.get() != srcIssue) return temBAD_PATH_LOOP; } } diff --git a/src/xrpld/app/paths/detail/MPTEndpointStep.cpp b/src/xrpld/app/paths/detail/MPTEndpointStep.cpp index 7b6b1fad1aa..178560bb1d0 100644 --- a/src/xrpld/app/paths/detail/MPTEndpointStep.cpp +++ b/src/xrpld/app/paths/detail/MPTEndpointStep.cpp @@ -22,8 +22,8 @@ #include #include #include -#include #include +#include #include #include diff --git a/src/xrpld/app/paths/detail/PaySteps.cpp b/src/xrpld/app/paths/detail/PaySteps.cpp index 925f714e68b..6c12514ab12 100644 --- a/src/xrpld/app/paths/detail/PaySteps.cpp +++ b/src/xrpld/app/paths/detail/PaySteps.cpp @@ -57,18 +57,6 @@ checkNear(IOUAmount const& expected, IOUAmount const& actual) return r <= ratTol; }; -bool -checkNear(XRPAmount const& expected, XRPAmount const& actual) -{ - return expected == actual; -}; - -bool -checkNear(MPTAmount const& expected, MPTAmount const& actual) -{ - return expected == actual; -}; - static bool isXRPAccount(STPathElement const& pe) { @@ -228,11 +216,7 @@ toStrand( if (hasAccount && (pe.getAccountID() == noAccount())) return {temBAD_PATH, Strand{}}; - if (hasMPT && (hasCurrency || hasAccount)) - return {temBAD_PATH, Strand{}}; - - if (hasMPT && hasIssuer && - (pe.getIssuerID() != getMPTIssuer(pe.getMPTID()))) + if (hasMPT && (hasCurrency || hasAccount || hasIssuer)) return {temBAD_PATH, Strand{}}; } @@ -242,6 +226,7 @@ toStrand( return xrpIssue(); if (asset.holds()) return asset; + // First step ripples from the source to the issuer. return Issue{asset.get().currency, src}; }(); @@ -255,8 +240,9 @@ toStrand( // sendmax and deliver. normPath.reserve(4 + path.size()); { - // Implied step: sender of the transaction and either sendmax or deliver - // asset + // The first step of a path is always implied to be the sender of the + // transaction, as defined by the transaction's Account field. The Asset + // is either SendMax or Deliver. auto const t = [&]() { auto const t = STPathElement::typeAccount | STPathElement::typeIssuer; @@ -266,9 +252,10 @@ toStrand( }(); normPath.emplace_back(t, src, curAsset, curAsset.getIssuer()); - // If transaction includes sendmax with the issuer, which is not - // the sender then the issuer is the second implied step, unless - // the path starts at address, which is the issuer of sendmax + // If transaction includes SendMax with the issuer, which is not + // the sender of the transaction, that issuer is implied to be + // the second step of the path. Unless the path starts at an address, + // which is the issuer of SendMax. if (sendMaxAsset && sendMaxAsset->getIssuer() != src && (path.empty() || !path[0].isAccount() || path[0].getAccountID() != sendMaxAsset->getIssuer())) @@ -294,22 +281,23 @@ toStrand( } } + // If the Amount field of the transaction includes an issuer that is not + // the same as the Destination of the transaction, that issuer is + // implied to be the second-to-last step of the path. if (!((normPath.back().isAccount() && normPath.back().getAccountID() == deliver.getIssuer()) || (dst == deliver.getIssuer()))) { normPath.emplace_back( - deliver.getIssuer(), - std::nullopt, - std::nullopt, - STPathElement::PathAssetTag{}); + deliver.getIssuer(), std::nullopt, std::nullopt); } + // Last step of a path is always implied to be the receiver of a + // transaction, as defined by the transaction's Destination field. if (!normPath.back().isAccount() || normPath.back().getAccountID() != dst) { - normPath.emplace_back( - dst, std::nullopt, std::nullopt, STPathElement::PathAssetTag{}); + normPath.emplace_back(dst, std::nullopt, std::nullopt); } } @@ -545,11 +533,7 @@ toStrand( curAsset.get().currency != deliver.get().currency) || (curAsset.holds() && curAsset.get() != deliver.get())) - { - std::cout << to_string(curAsset) << std::endl; - std::cout << to_string(deliver) << std::endl; return false; - } if (curAsset.getIssuer() != deliver.getIssuer() && curAsset.getIssuer() != dst) return false; diff --git a/src/xrpld/app/paths/detail/Steps.h b/src/xrpld/app/paths/detail/Steps.h index 5f700ce8e8d..ac5b15270a6 100644 --- a/src/xrpld/app/paths/detail/Steps.h +++ b/src/xrpld/app/paths/detail/Steps.h @@ -31,8 +31,6 @@ #include #include -extern bool gf; - namespace ripple { class PaymentSandbox; @@ -519,10 +517,13 @@ class FlowException : public std::runtime_error // Check equal with tolerance bool checkNear(IOUAmount const& expected, IOUAmount const& actual); +template + requires(std::is_same_v || std::is_same_v) bool -checkNear(XRPAmount const& expected, XRPAmount const& actual); -bool -checkNear(MPTAmount const& expected, MPTAmount const& actual); +checkNear(T const& expected, T const& actual) +{ + return expected == actual; +} /// @endcond /** diff --git a/src/xrpld/app/paths/detail/StrandFlow.h b/src/xrpld/app/paths/detail/StrandFlow.h index ccd188b2894..1720dbcc9c4 100644 --- a/src/xrpld/app/paths/detail/StrandFlow.h +++ b/src/xrpld/app/paths/detail/StrandFlow.h @@ -367,15 +367,6 @@ qualityUpperBound(ReadView const& v, Strand const& strand) * increases quality of AMM steps, increasing the strand's composite * quality as the result. */ -inline MPTAmount -limitOut( - ReadView const& v, - Strand const& strand, - MPTAmount const& remainingOut, - Quality const& limitQuality) -{ - return remainingOut; -} template inline TOutAmt limitOut( diff --git a/src/xrpld/app/tx/detail/AMMBid.cpp b/src/xrpld/app/tx/detail/AMMBid.cpp index bbec5d5ffac..856b2dbb4c5 100644 --- a/src/xrpld/app/tx/detail/AMMBid.cpp +++ b/src/xrpld/app/tx/detail/AMMBid.cpp @@ -51,8 +51,7 @@ AMMBid::preflight(PreflightContext const& ctx) return temINVALID_FLAG; } - if (auto const res = invalidAMMAssetPair( - ctx.tx[sfAsset].get(), ctx.tx[sfAsset2].get())) + if (auto const res = invalidAMMAssetPair(ctx.tx[sfAsset], ctx.tx[sfAsset2])) { JLOG(ctx.j.debug()) << "AMM Bid: Invalid asset pair."; return res; diff --git a/src/xrpld/app/tx/detail/AMMClawback.cpp b/src/xrpld/app/tx/detail/AMMClawback.cpp index 2fede9ff85c..bba09a80040 100644 --- a/src/xrpld/app/tx/detail/AMMClawback.cpp +++ b/src/xrpld/app/tx/detail/AMMClawback.cpp @@ -40,8 +40,12 @@ AMMClawback::preflight(PreflightContext const& ctx) return temDISABLED; std::optional const clawAmount = ctx.tx[~sfAmount]; - if (!ctx.rules.enabled(featureMPTokensV2) && clawAmount && - clawAmount->holds()) + auto const asset = ctx.tx[sfAsset]; + auto const asset2 = ctx.tx[sfAsset2]; + + if (!ctx.rules.enabled(featureMPTokensV2) && + ((clawAmount && clawAmount->holds()) || + asset.holds() || asset2.holds())) return temDISABLED; if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) @@ -61,9 +65,6 @@ AMMClawback::preflight(PreflightContext const& ctx) return temMALFORMED; } - auto const asset = ctx.tx[sfAsset].get(); - auto const asset2 = ctx.tx[sfAsset2].get(); - if (isXRP(asset)) return temMALFORMED; @@ -82,7 +83,7 @@ AMMClawback::preflight(PreflightContext const& ctx) return temMALFORMED; } - if (clawAmount && clawAmount->get() != asset) + if (clawAmount && clawAmount->issue() != asset) { JLOG(ctx.j.trace()) << "AMMClawback: Amount's issuer/currency subfield " "does not match Asset field"; @@ -98,8 +99,8 @@ AMMClawback::preflight(PreflightContext const& ctx) TER AMMClawback::preclaim(PreclaimContext const& ctx) { - auto const asset = ctx.tx[sfAsset].get(); - auto const asset2 = ctx.tx[sfAsset2].get(); + auto const asset = ctx.tx[sfAsset]; + auto const asset2 = ctx.tx[sfAsset2]; auto const sleIssuer = ctx.view.read(keylet::account(ctx.tx[sfAccount])); if (!sleIssuer) return terNO_ACCOUNT; // LCOV_EXCL_LINE diff --git a/src/xrpld/app/tx/detail/AMMCreate.cpp b/src/xrpld/app/tx/detail/AMMCreate.cpp index 0fc167e70cd..1b37836cc7d 100644 --- a/src/xrpld/app/tx/detail/AMMCreate.cpp +++ b/src/xrpld/app/tx/detail/AMMCreate.cpp @@ -345,8 +345,14 @@ applyCreate( // Authorize MPT if (amount.holds()) { - auto const mptokenKey = - keylet::mptoken(amount.get().getMptID(), *ammAccount); + auto const& mptIssue = amount.get(); + if (auto const err = requireAuth( + ctx_.view(), mptIssue, account_, MPTAuthType::WeakAuth); + err != tesSUCCESS) + return err; + + auto const& mptID = mptIssue.getMptID(); + auto const mptokenKey = keylet::mptoken(mptID, *ammAccount); auto const ownerNode = sb.dirInsert( keylet::ownerDir(*ammAccount), @@ -358,7 +364,7 @@ applyCreate( auto mptoken = std::make_shared(mptokenKey); (*mptoken)[sfAccount] = *ammAccount; - (*mptoken)[sfMPTokenIssuanceID] = amount.get().getMptID(); + (*mptoken)[sfMPTokenIssuanceID] = mptID; (*mptoken)[sfFlags] = 0; (*mptoken)[sfOwnerNode] = *ownerNode; sb.insert(mptoken); diff --git a/src/xrpld/app/tx/detail/AMMDelete.cpp b/src/xrpld/app/tx/detail/AMMDelete.cpp index e25cfcc494d..4592bbedf2c 100644 --- a/src/xrpld/app/tx/detail/AMMDelete.cpp +++ b/src/xrpld/app/tx/detail/AMMDelete.cpp @@ -77,8 +77,8 @@ AMMDelete::doApply() // as we go on processing transactions. Sandbox sb(&ctx_.view()); - auto const ter = deleteAMMAccount( - sb, ctx_.tx[sfAsset].get(), ctx_.tx[sfAsset2].get(), j_); + auto const ter = + deleteAMMAccount(sb, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_); if (ter == tesSUCCESS || ter == tecINCOMPLETE) sb.apply(ctx_.rawView()); diff --git a/src/xrpld/app/tx/detail/AMMDeposit.cpp b/src/xrpld/app/tx/detail/AMMDeposit.cpp index 752680301c8..4e543396d66 100644 --- a/src/xrpld/app/tx/detail/AMMDeposit.cpp +++ b/src/xrpld/app/tx/detail/AMMDeposit.cpp @@ -108,8 +108,8 @@ AMMDeposit::preflight(PreflightContext const& ctx) return temMALFORMED; } - auto const asset = ctx.tx[sfAsset].get(); - auto const asset2 = ctx.tx[sfAsset2].get(); + auto const asset = ctx.tx[sfAsset]; + auto const asset2 = ctx.tx[sfAsset2]; if (auto const res = invalidAMMAssetPair(asset, asset2)) { JLOG(ctx.j.debug()) << "AMM Deposit: invalid asset pair."; @@ -259,7 +259,8 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) // Check if either of the assets is frozen, AMMDeposit is not allowed // if either asset is frozen auto checkAsset = [&](Asset const& asset) -> TER { - if (auto const ter = requireAuth(ctx.view, asset, accountID)) + if (auto const ter = requireAuth( + ctx.view, asset, accountID, MPTAuthType::WeakAuth)) { JLOG(ctx.j.debug()) << "AMM Deposit: account is not authorized, " << asset; @@ -278,10 +279,10 @@ AMMDeposit::preclaim(PreclaimContext const& ctx) return tesSUCCESS; }; - if (auto const ter = checkAsset(ctx.tx[sfAsset].get())) + if (auto const ter = checkAsset(ctx.tx[sfAsset])) return ter; - if (auto const ter = checkAsset(ctx.tx[sfAsset2].get())) + if (auto const ter = checkAsset(ctx.tx[sfAsset2])) return ter; } diff --git a/src/xrpld/app/tx/detail/AMMVote.cpp b/src/xrpld/app/tx/detail/AMMVote.cpp index e20fc10620c..e9c8b15be43 100644 --- a/src/xrpld/app/tx/detail/AMMVote.cpp +++ b/src/xrpld/app/tx/detail/AMMVote.cpp @@ -43,8 +43,7 @@ AMMVote::preflight(PreflightContext const& ctx) if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; - if (auto const res = invalidAMMAssetPair( - ctx.tx[sfAsset].get(), ctx.tx[sfAsset2].get())) + if (auto const res = invalidAMMAssetPair(ctx.tx[sfAsset], ctx.tx[sfAsset2])) { JLOG(ctx.j.debug()) << "AMM Vote: invalid asset pair."; return res; diff --git a/src/xrpld/app/tx/detail/AMMWithdraw.cpp b/src/xrpld/app/tx/detail/AMMWithdraw.cpp index 03bbcaa44c5..25c58825748 100644 --- a/src/xrpld/app/tx/detail/AMMWithdraw.cpp +++ b/src/xrpld/app/tx/detail/AMMWithdraw.cpp @@ -109,8 +109,8 @@ AMMWithdraw::preflight(PreflightContext const& ctx) return temMALFORMED; } - auto const asset = ctx.tx[sfAsset].get(); - auto const asset2 = ctx.tx[sfAsset2].get(); + auto const asset = ctx.tx[sfAsset]; + auto const asset2 = ctx.tx[sfAsset2]; if (auto const res = invalidAMMAssetPair(asset, asset2)) { JLOG(ctx.j.debug()) << "AMM Withdraw: Invalid asset pair."; @@ -226,8 +226,11 @@ AMMWithdraw::preclaim(PreclaimContext const& ctx) << *amount; return tecAMM_BALANCE; } - if (auto const ter = - requireAuth(ctx.view, amount->asset(), accountID)) + if (auto const ter = requireAuth( + ctx.view, + amount->asset(), + accountID, + MPTAuthType::WeakAuth)) { JLOG(ctx.j.debug()) << "AMM Withdraw: account is not authorized, " @@ -438,12 +441,7 @@ AMMWithdraw::applyGuts(Sandbox& sb) return {result, false}; auto const res = deleteAMMAccountIfEmpty( - sb, - ammSle, - newLPTokenBalance, - ctx_.tx[sfAsset].get(), - ctx_.tx[sfAsset2].get(), - j_); + sb, ammSle, newLPTokenBalance, ctx_.tx[sfAsset], ctx_.tx[sfAsset2], j_); // LCOV_EXCL_START if (!res.second) return {res.first, false}; @@ -644,6 +642,61 @@ AMMWithdraw::withdraw( if (auto const err = sufficientReserve(amountWithdrawActual.issue())) return {err, STAmount{}, STAmount{}, STAmount{}}; + // Create MPToken if doesn't exist + // TODO make a library, AMMCreate, AMMAuthorize use almost identical code + auto createMPToken = [&](Asset const& asset) -> TER { + if (asset.holds()) + { + auto const& mptIssue = asset.get(); + auto const issuanceKey = keylet::mptIssuance(mptIssue.getMptID()); + auto const mptokenKey = keylet::mptoken(issuanceKey.key, account); + if (!view.exists(mptokenKey)) + { + if (auto err = requireAuth( + view, mptIssue, account, MPTAuthType::WeakAuth); + err != tesSUCCESS) + return err; + } + + auto const sleAcct = view.peek(keylet::account(account)); + if (!sleAcct) + return tefINTERNAL; + + std::uint32_t const uOwnerCount = + sleAcct->getFieldU32(sfOwnerCount); + XRPAmount const reserveCreate( + (uOwnerCount < 2) + ? XRPAmount(beast::zero) + : view.fees().accountReserve(uOwnerCount + 1)); + + if (priorBalance < reserveCreate) + return tecINSUFFICIENT_RESERVE; + + auto const ownerNode = view.dirInsert( + keylet::ownerDir(account), + mptokenKey, + describeOwnerDir(account)); + + if (!ownerNode) + return tecDIR_FULL; + + auto mptoken = std::make_shared(mptokenKey); + (*mptoken)[sfAccount] = account; + (*mptoken)[sfMPTokenIssuanceID] = mptIssue.getMptID(); + (*mptoken)[sfFlags] = 0; + (*mptoken)[sfOwnerNode] = *ownerNode; + view.insert(mptoken); + + // Update owner count. + adjustOwnerCount(view, sleAcct, 1, journal); + } + return tesSUCCESS; + }; + + if (auto const res = createMPToken(amountWithdrawActual.asset()); + res != tesSUCCESS) + return {res, STAmount{}, STAmount{}, STAmount{}}; + // Withdraw amountWithdraw auto res = accountSend( view, @@ -668,6 +721,10 @@ AMMWithdraw::withdraw( err != tesSUCCESS) return {err, STAmount{}, STAmount{}, STAmount{}}; + if (auto const res = createMPToken(amountWithdrawActual.asset()); + res != tesSUCCESS) + return {res, STAmount{}, STAmount{}, STAmount{}}; + res = accountSend( view, ammAccount, diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp index 8a5139f8c8d..798a5bfd8ea 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.cpp +++ b/src/xrpld/app/tx/detail/CreateOffer.cpp @@ -354,11 +354,7 @@ CreateOffer::flowCross( if (!takerAmount.in.native() && !takerAmount.out.native()) { STPath path; - path.emplace_back( - std::nullopt, - xrpCurrency(), - std::nullopt, - STPathElement::PathAssetTag{}); + path.emplace_back(std::nullopt, xrpCurrency(), std::nullopt); paths.emplace_back(std::move(path)); } // Special handling for the tfSell flag. diff --git a/src/xrpld/app/tx/detail/Taker.cpp b/src/xrpld/app/tx/detail/Taker.cpp index 6e4b7f5cd57..533dced71be 100644 --- a/src/xrpld/app/tx/detail/Taker.cpp +++ b/src/xrpld/app/tx/detail/Taker.cpp @@ -26,10 +26,11 @@ namespace ripple { static std::string format_amount(STAmount const& amount) { - std::string txt = amount.getText(); - txt += "/"; - txt += to_string(amount.issue().currency); - return txt; + if (amount.holds()) + return std::format( + "{}/{}", amount.getText(), to_string(amount.get().currency)); + return std::format( + "{}/{}", amount.getText(), to_string(amount.get())); } BasicTaker::BasicTaker( diff --git a/src/xrpld/ledger/View.h b/src/xrpld/ledger/View.h index dc453397ace..4d0fc7798b5 100644 --- a/src/xrpld/ledger/View.h +++ b/src/xrpld/ledger/View.h @@ -542,25 +542,41 @@ transferXRP( STAmount const& amount, beast::Journal j); +/* Check if MPToken exists: + * - StrongAuth - before checking lsfMPTRequireAuth is set + * - WeakAuth - after checking if lsfMPTRequireAuth is set + */ +enum class MPTAuthType : bool { StrongAuth = true, WeakAuth = false }; + /** Check if the account lacks required authorization. * Return tecNO_AUTH or tecNO_LINE if it does * and tesSUCCESS otherwise. */ [[nodiscard]] TER requireAuth(ReadView const& view, Issue const& issue, AccountID const& account); +/* If StrongAuth then return tecNO_AUTH if MPToken doesn't exist or + * lsfMPTRequireAuth is set and MPToken is not authorized. If WeakAuth then + * return tecNO_AUTH if lsfMPTRequireAuth is set and MPToken doesn't exist or is + * not authorized. + */ [[nodiscard]] TER requireAuth( ReadView const& view, MPTIssue const& mptIssue, - AccountID const& account); + AccountID const& account, + MPTAuthType authType = MPTAuthType::StrongAuth); [[nodiscard]] TER inline requireAuth( ReadView const& view, Asset const& asset, - AccountID const& account) + AccountID const& account, + MPTAuthType authType = MPTAuthType::StrongAuth) { return std::visit( [&](TIss const& issue_) { - return requireAuth(view, issue_, account); + if constexpr (std::is_same_v) + return requireAuth(view, issue_, account); + else + return requireAuth(view, issue_, account, authType); }, asset.value()); } diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp index e224ffbe380..e968540e209 100644 --- a/src/xrpld/ledger/detail/View.cpp +++ b/src/xrpld/ledger/detail/View.cpp @@ -1867,7 +1867,8 @@ TER requireAuth( ReadView const& view, MPTIssue const& mptIssue, - AccountID const& account) + AccountID const& account, + MPTAuthType authType) { auto const mptID = keylet::mptIssuance(mptIssue.getMptID()); auto const sleIssuance = view.read(mptID); @@ -1885,12 +1886,12 @@ requireAuth( auto const sleToken = view.read(mptokenID); // if account has no MPToken, fail - if (!sleToken) + if (!sleToken && authType == MPTAuthType::StrongAuth) return tecNO_AUTH; // mptoken must be authorized if issuance enabled requireAuth if (sleIssuance->getFieldU32(sfFlags) & lsfMPTRequireAuth && - !(sleToken->getFlags() & lsfMPTAuthorized)) + (!sleToken || (!(sleToken->getFlags() & lsfMPTAuthorized)))) return tecNO_AUTH; return tesSUCCESS; diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index feba24c9cb2..3e9c1a22d69 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -260,8 +260,8 @@ checkPayment( ledger, app.journal("AssetCache")), srcAddressID, *dstAccountID, - sendMax.issue().currency, - sendMax.issue().account, + sendMax.asset(), + sendMax.getIssuer(), amount, std::nullopt, app); @@ -272,7 +272,7 @@ checkPayment( STPath fullLiquidityPath; STPathSet paths; result = pf.getBestPaths( - 4, fullLiquidityPath, paths, sendMax.issue().account); + 4, fullLiquidityPath, paths, sendMax.getIssuer()); } } diff --git a/src/xrpld/rpc/handlers/AccountLines.cpp b/src/xrpld/rpc/handlers/AccountLines.cpp index e2e6ce19ded..7222788d6cd 100644 --- a/src/xrpld/rpc/handlers/AccountLines.cpp +++ b/src/xrpld/rpc/handlers/AccountLines.cpp @@ -45,7 +45,7 @@ addLine(Json::Value& jsonLines, RPCTrustLine const& line) // Amount reported is negative if other account holds current // account's IOUs. jPeer[jss::balance] = saBalance.getText(); - jPeer[jss::currency] = to_string(saBalance.issue().currency); + jPeer[jss::currency] = to_string(saBalance.get().currency); jPeer[jss::limit] = saLimit.getText(); jPeer[jss::limit_peer] = saLimitPeer.getText(); jPeer[jss::quality_in] = line.getQualityIn().value;