ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-04 19:25:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Add delivered_amount to tx result for CheckCash (RIPD-1623)

Browse Source
This commit is contained in:
Scott Schurr
2018-04-03 08:21:05 -07:00
committed by Nikolaos D. Bougalis
parent 6bd0b850a0
commit 7bc163ee4c
5 changed files with 142 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

64
src/ripple/app/tx/impl/CashCheck.cpp
View File

@@ -282,6 +282,8 @@ CashCheck::doApply ()
// If it is not a check to self (as should be the case), then there's
// work to do...
auto viewJ = ctx_.app.journal ("View");
auto const optDeliverMin = ctx_.tx[~sfDeliverMin];
bool const doFix1623 {ctx_.view().rules().enabled (fix1623)};
if (srcId != account_)
{
STAmount const sendMax {sleCheck->getFieldAmount (sfSendMax)};
@@ -300,17 +302,9 @@ CashCheck::doApply ()
STAmount const srcLiquid {xrpLiquid (psb, srcId, -1, viewJ)};
// Now, how much do they need in order to be successful?
STAmount const xrpDeliver {
[&sendMax, &srcLiquid] (STTx const& tx)
{
// If the check cash specified Amount deliver exactly that.
auto const optDeliverMin = tx[~sfDeliverMin];
if (! optDeliverMin)
return tx.getFieldAmount (sfAmount);
return (std::max (
*optDeliverMin, std::min (sendMax, srcLiquid)));
}(ctx_.tx)};
STAmount const xrpDeliver {optDeliverMin ?
std::max (*optDeliverMin, std::min (sendMax, srcLiquid)) :
ctx_.tx.getFieldAmount (sfAmount)};
if (srcLiquid < xrpDeliver)
{
@@ -321,29 +315,27 @@ CashCheck::doApply ()
<< " < " << xrpDeliver.getFullText();
return tecUNFUNDED_PAYMENT;
}
else
{
// The source account has enough XRP so make the ledger change.
transferXRP (psb, srcId, account_, xrpDeliver, viewJ);
}
if (optDeliverMin && doFix1623)
// Set the DeliveredAmount metadata.
ctx_.deliver (xrpDeliver);
// The source account has enough XRP so make the ledger change.
transferXRP (psb, srcId, account_, xrpDeliver, viewJ);
}
else
{
// Let flow() do the heavy lifting on a check for an IOU.
auto const optDeliverMin = ctx_.tx[~sfDeliverMin];
STAmount const flowDeliver {[&optDeliverMin] (STTx const& tx)
{
// If the check cash specified Amount deliver exactly that.
if (! optDeliverMin)
return static_cast<STAmount>(tx[sfAmount]);
// We can't use the maximum possible currency here because
// there might be a gateway transfer rate to account for.
// Since the transfer rate cannot exceed 200%, we use 1/2
// maxValue for our limit.
return STAmount { optDeliverMin->issue(),
STAmount::cMaxValue / 2, STAmount::cMaxOffset };
}(ctx_.tx)};
//
// Note that for DeliverMin we don't know exactly how much
// currency we want flow to deliver. We can't ask for the
// maximum possible currency because there might be a gateway
// transfer rate to account for. Since the transfer rate cannot
// exceed 200%, we use 1/2 maxValue as our limit.
STAmount const flowDeliver {optDeliverMin ?
STAmount { optDeliverMin->issue(),
STAmount::cMaxValue / 2, STAmount::cMaxOffset } :
static_cast<STAmount>(ctx_.tx[sfAmount])};
// Call the payment engine's flow() to do the actual work.
auto const result = flow (psb, flowDeliver, srcId, account_,
@@ -364,10 +356,16 @@ CashCheck::doApply ()
}
// Make sure that deliverMin was satisfied.
if (optDeliverMin && result.actualAmountOut < *optDeliverMin)
if (optDeliverMin)
{
JLOG(ctx_.journal.warn()) << "flow did not produce DeliverMin.";
return tecPATH_PARTIAL;
if (result.actualAmountOut < *optDeliverMin)
{
JLOG(ctx_.journal.warn()) << "flow did not produce DeliverMin.";
return tecPATH_PARTIAL;
}
if (doFix1623)
// Set the delivered_amount metadata.
ctx_.deliver (result.actualAmountOut);
}
}
}

