diff --git a/src/ripple/app/paths/impl/BookStep.cpp b/src/ripple/app/paths/impl/BookStep.cpp index 4ad559855c..ea241c2b86 100644 --- a/src/ripple/app/paths/impl/BookStep.cpp +++ b/src/ripple/app/paths/impl/BookStep.cpp @@ -412,7 +412,15 @@ BookStep::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::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; }; diff --git a/src/ripple/ledger/View.h b/src/ripple/ledger/View.h index 94c848462d..246e3b4a5e 100644 --- a/src/ripple/ledger/View.h +++ b/src/ripple/ledger/View.h @@ -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 diff --git a/src/ripple/ledger/impl/View.cpp b/src/ripple/ledger/impl/View.cpp index c5ecdd6e2e..83ebb86b7f 100644 --- a/src/ripple/ledger/impl/View.cpp +++ b/src/ripple/ledger/impl/View.cpp @@ -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. diff --git a/src/test/app/Flow_test.cpp b/src/test/app/Flow_test.cpp index f81b32bb88..0b5e49291f 100644 --- a/src/test/app/Flow_test.cpp +++ b/src/test/app/Flow_test.cpp @@ -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); } };