ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-12-06 17:27:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix funded offer removal during payment (RIPD-113):

Browse Source 
In some cases, funded offers were incorrectly removed. This happened
when:

1) There are multiple payment paths.
2) A payment path has several offers in a row.
3) An offer causes a previous offer to become unfunded when calculating
   reverse liquidity and that offer does not satisfy the payment and
   there is another offer at the same quality.
4) The payment path is not used to satisfy the payment (there are other
   paths at better quality that do the job).
This commit is contained in:
seelabs
2015-06-22 17:17:22 -04:00
committed by Vinnie Falco
parent bc5a25168a
commit 2b91e62d5d
3 changed files with 85 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
src/ripple/app/paths/cursor/DeliverNodeReverse.cpp
View File

@@ -35,17 +35,15 @@ namespace path {
// as the rate does not increase past the initial rate.
// To deliver from an order book, when computing
TER PathCursor::deliverNodeReverse (
AccountID const& uOutAccountID, // --> Output owner's account.
STAmount const& saOutReq, // --> Funds requested to be
// delivered for an increment.
STAmount& saOutAct) const // <-- Funds actually delivered for an
// increment.
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
// increment
) const
{
TER resultCode = tesSUCCESS;
node().directory.restart(multiQuality_);
// Accumulation of what the previous node must deliver.
// Possible optimization: Note this gets zeroed on each increment, ideally
// only on first increment, then it could be a limit on the forward pass.
@@ -235,7 +233,7 @@ TER PathCursor::deliverNodeReverse (
// Compute in previous offer node how much could come in.
// TODO(tom): Fix nasty recursion here!
resultCode = increment(-1).deliverNodeReverse(
resultCode = increment(-1).deliverNodeReverseImpl(
node().offerOwnerAccount_,
saInPassReq,
saInPassAct);
@@ -345,5 +343,19 @@ TER PathCursor::deliverNodeReverse (
return resultCode;
}
TER PathCursor::deliverNodeReverse (
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
// increment
) const
{
for (int i = nodeIndex_; i >= 0 && !node (i).isAccount(); --i)
node (i).directory.restart (multiQuality_);
return deliverNodeReverseImpl(uOutAccountID, saOutReq, saOutAct);
}
} // path
} // ripple

6
src/ripple/app/paths/cursor/PathCursor.h
View File

@@ -85,6 +85,12 @@ private:
STAmount const& saOutReq,
STAmount& saOutAct) const;
// To deliver from an order book, when computing
TER deliverNodeReverseImpl (
AccountID const& uOutAccountID,
STAmount const& saOutReq,
STAmount& saOutAct) const;
TER deliverNodeForward (
AccountID const& uInAccountID,
STAmount const& saInReq,

75
src/ripple/app/tx/tests/Offer.test.cpp
View File

@@ -23,31 +23,69 @@
#include <ripple/basics/Log.h>
#include <ripple/test/jtx.h>
#include <ripple/test/jtx/Account.h>
#include <ripple/ledger/tests/PathSet.h>
namespace ripple {
namespace test {
// An offer exists
bool isOffer (jtx::Env const& env,
jtx::Account const& account,
STAmount const& takerPays,
STAmount const& takerGets)
{
bool exists = false;
forEachItem (*env.open(), account,
[&](std::shared_ptr<SLE const> const& sle)
{
if (sle->getType () == ltOFFER &&
sle->getFieldAmount (sfTakerPays) == takerPays &&
sle->getFieldAmount (sfTakerGets) == takerGets)
exists = true;
});
return exists;
}
class Offer_test : public beast::unit_test::suite
{
public:
void testRmFundedOffer ()
{
// We need at least two paths. One at good quality and one at bad quality.
// The bad quality path needs two offer books in a row. Each offer book
// should have two offers at the same quality, the offers should be
// completely consumed, and the payment should should require both offers to
// be satisified. The first offer must be "taker gets" XRP. Old, broken
// would remove the first "taker gets" xrp offer, even though the offer is
// still funded and not used for the payment.
using namespace jtx;
Env env (*this);
auto const gw = Account ("gateway");
auto const USD = gw["USD"];
auto const BTC = gw["BTC"];
Account const alice ("alice");
Account const bob ("bob");
Account const carol ("carol");
env.fund (XRP (10000), alice, bob, carol, gw);
env.trust (USD (1000), alice, bob, carol);
env.trust (BTC (1000), alice, bob, carol);
env (pay (gw, alice, BTC (1000)));
env (pay (gw, carol, USD (1000)));
env (pay (gw, carol, BTC (1000)));
// Must be two offers at the same quality
// "taker gets" must be XRP
// (Different amounts so I can distinguish the offers)
env (offer (carol, BTC (49), XRP (49)));
env (offer (carol, BTC (51), XRP (51)));
// Offers for the poor quality path
// Must be two offers at the same quality
env (offer (carol, XRP (50), USD (50)));
env (offer (carol, XRP (50), USD (50)));
// Offers for the good quality path
env (offer (carol, BTC (1), USD (100)));
PathSet paths (
Path (XRP, USD),
Path (USD));
env (pay ("alice", "bob", USD (100)),
json (paths.json ()),
sendmax (BTC (1000)),
txflags (tfPartialPayment));
require (balance ("bob", USD (100)));
expect (!isOffer (env, "carol", BTC (1), USD (100)) &&
isOffer (env, "carol", BTC (49), XRP (49)));
}
void testCanceledOffer ()
{
using namespace jtx;
@@ -87,6 +125,7 @@ public:
// Hack to silence logging
deprecatedLogs ().severity (beast::Journal::Severity::kNone);
testCanceledOffer ();
testRmFundedOffer ();
}
};