4
src/ripple/protocol/Feature.h
View File

@@ -78,7 +78,8 @@ class FeatureCollections
"Checks",
"fix1571",
"fix1543",
"ValidationCookies"
"ValidationCookies",
"fix1623"
};
std::vector<uint256> features;
@@ -363,6 +364,7 @@ extern uint256 const featureChecks;
extern uint256 const fix1571;
extern uint256 const fix1543;
extern uint256 const featureValidationCookies;
extern uint256 const fix1623;
} // ripple

8
src/ripple/protocol/impl/Feature.cpp
View File

@@ -106,11 +106,12 @@ detail::supportedAmendments ()
{ "67A34F2CF55BFC0F93AACD5B281413176FEE195269FA6D95219A2DF738671172 fix1513" },
{ "B9E739B8296B4A1BB29BE990B17D66E21B62A300A909F25AC55C22D6C72E1F9D fix1523" },
{ "1D3463A5891F9E589C5AE839FFAC4A917CE96197098A1EF22304E1BC5B98A454 fix1528" },
{ "F64E1EABBE79D55B3BB82020516CEC2C582A98A6BFE20FBE9BB6A0D233418064 DepositAuth"},
{ "157D2D480E006395B76F948E3E07A45A05FE10230D88A7993C71F97AE4B1F2D1 Checks"},
{ "F64E1EABBE79D55B3BB82020516CEC2C582A98A6BFE20FBE9BB6A0D233418064 DepositAuth" },
{ "157D2D480E006395B76F948E3E07A45A05FE10230D88A7993C71F97AE4B1F2D1 Checks" },
{ "7117E2EC2DBF119CA55181D69819F1999ECEE1A0225A7FD2B9ED47940968479C fix1571" },
{ "CA7C02118BA27599528543DFE77BA6838D1B0F43B447D4D7F53523CE6A0E9AC2 fix1543" },
{ "8413364A1E27704241F7106789570A4B36EE2AFA14F94828E78BE942AB2F24BE ValidationCookies"}
{ "8413364A1E27704241F7106789570A4B36EE2AFA14F94828E78BE942AB2F24BE ValidationCookies" },
{ "58BE9B5968C4DA7C59BA900961828B113E5490699B21877DEF9A31E9D0FE5D5F fix1623" }
};
return supported;
}
@@ -163,5 +164,6 @@ uint256 const featureChecks = *getRegisteredFeature("Checks");
uint256 const fix1571 = *getRegisteredFeature("fix1571");
uint256 const fix1543 = *getRegisteredFeature("fix1543");
uint256 const featureValidationCookies = *getRegisteredFeature("ValidationCookies");
uint256 const fix1623 = *getRegisteredFeature("fix1623");
} // ripple

24
src/ripple/rpc/impl/RPCHelpers.cpp
View File

