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 unfunded offer not removed (RIPD-1298):

Browse Source 
If the mantissas of two non-native amounts differ by less than 10, then
subtracting them leaves a result of zero. This can cause situations
where `a>b`, yet `a-b == 0`.

One consequence of this is unfunded offers were incorrectly left in
order books. The code would check if the offer would be
consumed (`amount in offer > amount needed`), assume it wouldn't be,
yet when `amount needed` was subtracted from `amount in offer` the
result was zero and the offer was unfunded. This unfunded offer
incorrectly remained on the order book.

This patch fixes this bug.
This commit is contained in:
seelabs
2016-09-16 11:19:51 -04:00
committed by Vinnie Falco
parent 3b639afac2
commit bb0b97f46b
4 changed files with 109 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
src/ripple/app/paths/impl/BookStep.cpp
View File

@@ -412,7 +412,15 @@ BookStep<TIn, TOut>::revImp (
result.in = sum(savedIns);
result.out = out;
this->consumeOffer (sb, offer, ofrAdjAmt, stpAdjAmt, ownerGivesAdj);
return false;
// When the mantissas of two iou amounts differ by less than ten, then
// subtracting them leaves a result of zero. This can cause the check for
// (stpAmt.out > remainingOut) to incorrectly think an offer will be funded
// after subtracting remainingIn.
if (amendmentRIPD1298(sb.parentCloseTime()))
return offer.fully_consumed();
else
return false;
}
};
@@ -561,6 +569,14 @@ BookStep<TIn, TOut>::fwdImp (
remainingIn = in - result.in;
this->consumeOffer (sb, offer, ofrAdjAmt, stpAdjAmt, ownerGivesAdj);
// When the mantissas of two iou amounts differ by less than ten, then
// subtracting them leaves a result of zero. This can cause the check for
// (stpAmt.in > remainingIn) to incorrectly think an offer will be funded
// after subtracting remainingIn.
if (amendmentRIPD1298(sb.parentCloseTime()))
processMore = processMore || offer.fully_consumed();
return processMore;
};

7
src/ripple/ledger/View.h
View File

@@ -338,14 +338,11 @@ transferXRP (ApplyView& view,
NetClock::time_point const& amendmentRIPD1141SoTime ();
bool amendmentRIPD1141 (NetClock::time_point const closeTime);
NetClock::time_point const& amendmentRIPD1141SoTime ();
bool amendmentRIPD1141 (NetClock::time_point const closeTime);
NetClock::time_point const& amendmentRIPD1274SoTime ();
bool amendmentRIPD1274 (NetClock::time_point const closeTime);
NetClock::time_point const& amendmentRIPD1274SoTime ();
bool amendmentRIPD1274 (NetClock::time_point const closeTime);
NetClock::time_point const& amendmentRIPD1298SoTime ();
bool amendmentRIPD1298 (NetClock::time_point const closeTime);
} // ripple

14
src/ripple/ledger/impl/View.cpp
View File

@@ -58,6 +58,20 @@ bool amendmentRIPD1274 (NetClock::time_point const closeTime)
return closeTime > amendmentRIPD1274SoTime();
}
NetClock::time_point const& amendmentRIPD1298SoTime ()
{
using namespace std::chrono_literals;
// TBD Move this date up when we know the release date
// Thu Jan 5, 2017 10:00:00am PDT
static NetClock::time_point const soTime{536954400s};
return soTime;
}
bool amendmentRIPD1298 (NetClock::time_point const closeTime)
{
return closeTime > amendmentRIPD1298SoTime();
}
// VFALCO NOTE A copy of the other one for now
/** Maximum number of entries in a directory page
A change would be protocol-breaking.

76
src/test/app/Flow_test.cpp
View File

@@ -1305,6 +1305,80 @@ struct Flow_test : public beast::unit_test::suite
txflags(tfPartialPayment | tfNoRippleDirect));
}
void testUnfundedOffer (bool withFix)
{
testcase(std::string("Unfunded Offer ") +
(withFix ? "with fix" : "without fix"));
using namespace jtx;
{
// Test reverse
Env env(*this, features(featureFlow));
auto closeTime = amendmentRIPD1298SoTime();
if (withFix)
closeTime += env.closed()->info().closeTimeResolution;
else
closeTime -= env.closed()->info().closeTimeResolution;
env.close(closeTime);
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const gw = Account("gw");
auto const USD = gw["USD"];
env.fund(XRP(100000), alice, bob, gw);
env(trust(bob, USD(20)));
STAmount tinyAmt1{USD.issue(), 9000000000000000ll, -17, false,
false, STAmount::unchecked{}};
STAmount tinyAmt3{USD.issue(), 9000000000000003ll, -17, false,
false, STAmount::unchecked{}};
env(offer(gw, drops(9000000000), tinyAmt3));
env(pay(alice, bob, tinyAmt1), path(~USD),
sendmax(drops(9000000000)), txflags(tfNoRippleDirect));
if (withFix)
BEAST_EXPECT(!isOffer(env, gw, XRP(0), USD(0)));
else
BEAST_EXPECT(isOffer(env, gw, XRP(0), USD(0)));
}
{
// Test forward
Env env(*this, features(featureFlow));
auto closeTime = amendmentRIPD1298SoTime();
if (withFix)
closeTime += env.closed()->info().closeTimeResolution;
else
closeTime -= env.closed()->info().closeTimeResolution;
env.close(closeTime);
auto const alice = Account("alice");
auto const bob = Account("bob");
auto const gw = Account("gw");
auto const USD = gw["USD"];
env.fund(XRP(100000), alice, bob, gw);
env(trust(alice, USD(20)));
STAmount tinyAmt1{USD.issue(), 9000000000000000ll, -17, false,
false, STAmount::unchecked{}};
STAmount tinyAmt3{USD.issue(), 9000000000000003ll, -17, false,
false, STAmount::unchecked{}};
env(pay(gw, alice, tinyAmt1));
env(offer(gw, tinyAmt3, drops(9000000000)));
env(pay(alice, bob, drops(9000000000)), path(~XRP),
sendmax(USD(1)), txflags(tfNoRippleDirect));
if (withFix)
BEAST_EXPECT(!isOffer(env, gw, USD(0), XRP(0)));
else
BEAST_EXPECT(isOffer(env, gw, USD(0), XRP(0)));
}
}
void run() override
{
testDirectStep ();
@@ -1318,6 +1392,8 @@ struct Flow_test : public beast::unit_test::suite
testSelfPayment2();
testSelfFundedXRPEndpoint(false);
testSelfFundedXRPEndpoint(true);
testUnfundedOffer(true);
testUnfundedOffer(false);
}
};