Skip to content

Commit

Permalink
Add fixTakerDryOfferRemoval amendment
Browse files Browse the repository at this point in the history
  • Loading branch information
demonstefan authored and mellery451 committed Oct 19, 2018
1 parent d5c0e12 commit 7b48dc3
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 28 deletions.
29 changes: 23 additions & 6 deletions src/ripple/app/tx/impl/CreateOffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,15 +446,32 @@ CreateOffer::bridged_cross (

JLOG (j_.debug()) << "Bridge Result: " << transToken (cross_result);

if (dry_offer (view, offers_leg1.tip ()))
if (view.rules().enabled (fixTakerDryOfferRemoval))
{
leg1_consumed = true;
have_bridge = (have_bridge && offers_leg1.step ());
// have_bridge can be true the next time 'round only if
// neither of the OfferStreams are dry.
leg1_consumed = dry_offer (view, offers_leg1.tip());
if (leg1_consumed)
have_bridge &= offers_leg1.step();

leg2_consumed = dry_offer (view, offers_leg2.tip());
if (leg2_consumed)
have_bridge &= offers_leg2.step();
}
if (dry_offer (view, offers_leg2.tip ()))
else
{
leg2_consumed = true;
have_bridge = (have_bridge && offers_leg2.step ());
// This old behavior may leave an empty offer in the book for
// the second leg.
if (dry_offer (view, offers_leg1.tip ()))
{
leg1_consumed = true;
have_bridge = (have_bridge && offers_leg1.step ());
}
if (dry_offer (view, offers_leg2.tip ()))
{
leg2_consumed = true;
have_bridge = (have_bridge && offers_leg2.step ());
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/ripple/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ class FeatureCollections
"DepositPreauth",
"fix1515",
"fix1578",
"MultiSignReserve"
"MultiSignReserve",
"fixTakerDryOfferRemoval"
};

std::vector<uint256> features;
Expand Down Expand Up @@ -371,6 +372,7 @@ extern uint256 const featureDepositPreauth;
extern uint256 const fix1515;
extern uint256 const fix1578;
extern uint256 const featureMultiSignReserve;
extern uint256 const fixTakerDryOfferRemoval;

} // ripple

Expand Down
4 changes: 3 additions & 1 deletion src/ripple/protocol/impl/Feature.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ detail::supportedAmendments ()
// Use liquidity from strands that consume max offers, but mark as dry
{ "5D08145F0A4983F23AFFFF514E83FAD355C5ABFBB6CAB76FB5BC8519FF5F33BE fix1515" },
{ "FBD513F1B893AC765B78F250E6FFA6A11B573209D1842ADC787C850696741288 fix1578" },
{ "586480873651E106F1D6339B0C4A8945BA705A777F3F4524626FF1FC07EFE41D MultiSignReserve" }
{ "586480873651E106F1D6339B0C4A8945BA705A777F3F4524626FF1FC07EFE41D MultiSignReserve" },
{ "2CD5286D8D687E98B41102BDD797198E81EA41DF7BD104E6561FEB104EFF2561 fixTakerDryOfferRemoval"}
};
return supported;
}
Expand Down Expand Up @@ -172,5 +173,6 @@ uint256 const featureDepositPreauth = *getRegisteredFeature("DepositPreauth");
uint256 const fix1515 = *getRegisteredFeature("fix1515");
uint256 const fix1578 = *getRegisteredFeature("fix1578");
uint256 const featureMultiSignReserve = *getRegisteredFeature("MultiSignReserve");
uint256 const fixTakerDryOfferRemoval = *getRegisteredFeature("fixTakerDryOfferRemoval");

} // ripple
120 changes: 100 additions & 20 deletions src/test/app/Offer_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1154,15 +1154,19 @@ class Offer_test : public beast::unit_test::suite
BEAST_EXPECT(jrr[jss::offers].size() == 0);

// NOTE :
// at this point, all offers are expected to be consumed.
// alas, they are not - because of a bug in the Taker auto-bridging
// implementation (to be replaced in the not-so-distant future).
// the current implementation (incorrect) leaves an empty offer in the
// second leg of the bridge. validate both the old and the new
// behavior.
// At this point, all offers are expected to be consumed.
// Alas, they are not - because of a bug in the Taker auto-bridging
// implementation which is addressed by fixTakerDryOfferRemoval.
// The pre-fixTakerDryOfferRemoval implementation (incorrect) leaves
// an empty offer in the second leg of the bridge. Validate both the
// old and the new behavior.
{
auto acctOffers = offersOnAccount (env, account_to_test);
BEAST_EXPECT(acctOffers.size() == (features[featureFlowCross] ? 0 : 1));
bool const noStaleOffers {
features[featureFlowCross] ||
features[fixTakerDryOfferRemoval]};

BEAST_EXPECT(acctOffers.size() == (noStaleOffers ? 0 : 1));
for (auto const& offerPtr : acctOffers)
{
auto const& offer = *offerPtr;
Expand Down Expand Up @@ -1831,6 +1835,81 @@ class Offer_test : public beast::unit_test::suite
jro[jss::node][jss::TakerPays] == XRP (200).value ().getText ());
}

