mirror of
https://github.com/XRPLF/rippled.git
synced 2026-04-29 15:37:57 +00:00
Fix false dry and other payment bugs:
The Owner count could decrease while evaluating a strand, causing different behavior in forward passes and reverses passes. The fix treats a decreased owner count like a deferred credit. In some situations, deferred credits could cause an XRP balance to be calculated as negative, triggering some asserts. When XRP is used as a bridge currency, a path could be falsely marked as dry. This happens when the XRP/XXX offer recursively checks the XXX/XRP offer and the XXX/XRP offer could not satisfy the request in a single call. With a single strand and limit quality the old payment code incorrectly computed with multiquailty set to true. This could cause the total quality to go below the requested quality even if there was liquidity available above the requested quality value.
This commit is contained in:
@@ -83,15 +83,4 @@ STAmount creditBalance (
|
||||
return result;
|
||||
}
|
||||
|
||||
IOUAmount
|
||||
creditBalance2 (
|
||||
ReadView const& v,
|
||||
AccountID const& acc,
|
||||
AccountID const& iss,
|
||||
Currency const& cur)
|
||||
{
|
||||
return toAmount<IOUAmount> (creditBalance (v, acc, iss, cur));
|
||||
}
|
||||
|
||||
|
||||
} // ripple
|
||||
|
||||
@@ -60,13 +60,6 @@ STAmount creditBalance (
|
||||
AccountID const& account,
|
||||
AccountID const& issuer,
|
||||
Currency const& currency);
|
||||
|
||||
IOUAmount
|
||||
creditBalance2 (
|
||||
ReadView const& v,
|
||||
AccountID const& acc,
|
||||
AccountID const& iss,
|
||||
Currency const& cur);
|
||||
/** @} */
|
||||
|
||||
} // ripple
|
||||
|
||||
@@ -291,6 +291,7 @@ TER RippleCalc::rippleCalculate ()
|
||||
boost::container::flat_set<uint256> unfundedOffersFromBestPaths;
|
||||
|
||||
int iPass = 0;
|
||||
auto const dcSwitch = dcSwitchover(view.info().parentCloseTime);
|
||||
|
||||
while (resultCode == temUNCERTAIN)
|
||||
{
|
||||
@@ -306,8 +307,12 @@ TER RippleCalc::rippleCalculate ()
|
||||
if (pathState->quality())
|
||||
// Only do active paths.
|
||||
{
|
||||
// If computing the only non-dry path, compute multi-quality.
|
||||
multiQuality = ((pathStateList_.size () - iDry) == 1);
|
||||
// If computing the only non-dry path, and not limiting quality,
|
||||
// compute multi-quality.
|
||||
multiQuality = dcSwitch
|
||||
? !inputFlags.limitQuality &&
|
||||
((pathStateList_.size () - iDry) == 1)
|
||||
: ((pathStateList_.size () - iDry) == 1);
|
||||
|
||||
// Update to current amount processed.
|
||||
pathState->reset (actualAmountIn_, actualAmountOut_);
|
||||
|
||||
@@ -20,13 +20,16 @@
|
||||
#include <BeastConfig.h>
|
||||
#include <ripple/app/paths/cursor/RippleLiquidity.h>
|
||||
#include <ripple/basics/Log.h>
|
||||
#include <ripple/ledger/View.h>
|
||||
|
||||
namespace ripple {
|
||||
namespace path {
|
||||
|
||||
TER PathCursor::advanceNode (STAmount const& amount, bool reverse) const
|
||||
TER PathCursor::advanceNode (STAmount const& amount, bool reverse, bool callerHasLiquidity) const
|
||||
{
|
||||
bool multi = multiQuality_ || amount == zero;
|
||||
bool const multi = dcSwitchover (view ().info ().parentCloseTime)
|
||||
? (multiQuality_ || (!callerHasLiquidity && amount == zero))
|
||||
: (multiQuality_ || amount == zero);
|
||||
|
||||
// If the multiQuality_ is unchanged, use the PathCursor we're using now.
|
||||
if (multi == multiQuality_)
|
||||
|
||||
@@ -35,7 +35,8 @@ TER PathCursor::deliverNodeForward (
|
||||
AccountID const& uInAccountID, // --> Input owner's account.
|
||||
STAmount const& saInReq, // --> Amount to deliver.
|
||||
STAmount& saInAct, // <-- Amount delivered, this invocation.
|
||||
STAmount& saInFees) const // <-- Fees charged, this invocation.
|
||||
STAmount& saInFees, // <-- Fees charged, this invocation.
|
||||
bool callerHasLiquidity) const
|
||||
{
|
||||
TER resultCode = tesSUCCESS;
|
||||
|
||||
@@ -66,7 +67,7 @@ TER PathCursor::deliverNodeForward (
|
||||
|
||||
// Determine values for pass to adjust saInAct, saInFees, and
|
||||
// node().saFwdDeliver.
|
||||
advanceNode (saInAct, false);
|
||||
advanceNode (saInAct, false, callerHasLiquidity);
|
||||
|
||||
// If needed, advance to next funded offer.
|
||||
|
||||
@@ -224,7 +225,8 @@ TER PathCursor::deliverNodeForward (
|
||||
node().offerOwnerAccount_, // --> Current holder.
|
||||
saOutPassMax, // --> Amount available.
|
||||
saOutPassAct, // <-- Amount delivered.
|
||||
saOutPassFees); // <-- Fees charged.
|
||||
saOutPassFees, // <-- Fees charged.
|
||||
saInAct > zero);
|
||||
|
||||
if (resultCode != tesSUCCESS)
|
||||
break;
|
||||
|
||||
@@ -39,8 +39,9 @@ TER PathCursor::deliverNodeReverseImpl (
|
||||
AccountID const& uOutAccountID, // --> Output owner's account.
|
||||
STAmount const& saOutReq, // --> Funds requested to be
|
||||
// delivered for an increment.
|
||||
STAmount& saOutAct // <-- Funds actually delivered for an
|
||||
STAmount& saOutAct, // <-- Funds actually delivered for an
|
||||
// increment
|
||||
bool callerHasLiquidity
|
||||
) const
|
||||
{
|
||||
TER resultCode = tesSUCCESS;
|
||||
@@ -73,7 +74,7 @@ TER PathCursor::deliverNodeReverseImpl (
|
||||
return telFAILED_PROCESSING;
|
||||
}
|
||||
|
||||
resultCode = advanceNode (saOutAct, true);
|
||||
resultCode = advanceNode (saOutAct, true, callerHasLiquidity);
|
||||
// If needed, advance to next funded offer.
|
||||
|
||||
if (resultCode != tesSUCCESS || !node().offerIndex_)
|
||||
@@ -251,7 +252,19 @@ TER PathCursor::deliverNodeReverseImpl (
|
||||
resultCode = increment(-1).deliverNodeReverseImpl(
|
||||
node().offerOwnerAccount_,
|
||||
saInPassReq,
|
||||
saInPassAct);
|
||||
saInPassAct,
|
||||
saOutAct > zero);
|
||||
|
||||
if (dcSwitchover(view().info().parentCloseTime))
|
||||
{
|
||||
// The recursive call is dry this time, but we have liquidity
|
||||
// from previous calls
|
||||
if (resultCode == tecPATH_DRY && saOutAct > zero)
|
||||
{
|
||||
resultCode = tesSUCCESS;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
JLOG (j_.trace())
|
||||
<< "deliverNodeReverse: offer --> OFFER --> ? :"
|
||||
@@ -372,7 +385,7 @@ TER PathCursor::deliverNodeReverse (
|
||||
for (int i = nodeIndex_; i >= 0 && !node (i).isAccount(); --i)
|
||||
node (i).directory.restart (multiQuality_);
|
||||
|
||||
return deliverNodeReverseImpl(uOutAccountID, saOutReq, saOutAct);
|
||||
return deliverNodeReverseImpl(uOutAccountID, saOutReq, saOutAct, /*callerHasLiquidity*/false);
|
||||
}
|
||||
|
||||
} // path
|
||||
|
||||
@@ -42,7 +42,8 @@ TER PathCursor::forwardLiquidity () const
|
||||
previousNode().account_,
|
||||
previousNode().saFwdDeliver, // Previous is sending this much.
|
||||
saInAct,
|
||||
saInFees);
|
||||
saInFees,
|
||||
false);
|
||||
|
||||
assert (resultCode != tesSUCCESS ||
|
||||
previousNode().saFwdDeliver == saInAct + saInFees);
|
||||
|
||||
@@ -79,7 +79,7 @@ private:
|
||||
<-- uOfferIndex : 0=end of list.
|
||||
*/
|
||||
TER advanceNode (bool reverse) const;
|
||||
TER advanceNode (STAmount const& amount, bool reverse) const;
|
||||
TER advanceNode (STAmount const& amount, bool reverse, bool callerHasLiquidity) const;
|
||||
|
||||
// To deliver from an order book, when computing
|
||||
TER deliverNodeReverse (
|
||||
@@ -91,13 +91,15 @@ private:
|
||||
TER deliverNodeReverseImpl (
|
||||
AccountID const& uOutAccountID,
|
||||
STAmount const& saOutReq,
|
||||
STAmount& saOutAct) const;
|
||||
STAmount& saOutAct,
|
||||
bool callerHasLiquidity) const;
|
||||
|
||||
TER deliverNodeForward (
|
||||
AccountID const& uInAccountID,
|
||||
STAmount const& saInReq,
|
||||
STAmount& saInAct,
|
||||
STAmount& saInFees) const;
|
||||
STAmount& saInFees,
|
||||
bool callerHasLiquidity) const;
|
||||
|
||||
// VFALCO TODO Rename this to view()
|
||||
PaymentSandbox&
|
||||
|
||||
@@ -197,7 +197,8 @@ maxFlow (
|
||||
AccountID const& dst,
|
||||
Currency const& cur)
|
||||
{
|
||||
auto const srcOwed = creditBalance2 (sb, dst, src, cur);
|
||||
auto const srcOwed = toAmount<IOUAmount> (
|
||||
accountHolds (sb, src, cur, dst, fhIGNORE_FREEZE, beast::Journal{}));
|
||||
|
||||
if (srcOwed.signum () > 0)
|
||||
return {srcOwed, true};
|
||||
@@ -211,7 +212,8 @@ DirectStepI::redeems (ReadView const& sb, bool fwd) const
|
||||
{
|
||||
if (!fwd)
|
||||
{
|
||||
auto const srcOwed = creditBalance2 (sb, dst_, src_, currency_);
|
||||
auto const srcOwed = accountHolds (
|
||||
sb, src_, currency_, dst_, fhIGNORE_FREEZE, beast::Journal{});
|
||||
return srcOwed.signum () > 0;
|
||||
}
|
||||
else
|
||||
|
||||
@@ -901,12 +901,289 @@ struct Flow_test : public beast::unit_test::suite
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
testFalseDryHelper (jtx::Env& env)
|
||||
{
|
||||
using namespace jtx;
|
||||
|
||||
auto const gw = Account ("gateway");
|
||||
auto const USD = gw["USD"];
|
||||
auto const EUR = gw["EUR"];
|
||||
Account const alice ("alice");
|
||||
Account const bob ("bob");
|
||||
Account const carol ("carol");
|
||||
|
||||
auto const closeTime = dcSoTime() +
|
||||
100 * env.closed ()->info ().closeTimeResolution;
|
||||
env.close (closeTime);
|
||||
|
||||
env.fund (XRP(10000), alice, carol, gw);
|
||||
env.fund (reserve(env, 5), bob);
|
||||
env.trust (USD (1000), alice, bob, carol);
|
||||
env.trust (EUR (1000), alice, bob, carol);
|
||||
|
||||
|
||||
env (pay (gw, alice, EUR (50)));
|
||||
env (pay (gw, bob, USD (50)));
|
||||
|
||||
// Bob has _just_ slightly less than 50 xrp available
|
||||
// If his owner count changes, he will have more liquidity.
|
||||
// This is one error case to test (when FlowV2 is used).
|
||||
// Computing the incomming xrp to the XRP/USD offer will require two
|
||||
// recursive calls to the EUR/XRP offer. The second call will return
|
||||
// tecPATH_DRY, but the entire path should not be marked as dry. This
|
||||
// is the second error case to test (when flowV1 is used).
|
||||
env (offer (bob, EUR (50), XRP (50)));
|
||||
env (offer (bob, XRP (50), USD (50)));
|
||||
|
||||
env (pay (alice, carol, USD (1000000)), path (~XRP, ~USD),
|
||||
sendmax (EUR (500)),
|
||||
txflags (tfNoRippleDirect | tfPartialPayment));
|
||||
|
||||
auto const carolUSD = env.balance(carol, USD).value();
|
||||
expect (carolUSD > USD (0) && carolUSD < USD (50));
|
||||
}
|
||||
|
||||
void
|
||||
testFalseDry ()
|
||||
{
|
||||
testcase ("falseDryChanges");
|
||||
using namespace jtx;
|
||||
{
|
||||
Env env (*this, features (featureFlowV2));
|
||||
testFalseDryHelper (env);
|
||||
}
|
||||
{
|
||||
Env env (*this);
|
||||
testFalseDryHelper (env);
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
testLimitQuality ()
|
||||
{
|
||||
// Single path with two offers and limit quality. The quality limit is
|
||||
// such that the first offer should be taken but the second should not.
|
||||
// The total amount delivered should be the sum of the two offers and
|
||||
// sendMax should be more than the first offer.
|
||||
testcase ("limitQuality");
|
||||
using namespace jtx;
|
||||
|
||||
auto const gw = Account ("gateway");
|
||||
auto const USD = gw["USD"];
|
||||
Account const alice ("alice");
|
||||
Account const bob ("bob");
|
||||
Account const carol ("carol");
|
||||
|
||||
auto const timeDelta = Env{*this}.closed ()->info ().closeTimeResolution;
|
||||
|
||||
for(auto const& t :{dcSoTime(), flowV2SoTime()}){
|
||||
for(auto const& d: {-timeDelta*100, +timeDelta*100}){
|
||||
auto const closeTime = t+d;
|
||||
Env env (*this);
|
||||
env.close (closeTime);
|
||||
|
||||
env.fund (XRP(10000), alice, bob, carol, gw);
|
||||
|
||||
env.trust (USD(100), alice, bob, carol);
|
||||
env (pay (gw, bob, USD (100)));
|
||||
env (offer (bob, XRP (50), USD (50)));
|
||||
env (offer (bob, XRP (100), USD (50)));
|
||||
|
||||
auto expectedResult =
|
||||
closeTime < dcSoTime () ? tecPATH_DRY : tesSUCCESS;
|
||||
env (pay (alice, carol, USD (100)), path (~USD), sendmax (XRP (100)),
|
||||
txflags (tfNoRippleDirect | tfPartialPayment | tfLimitQuality),
|
||||
ter (expectedResult));
|
||||
|
||||
if (expectedResult == tesSUCCESS)
|
||||
env.require (balance (carol, USD (50)));
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
// Helper function that returns the reserve on an account based on
|
||||
// the passed in number of owners.
|
||||
static XRPAmount reserve(jtx::Env& env, std::uint32_t count)
|
||||
{
|
||||
return env.current()->fees().accountReserve (count);
|
||||
}
|
||||
|
||||
// Helper function that returns the Offers on an account.
|
||||
static std::vector<std::shared_ptr<SLE const>>
|
||||
offersOnAccount (jtx::Env& env, jtx::Account account)
|
||||
{
|
||||
std::vector<std::shared_ptr<SLE const>> result;
|
||||
forEachItem (*env.current (), account,
|
||||
[&env, &result](std::shared_ptr<SLE const> const& sle)
|
||||
{
|
||||
if (sle->getType() == ltOFFER)
|
||||
result.push_back (sle);
|
||||
});
|
||||
return result;
|
||||
}
|
||||
|
||||
void
|
||||
testSelfPayment1()
|
||||
{
|
||||
testcase ("Self-payment 1");
|
||||
|
||||
// In this test case the new flow code mis-computes the amount
|
||||
// of money to move. Fortunately the new code's re-execute
|
||||
// check catches the problem and throws out the transaction.
|
||||
//
|
||||
// The old payment code handles the payment correctly.
|
||||
using namespace jtx;
|
||||
|
||||
auto const gw1 = Account ("gw1");
|
||||
auto const gw2 = Account ("gw2");
|
||||
auto const alice = Account ("alice");
|
||||
auto const USD = gw1["USD"];
|
||||
auto const EUR = gw2["EUR"];
|
||||
|
||||
Env env (*this, features (featureFlowV2));
|
||||
|
||||
auto const closeTime =
|
||||
flowV2SoTime () + 100 * env.closed ()->info ().closeTimeResolution;
|
||||
env.close (closeTime);
|
||||
|
||||
env.fund (XRP (1000000), gw1, gw2);
|
||||
env.close ();
|
||||
|
||||
// The fee that's charged for transactions.
|
||||
auto const f = env.current ()->fees ().base;
|
||||
|
||||
env.fund (reserve (env, 3) + f * 4, alice);
|
||||
env.close ();
|
||||
|
||||
env (trust (alice, USD (2000)));
|
||||
env (trust (alice, EUR (2000)));
|
||||
env.close ();
|
||||
|
||||
env (pay (gw1, alice, USD (1)));
|
||||
env (pay (gw2, alice, EUR (1000)));
|
||||
env.close ();
|
||||
|
||||
env (offer (alice, USD (500), EUR (600)));
|
||||
env.close ();
|
||||
|
||||
env.require (owners (alice, 3));
|
||||
env.require (balance (alice, USD (1)));
|
||||
env.require (balance (alice, EUR (1000)));
|
||||
|
||||
auto aliceOffers = offersOnAccount (env, alice);
|
||||
expect (aliceOffers.size () == 1);
|
||||
for (auto const& offerPtr : aliceOffers)
|
||||
{
|
||||
auto const offer = *offerPtr;
|
||||
expect (offer[sfLedgerEntryType] == ltOFFER);
|
||||
expect (offer[sfTakerGets] == EUR (600));
|
||||
expect (offer[sfTakerPays] == USD (500));
|
||||
}
|
||||
|
||||
env (pay (alice, alice, EUR (600)), sendmax (USD (500)),
|
||||
txflags (tfPartialPayment));
|
||||
env.close ();
|
||||
|
||||
env.require (owners (alice, 3));
|
||||
env.require (balance (alice, USD (1)));
|
||||
env.require (balance (alice, EUR (1000)));
|
||||
aliceOffers = offersOnAccount (env, alice);
|
||||
expect (aliceOffers.size () == 1);
|
||||
for (auto const& offerPtr : aliceOffers)
|
||||
{
|
||||
auto const offer = *offerPtr;
|
||||
expect (offer[sfLedgerEntryType] == ltOFFER);
|
||||
expect (offer[sfTakerGets] == EUR (598.8));
|
||||
expect (offer[sfTakerPays] == USD (499));
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
testSelfPayment2()
|
||||
{
|
||||
testcase ("Self-payment 2");
|
||||
|
||||
// In this case the difference between the old payment code and
|
||||
// the new is the values left behind in the offer. Not saying either
|
||||
// ios ring, they are just different.
|
||||
using namespace jtx;
|
||||
|
||||
auto const gw1 = Account ("gw1");
|
||||
auto const gw2 = Account ("gw2");
|
||||
auto const alice = Account ("alice");
|
||||
auto const USD = gw1["USD"];
|
||||
auto const EUR = gw2["EUR"];
|
||||
|
||||
Env env (*this, features (featureFlowV2));
|
||||
|
||||
auto const closeTime =
|
||||
flowV2SoTime () + 100 * env.closed ()->info ().closeTimeResolution;
|
||||
env.close (closeTime);
|
||||
|
||||
env.fund (XRP (1000000), gw1, gw2);
|
||||
env.close ();
|
||||
|
||||
// The fee that's charged for transactions.
|
||||
auto const f = env.current ()->fees ().base;
|
||||
|
||||
env.fund (reserve (env, 3) + f * 4, alice);
|
||||
env.close ();
|
||||
|
||||
env (trust (alice, USD (506)));
|
||||
env (trust (alice, EUR (606)));
|
||||
env.close ();
|
||||
|
||||
env (pay (gw1, alice, USD (500)));
|
||||
env (pay (gw2, alice, EUR (600)));
|
||||
env.close ();
|
||||
|
||||
env (offer (alice, USD (500), EUR (600)));
|
||||
env.close ();
|
||||
|
||||
env.require (owners (alice, 3));
|
||||
env.require (balance (alice, USD (500)));
|
||||
env.require (balance (alice, EUR (600)));
|
||||
|
||||
auto aliceOffers = offersOnAccount (env, alice);
|
||||
expect (aliceOffers.size () == 1);
|
||||
for (auto const& offerPtr : aliceOffers)
|
||||
{
|
||||
auto const offer = *offerPtr;
|
||||
expect (offer[sfLedgerEntryType] == ltOFFER);
|
||||
expect (offer[sfTakerGets] == EUR (600));
|
||||
expect (offer[sfTakerPays] == USD (500));
|
||||
}
|
||||
|
||||
env (pay (alice, alice, EUR (60)), sendmax (USD (50)),
|
||||
txflags (tfPartialPayment));
|
||||
env.close ();
|
||||
|
||||
env.require (owners (alice, 3));
|
||||
env.require (balance (alice, USD (500)));
|
||||
env.require (balance (alice, EUR (600)));
|
||||
aliceOffers = offersOnAccount (env, alice);
|
||||
expect (aliceOffers.size () == 1);
|
||||
for (auto const& offerPtr : aliceOffers)
|
||||
{
|
||||
auto const offer = *offerPtr;
|
||||
expect (offer[sfLedgerEntryType] == ltOFFER);
|
||||
expect (offer[sfTakerGets] == EUR (594));
|
||||
expect (offer[sfTakerPays] == USD (495));
|
||||
}
|
||||
}
|
||||
|
||||
void run() override
|
||||
{
|
||||
testDirectStep ();
|
||||
testBookStep ();
|
||||
testTransferRate ();
|
||||
testToStrand ();
|
||||
testFalseDry();
|
||||
testLimitQuality();
|
||||
testSelfPayment1();
|
||||
testSelfPayment2();
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -204,6 +204,14 @@ public:
|
||||
STAmount const& preCreditBalance)
|
||||
{
|
||||
}
|
||||
|
||||
// Called when the owner count changes
|
||||
// This is required to support PaymentSandbox
|
||||
virtual
|
||||
void adjustOwnerCountHook (AccountID const& account,
|
||||
std::uint32_t cur, std::uint32_t next)
|
||||
{};
|
||||
|
||||
};
|
||||
|
||||
} // ripple
|
||||
|
||||
@@ -57,6 +57,16 @@ public:
|
||||
STAmount const& amount,
|
||||
STAmount const& preCreditSenderBalance);
|
||||
|
||||
void ownerCount (AccountID const& id,
|
||||
std::uint32_t cur,
|
||||
std::uint32_t next);
|
||||
|
||||
// Get the adjusted owner count. Since DeferredCredits is meant to be used
|
||||
// in payments, and payments only decrease owner counts, return the max
|
||||
// remembered owner count.
|
||||
boost::optional<std::uint32_t>
|
||||
ownerCount (AccountID const& id) const;
|
||||
|
||||
void apply (DeferredCredits& to);
|
||||
private:
|
||||
// lowAccount, highAccount
|
||||
@@ -75,7 +85,8 @@ private:
|
||||
AccountID const& a2,
|
||||
Currency const& c);
|
||||
|
||||
std::map<Key, Value> map_;
|
||||
std::map<Key, Value> credits_;
|
||||
std::map<AccountID, std::uint32_t> ownerCounts_;
|
||||
};
|
||||
|
||||
} // detail
|
||||
@@ -155,6 +166,15 @@ public:
|
||||
STAmount const& amount,
|
||||
STAmount const& preCreditBalance) override;
|
||||
|
||||
void
|
||||
adjustOwnerCountHook (AccountID const& account,
|
||||
std::uint32_t cur,
|
||||
std::uint32_t next) override;
|
||||
|
||||
std::uint32_t
|
||||
ownerCountHook (AccountID const& account,
|
||||
std::uint32_t count) const override;
|
||||
|
||||
/** Apply changes to base view.
|
||||
|
||||
`to` must contain contents identical to the parent
|
||||
|
||||
@@ -303,8 +303,11 @@ public:
|
||||
std::shared_ptr<SLE const>
|
||||
read (Keylet const& k) const = 0;
|
||||
|
||||
// Called to adjust returned balances
|
||||
// This is required to support PaymentSandbox
|
||||
// Accounts in a payment are not allowed to use assets acquired during that
|
||||
// payment. The PaymentSandbox tracks the debits, credits, and owner count
|
||||
// changes that accounts make during a payment. `balanceHook` adjusts balances
|
||||
// so newly acquired assets are not counted toward the balance.
|
||||
// This is required to support PaymentSandbox.
|
||||
virtual
|
||||
STAmount
|
||||
balanceHook (AccountID const& account,
|
||||
@@ -314,6 +317,19 @@ public:
|
||||
return amount;
|
||||
}
|
||||
|
||||
// Accounts in a payment are not allowed to use assets acquired during that
|
||||
// payment. The PaymentSandbox tracks the debits, credits, and owner count
|
||||
// changes that accounts make during a payment. `ownerCountHook` adjusts the
|
||||
// ownerCount so it returns the max value of the ownerCount so far.
|
||||
// This is required to support PaymentSandbox.
|
||||
virtual
|
||||
std::uint32_t
|
||||
ownerCountHook (AccountID const& account,
|
||||
std::uint32_t count) const
|
||||
{
|
||||
return count;
|
||||
}
|
||||
|
||||
// used by the implementation
|
||||
virtual
|
||||
std::unique_ptr<sles_type::iter_base>
|
||||
|
||||
@@ -343,6 +343,10 @@ transferXRP (ApplyView& view,
|
||||
NetClock::time_point const& flowV2SoTime ();
|
||||
bool flowV2Switchover (NetClock::time_point const closeTime);
|
||||
|
||||
NetClock::time_point const& dcSoTime ();
|
||||
bool dcSwitchover (NetClock::time_point const closeTime);
|
||||
|
||||
|
||||
} // ripple
|
||||
|
||||
#endif
|
||||
|
||||
@@ -49,8 +49,8 @@ DeferredCredits::credit (AccountID const& sender,
|
||||
assert (!amount.negative());
|
||||
|
||||
auto const k = makeKey (sender, receiver, amount.getCurrency ());
|
||||
auto i = map_.find (k);
|
||||
if (i == map_.end ())
|
||||
auto i = credits_.find (k);
|
||||
if (i == credits_.end ())
|
||||
{
|
||||
Value v;
|
||||
|
||||
@@ -67,7 +67,7 @@ DeferredCredits::credit (AccountID const& sender,
|
||||
v.lowAcctOrigBalance = -preCreditSenderBalance;
|
||||
}
|
||||
|
||||
map_[k] = v;
|
||||
credits_[k] = v;
|
||||
}
|
||||
else
|
||||
{
|
||||
@@ -80,6 +80,29 @@ DeferredCredits::credit (AccountID const& sender,
|
||||
}
|
||||
}
|
||||
|
||||
void
|
||||
DeferredCredits::ownerCount (AccountID const& id,
|
||||
std::uint32_t cur,
|
||||
std::uint32_t next)
|
||||
{
|
||||
auto const v = std::max (cur, next);
|
||||
auto r = ownerCounts_.emplace (std::make_pair (id, v));
|
||||
if (!r.second)
|
||||
{
|
||||
auto& mapVal = r.first->second;
|
||||
mapVal = std::max (v, mapVal);
|
||||
}
|
||||
}
|
||||
|
||||
boost::optional<std::uint32_t>
|
||||
DeferredCredits::ownerCount (AccountID const& id) const
|
||||
{
|
||||
auto i = ownerCounts_.find (id);
|
||||
if (i != ownerCounts_.end ())
|
||||
return i->second;
|
||||
return boost::none;
|
||||
}
|
||||
|
||||
// Get the adjustments for the balance between main and other.
|
||||
auto
|
||||
DeferredCredits::adjustments (AccountID const& main,
|
||||
@@ -89,8 +112,8 @@ DeferredCredits::adjustments (AccountID const& main,
|
||||
boost::optional<Adjustment> result;
|
||||
|
||||
Key const k = makeKey (main, other, currency);
|
||||
auto i = map_.find (k);
|
||||
if (i == map_.end ())
|
||||
auto i = credits_.find (k);
|
||||
if (i == credits_.end ())
|
||||
return result;
|
||||
|
||||
auto const& v = i->second;
|
||||
@@ -110,19 +133,29 @@ DeferredCredits::adjustments (AccountID const& main,
|
||||
void DeferredCredits::apply(
|
||||
DeferredCredits& to)
|
||||
{
|
||||
for (auto& p : map_)
|
||||
for (auto const& i : credits_)
|
||||
{
|
||||
auto r =
|
||||
to.map_.emplace(p);
|
||||
if (! r.second)
|
||||
auto r = to.credits_.emplace (i);
|
||||
if (!r.second)
|
||||
{
|
||||
auto& toVal = r.first->second;
|
||||
auto const& fromVal = p.second;
|
||||
auto const& fromVal = i.second;
|
||||
toVal.lowAcctCredits += fromVal.lowAcctCredits;
|
||||
toVal.highAcctCredits += fromVal.highAcctCredits;
|
||||
// Do not update the orig balance, it's already correct
|
||||
}
|
||||
}
|
||||
|
||||
for (auto const& i : ownerCounts_)
|
||||
{
|
||||
auto r = to.ownerCounts_.emplace (i);
|
||||
if (!r.second)
|
||||
{
|
||||
auto& toVal = r.first->second;
|
||||
auto const& fromVal = i.second;
|
||||
toVal = std::max (toVal, fromVal);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
} // detail
|
||||
@@ -172,10 +205,29 @@ PaymentSandbox::balanceHook (AccountID const& account,
|
||||
}
|
||||
}
|
||||
|
||||
assert (!isXRP(issuer) || adjustedAmt >= beast::zero);
|
||||
if (isXRP(issuer) && adjustedAmt < beast::zero)
|
||||
// A calculated negative XRP balance is not an error case. Consider a
|
||||
// payment snippet that credits a large XRP amount and then debits the
|
||||
// same amount. The credit can't be used but we subtract the debit and
|
||||
// calculate a negative value. It's not an error case.
|
||||
adjustedAmt.clear();
|
||||
|
||||
return adjustedAmt;
|
||||
}
|
||||
|
||||
std::uint32_t
|
||||
PaymentSandbox::ownerCountHook (AccountID const& account,
|
||||
std::uint32_t count) const
|
||||
{
|
||||
std::uint32_t result = count;
|
||||
for (auto curSB = this; curSB; curSB = curSB->ps_)
|
||||
{
|
||||
if (auto adj = curSB->tab_.ownerCount (account))
|
||||
result = std::max (result, *adj);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
void
|
||||
PaymentSandbox::creditHook (AccountID const& from,
|
||||
AccountID const& to,
|
||||
@@ -185,6 +237,14 @@ PaymentSandbox::creditHook (AccountID const& from,
|
||||
tab_.credit (from, to, amount, preCreditBalance);
|
||||
}
|
||||
|
||||
void
|
||||
PaymentSandbox::adjustOwnerCountHook (AccountID const& account,
|
||||
std::uint32_t cur,
|
||||
std::uint32_t next)
|
||||
{
|
||||
tab_.ownerCount (account, cur, next);
|
||||
}
|
||||
|
||||
void
|
||||
PaymentSandbox::apply (RawView& to)
|
||||
{
|
||||
|
||||
@@ -34,8 +34,8 @@ namespace ripple {
|
||||
NetClock::time_point const& flowV2SoTime ()
|
||||
{
|
||||
using namespace std::chrono_literals;
|
||||
// Wed May 25, 2016 10:00:00am PDT
|
||||
static NetClock::time_point const soTime{517510800s};
|
||||
// Mon Aug 29, 2016 10:00:00am PDT
|
||||
static NetClock::time_point const soTime{525805200s};
|
||||
return soTime;
|
||||
}
|
||||
|
||||
@@ -44,6 +44,19 @@ bool flowV2Switchover (NetClock::time_point const closeTime)
|
||||
return closeTime > flowV2SoTime();
|
||||
}
|
||||
|
||||
NetClock::time_point const& dcSoTime ()
|
||||
{
|
||||
using namespace std::chrono_literals;
|
||||
// Mon May 23, 2016 10:00:00am PDT
|
||||
static NetClock::time_point const soTime{517338000s};
|
||||
return soTime;
|
||||
}
|
||||
|
||||
bool dcSwitchover (NetClock::time_point const closeTime)
|
||||
{
|
||||
return closeTime > dcSoTime();
|
||||
}
|
||||
|
||||
// VFALCO NOTE A copy of the other one for now
|
||||
/** Maximum number of entries in a directory page
|
||||
A change would be protocol-breaking.
|
||||
@@ -121,51 +134,85 @@ accountHolds (ReadView const& view,
|
||||
if (isXRP(currency))
|
||||
{
|
||||
// XRP: return balance minus reserve
|
||||
auto const sle = view.read(
|
||||
keylet::account(account));
|
||||
auto const reserve =
|
||||
view.fees().accountReserve(
|
||||
sle->getFieldU32(sfOwnerCount));
|
||||
auto const balance =
|
||||
sle->getFieldAmount(sfBalance).xrp ();
|
||||
if (balance < reserve)
|
||||
amount.clear ();
|
||||
if (dcSwitchover (view.info ().parentCloseTime))
|
||||
{
|
||||
auto const sle = view.read(
|
||||
keylet::account(account));
|
||||
auto const ownerCount =
|
||||
view.ownerCountHook (account, sle->getFieldU32 (sfOwnerCount));
|
||||
auto const reserve =
|
||||
view.fees().accountReserve(ownerCount);
|
||||
|
||||
auto const fullBalance =
|
||||
sle->getFieldAmount(sfBalance);
|
||||
|
||||
auto const balance = view.balanceHook(
|
||||
account, issuer, fullBalance).xrp();
|
||||
|
||||
if (balance < reserve)
|
||||
amount.clear ();
|
||||
else
|
||||
amount = balance - reserve;
|
||||
|
||||
JLOG (j.trace()) << "accountHolds:" <<
|
||||
" account=" << to_string (account) <<
|
||||
" amount=" << amount.getFullText () <<
|
||||
" fullBalance=" << to_string (fullBalance.xrp()) <<
|
||||
" balance=" << to_string (balance) <<
|
||||
" reserve=" << to_string (reserve);
|
||||
|
||||
return amount;
|
||||
}
|
||||
else
|
||||
amount = balance - reserve;
|
||||
JLOG (j.trace()) << "accountHolds:" <<
|
||||
" account=" << to_string (account) <<
|
||||
" amount=" << amount.getFullText () <<
|
||||
" balance=" << to_string (balance) <<
|
||||
" reserve=" << to_string (reserve);
|
||||
{
|
||||
// pre-switchover
|
||||
// XRP: return balance minus reserve
|
||||
auto const sle = view.read(
|
||||
keylet::account(account));
|
||||
auto const reserve =
|
||||
view.fees().accountReserve(
|
||||
sle->getFieldU32(sfOwnerCount));
|
||||
auto const balance =
|
||||
sle->getFieldAmount(sfBalance).xrp ();
|
||||
if (balance < reserve)
|
||||
amount.clear ();
|
||||
else
|
||||
amount = balance - reserve;
|
||||
JLOG (j.trace()) << "accountHolds:" <<
|
||||
" account=" << to_string (account) <<
|
||||
" amount=" << amount.getFullText () <<
|
||||
" balance=" << to_string (balance) <<
|
||||
" reserve=" << to_string (reserve);
|
||||
return view.balanceHook(
|
||||
account, issuer, amount);
|
||||
}
|
||||
}
|
||||
|
||||
// IOU: Return balance on trust line modulo freeze
|
||||
auto const sle = view.read(keylet::line(
|
||||
account, issuer, currency));
|
||||
if (! sle)
|
||||
{
|
||||
amount.clear ({currency, issuer});
|
||||
}
|
||||
else if ((zeroIfFrozen == fhZERO_IF_FROZEN) &&
|
||||
isFrozen(view, account, currency, issuer))
|
||||
{
|
||||
amount.clear (Issue (currency, issuer));
|
||||
}
|
||||
else
|
||||
{
|
||||
// IOU: Return balance on trust line modulo freeze
|
||||
auto const sle = view.read(keylet::line(
|
||||
account, issuer, currency));
|
||||
if (! sle)
|
||||
amount = sle->getFieldAmount (sfBalance);
|
||||
if (account > issuer)
|
||||
{
|
||||
amount.clear ({currency, issuer});
|
||||
// Put balance in account terms.
|
||||
amount.negate();
|
||||
}
|
||||
else if ((zeroIfFrozen == fhZERO_IF_FROZEN) &&
|
||||
isFrozen(view, account, currency, issuer))
|
||||
{
|
||||
amount.clear (Issue (currency, issuer));
|
||||
}
|
||||
else
|
||||
{
|
||||
amount = sle->getFieldAmount (sfBalance);
|
||||
if (account > issuer)
|
||||
{
|
||||
// Put balance in account terms.
|
||||
amount.negate();
|
||||
}
|
||||
amount.setIssuer (issuer);
|
||||
}
|
||||
JLOG (j.trace()) << "accountHolds:" <<
|
||||
" account=" << to_string (account) <<
|
||||
" amount=" << amount.getFullText ();
|
||||
amount.setIssuer (issuer);
|
||||
}
|
||||
JLOG (j.trace()) << "accountHolds:" <<
|
||||
" account=" << to_string (account) <<
|
||||
" amount=" << amount.getFullText ();
|
||||
|
||||
return view.balanceHook(
|
||||
account, issuer, amount);
|
||||
@@ -609,13 +656,14 @@ adjustOwnerCount (ApplyView& view,
|
||||
auto const current =
|
||||
sle->getFieldU32 (sfOwnerCount);
|
||||
auto adjusted = current + amount;
|
||||
AccountID const id = (*sle)[sfAccount];
|
||||
if (amount > 0)
|
||||
{
|
||||
// Overflow is well defined on unsigned
|
||||
if (adjusted < current)
|
||||
{
|
||||
JLOG (j.fatal()) <<
|
||||
"Account " << sle->getAccountID(sfAccount) <<
|
||||
"Account " << id <<
|
||||
" owner count exceeds max!";
|
||||
adjusted =
|
||||
std::numeric_limits<std::uint32_t>::max ();
|
||||
@@ -627,12 +675,13 @@ adjustOwnerCount (ApplyView& view,
|
||||
if (adjusted > current)
|
||||
{
|
||||
JLOG (j.fatal()) <<
|
||||
"Account " << sle->getAccountID (sfAccount) <<
|
||||
"Account " << id <<
|
||||
" owner count set below 0!";
|
||||
adjusted = 0;
|
||||
assert(false);
|
||||
}
|
||||
}
|
||||
view.adjustOwnerCountHook (id, current, adjusted);
|
||||
sle->setFieldU32 (sfOwnerCount, adjusted);
|
||||
view.update(sle);
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@
|
||||
#include <ripple/ledger/PaymentSandbox.h>
|
||||
#include <ripple/ledger/tests/PathSet.h>
|
||||
#include <ripple/ledger/View.h>
|
||||
#include <ripple/protocol/AmountConversions.h>
|
||||
|
||||
namespace ripple {
|
||||
namespace test {
|
||||
@@ -290,6 +291,46 @@ class PaymentSandbox_test : public beast::unit_test::suite
|
||||
expect (pv.balanceHook (alice, gw, hugeAmt) != tinyAmt);
|
||||
}
|
||||
}
|
||||
void testReserve()
|
||||
{
|
||||
testcase ("Reserve");
|
||||
using namespace jtx;
|
||||
|
||||
beast::Journal dj;
|
||||
|
||||
auto accountFundsXRP = [&dj](
|
||||
ReadView const& view, AccountID const& id) -> XRPAmount
|
||||
{
|
||||
return toAmount<XRPAmount> (accountHolds (
|
||||
view, id, xrpCurrency (), xrpAccount (), fhZERO_IF_FROZEN, dj));
|
||||
};
|
||||
|
||||
auto reserve = [](jtx::Env& env, std::uint32_t count) -> XRPAmount
|
||||
{
|
||||
return env.current ()->fees ().accountReserve (count);
|
||||
};
|
||||
|
||||
Env env (*this);
|
||||
|
||||
Account const alice ("alice");
|
||||
env.fund (reserve(env, 1), alice);
|
||||
|
||||
auto const closeTime = dcSoTime () +
|
||||
100 * env.closed ()->info ().closeTimeResolution;
|
||||
env.close (closeTime);
|
||||
ApplyViewImpl av (&*env.current (), tapNONE);
|
||||
PaymentSandbox sb (&av);
|
||||
{
|
||||
// Send alice an amount and spend it. The deferredCredits will cause her balance
|
||||
// to drop below the reserve. Make sure her funds are zero (there was a bug that
|
||||
// caused her funds to become negative).
|
||||
|
||||
accountSend (sb, xrpAccount (), alice, XRP(100), dj);
|
||||
accountSend (sb, alice, xrpAccount (), XRP(100), dj);
|
||||
expect (accountFundsXRP (sb, alice) == beast::zero);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
public:
|
||||
void run ()
|
||||
@@ -297,6 +338,7 @@ public:
|
||||
testSelfFunding ();
|
||||
testSubtractCredits ();
|
||||
testTinyBalance ();
|
||||
testReserve();
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user