ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-04-29 15:37:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'XRPLF/ximinez/lending-refactoring-4' into ximinez/lending-XLS-66

Browse Source 
* XRPLF/ximinez/lending-refactoring-4:
  fix `DeliveredAmount` and `delivered_amount` in transaction metadata for direct MPT transfer (#5569)
This commit is contained in:
Ed Hennis
2025-07-30 11:07:03 -04:00
parent ec8eaf0d73 97469a1c02
commit 83ba11d505
3 changed files with 190 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

84
src/test/rpc/DeliveredAmount_test.cpp
View File

@@ -21,6 +21,7 @@
#include <test/jtx/WSClient.h>
#include <xrpl/beast/unit_test.h>
#include <xrpl/beast/unit_test/suite.h>
#include <xrpl/protocol/jss.h>
namespace ripple {
@@ -329,12 +330,95 @@ class DeliveredAmount_test : public beast::unit_test::suite
}
}
void
testMPTDeliveredAmountRPC(FeatureBitset features)
{
testcase("MPT DeliveredAmount");
using namespace jtx;
Account const alice("alice");
Account const carol("carol");
Account const bob("bob");
Env env{*this, features};
MPTTester mptAlice(
env, alice, {.holders = {bob, carol}, .close = false});
mptAlice.create(
{.transferFee = 25000,
.ownerCount = 1,
.holderCount = 0,
.flags = tfMPTCanTransfer});
auto const MPT = mptAlice["MPT"];
mptAlice.authorize({.account = bob});
mptAlice.authorize({.account = carol});
// issuer to holder
mptAlice.pay(alice, bob, 10000);
// holder to holder
env(pay(bob, carol, mptAlice.mpt(1000)), txflags(tfPartialPayment));
env.close();
// Get the hash for the most recent transaction.
std::string txHash{
env.tx()->getJson(JsonOptions::none)[jss::hash].asString()};
Json::Value meta = env.rpc("tx", txHash)[jss::result][jss::meta];
if (features[fixMPTDeliveredAmount])
{
BEAST_EXPECT(
meta[sfDeliveredAmount.jsonName] ==
STAmount{MPT(800)}.getJson(JsonOptions::none));
BEAST_EXPECT(
meta[jss::delivered_amount] ==
STAmount{MPT(800)}.getJson(JsonOptions::none));
}
else
{
BEAST_EXPECT(!meta.isMember(sfDeliveredAmount.jsonName));
BEAST_EXPECT(
meta[jss::delivered_amount] = Json::Value("unavailable"));
}
env(pay(bob, carol, MPT(1000)),
sendmax(MPT(1200)),
txflags(tfPartialPayment));
env.close();
txHash = env.tx()->getJson(JsonOptions::none)[jss::hash].asString();
meta = env.rpc("tx", txHash)[jss::result][jss::meta];
if (features[fixMPTDeliveredAmount])
{
BEAST_EXPECT(
meta[sfDeliveredAmount.jsonName] ==
STAmount{MPT(960)}.getJson(JsonOptions::none));
BEAST_EXPECT(
meta[jss::delivered_amount] ==
STAmount{MPT(960)}.getJson(JsonOptions::none));
}
else
{
BEAST_EXPECT(!meta.isMember(sfDeliveredAmount.jsonName));
BEAST_EXPECT(
meta[jss::delivered_amount] = Json::Value("unavailable"));
}
}
public:
void
run() override
{
using namespace test::jtx;
FeatureBitset const all{testable_amendments()};
testTxDeliveredAmountRPC();
testAccountDeliveredAmountSubscribe();
testMPTDeliveredAmountRPC(all - fixMPTDeliveredAmount);
testMPTDeliveredAmountRPC(all);
}
};

160
src/xrpld/app/tx/detail/Payment.cpp
View File