void
testBridgedSecondLegDry(FeatureBitset features)
{
// At least with Taker bridging, a sensitivity was identified if the
// second leg goes dry before the first one. This test exercises that
// case.
testcase ("Auto Bridged Second Leg Dry");

using namespace jtx;
Env env(*this, features);
auto const closeTime =
fix1449Time() +
100 * env.closed()->info().closeTimeResolution;
env.close (closeTime);

Account const alice {"alice"};
Account const bob {"bob"};
Account const carol {"carol"};
Account const gw {"gateway"};
auto const USD = gw["USD"];
auto const EUR = gw["EUR"];

env.fund(XRP(100000000), alice, bob, carol, gw);

env.trust(USD(10), alice);
env.close();
env(pay(gw, alice, USD(10)));
env.trust(USD(10), carol);
env.close();
env(pay(gw, carol, USD(3)));

env (offer (alice, EUR(2), XRP(1)));
env (offer (alice, EUR(2), XRP(1)));

env (offer (alice, XRP(1), USD(4)));
env (offer (carol, XRP(1), USD(3)));
env.close();

// Bob offers to buy 10 USD for 10 EUR.
// 1. He spends 2 EUR taking Alice's auto-bridged offers and
// gets 4 USD for that.
// 2. He spends another 2 EUR taking Alice's last EUR->XRP offer and
// Carol's XRP-USD offer. He gets 3 USD for that.
// The key for this test is that Alice's XRP->USD leg goes dry before
// Alice's EUR->XRP. The XRP->USD leg is the second leg which showed
// some sensitivity.
env.trust(EUR(10), bob);
env.close();
env(pay(gw, bob, EUR(10)));
env.close();
env(offer(bob, USD(10), EUR(10)));
env.close();

env.require (balance(bob, USD(7)));
env.require (balance(bob, EUR(6)));
env.require (offers (bob, 1));
env.require (owners (bob, 3));

env.require (balance(alice, USD(6)));
env.require (balance(alice, EUR(4)));
env.require (offers(alice, 0));
env.require (owners(alice, 2));

env.require (balance(carol, USD(0)));
env.require (balance(carol, EUR(none)));
// If neither featureFlowCross nor fixTakerDryOfferRemoval are defined
// then carol's offer will be left on the books, but with zero value.
int const emptyOfferCount {
features[featureFlowCross] ||
features[fixTakerDryOfferRemoval] ? 0 : 1};

env.require (offers(carol, 0 + emptyOfferCount));
env.require (owners(carol, 1 + emptyOfferCount));
}

void
testOfferFeesConsumeFunds(FeatureBitset features)
{
Expand Down Expand Up @@ -4607,6 +4686,7 @@ class Offer_test : public beast::unit_test::suite
testCrossCurrencyStartXRP(features);
testCrossCurrencyEndXRP(features);
testCrossCurrencyBridged(features);
testBridgedSecondLegDry(features);
testOfferFeesConsumeFunds(features);
testOfferCreateThenCross(features);
testSellFlagBasic(features);
Expand Down Expand Up @@ -4640,20 +4720,15 @@ class Offer_test : public beast::unit_test::suite
void run () override
{
using namespace jtx;
auto const all = supported_amendments();
FeatureBitset const flow{featureFlow};
FeatureBitset const all{supported_amendments()};
FeatureBitset const f1373{fix1373};
FeatureBitset const flowCross{featureFlowCross};
(void) flow;

// The first three test variants below passed at one time in the past
// (and should still pass) but are commented out to conserve test time.
// testAll(all - flow - f1373 - flowCross);
// testAll(all - flow - f1373 );
// testAll(all - f1373 - flowCross);
testAll(all - f1373 );
testAll(all - flowCross);
testAll(all );
FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval};

testAll(all - f1373 - takerDryOffer);
testAll(all - flowCross - takerDryOffer);
testAll(all - flowCross );
testAll(all );
}
};

Expand All @@ -4662,12 +4737,13 @@ class Offer_manual_test : public Offer_test
void run() override
{
using namespace jtx;
auto const all = supported_amendments();
FeatureBitset const all{supported_amendments()};
FeatureBitset const feeEscalation{featureFeeEscalation};
FeatureBitset const flow{featureFlow};
FeatureBitset const f1373{fix1373};
FeatureBitset const flowCross{featureFlowCross};
FeatureBitset const f1513{fix1513};
FeatureBitset const takerDryOffer{fixTakerDryOfferRemoval};

testAll(all -feeEscalation - flow - f1373 - flowCross - f1513);
testAll(all - flow - f1373 - flowCross - f1513);
Expand All @@ -4687,6 +4763,10 @@ class Offer_manual_test : public Offer_test
testAll(all -feeEscalation - f1513);
testAll(all - f1513);
testAll(all );

testAll(all - flow - f1373 - flowCross - takerDryOffer);
testAll(all - flow - f1373 - takerDryOffer);
testAll(all - f1373 - flowCross - takerDryOffer);
}
};

Expand Down

0 comments on commit 7b48dc3

Please sign in to comment.