@@ -18,10 +18,12 @@
//==============================================================================
#include <ripple/app/ledger/LedgerMaster.h>
#include <ripple/app/ledger/OpenLedger.h>
#include <ripple/app/misc/Transaction.h>
#include <ripple/ledger/View.h>
#include <ripple/net/RPCErr.h>
#include <ripple/protocol/AccountID.h>
#include <ripple/protocol/Feature.h>
#include <ripple/rpc/Context.h>
#include <ripple/rpc/impl/RPCHelpers.h>
#include <boost/algorithm/string/case_conv.hpp>
@@ -400,10 +402,26 @@ addPaymentDeliveredAmount(Json::Value& meta, RPC::Context& context,
if (! transaction)
return;
auto serializedTx = transaction->getSTransaction ();
if (! serializedTx || serializedTx->getTxnType () != ttPAYMENT)
auto const serializedTx = transaction->getSTransaction ();
if (! serializedTx)
return;
{
// Only include this field for Payment and CheckCash transactions.
TxType const tt {serializedTx->getTxnType()};
if ((tt != ttPAYMENT) && (tt != ttCHECK_CASH))
return;
// Only include this field for CheckCash transactions if the fix
// is enabled.
if (tt == ttCHECK_CASH)
{
auto const view = context.app.openLedger().current();
if (!view || !view->rules().enabled (fix1623))
return;
}
}
if (transactionMeta)
{
if (transactionMeta->getResultTER() != tesSUCCESS)
@@ -437,7 +455,7 @@ addPaymentDeliveredAmount(Json::Value& meta, RPC::Context& context,
// then its absence indicates that the amount delivered is listed in the
// Amount field. DeliveredAmount went live January 24, 2014.
using namespace std::chrono_literals;
auto ct =
auto const ct =
context.ledgerMaster.getCloseTimeBySeq (transaction->getLedger ());
if (ct && (*ct > NetClock::time_point{446000000s}))
{

92
src/test/app/Check_test.cpp
View File

@@ -199,6 +199,29 @@ class Check_test : public beast::unit_test::suite
return ret;
}
// Helper function that verifies the expected DeliveredAmount is present.
//
// NOTE: the function _infers_ the transaction to operate on by calling
// env.tx(), which returns the result from the most recent transaction.
void verifyDeliveredAmount (test::jtx::Env& env, STAmount const& amount)
{
// Get the hash for the most recent transaction.
std::string const txHash {env.tx()->getJson (0)[jss::hash].asString()};
// Verify DeliveredAmount and delivered_amount metadata are correct.
env.close();
Json::Value const meta = env.rpc ("tx", txHash)[jss::result][jss::meta];
// Expect there to be a DeliveredAmount field.
if (! BEAST_EXPECT(meta.isMember (sfDeliveredAmount.jsonName)))
return;
// DeliveredAmount and delivered_amount should both be present and
// equal amount.
BEAST_EXPECT (meta[sfDeliveredAmount.jsonName] == amount.getJson (0));
BEAST_EXPECT (meta[jss::delivered_amount] == amount.getJson (0));
}
void testEnabled()
{
testcase ("Enabled");
@@ -607,7 +630,7 @@ class Check_test : public beast::unit_test::suite
// because one unit of alice's reserve is released when the
// check is consumed.
env (check::cash (bob, chkId, check::DeliverMin (checkAmount)));
env.close();
verifyDeliveredAmount (env, drops(checkAmount.mantissa()));
env.require (balance (alice, reserve));
env.require (balance (bob,
startBalance + checkAmount - drops (baseFeeDrops * 3)));
@@ -639,7 +662,7 @@ class Check_test : public beast::unit_test::suite
// bob decides to get what he can from the bounced check.
env (check::cash (bob, chkId, check::DeliverMin (drops(1))));
env.close();
verifyDeliveredAmount (env, drops(checkAmount.mantissa() - 1));
env.require (balance (alice, reserve));
env.require (balance (bob,
startBalance + checkAmount - drops ((baseFeeDrops * 2) + 1)));
@@ -829,7 +852,7 @@ class Check_test : public beast::unit_test::suite
// bob sets a DeliverMin of 7 and gets all that alice has.
env (check::cash (bob, chkId9, check::DeliverMin (USD(7))));
env.close();
verifyDeliveredAmount (env, USD(8));
env.require (balance (alice, USD(0)));
env.require (balance (bob, USD(8)));
BEAST_EXPECT (checksOnAccount (env, alice).size() == 3);
@@ -844,7 +867,7 @@ class Check_test : public beast::unit_test::suite
// Using DeliverMin for the SendMax value of the check (and no
// transfer fees) should work just like setting Amount.
env (check::cash (bob, chkId7, check::DeliverMin (USD(7))));
env.close();
verifyDeliveredAmount (env, USD(7));
env.require (balance (alice, USD(0)));
env.require (balance (bob, USD(8)));
BEAST_EXPECT (checksOnAccount (env, alice).size() == 2);
@@ -859,7 +882,7 @@ class Check_test : public beast::unit_test::suite
// alice has USD(8). If bob uses the check for USD(6) and uses a
// DeliverMin of 4, he should get the SendMax value of the check.
env (check::cash (bob, chkId6, check::DeliverMin (USD(4))));
env.close();
verifyDeliveredAmount (env, USD(6));
env.require (balance (alice, USD(2)));
env.require (balance (bob, USD(6)));
BEAST_EXPECT (checksOnAccount (env, alice).size() == 1);
@@ -870,7 +893,7 @@ class Check_test : public beast::unit_test::suite
// bob cashes the last remaining check setting a DeliverMin.
// of exactly alice's remaining USD.
env (check::cash (bob, chkId8, check::DeliverMin (USD(2))));
env.close();
verifyDeliveredAmount (env, USD(2));
env.require (balance (alice, USD(0)));
env.require (balance (bob, USD(8)));
BEAST_EXPECT (checksOnAccount (env, alice).size() == 0);
@@ -923,7 +946,7 @@ class Check_test : public beast::unit_test::suite
// Since bob set his limit low, he cashes the check with a
// DeliverMin and hits his trust limit.
env (check::cash (bob, chkId, check::DeliverMin (USD(4))));
env.close();
verifyDeliveredAmount (env, USD(5));
env.require (balance (alice, USD(3)));
env.require (balance (bob, USD(5)));
BEAST_EXPECT (checksOnAccount (env, alice).size() == 0);
@@ -1042,7 +1065,7 @@ class Check_test : public beast::unit_test::suite
// bob decides that he'll accept anything USD(75) or up.
// He gets USD(100).
env (check::cash (bob, chkId125, check::DeliverMin (USD(75))));
env.close();
verifyDeliveredAmount (env, USD(100));
env.require (balance (alice, USD(1000 - 125)));
env.require (balance (bob, USD( 0 + 100)));
BEAST_EXPECT (checksOnAccount (env, alice).size() == 1);
@@ -1440,7 +1463,7 @@ class Check_test : public beast::unit_test::suite
env (check::cash (bob, chkIdU, USD(20)));
env.close();
env (check::cash (bob, chkIdX, check::DeliverMin (XRP(10))));
env.close();
verifyDeliveredAmount (env, XRP(10));
// Try to cash an expired check.
env (check::cash (bob, chkIdExp, XRP(10)), ter (tecEXPIRED));
@@ -1506,7 +1529,7 @@ class Check_test : public beast::unit_test::suite
env (trust(gw, bob["USD"](0), tfClearFreeze));
env.close();
env (check::cash (bob, chkIdFroz3, check::DeliverMin (USD(1))));
env.close();
verifyDeliveredAmount (env, USD(3));
env.require (balance (alice, USD(14)));
env.require (balance (bob, USD( 6)));
@@ -1745,6 +1768,54 @@ class Check_test : public beast::unit_test::suite
env.close();
}
void testFix1623Enable ()
{
testcase ("Fix1623 enable");
using namespace test::jtx;
auto testEnable = [this] (FeatureBitset const& features, bool hasFields)
{
// Unless fix1623 is enabled a "tx" RPC command should return
// neither "DeliveredAmount" nor "delivered_amount" on a CheckCash
// transaction.
Account const alice {"alice"};
Account const bob {"bob"};
Env env {*this, features};
auto const closeTime =
fix1449Time() + 100 * env.closed()->info().closeTimeResolution;
env.close (closeTime);
env.fund (XRP(1000), alice, bob);
env.close();
uint256 const chkId {getCheckIndex (alice, env.seq (alice))};
env (check::create (alice, bob, XRP(200)));
env.close();
env (check::cash (bob, chkId, check::DeliverMin (XRP(100))));
// Get the hash for the most recent transaction.
std::string const txHash {
env.tx()->getJson (0)[jss::hash].asString()};
// DeliveredAmount and delivered_amount are either present or
// not present in the metadata returned by "tx" based on fix1623.
env.close();
Json::Value const meta =
env.rpc ("tx", txHash)[jss::result][jss::meta];
BEAST_EXPECT(meta.isMember (
sfDeliveredAmount.jsonName) == hasFields);
BEAST_EXPECT(meta.isMember (jss::delivered_amount) == hasFields);
};
// Run both the disabled and enabled cases.
testEnable (supported_amendments() - fix1623, false);
testEnable (supported_amendments(), true);
}
public:
void run ()
{
@@ -1758,6 +1829,7 @@ public:
testCashInvalid();
testCancelValid();
testCancelInvalid();
testFix1623Enable();
}
};