@@ -575,13 +575,38 @@ Payment::doApply()
TER
Payment::makeRipplePayment(Payment::RipplePaymentParams const& p)
{
// Ripple payment with at least one intermediate step and uses
// transitive balances.
// Set up some copies/references so the code can be moved from
// Payment::doApply otherwise unmodified
//
// Note that some of these variable names use trailing '_', which is
// usually reserved for member variables. After, or just before these
// changes are merged, a follow-up can clean up the names for consistency.
ApplyContext& ctx_ = p.ctx;
STAmount const& maxSourceAmount = p.maxSourceAmount;
AccountID const& account_ = p.srcAccountID;
AccountID const& dstAccountID = p.dstAccountID;
SLE::pointer sleDst = p.sleDst;
STAmount const& dstAmount = p.dstAmount;
STPathSet const& paths = p.paths;
std::optional<STAmount> const& deliverMin = p.deliverMin;
bool partialPaymentAllowed = p.partialPaymentAllowed;
bool defaultPathsAllowed = p.defaultPathsAllowed;
bool limitQuality = p.limitQuality;
beast::Journal j_ = p.j;
auto view = [&p]() -> ApplyView& { return p.ctx.view(); };
bool const depositPreauth =
p.ctx.view().rules().enabled(featureDepositPreauth);
bool const depositAuth = p.ctx.view().rules().enabled(featureDepositAuth);
// Below this line, copied straight from Payment::doApply
// except `ctx_.tx.getFieldPathSet(sfPaths)` replaced with `paths`,
// because not all transactions have that field available, and using
// it will throw
//-------------------------------------------------------
// Ripple payment with at least one intermediate step and uses
// transitive balances.
if (depositPreauth && depositAuth)
{
// If depositPreauth is enabled, then an account that requires
@@ -590,51 +615,51 @@ Payment::makeRipplePayment(Payment::RipplePaymentParams const& p)
// 2. If Account is deposit preauthorized by destination.
if (auto err = verifyDepositPreauth(
p.ctx.tx,
p.ctx.view(),
p.srcAccountID,
p.dstAccountID,
p.sleDst,
p.ctx.journal);
ctx_.tx,
ctx_.view(),
account_,
dstAccountID,
sleDst,
ctx_.journal);
!isTesSuccess(err))
return err;
}
path::RippleCalc::Input rcInput;
rcInput.partialPaymentAllowed = p.partialPaymentAllowed;
rcInput.defaultPathsAllowed = p.defaultPathsAllowed;
rcInput.limitQuality = p.limitQuality;
rcInput.isLedgerOpen = p.ctx.view().open();
rcInput.partialPaymentAllowed = partialPaymentAllowed;
rcInput.defaultPathsAllowed = defaultPathsAllowed;
rcInput.limitQuality = limitQuality;
rcInput.isLedgerOpen = view().open();
path::RippleCalc::Output rc;
{
PaymentSandbox pv(&p.ctx.view());
JLOG(p.j.debug()) << "Entering RippleCalc in payment: "
<< p.ctx.tx.getTransactionID();
PaymentSandbox pv(&view());
JLOG(j_.debug()) << "Entering RippleCalc in payment: "
<< ctx_.tx.getTransactionID();
rc = path::RippleCalc::rippleCalculate(
pv,
p.maxSourceAmount,
p.dstAmount,
p.dstAccountID,
p.srcAccountID,
p.paths,
p.ctx.tx[~sfDomainID],
p.ctx.app.logs(),
maxSourceAmount,
dstAmount,
dstAccountID,
account_,
paths,
ctx_.tx[~sfDomainID],
ctx_.app.logs(),
&rcInput);
// VFALCO NOTE We might not need to apply, depending
// on the TER. But always applying *should*
// be safe.
pv.apply(p.ctx.rawView());
pv.apply(ctx_.rawView());
}
// TODO: is this right? If the amount is the correct
// amount, was the delivered amount previously set?
if (rc.result() == tesSUCCESS && rc.actualAmountOut != p.dstAmount)
// TODO: is this right? If the amount is the correct amount, was
// the delivered amount previously set?
if (rc.result() == tesSUCCESS && rc.actualAmountOut != dstAmount)
{
if (p.deliverMin && rc.actualAmountOut < *p.deliverMin)
if (deliverMin && rc.actualAmountOut < *deliverMin)
rc.setResult(tecPATH_PARTIAL);
else
p.ctx.deliver(rc.actualAmountOut);
ctx_.deliver(rc.actualAmountOut);
}
auto terResult = rc.result();
@@ -651,29 +676,46 @@ Payment::makeRipplePayment(Payment::RipplePaymentParams const& p)
TER
Payment::makeMPTDirectPayment(Payment::RipplePaymentParams const& p)
{
JLOG(p.j.trace()) << " p.dstAmount=" << p.dstAmount.getFullText();
auto const& mptIssue = p.dstAmount.get<MPTIssue>();
// Set up some copies/references so the code can be moved from
// Payment::doApply otherwise unmodified
//
// Note that some of these variable names use trailing '_', which is
// usually reserved for member variables. After, or just before these
// changes are merged, a follow-up can clean up the names for consistency.
ApplyContext& ctx_ = p.ctx;
STAmount const& maxSourceAmount = p.maxSourceAmount;
AccountID const& account_ = p.srcAccountID;
AccountID const& dstAccountID = p.dstAccountID;
SLE::pointer sleDst = p.sleDst;
STAmount const& dstAmount = p.dstAmount;
// STPathSet const& paths = p.paths;
std::optional<STAmount> const& deliverMin = p.deliverMin;
bool partialPaymentAllowed = p.partialPaymentAllowed;
// bool defaultPathsAllowed = p.defaultPathsAllowed;
// bool limitQuality = p.limitQuality;
beast::Journal j_ = p.j;
if (auto const ter = requireAuth(p.ctx.view(), mptIssue, p.srcAccountID);
auto view = [&p]() -> ApplyView& { return p.ctx.view(); };
// Below this line, copied straight from Payment::doApply
//-------------------------------------------------------
JLOG(j_.trace()) << " dstAmount=" << dstAmount.getFullText();
auto const& mptIssue = dstAmount.get<MPTIssue>();
if (auto const ter = requireAuth(view(), mptIssue, account_);
ter != tesSUCCESS)
return ter;
if (auto const ter = requireAuth(p.ctx.view(), mptIssue, p.dstAccountID);
if (auto const ter = requireAuth(view(), mptIssue, dstAccountID);
ter != tesSUCCESS)
return ter;
if (auto const ter =
canTransfer(p.ctx.view(), mptIssue, p.srcAccountID, p.dstAccountID);
if (auto const ter = canTransfer(view(), mptIssue, account_, dstAccountID);
ter != tesSUCCESS)
return ter;
if (auto err = verifyDepositPreauth(
p.ctx.tx,
p.ctx.view(),
p.srcAccountID,
p.dstAccountID,
p.sleDst,
p.ctx.journal);
ctx_.tx, ctx_.view(), account_, dstAccountID, sleDst, ctx_.journal);
!isTesSuccess(err))
return err;
@@ -682,45 +724,53 @@ Payment::makeMPTDirectPayment(Payment::RipplePaymentParams const& p)
// Transfer rate
Rate rate{QUALITY_ONE};
// Payment between the holders
if (p.srcAccountID != issuer && p.dstAccountID != issuer)
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(
p.ctx.view(), {p.srcAccountID, p.dstAccountID}, mptIssue))
if (isAnyFrozen(view(), {account_, dstAccountID}, mptIssue))
return tecLOCKED;
// Get the rate for a payment between the holders.
rate = transferRate(p.ctx.view(), mptIssue.getMptID());
rate = transferRate(view(), mptIssue.getMptID());
}
// Amount to deliver.
STAmount amountDeliver = p.dstAmount;
STAmount amountDeliver = dstAmount;
// Factor in the transfer rate.
// No rounding. It'll change once MPT integrated into DEX.
STAmount requiredMaxSourceAmount = multiply(p.dstAmount, rate);
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 (p.partialPaymentAllowed && requiredMaxSourceAmount > p.maxSourceAmount)
if (partialPaymentAllowed && requiredMaxSourceAmount > maxSourceAmount)
{
requiredMaxSourceAmount = p.maxSourceAmount;
requiredMaxSourceAmount = maxSourceAmount;
// No rounding. It'll change once MPT integrated into DEX.
amountDeliver = divide(p.maxSourceAmount, rate);
amountDeliver = divide(maxSourceAmount, rate);
}
if (requiredMaxSourceAmount > p.maxSourceAmount ||
(p.deliverMin && amountDeliver < *p.deliverMin))
if (requiredMaxSourceAmount > maxSourceAmount ||
(deliverMin && amountDeliver < *deliverMin))
return tecPATH_PARTIAL;
PaymentSandbox pv(&p.ctx.view());
auto res = accountSend(
pv, p.srcAccountID, p.dstAccountID, amountDeliver, p.ctx.journal);
PaymentSandbox pv(&view());
auto res =
accountSend(pv, account_, dstAccountID, amountDeliver, ctx_.journal);
if (res == tesSUCCESS)
pv.apply(p.ctx.rawView());
{
pv.apply(ctx_.rawView());
// If the actual amount delivered is different from the original
// amount due to partial payment or transfer fee, we need to update
// DelieveredAmount using the actual delivered amount
if (view().rules().enabled(fixMPTDeliveredAmount) &&
amountDeliver != dstAmount)
ctx_.deliver(amountDeliver);
}
else if (res == tecINSUFFICIENT_FUNDS || res == tecPATH_DRY)
res = tecPATH_PARTIAL;