diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 69908c4b52..eb133628ef 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -244,9 +244,16 @@ class LoanBroker_test : public beast::unit_test::suite env(coverWithdraw(alice, keylet.key, vault.asset(900)), ter(tecINSUFFICIENT_FUNDS)); - env(coverWithdraw(alice, keylet.key, vault.asset(1)), - destination(bystander), - ter(tecNO_LINE)); + // Skip this test for XRP, because that can always be sent + if (!vault.asset.raw().native()) + { + TER const expected = vault.asset.raw().holds() + ? tecNO_AUTH + : tecNO_LINE; + env(coverWithdraw(alice, keylet.key, vault.asset(1)), + destination(bystander), + ter(expected)); + } verifyCoverAmount(10); // Withdraw some of the cover amount diff --git a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp index 3dff73ebd5..9b155b32c2 100644 --- a/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp +++ b/src/xrpld/app/tx/detail/LoanBrokerCoverWithdraw.cpp @@ -55,7 +55,11 @@ LoanBrokerCoverWithdraw::preflight(PreflightContext const& ctx) if (ctx.tx[sfLoanBrokerID] == beast::zero) return temINVALID; - if (ctx.tx[sfAmount] <= beast::zero) + auto const dstAmount = ctx.tx[sfAmount]; + if (dstAmount <= beast::zero) + return temBAD_AMOUNT; + + if (!isLegalNet(dstAmount)) return temBAD_AMOUNT; if (auto const destination = ctx.tx[~sfDestination]; @@ -183,41 +187,54 @@ LoanBrokerCoverWithdraw::doApply() if (!broker) return tecINTERNAL; // LCOV_EXCL_LINE - auto const brokerPseudoID = broker->at(sfAccount); + auto const brokerPseudoID = *broker->at(sfAccount); // Decrease the LoanBroker's CoverAvailable by Amount broker->at(sfCoverAvailable) -= amount; view().update(broker); - if (dstAcct != account_ && !amount.native() && !amount.holds()) + // Move the funds from the broker's pseudo-account to the dstAcct + + if (dstAcct != account_ && !amount.native()) { - STAmount const maxSourceAmount( - Issue{amount.get().currency, brokerPseudoID}, - amount.mantissa(), - amount.exponent(), - amount < beast::zero); + bool const mptDirect = amount.holds(); + STAmount const maxSourceAmount = + Payment::getMaxSourceAmount(brokerPseudoID, amount); SLE::pointer sleDst = view().peek(keylet::account(dstAcct)); + if (!sleDst) + return tecINTERNAL; - auto ret = Payment::makeRipplePayment(Payment::RipplePaymentParams{ - .ctx = ctx_, - .maxSourceAmount = maxSourceAmount, - .srcAccountID = account_, - .dstAccountID = dstAcct, - .sleDst = sleDst, - .dstAmount = amount, - .deliverMin = std::nullopt, - .j = j_}); - - // Always claim a fee - if (!isTesSuccess(ret) && !isTecClaim(ret)) + if (!mptDirect) { - JLOG(j_.info()) - << "LoanBrokerCoverWithdraw: changing result from " - << transToken(ret) - << " to tecPATH_DRY for IOU payment with Destination"; - ret = tecPATH_DRY; + // If sending the Cover to a different account, then this is + // effectively a payment. Use the Payment transaction code to call + // the payment engine, though only a subset of the functionality is + // supported in this transaction. e.g. No paths, no partial + // payments. + + auto const ret = + Payment::makeRipplePayment(Payment::RipplePaymentParams{ + .ctx = ctx_, + .maxSourceAmount = maxSourceAmount, + .srcAccountID = brokerPseudoID, + .dstAccountID = dstAcct, + .sleDst = sleDst, + .dstAmount = amount, + .paths = STPathSet{}, + .deliverMin = std::nullopt, + .j = j_}); + + // Always claim a fee + if (!isTesSuccess(ret) && !isTecClaim(ret)) + { + JLOG(j_.info()) + << "LoanBrokerCoverWithdraw: changing result from " + << transToken(ret) + << " to tecPATH_DRY for IOU payment with Destination"; + return tecPATH_DRY; + } + return ret; } - return ret; } // Transfer assets from pseudo-account to depositor. diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index bd20ba36ed..a92fdc6c04 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -48,8 +48,8 @@ Payment::makeTxConsequences(PreflightContext const& ctx) } STAmount -getMaxSourceAmount( - AccountID const& account, +Payment::getMaxSourceAmount( + AccountID const& senderAccount, STAmount const& dstAmount, std::optional const& sendMax) { @@ -59,7 +59,7 @@ getMaxSourceAmount( return dstAmount; else return STAmount( - Issue{dstAmount.get().currency, account}, + Issue{dstAmount.get().currency, senderAccount}, dstAmount.mantissa(), dstAmount.exponent(), dstAmount < beast::zero); @@ -454,6 +454,7 @@ Payment::doApply() .dstAccountID = dstAccountID, .sleDst = sleDst, .dstAmount = dstAmount, + .paths = ctx_.tx.getFieldPathSet(sfPaths), .deliverMin = deliverMin, .partialPaymentAllowed = partialPaymentAllowed, .defaultPathsAllowed = defaultPathsAllowed, @@ -462,79 +463,19 @@ Payment::doApply() } else if (mptDirect) { - JLOG(j_.trace()) << " dstAmount=" << dstAmount.getFullText(); - auto const& mptIssue = dstAmount.get(); - - if (auto const ter = requireAuth(view(), mptIssue, account_); - ter != tesSUCCESS) - return ter; - - if (auto const ter = requireAuth(view(), mptIssue, dstAccountID); - ter != tesSUCCESS) - return ter; - - if (auto const ter = - canTransfer(view(), mptIssue, account_, dstAccountID); - ter != tesSUCCESS) - return ter; - - if (auto err = verifyDepositPreauth( - ctx_.tx, - ctx_.view(), - account_, - dstAccountID, - sleDst, - ctx_.journal); - !isTesSuccess(err)) - return err; - - auto const& issuer = mptIssue.getIssuer(); - - // Transfer rate - Rate rate{QUALITY_ONE}; - // Payment between the holders - if (account_ != issuer && dstAccountID != issuer) - { - // If globally/individually locked then - // - can't send between holders - // - holder can send back to issuer - // - issuer can send to holder - if (isAnyFrozen(view(), {account_, dstAccountID}, mptIssue)) - return tecLOCKED; - - // Get the rate for a payment between the holders. - rate = transferRate(view(), mptIssue.getMptID()); - } - - // Amount to deliver. - STAmount amountDeliver = dstAmount; - // Factor in the transfer rate. - // No rounding. It'll change once MPT integrated into DEX. - STAmount requiredMaxSourceAmount = multiply(dstAmount, rate); - - // Send more than the account wants to pay or less than - // the account wants to deliver (if no SendMax). - // Adjust the amount to deliver. - if (partialPaymentAllowed && requiredMaxSourceAmount > maxSourceAmount) - { - requiredMaxSourceAmount = maxSourceAmount; - // No rounding. It'll change once MPT integrated into DEX. - amountDeliver = divide(maxSourceAmount, rate); - } - - if (requiredMaxSourceAmount > maxSourceAmount || - (deliverMin && amountDeliver < *deliverMin)) - return tecPATH_PARTIAL; - - PaymentSandbox pv(&view()); - auto res = accountSend( - pv, account_, dstAccountID, amountDeliver, ctx_.journal); - if (res == tesSUCCESS) - pv.apply(ctx_.rawView()); - else if (res == tecINSUFFICIENT_FUNDS || res == tecPATH_DRY) - res = tecPATH_PARTIAL; - - return res; + return makeMPTDirectPayment(RipplePaymentParams{ + .ctx = ctx_, + .maxSourceAmount = maxSourceAmount, + .srcAccountID = account_, + .dstAccountID = dstAccountID, + .sleDst = sleDst, + .dstAmount = dstAmount, + .paths = ctx_.tx.getFieldPathSet(sfPaths), + .deliverMin = deliverMin, + .partialPaymentAllowed = partialPaymentAllowed, + .defaultPathsAllowed = defaultPathsAllowed, + .limitQuality = limitQuality, + .j = j_}); } XRPL_ASSERT(dstAmount.native(), "ripple::Payment::doApply : amount is XRP"); @@ -676,7 +617,7 @@ Payment::makeRipplePayment(Payment::RipplePaymentParams const& p) p.dstAmount, p.dstAccountID, p.srcAccountID, - p.ctx.tx.getFieldPathSet(sfPaths), + p.paths, p.ctx.tx[~sfDomainID], p.ctx.app.logs(), &rcInput); @@ -707,4 +648,83 @@ Payment::makeRipplePayment(Payment::RipplePaymentParams const& p) return terResult; } +TER +Payment::makeMPTDirectPayment(Payment::RipplePaymentParams const& p) +{ + JLOG(p.j.trace()) << " p.dstAmount=" << p.dstAmount.getFullText(); + auto const& mptIssue = p.dstAmount.get(); + + if (auto const ter = requireAuth(p.ctx.view(), mptIssue, p.srcAccountID); + ter != tesSUCCESS) + return ter; + + if (auto const ter = requireAuth(p.ctx.view(), mptIssue, p.dstAccountID); + ter != tesSUCCESS) + return ter; + + if (auto const ter = + canTransfer(p.ctx.view(), mptIssue, p.srcAccountID, p.dstAccountID); + ter != tesSUCCESS) + return ter; + + if (auto err = verifyDepositPreauth( + p.ctx.tx, + p.ctx.view(), + p.srcAccountID, + p.dstAccountID, + p.sleDst, + p.ctx.journal); + !isTesSuccess(err)) + return err; + + auto const& issuer = mptIssue.getIssuer(); + + // Transfer rate + Rate rate{QUALITY_ONE}; + // Payment between the holders + if (p.srcAccountID != issuer && p.dstAccountID != issuer) + { + // If globally/individually locked then + // - can't send between holders + // - holder can send back to issuer + // - issuer can send to holder + if (isAnyFrozen( + p.ctx.view(), {p.srcAccountID, p.dstAccountID}, mptIssue)) + return tecLOCKED; + + // Get the rate for a payment between the holders. + rate = transferRate(p.ctx.view(), mptIssue.getMptID()); + } + + // Amount to deliver. + STAmount amountDeliver = p.dstAmount; + // Factor in the transfer rate. + // No rounding. It'll change once MPT integrated into DEX. + STAmount requiredMaxSourceAmount = multiply(p.dstAmount, rate); + + // Send more than the account wants to pay or less than + // the account wants to deliver (if no SendMax). + // Adjust the amount to deliver. + if (p.partialPaymentAllowed && requiredMaxSourceAmount > p.maxSourceAmount) + { + requiredMaxSourceAmount = p.maxSourceAmount; + // No rounding. It'll change once MPT integrated into DEX. + amountDeliver = divide(p.maxSourceAmount, rate); + } + + if (requiredMaxSourceAmount > p.maxSourceAmount || + (p.deliverMin && amountDeliver < *p.deliverMin)) + return tecPATH_PARTIAL; + + PaymentSandbox pv(&p.ctx.view()); + auto res = accountSend( + pv, p.srcAccountID, p.dstAccountID, amountDeliver, p.ctx.journal); + if (res == tesSUCCESS) + pv.apply(p.ctx.rawView()); + else if (res == tecINSUFFICIENT_FUNDS || res == tecPATH_DRY) + res = tecPATH_PARTIAL; + + return res; +} + } // namespace ripple diff --git a/src/xrpld/app/tx/detail/Payment.h b/src/xrpld/app/tx/detail/Payment.h index f201f3f9be..2a6751f88b 100644 --- a/src/xrpld/app/tx/detail/Payment.h +++ b/src/xrpld/app/tx/detail/Payment.h @@ -69,6 +69,9 @@ public: AccountID const& dstAccountID; SLE::pointer sleDst; STAmount const& dstAmount; + // Paths need to be explicitly included because other transactions don't + // have them defined + STPathSet const& paths; std::optional const& deliverMin; bool partialPaymentAllowed = false; bool defaultPathsAllowed = true; @@ -76,8 +79,17 @@ public: beast::Journal j; }; + static STAmount + getMaxSourceAmount( + AccountID const& senderAccount, + STAmount const& dstAmount, + std::optional const& sendMax = {}); + static TER makeRipplePayment(RipplePaymentParams const& p); + + static TER + makeMPTDirectPayment(RipplePaymentParams const& p); }; } // namespace ripple