From 76cdc59b10335286e9215288950d26e35b0cbfce Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Fri, 27 Sep 2024 17:29:05 -0400 Subject: [PATCH] Add MPT support to Payment. --- include/xrpl/protocol/AmountConversions.h | 3 +- include/xrpl/protocol/MPTIssue.h | 3 + src/libxrpl/protocol/MPTIssue.cpp | 7 + src/test/app/MPToken_test.cpp | 57 ++- src/test/jtx/impl/mpt.cpp | 11 + src/test/jtx/mpt.h | 5 + src/xrpld/app/paths/Flow.cpp | 12 +- .../app/paths/detail/MPTEndpointStep.cpp | 75 +++- src/xrpld/app/tx/detail/Clawback.cpp | 3 +- src/xrpld/app/tx/detail/CreateOffer.cpp | 2 +- src/xrpld/app/tx/detail/Payment.cpp | 330 ++++-------------- src/xrpld/ledger/View.h | 11 +- src/xrpld/ledger/detail/View.cpp | 17 +- src/xrpld/rpc/detail/TransactionSign.cpp | 4 +- 14 files changed, 211 insertions(+), 329 deletions(-) diff --git a/include/xrpl/protocol/AmountConversions.h b/include/xrpl/protocol/AmountConversions.h index 3ec3073c06c..bdf1b74d418 100644 --- a/include/xrpl/protocol/AmountConversions.h +++ b/include/xrpl/protocol/AmountConversions.h @@ -113,8 +113,7 @@ template <> inline MPTAmount toAmount(STAmount const& amt) { - assert(amt.mantissa() < std::numeric_limits::max()); - assert(amt.holds()); + assert(amt.holds() && amt.mantissa() <= maxMPTokenAmount); bool const isNeg = amt.negative(); std::int64_t const sMant = isNeg ? -std::int64_t(amt.mantissa()) : amt.mantissa(); diff --git a/include/xrpl/protocol/MPTIssue.h b/include/xrpl/protocol/MPTIssue.h index 652846d5532..21300dad326 100644 --- a/include/xrpl/protocol/MPTIssue.h +++ b/include/xrpl/protocol/MPTIssue.h @@ -98,6 +98,9 @@ to_json(MPTIssue const& issue); std::string to_string(MPTIssue const& mpt); +std::ostream& +operator<<(std::ostream& os, MPTIssue const& x); + } // namespace ripple namespace std { diff --git a/src/libxrpl/protocol/MPTIssue.cpp b/src/libxrpl/protocol/MPTIssue.cpp index addd4cd3afe..f7fb9825818 100644 --- a/src/libxrpl/protocol/MPTIssue.cpp +++ b/src/libxrpl/protocol/MPTIssue.cpp @@ -62,4 +62,11 @@ to_string(MPTIssue const& mptIssue) return to_string(mptIssue.getMptID()); } +std::ostream& +operator<<(std::ostream& os, MPTIssue const& x) +{ + os << to_string(x); + return os; +} + } // namespace ripple diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 8b1572801d3..1bd94df7849 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -742,10 +742,10 @@ class MPToken_test : public beast::unit_test::suite mptAlice.pay(alice, bob, 100); // Pay to another holder - mptAlice.pay(bob, carol, 101, tecINSUFFICIENT_FUNDS); + mptAlice.pay(bob, carol, 101, tecPATH_PARTIAL); // Pay to the issuer - mptAlice.pay(bob, alice, 101, tecINSUFFICIENT_FUNDS); + mptAlice.pay(bob, alice, 101, tecPATH_PARTIAL); } // MPT is locked @@ -805,7 +805,11 @@ class MPToken_test : public beast::unit_test::suite mptAlice.pay(alice, bob, 100); // issuer tries to exceed max amount - mptAlice.pay(alice, bob, 1, tecMPT_MAX_AMOUNT_EXCEEDED); + auto const MPTA = mptAlice["MPTA"]; + mptAlice.pay(alice, bob, 1, tecPATH_PARTIAL); + env(pay(alice, bob, MPTA(1)), + txflags(tfPartialPayment), + ter(tecPATH_DRY)); } // Issuer fails trying to send more than the default maximum @@ -823,7 +827,7 @@ class MPToken_test : public beast::unit_test::suite mptAlice.pay(alice, bob, maxMPTokenAmount); // issuer tries to exceed max amount - mptAlice.pay(alice, bob, 1, tecMPT_MAX_AMOUNT_EXCEEDED); + mptAlice.pay(alice, bob, 1, tecPATH_PARTIAL); } // Can't pay negative amount @@ -878,23 +882,24 @@ class MPToken_test : public beast::unit_test::suite // Payment between the holder and the issuer, no transfer fee. mptAlice.pay(alice, bob, 1'000); - // Payment between the holders. The sender doesn't have - // enough funds to cover the transfer fee. - mptAlice.pay(bob, carol, 1'000); + // Payment between the holders. The sender has to include sendmax + // to cover the transfer fee. + auto const MPTA = mptAlice["MPTA"]; + env(pay(bob, carol, MPTA(1'000)), ter(tecPATH_PARTIAL)); + env(pay(bob, carol, MPTA(1'000)), sendmax(MPTA(1'100))); - // Payment between the holders. The sender pays 10% transfer fee. - mptAlice.pay(bob, carol, 100); + // Payment between the holders. The sender has to include sendmax + // to cover the transfer fee. + env(pay(bob, carol, MPTA(100)), ter(tecPATH_PARTIAL)); + env(pay(bob, carol, MPTA(100)), sendmax(MPTA(110))); } // Test that non-issuer cannot send to each other if MPTCanTransfer // isn't set { Env env(*this, features); - Account const alice{"alice"}; - Account const bob{"bob"}; - Account const cindy{"cindy"}; - MPTTester mptAlice(env, alice, {.holders = {&bob, &cindy}}); + MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); // alice creates issuance without MPTCanTransfer mptAlice.create({.ownerCount = 1, .holderCount = 0}); @@ -903,14 +908,14 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize({.account = &bob}); // cindy creates a MPToken - mptAlice.authorize({.account = &cindy}); + mptAlice.authorize({.account = &carol}); // alice pays bob 100 tokens mptAlice.pay(alice, bob, 100); // bob tries to send cindy 10 tokens, but fails because canTransfer // is off - mptAlice.pay(bob, cindy, 10, tecNO_AUTH); + mptAlice.pay(bob, carol, 10, tecNO_AUTH); // bob can send back to alice(issuer) just fine mptAlice.pay(bob, alice, 10); @@ -919,8 +924,6 @@ class MPToken_test : public beast::unit_test::suite // MPT is disabled { Env env{*this, features - featureMPTokensV1}; - Account const alice("alice"); - Account const bob("bob"); env.fund(XRP(1'000), alice); env.fund(XRP(1'000), bob); @@ -932,8 +935,6 @@ class MPToken_test : public beast::unit_test::suite // MPT is disabled, unsigned request { Env env{*this, features - featureMPTokensV1}; - Account const alice("alice"); // issuer - Account const carol("carol"); auto const USD = alice["USD"]; env.fund(XRP(1'000), alice); @@ -951,8 +952,6 @@ class MPToken_test : public beast::unit_test::suite // Invalid combination of send, sendMax, deliverMin { Env env{*this, features}; - Account const alice("alice"); - Account const carol("carol"); MPTTester mptAlice(env, alice, {.holders = {&carol}}); @@ -960,21 +959,22 @@ class MPToken_test : public beast::unit_test::suite mptAlice.authorize({.account = &carol}); + env.disableFeature(featureMPTokensV2); + // sendMax and DeliverMin are valid XRP amount, // but is invalid combination with MPT amount - env(pay(alice, carol, mptAlice.mpt(100)), + auto const MPTA = mptAlice["MPTA"]; + env(pay(alice, carol, MPTA(100)), sendmax(XRP(100)), ter(temMALFORMED)); - env(pay(alice, carol, mptAlice.mpt(100)), + env(pay(alice, carol, MPTA(100)), delivermin(XRP(100)), ter(temMALFORMED)); } // build_path is invalid if MPT { - Env env{*this, features}; - Account const alice("alice"); - Account const carol("carol"); + Env env{*this, features - featureMPTokensV2}; MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); @@ -1007,7 +1007,7 @@ class MPToken_test : public beast::unit_test::suite // alice destroys issuance mptAlice.destroy({.ownerCount = 0}); - // alice tries to send bob fund after issuance is destroy, should + // alice tries to send bob fund after issuance is destroyed, should // fail. mptAlice.pay(alice, bob, 100, tecMPT_ISSUANCE_NOT_FOUND); } @@ -1033,9 +1033,6 @@ class MPToken_test : public beast::unit_test::suite // be able to transfer the max amount to someone else { Env env{*this, features}; - Account const alice("alice"); - Account const carol("bob"); - Account const bob("carol"); MPTTester mptAlice(env, alice, {.holders = {&bob, &carol}}); diff --git a/src/test/jtx/impl/mpt.cpp b/src/test/jtx/impl/mpt.cpp index 7cd60afed67..df08018ea76 100644 --- a/src/test/jtx/impl/mpt.cpp +++ b/src/test/jtx/impl/mpt.cpp @@ -80,6 +80,17 @@ MPTTester::MPTTester(Env& env, Account const& issuer, MPTConstr const& arg) } } +MPTTester::operator MPTIssue() const +{ + return issuanceID(); +} + +MPT +MPTTester::operator[](std::string const& name) const +{ + return MPT(name, issuanceID()); +} + void MPTTester::create(const MPTCreate& arg) { diff --git a/src/test/jtx/mpt.h b/src/test/jtx/mpt.h index e66433260eb..9dd0502399a 100644 --- a/src/test/jtx/mpt.h +++ b/src/test/jtx/mpt.h @@ -152,6 +152,11 @@ class MPTTester public: MPTTester(Env& env, Account const& issuer, MPTConstr const& constr = {}); + operator MPTIssue() const; + + MPT + operator[](std::string const& name) const; + void create(MPTCreate const& arg = MPTCreate{}); diff --git a/src/xrpld/app/paths/Flow.cpp b/src/xrpld/app/paths/Flow.cpp index e0b3e41f062..c53a4089981 100644 --- a/src/xrpld/app/paths/Flow.cpp +++ b/src/xrpld/app/paths/Flow.cpp @@ -74,11 +74,11 @@ flow( Asset const srcAsset = [&]() -> Asset { if (sendMax) return sendMax->asset(); + if (isXRP(deliver)) + return xrpIssue(); if (deliver.holds()) - return Issue(deliver.issue().currency, src); - if (deliver.holds()) - return deliver.asset(); - return xrpIssue(); + return Issue(deliver.get().currency, src); + return deliver.asset(); }(); Asset const dstAsset = deliver.asset(); @@ -134,12 +134,12 @@ flow( std::variant; auto getTypedAmt = [&](Asset const& iss) -> Var { static auto xrp = XRPAmount{}; - static auto cft = MPTAmount{}; + static auto mpt = MPTAmount{}; static auto iou = IOUAmount{}; if (isXRP(iss)) return &xrp; if (iss.holds()) - return &cft; + return &mpt; return &iou; }; diff --git a/src/xrpld/app/paths/detail/MPTEndpointStep.cpp b/src/xrpld/app/paths/detail/MPTEndpointStep.cpp index 7b38a7e6a08..e8ddf2f8f51 100644 --- a/src/xrpld/app/paths/detail/MPTEndpointStep.cpp +++ b/src/xrpld/app/paths/detail/MPTEndpointStep.cpp @@ -40,11 +40,13 @@ class MPTEndpointStep protected: AccountID const src_; AccountID const dst_; - MPTIssue mptIssue_; + MPTIssue const mptIssue_; // Charge transfer fees when the prev step redeems Step const* const prevStep_ = nullptr; bool const isLast_; + // Used by maxFlow's last step. + bool const isDirectBetweenHolders_; beast::Journal const j_; struct Cache @@ -100,6 +102,11 @@ class MPTEndpointStep , mptIssue_(mpt) , prevStep_(ctx.prevStep) , isLast_(ctx.isLast) + , isDirectBetweenHolders_( + ctx.prevStep && !ctx.prevStep->bookStepBook() && ctx.isLast && + ctx.strandDeliver.holds() && + mptIssue_ == ctx.strandDeliver.get() && + dst_ == ctx.strandDst) , j_(ctx.j) { } @@ -239,7 +246,7 @@ class DirectMPTPaymentStep : public MPTEndpointStep bool verifyPrevStepDebtDirection(DebtDirection) const { - // A payment doesn't care whether or not prevStepRedeems. + // A payment doesn't care regardless of prevStepRedeems. return true; } @@ -367,12 +374,12 @@ DirectMPTPaymentStep::check( // Since this is a payment a MPToken must be present. Perform all // MPToken related checks. if (!ctx.view.exists(keylet::mptIssuance(mptIssue_.getMptID()))) - return tecOBJECT_NOT_FOUND; + return tecMPT_ISSUANCE_NOT_FOUND; if (src_ != mptIssue_.getIssuer()) { auto const mptokenID = keylet::mptoken(mptIssue_.getMptID(), src_); if (!ctx.view.exists(mptokenID)) - return tecOBJECT_NOT_FOUND; + return tecNO_AUTH; if (auto const ter = requireAuth(ctx.view, mptIssue_, src_); ter != tesSUCCESS) @@ -383,7 +390,7 @@ DirectMPTPaymentStep::check( { auto const mptokenID = keylet::mptoken(mptIssue_.getMptID(), dst_); if (!ctx.view.exists(mptokenID)) - return tecOBJECT_NOT_FOUND; + return tecNO_AUTH; if (auto const ter = requireAuth(ctx.view, mptIssue_, dst_); ter != tesSUCCESS) @@ -397,6 +404,11 @@ DirectMPTPaymentStep::check( if (isFrozen(ctx.view, src_, mptIssue_) || isFrozen(ctx.view, ctx.strandDst, mptIssue_)) return tecMPT_LOCKED; + + if (auto const ter = + canTransfer(ctx.view, mptIssue_, src_, ctx.strandDst); + ter != tesSUCCESS) + return ter; } return tesSUCCESS; @@ -416,11 +428,20 @@ template std::pair MPTEndpointStep::maxPaymentFlow(ReadView const& sb) const { - if (src_ != mptIssue_.getIssuer()) - { + // If direct payment between the holders then the outstanding amount balance + // doesn't change and the max flow is the available balance of the source + // account. + if (src_ != mptIssue_.getIssuer() || + (prevStep_ != nullptr && isDirectBetweenHolders_)) + { + assert(!prevStep_ || prevStep_->directStepSrcAcct().has_value()); + auto const account = + prevStep_ != nullptr ? *(prevStep_->directStepSrcAcct()) : src_; auto const srcOwed = toAmount(accountHolds( - sb, src_, mptIssue_, fhIGNORE_FREEZE, ahIGNORE_AUTH, j_)); + sb, account, mptIssue_, fhIGNORE_FREEZE, ahIGNORE_AUTH, j_)); + if (src_ == mptIssue_.getIssuer()) + return {srcOwed, DebtDirection::issues}; return {srcOwed, DebtDirection::redeems}; } @@ -429,7 +450,7 @@ MPTEndpointStep::maxPaymentFlow(ReadView const& sb) const std::int64_t const max = [&]() { auto const max = sle->getFieldU64(sfMaximumAmount); - return max > 0 ? max : STAmount::cMaxNativeN; // TODO MPT + return max > 0 ? max : maxMPTokenAmount; }() - sle->getFieldU64(sfOutstandingAmount); return {MPTAmount{max}, DebtDirection::issues}; @@ -496,8 +517,13 @@ MPTEndpointStep::revImp( MPTAmount const in = mulRatio(srcToDst, srcQOut, QUALITY_ONE, /*roundUp*/ true); cache_.emplace(in, srcToDst, srcToDst, srcDebtDir); - auto const ter = accountSendMPT( - sb, src_, dst_, toSTAmount(srcToDst, srcToDstIss), j_); + auto const ter = rippleCreditMPT( + sb, + src_, + dst_, + toSTAmount(srcToDst, srcToDstIss), + /*checkIssuer*/ true, + j_); (void)ter; JLOG(j_.trace()) << "MPTEndpointStep::rev: Non-limiting" << " srcRedeems: " << redeems(srcDebtDir) @@ -514,8 +540,13 @@ MPTEndpointStep::revImp( mulRatio(maxSrcToDst, dstQIn, QUALITY_ONE, /*roundUp*/ false); cache_.emplace(in, maxSrcToDst, actualOut, srcDebtDir); - auto const ter = rippleMPTCredit( - sb, src_, dst_, toSTAmount(maxSrcToDst, srcToDstIss), j_); + auto const ter = rippleCreditMPT( + sb, + src_, + dst_, + toSTAmount(maxSrcToDst, srcToDstIss), + /*checkIssuer*/ true, + j_); (void)ter; JLOG(j_.trace()) << "MPTEndpointStep::rev: Limiting" << " srcRedeems: " << redeems(srcDebtDir) @@ -612,8 +643,13 @@ MPTEndpointStep::fwdImp( MPTAmount const out = mulRatio(srcToDst, dstQIn, QUALITY_ONE, /*roundUp*/ false); setCacheLimiting(in, srcToDst, out, srcDebtDir); - auto const ter = rippleMPTCredit( - sb, src_, dst_, toSTAmount(cache_->srcToDst, srcToDstIss), j_); + auto const ter = rippleCreditMPT( + sb, + src_, + dst_, + toSTAmount(cache_->srcToDst, srcToDstIss), + /*checkIssuer*/ true, + j_); (void)ter; JLOG(j_.trace()) << "MPTEndpointStep::fwd: Non-limiting" << " srcRedeems: " << redeems(srcDebtDir) @@ -629,8 +665,13 @@ MPTEndpointStep::fwdImp( MPTAmount const out = mulRatio(maxSrcToDst, dstQIn, QUALITY_ONE, /*roundUp*/ false); setCacheLimiting(actualIn, maxSrcToDst, out, srcDebtDir); - auto const ter = rippleMPTCredit( - sb, src_, dst_, toSTAmount(cache_->srcToDst, srcToDstIss), j_); + auto const ter = rippleCreditMPT( + sb, + src_, + dst_, + toSTAmount(cache_->srcToDst, srcToDstIss), + /*checkIssuer*/ true, + j_); (void)ter; JLOG(j_.trace()) << "MPTEndpointStep::rev: Limiting" << " srcRedeems: " << redeems(srcDebtDir) diff --git a/src/xrpld/app/tx/detail/Clawback.cpp b/src/xrpld/app/tx/detail/Clawback.cpp index c11dfbd8c11..c999ba4fb1d 100644 --- a/src/xrpld/app/tx/detail/Clawback.cpp +++ b/src/xrpld/app/tx/detail/Clawback.cpp @@ -252,11 +252,12 @@ applyHelper(ApplyContext& ctx) ahIGNORE_AUTH, ctx.journal); - return rippleMPTCredit( + return rippleCreditMPT( ctx.view(), holder, issuer, std::min(spendableAmount, clawAmount), + /*checkIssuer*/ true, ctx.journal); } diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp index 435e49ea425..71dbdfb363d 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.cpp +++ b/src/xrpld/app/tx/detail/CreateOffer.cpp @@ -217,7 +217,7 @@ CreateOffer::checkAcceptAsset( Asset const& asset) { // Only valid for custom currencies - assert(!isXRP(issue.currency)); + assert(!isXRP(asset)); auto const issuerAccount = view.read(keylet::account(asset.getIssuer())); diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index 6f68789357e..c112454f9ea 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -44,14 +44,20 @@ Payment::makeTxConsequences(PreflightContext const& ctx) return TxConsequences{ctx.tx, calculateMaxXRPSpend(ctx.tx)}; } -template -static NotTEC -preflightHelper(PreflightContext const& ctx); - -template <> NotTEC -preflightHelper(PreflightContext const& ctx) +Payment::preflight(PreflightContext const& ctx) { + if (!ctx.rules.enabled(featureMPTokensV1) && + ctx.tx[sfAmount].holds()) + return temDISABLED; + + // It's malformed in MPTokensV1 + if (!ctx.rules.enabled(featureMPTokensV2) && + ctx.tx[sfAmount].holds() && + (ctx.tx.isFieldPresent(sfSendMax) || + ctx.tx.isFieldPresent(sfDeliverMin) || ctx.tx.isFieldPresent(sfPaths))) + return temMALFORMED; + if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) return ret; @@ -80,22 +86,21 @@ preflightHelper(PreflightContext const& ctx) if (bMax) maxSourceAmount = tx.getFieldAmount(sfSendMax); - else if (saDstAmount.native()) + else if (saDstAmount.native() || saDstAmount.holds()) maxSourceAmount = saDstAmount; else maxSourceAmount = STAmount( - Issue{saDstAmount.getCurrency(), account}, + Issue{saDstAmount.get().currency, account}, saDstAmount.mantissa(), saDstAmount.exponent(), saDstAmount < beast::zero); - auto const& uSrcCurrency = maxSourceAmount.getCurrency(); - auto const& uDstCurrency = saDstAmount.getCurrency(); + auto const& uSrcAsset = maxSourceAmount.asset(); + auto const& uDstAsset = saDstAmount.asset(); - // isZero() is XRP. FIX! - bool const bXRPDirect = uSrcCurrency.isZero() && uDstCurrency.isZero(); + bool const bXRPDirect = isXRP(maxSourceAmount) && isXRP(saDstAmount); - if (!isLegalNet(saDstAmount) || !isLegalNet(maxSourceAmount)) + if (!isLegalNet(uDstAsset) || !isLegalNet(uSrcAsset)) return temBAD_AMOUNT; auto const uDstAccountID = tx.getAccountID(sfDestination); @@ -118,20 +123,20 @@ preflightHelper(PreflightContext const& ctx) << "bad dst amount: " << saDstAmount.getFullText(); return temBAD_AMOUNT; } - if (badCurrency() == uSrcCurrency || badCurrency() == uDstCurrency) + if (badCurrency() == uSrcAsset || badCurrency() == uDstAsset) { JLOG(j.trace()) << "Malformed transaction: " << "Bad currency."; return temBAD_CURRENCY; } - if (account == uDstAccountID && uSrcCurrency == uDstCurrency && !bPaths) + if (account == uDstAccountID && + equalCurrencyOrMPTID(uSrcAsset, uDstAsset) && !bPaths) { // You're signing yourself a payment. // If bPaths is true, you might be trying some arbitrage. JLOG(j.trace()) << "Malformed transaction: " << "Redundant payment from " << to_string(account) - << " to self without path for " - << to_string(uDstCurrency); + << " to self without path for " << to_string(uDstAsset); return temREDUNDANT; } if (bXRPDirect && bMax) @@ -189,7 +194,7 @@ preflightHelper(PreflightContext const& ctx) << " amount. " << dMin.getFullText(); return temBAD_AMOUNT; } - if (dMin.issue() != saDstAmount.issue()) + if (dMin.asset() != saDstAmount.asset()) { JLOG(j.trace()) << "Malformed transaction: Dst issue differs " @@ -204,88 +209,14 @@ preflightHelper(PreflightContext const& ctx) << jss::DeliverMin.c_str() << ". " << dMin.getFullText(); return temBAD_AMOUNT; } + // TODO is tfNoRippleDirect valid with MPT? } return preflight2(ctx); } -template <> -NotTEC -preflightHelper(PreflightContext const& ctx) -{ - if (auto const ret = preflight1(ctx); !isTesSuccess(ret)) - return ret; - - if (!ctx.rules.enabled(featureMPTokensV1)) - return temDISABLED; - - if (ctx.tx.isFieldPresent(sfDeliverMin) || - ctx.tx.isFieldPresent(sfSendMax) || ctx.tx.isFieldPresent(sfPaths)) - return temMALFORMED; - - auto& tx = ctx.tx; - auto& j = ctx.j; - - std::uint32_t const uTxFlags = tx.getFlags(); - - if (uTxFlags & tfPaymentMask) - { - JLOG(j.trace()) << "Malformed transaction: " - << "Invalid flags set."; - return temINVALID_FLAG; - } - - STAmount const saDstAmount(tx.getFieldAmount(sfAmount)); - - auto const account = tx.getAccountID(sfAccount); - - auto const& uDstMptID = saDstAmount.get().getMptID(); - - auto const uDstAccountID = tx.getAccountID(sfDestination); - - if (!uDstAccountID) - { - JLOG(j.trace()) << "Malformed transaction: " - << "Payment destination account not specified."; - return temDST_NEEDED; - } - if (saDstAmount <= beast::zero) - { - JLOG(j.trace()) << "Malformed transaction: " - << "bad dst amount: " << saDstAmount.getFullText(); - return temBAD_AMOUNT; - } - if (account == uDstAccountID) - { - // You're signing yourself a payment. - JLOG(j.trace()) << "Malformed transaction: " - << "Redundant payment from " << to_string(account) - << " to self without path for " << to_string(uDstMptID); - return temREDUNDANT; - } - if (uTxFlags & (tfPartialPayment | tfLimitQuality | tfNoRippleDirect)) - { - JLOG(j.trace()) << "Malformed transaction: invalid MPT flags: " - << uTxFlags; - return temMALFORMED; - } - - return preflight2(ctx); -} - -template -static TER -preclaimHelper( - PreclaimContext const& ctx, - std::size_t maxPathSize, - std::size_t maxPathLength); - -template <> TER -preclaimHelper( - PreclaimContext const& ctx, - std::size_t maxPathSize, - std::size_t maxPathLength) +Payment::preclaim(PreclaimContext const& ctx) { // Ripple if source or destination is non-native or if there are paths. std::uint32_t const uTxFlags = ctx.tx.getFlags(); @@ -356,9 +287,9 @@ preclaimHelper( { STPathSet const& paths = ctx.tx.getFieldPathSet(sfPaths); - if (paths.size() > maxPathSize || + if (paths.size() > MaxPathSize || std::any_of(paths.begin(), paths.end(), [&](STPath const& path) { - return path.size() > maxPathLength; + return path.size() > MaxPathLength; })) { return telBAD_PATH_COUNT; @@ -368,94 +299,46 @@ preclaimHelper( return tesSUCCESS; } -template <> TER -preclaimHelper(PreclaimContext const& ctx, std::size_t, std::size_t) -{ - AccountID const uDstAccountID(ctx.tx[sfDestination]); - - auto const k = keylet::account(uDstAccountID); - auto const sleDst = ctx.view.read(k); - - if (!sleDst) - { - JLOG(ctx.j.trace()) - << "Delay transaction: Destination account does not exist."; - - // Another transaction could create the account and then this - // transaction would succeed. - return tecNO_DST; - } - else if ( - (sleDst->getFlags() & lsfRequireDestTag) && - !ctx.tx.isFieldPresent(sfDestinationTag)) - { - // The tag is basically account-specific information we don't - // understand, but we can require someone to fill it in. - - // We didn't make this test for a newly-formed account because there's - // no way for this field to be set. - JLOG(ctx.j.trace()) - << "Malformed transaction: DestinationTag required."; - - return tecDST_TAG_NEEDED; - } - - return tesSUCCESS; -} - -template -static TER -applyHelper( - ApplyContext& ctx, - XRPAmount const& priorBalance, - XRPAmount const& sourceBalance); - -template <> -TER -applyHelper( - ApplyContext& ctx, - XRPAmount const& priorBalance, - XRPAmount const& sourceBalance) +Payment::doApply() { - AccountID const account = ctx.tx[sfAccount]; - auto const deliverMin = ctx.tx[~sfDeliverMin]; + auto const deliverMin = ctx_.tx[~sfDeliverMin]; // Ripple if source or destination is non-native or if there are paths. - std::uint32_t const uTxFlags = ctx.tx.getFlags(); + std::uint32_t const uTxFlags = ctx_.tx.getFlags(); bool const partialPaymentAllowed = uTxFlags & tfPartialPayment; bool const limitQuality = uTxFlags & tfLimitQuality; bool const defaultPathsAllowed = !(uTxFlags & tfNoRippleDirect); - auto const paths = ctx.tx.isFieldPresent(sfPaths); - auto const sendMax = ctx.tx[~sfSendMax]; + auto const paths = ctx_.tx.isFieldPresent(sfPaths); + auto const sendMax = ctx_.tx[~sfSendMax]; - AccountID const uDstAccountID(ctx.tx.getAccountID(sfDestination)); - STAmount const saDstAmount(ctx.tx.getFieldAmount(sfAmount)); + AccountID const uDstAccountID(ctx_.tx.getAccountID(sfDestination)); + STAmount const saDstAmount(ctx_.tx.getFieldAmount(sfAmount)); STAmount maxSourceAmount; if (sendMax) maxSourceAmount = *sendMax; - else if (saDstAmount.native()) + else if (saDstAmount.native() || saDstAmount.holds()) maxSourceAmount = saDstAmount; else maxSourceAmount = STAmount( - Issue{saDstAmount.getCurrency(), account}, + Issue{saDstAmount.get().currency, account_}, saDstAmount.mantissa(), saDstAmount.exponent(), saDstAmount < beast::zero); - JLOG(ctx.journal.trace()) + JLOG(ctx_.journal.trace()) << "maxSourceAmount=" << maxSourceAmount.getFullText() << " saDstAmount=" << saDstAmount.getFullText(); // Open a ledger for editing. auto const k = keylet::account(uDstAccountID); - SLE::pointer sleDst = ctx.view().peek(k); + SLE::pointer sleDst = ctx_.view().peek(k); if (!sleDst) { std::uint32_t const seqno{ - ctx.view().rules().enabled(featureDeletableAccounts) - ? ctx.view().seq() + ctx_.view().rules().enabled(featureDeletableAccounts) + ? ctx_.view().seq() : 1}; // Create the account. @@ -463,22 +346,22 @@ applyHelper( sleDst->setAccountID(sfAccount, uDstAccountID); sleDst->setFieldU32(sfSequence, seqno); - ctx.view().insert(sleDst); + ctx_.view().insert(sleDst); } else { // Tell the engine that we are intending to change the destination // account. The source account gets always charged a fee so it's always // marked as modified. - ctx.view().update(sleDst); + ctx_.view().update(sleDst); } // Determine whether the destination requires deposit authorization. bool const reqDepositAuth = sleDst->getFlags() & lsfDepositAuth && - ctx.view().rules().enabled(featureDepositAuth); + ctx_.view().rules().enabled(featureDepositAuth); bool const depositPreauth = - ctx.view().rules().enabled(featureDepositPreauth); + ctx_.view().rules().enabled(featureDepositPreauth); bool const bRipple = paths || sendMax || !saDstAmount.native(); @@ -498,10 +381,10 @@ applyHelper( // authorization has two ways to get an IOU Payment in: // 1. If Account == Destination, or // 2. If Account is deposit preauthorized by destination. - if (uDstAccountID != account) + if (uDstAccountID != account_) { - if (!ctx.view().exists( - keylet::depositPreauth(uDstAccountID, account))) + if (!ctx_.view().exists( + keylet::depositPreauth(uDstAccountID, account_))) return tecNO_PERMISSION; } } @@ -510,26 +393,26 @@ applyHelper( rcInput.partialPaymentAllowed = partialPaymentAllowed; rcInput.defaultPathsAllowed = defaultPathsAllowed; rcInput.limitQuality = limitQuality; - rcInput.isLedgerOpen = ctx.view().open(); + rcInput.isLedgerOpen = ctx_.view().open(); path::RippleCalc::Output rc; { - PaymentSandbox pv(&ctx.view()); - JLOG(ctx.journal.debug()) << "Entering RippleCalc in payment: " - << ctx.tx.getTransactionID(); + PaymentSandbox pv(&ctx_.view()); + JLOG(ctx_.journal.debug()) << "Entering RippleCalc in payment: " + << ctx_.tx.getTransactionID(); rc = path::RippleCalc::rippleCalculate( pv, maxSourceAmount, saDstAmount, uDstAccountID, - account, - ctx.tx.getFieldPathSet(sfPaths), - ctx.app.logs(), + account_, + ctx_.tx.getFieldPathSet(sfPaths), + ctx_.app.logs(), &rcInput); // VFALCO NOTE We might not need to apply, depending // on the TER. But always applying *should* // be safe. - pv.apply(ctx.rawView()); + pv.apply(ctx_.rawView()); } // TODO: is this right? If the amount is the correct amount, was @@ -539,7 +422,7 @@ applyHelper( if (deliverMin && rc.actualAmountOut < *deliverMin) rc.setResult(tecPATH_PARTIAL); else - ctx.deliver(rc.actualAmountOut); + ctx_.deliver(rc.actualAmountOut); } auto terResult = rc.result(); @@ -557,7 +440,7 @@ applyHelper( // Direct XRP payment. - auto const sleSrc = ctx.view().peek(keylet::account(account)); + auto const sleSrc = ctx_.view().peek(keylet::account(account_)); if (!sleSrc) return tefINTERNAL; @@ -566,21 +449,21 @@ applyHelper( auto const uOwnerCount = sleSrc->getFieldU32(sfOwnerCount); // This is the total reserve in drops. - auto const reserve = ctx.view().fees().accountReserve(uOwnerCount); + auto const reserve = ctx_.view().fees().accountReserve(uOwnerCount); // mPriorBalance is the balance on the sending account BEFORE the // fees were charged. We want to make sure we have enough reserve // to send. Allow final spend to use reserve for fee. - auto const mmm = std::max(reserve, ctx.tx.getFieldAmount(sfFee).xrp()); + auto const mmm = std::max(reserve, ctx_.tx.getFieldAmount(sfFee).xrp()); - if (priorBalance < saDstAmount.xrp() + mmm) + if (mPriorBalance < saDstAmount.xrp() + mmm) { // Vote no. However the transaction might succeed, if applied in // a different order. - JLOG(ctx.journal.trace()) << "Delay transaction: Insufficient funds: " - << " " << to_string(priorBalance) << " / " - << to_string(saDstAmount.xrp() + mmm) << " (" - << to_string(reserve) << ")"; + JLOG(ctx_.journal.trace()) << "Delay transaction: Insufficient funds: " + << " " << to_string(mPriorBalance) << " / " + << to_string(saDstAmount.xrp() + mmm) << " (" + << to_string(reserve) << ")"; return tecUNFUNDED_PAYMENT; } @@ -612,13 +495,14 @@ applyHelper( // We choose the base reserve as our bound because it is // a small number that seldom changes but is always sufficient // to get the account un-wedged. - if (uDstAccountID != account) + if (uDstAccountID != account_) { - if (!ctx.view().exists( - keylet::depositPreauth(uDstAccountID, account))) + if (!ctx_.view().exists( + keylet::depositPreauth(uDstAccountID, account_))) { // Get the base reserve. - XRPAmount const dstReserve{ctx.view().fees().accountReserve(0)}; + XRPAmount const dstReserve{ + ctx_.view().fees().accountReserve(0)}; if (saDstAmount > dstReserve || sleDst->getFieldAmount(sfBalance) > dstReserve) @@ -628,7 +512,7 @@ applyHelper( } // Do the arithmetic for the transfer and make the ledger change. - sleSrc->setFieldAmount(sfBalance, sourceBalance - saDstAmount); + sleSrc->setFieldAmount(sfBalance, mSourceBalance - saDstAmount); sleDst->setFieldAmount( sfBalance, sleDst->getFieldAmount(sfBalance) + saDstAmount); @@ -639,78 +523,4 @@ applyHelper( return tesSUCCESS; } -template <> -TER -applyHelper(ApplyContext& ctx, XRPAmount const&, XRPAmount const&) -{ - auto const account = ctx.tx[sfAccount]; - - AccountID const uDstAccountID(ctx.tx.getAccountID(sfDestination)); - auto const saDstAmount(ctx.tx.getFieldAmount(sfAmount)); - - JLOG(ctx.journal.trace()) << " saDstAmount=" << saDstAmount.getFullText(); - - if (auto const ter = - requireAuth(ctx.view(), saDstAmount.get(), account); - ter != tesSUCCESS) - return ter; - - if (auto const ter = - requireAuth(ctx.view(), saDstAmount.get(), uDstAccountID); - ter != tesSUCCESS) - return ter; - - if (auto const ter = canTransfer( - ctx.view(), saDstAmount.get(), account, uDstAccountID); - ter != tesSUCCESS) - return ter; - - auto const& mpt = saDstAmount.get(); - auto const& issuer = mpt.getIssuer(); - // If globally/individually locked then - // - can't send between holders - // - holder can send back to issuer - // - issuer can send to holder - if (account != issuer && uDstAccountID != issuer && - (isFrozen(ctx.view(), account, mpt) || - isFrozen(ctx.view(), uDstAccountID, mpt))) - return tecMPT_LOCKED; - - PaymentSandbox pv(&ctx.view()); - auto const res = - accountSendMPT(pv, account, uDstAccountID, saDstAmount, ctx.journal); - pv.apply(ctx.rawView()); - return res; -} - -NotTEC -Payment::preflight(PreflightContext const& ctx) -{ - return std::visit( - [&](TDelIss const&) { - return preflightHelper(ctx); - }, - ctx.tx[sfAmount].asset().value()); -} - -TER -Payment::preclaim(PreclaimContext const& ctx) -{ - return std::visit( - [&](TDelIss const&) { - return preclaimHelper(ctx, MaxPathSize, MaxPathLength); - }, - ctx.tx[sfAmount].asset().value()); -} - -TER -Payment::doApply() -{ - return std::visit( - [&](TDelIss const&) { - return applyHelper(ctx_, mPriorBalance, mSourceBalance); - }, - ctx_.tx[sfAmount].asset().value()); -} - } // namespace ripple diff --git a/src/xrpld/ledger/View.h b/src/xrpld/ledger/View.h index e66c1cb07e1..30d201d6c2f 100644 --- a/src/xrpld/ledger/View.h +++ b/src/xrpld/ledger/View.h @@ -482,16 +482,17 @@ rippleCredit( ApplyView& view, AccountID const& uSenderID, AccountID const& uReceiverID, - const STAmount& saAmount, + STAmount const& saAmount, bool bCheckIssuer, beast::Journal j); [[nodiscard]] TER -rippleMPTCredit( +rippleCreditMPT( ApplyView& view, AccountID const& uSenderID, AccountID const& uReceiverID, - STAmount saAmount, + STAmount const& saAmount, + bool checkIssuer, beast::Journal j); [[nodiscard]] TER @@ -499,7 +500,7 @@ accountSend( ApplyView& view, AccountID const& from, AccountID const& to, - const STAmount& saAmount, + STAmount const& saAmount, beast::Journal j, WaiveTransferFee waiveFee = WaiveTransferFee::No); @@ -508,7 +509,7 @@ accountSendMPT( ApplyView& view, AccountID const& from, AccountID const& to, - const STAmount& saAmount, + STAmount const& saAmount, beast::Journal j, WaiveTransferFee waiveFee = WaiveTransferFee::No); diff --git a/src/xrpld/ledger/detail/View.cpp b/src/xrpld/ledger/detail/View.cpp index c4c6d180f9f..ac07475dbe7 100644 --- a/src/xrpld/ledger/detail/View.cpp +++ b/src/xrpld/ledger/detail/View.cpp @@ -314,7 +314,7 @@ accountHolds( AuthHandling zeroIfUnauthorized, beast::Journal j) { - STAmount amount; + STAmount amount{issue}; auto const sleMpt = view.read(keylet::mptoken(issue.getMptID(), account)); if (!sleMpt) @@ -1405,7 +1405,7 @@ rippleSendMPT( // Direct send: redeeming IOUs and/or sending own IOUs. auto const ter = - rippleMPTCredit(view, uSenderID, uReceiverID, saAmount, j); + rippleCreditMPT(view, uSenderID, uReceiverID, saAmount, false, j); if (ter != tesSUCCESS) return ter; saActual = saAmount; @@ -1428,11 +1428,11 @@ rippleSendMPT( << " cost=" << saActual.getFullText(); if (auto const terResult = - rippleMPTCredit(view, issuer, uReceiverID, saAmount, j); + rippleCreditMPT(view, issuer, uReceiverID, saAmount, true, j); terResult != tesSUCCESS) return terResult; - return rippleMPTCredit(view, uSenderID, issuer, saActual, j); + return rippleCreditMPT(view, uSenderID, issuer, saActual, true, j); } return tecINTERNAL; @@ -1901,13 +1901,18 @@ deleteAMMTrustLine( } TER -rippleMPTCredit( +rippleCreditMPT( ApplyView& view, AccountID const& uSenderID, AccountID const& uReceiverID, - STAmount saAmount, + STAmount const& saAmount, + bool checkIssuer, beast::Journal j) { + assert(saAmount.holds()); + assert( + !checkIssuer || uSenderID == saAmount.getIssuer() || + uReceiverID == saAmount.getIssuer()); auto const mptID = keylet::mptIssuance(saAmount.get().getMptID()); auto const issuer = saAmount.getIssuer(); if (!view.exists(mptID)) diff --git a/src/xrpld/rpc/detail/TransactionSign.cpp b/src/xrpld/rpc/detail/TransactionSign.cpp index 95258a4d4b3..fd1ee0cd85a 100644 --- a/src/xrpld/rpc/detail/TransactionSign.cpp +++ b/src/xrpld/rpc/detail/TransactionSign.cpp @@ -214,7 +214,9 @@ checkPayment( return RPC::invalid_field_error("tx_json.Destination"); if (((doPath == false) && params.isMember(jss::build_path)) || - (params.isMember(jss::build_path) && amount.holds())) + (params.isMember(jss::build_path) && + !app.openLedger().current()->rules().enabled(featureMPTokensV2) && + amount.holds())) return RPC::make_error( rpcINVALID_PARAMS, "Field 'build_path' not allowed in this context.");